Re: [patch] Set bfd field in target_section

[Aleksandar Ristovski <aristovski@qnx.com>]

Pedro Alves wrote:
> On Tuesday 28 July 2009 15:45:25, Aleksandar Ristovski wrote:
>> Pedro Alves wrote:
>>> On Tuesday 28 July 2009 15:28:33, Aleksandar Ristovski wrote:
>>>> Hello,
>>>>
>>>> I believe this is related to Pedro's patch from 03-Jun-09. I 
>>>> didn't see where we set target_section.bfd field - maybe I 
>>>> am overlooking something, but in bfd-target, in function 
>>>> target_bfd_xclose we will call bfd_close 
>>>> (table->sections->bfd); bfd_close doesn't like NULL argument.
>>>>
>>>> Am I missing something, or is this (the patch) missing?
>>> Doesn't add_to_section_table set the bfd in each new
>>> target section?
>> Indeed it does. However, the problem is if we don't find any 
>> sections in a bfd, it will exit and will leave bfd field 0.
> 
> Right, but table->sections will be equal to table->sections_end,
> meaning the table is empty.  Your fix isn't correct, since you
> should never write to *sections_end, which is one-past-the-end
> of the sections in the table.  In the degenerate case of
> bfd_count_sections == 0 (not 0 ALLOC sections), xmalloc will
> still return something non-NULL, but, writing to this pointer
> invokes undefined behaviour.
> 
>> I get this situation at the moment because I broke my 
>> xfer_partial, but I think it could happen in general?
> 
> Testing finished succesfully, so I've applied the patch
> with this ChangeLog entry.  Let me know if something is still wrong.
> 
> 2009-07-28  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* bfd-target.c (target_bfd_xclose): Only close the bfd if the
> 	section table is not empty.
> 	(target_bfd_reopen): If the section table ends up empty, close the
> 	bfd here.
> 

I think now you broke it in a different way. Now we can end 
up trying to read from a closed bfd.


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);



-- 
Aleksandar Ristovski
QNX Software Systems