Re: [patch] Set bfd field in target_section

Aleksandar Ristovski
2009-07-28T19:51:53Z

Pedro Alves wrote:
> On Tuesday 28 July 2009 16:16:23, Aleksandar Ristovski wrote:
>> Pedro Alves wrote:
>> I think now you broke it in a different way. Now we can end
>> up trying to read from a closed bfd.
>
> Ooops, you're right. There are uses of the bfd after transfering
> its ownership to the bfd target with target_bfd_reopen, and closing
> the bfd target (target_close)... This target is modeled on fdopen,
> which is why I blindly assumed it wasn't happening. I've reverted
> that patch.
>
>> And just wondering, why not simply:
>>
>> Index: gdb/exec.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/exec.c,v
>> retrieving revision 1.90
>> diff -u -p -r1.90 exec.c
>> --- gdb/exec.c 2 Jul 2009 17:21:06 -0000 1.90
>> +++ gdb/exec.c 28 Jul 2009 14:58:16 -0000
>> @@ -381,6 +381,7 @@ add_to_section_table (bfd *abfd, struct
>> struct target_section **table_pp = (struct
>> target_section **) table_pp_char;
>> flagword aflag;
>>
>> + (*table_pp)->bfd = abfd;
>> /* Check the section flags, but do not discard
>> zero-length sections, since
>> some symbols may still be attached to this section.
>> For instance, we
>> encountered on sparc-solaris 2.10 a shared library
>> with an empty .bss
>> @@ -390,7 +391,6 @@ add_to_section_table (bfd *abfd, struct
>> if (!(aflag & SEC_ALLOC))
>> return;
>>
>> - (*table_pp)->bfd = abfd;
>> (*table_pp)->the_bfd_section = asect;
>> (*table_pp)->addr = bfd_section_vma (abfd, asect);
>> (*table_pp)->endaddr = (*table_pp)->addr +
>> bfd_section_size (abfd, asect);
>
> Still not right, because that's still papering over the problem, and
> still hitting undefined behaviour, in the case of the
> table ending up being empty. When table->sections == table->sections_end,
> we should never dereference table->sections, like in 'table->sections->bfd'.
> Notice how the end pointer isn't advanced if !SEC_ALLOC.
> Consider the fact that we usually xmalloc enough memory that
> makes that not misbehave a coincidence. E.g., see that with
> a resize_section_table call, you can end up with table->sections == NULL,
> and table->section_end == NULL, which is legal, because table->sections
> is still equal to table->sections_end in that case (meaning empty table).
>
> There used to be an extra structure in bfd_target.c to hold
> the sections and the bfd, which didn't look necessary when I
> reimplemented bfd-target.c on top of exec.c's functions, but
> it is clear now that we need it anyway. Here's a patch that
> brings something like that back.
>
> Could you try it? Thanks.
>

I tried it, it works.

Thanks,
--
Aleksandar Ristovski
QNX Software Systems