Re: [PATCH] elf-eh-frame, move buffer alloc out of if block

[msnyder at sonic dot net]

> On Fri, Jul 27, 2007 at 02:21:19PM -0700, msnyder@sonic.net wrote:
>> The else branch also relies on this pointer being non-null, so
>> just move the allocation above the if.
>>
>
>> 2007-07-27  Michael Snyder  <msnyder@access-company.com>
>>
>> 	* elf-eh-frame.c (_bfd_elf_discard_section_eh_frame): Move alloc
>> 	above if block, since both branches rely on it.
>
> This is wrong.  Only if (hdr_id == 0) { ... } code ever adds new cies
> to the array, else branch will just goto free_no_table; (failed REQUIRE)
> if ecie_count == 0 (on an invalid .eh_frame section):
>
>           /* Find the corresponding CIE.  */
>           unsigned int cie_offset = this_inf->offset + 4 - hdr_id;
>           for (ecie = ecies; ecie < ecies + ecie_count; ++ecie)
>             if (cie_offset == ecie->offset)
>               break;
>
>           /* Ensure this FDE references one of the CIEs in this input
>              section.  */
>           REQUIRE (ecie != ecies + ecie_count);
>
> So, if ecies is NULL (implies invalid .eh_frame section and also
> ecie_count == 0), I don't see anything invalid on the
> ecie = NULL assignment or NULL < NULL + 0 comparison (false), then it
> will just do if (NULL == NULL + 0) goto free_no_table;

Well, that reasoning requires that you *know* that (ecies == NULL)
implies invalid .eh_frame section and ecie_count == 0.

OK then, how about this instead?

          /* Find the corresponding CIE.  */
          unsigned int cie_offset = this_inf->offset + 4 - hdr_id;
+         REQUIRE (ecies != NULL || ecie_count == 0);
          for (ecie = ecies; ecie < ecies + ecie_count; ++ecie)
            if (cie_offset == ecie->offset)
              break;