Re: [patch] Set bfd field in target_section

[Aleksandar Ristovski <aristovski@qnx.com>]

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