Broken Code in gas/symbols.c

Broken Code in gas/symbols.c

[Ravi]

Hi,

I was going over some of the binutils 2.15 code when I came across the 
following code :

<snip: gas/symbols.c>

void
symbol_mark_used_in_reloc (symbolS *s)
{
    if (LOCAL_SYMBOL_CHECK (s))
        s = local_symbol_convert ((struct local_symbol *) s);
    s->sy_used_in_reloc = 1;
}

</snip: gas/symbols.c>

Now local_symbol_convert creates a 'struct symbol' from a 'struct 
local_symbol' by doing a malloc. Since the function takes the pointer to 
a 'struct symbol', the newly created 'struct local symbol' does not get 
returned to the calling function.

The two options to make the newly created 'struct local symbol' 
available to the caller is either returning the value of the 'struct 
local symbol' or passing the 'struct symbol' by reference. The former 
would tend to break the consistency between all the functions defined in 
symbols.c as some of the functions already return values of their own. 
This leaves us with the second option of passing the 'struct symbol' by 
reference for all the functions in gas/symbols.c. For example the 
function symbol_mark_used_in_reloc would be changed to:

</snip: gas/symbols.h>

extern void symbol_mark_used_in_reloc (symbolS **);

</snip: gas/symbols.h>



<snip: gas/symbols.c>

void
symbol_mark_used_in_reloc (symbolS **s)
{
    symbolS *t = *s;

    if (LOCAL_SYMBOL_CHECK (t))
        t = local_symbol_convert ((struct local_symbol *) t);
    t->sy_used_in_reloc = 1;
    *s = t;
}

</snip: gas/symbols.c>

When I built the 2.15 sources for an arc-elf32 target I got the 
following problem:

<snip>

ravi@firebolt:/overflow/crap/tests$ cat test.s

.section .rodata.str, "aMS", @progbits, 1
.LC3: .string "main"
.section .text
mov r0, .LC3+1



ravi@firebolt:/overflow/crap/tests$ 
/overflow/crap/install/bin/arc-elf32-as test.s
Segmentation fault
ravi@firebolt:/overflow/crap/tests$

</snip>

My solution seems to solve this problem. Is this the best way of going 
about it though ?



Regards,
Ravi Ramaseshan.

Re: Broken Code in gas/symbols.c

[Alan Modra]

On Thu, Sep 30, 2004 at 07:27:40PM +0530, Ravi wrote:
> Now local_symbol_convert creates a 'struct symbol' from a 'struct 
> local_symbol' by doing a malloc. Since the function takes the pointer to 
> a 'struct symbol', the newly created 'struct local symbol' does not get 
> returned to the calling function.

You are not describing local_symbol_convert accurately.  When converted,
your original "struct local_symbol" is modified to contain a pointer to
the newly malloc'd "struct symbol".  I don't think there is any need to
modify any of the functions in gas/symbols.c.  More likely, you have hit
some code that accesses the fields of "struct symbol" directly rather
than going via accessor functions like symbol_get_bfdsym().

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: Broken Code in gas/symbols.c

[Ravi]

Hi Alan,

>You are not describing local_symbol_convert accurately.  When converted,
>your original "struct local_symbol" is modified to contain a pointer to
>the newly malloc'd "struct symbol".  I don't think there is any need to
>modify any of the functions in gas/symbols.c.  More likely, you have hit
>some code that accesses the fields of "struct symbol" directly rather
>than going via accessor functions like symbol_get_bfdsym().
>  
>
Thanks for the correction. My only concern was the real symbol not 
getting returned to the caller function. However as you pointed out, it 
does through local_symbol -> u .lsy_sym.I hadn't noticed the real symbol 
being stored in local_symbol -> u .lsy_sym. The problem then was that 
while the arc backend was generating a reloc using tc_gen_reloc the 
access to bsym was made directly. bsym would be NULL for a local symbol. 
This would then get stored in the reloc entry created and hence caused 
the seg fault in bfd_install_relocation. The patch below corrects this 
problem in tc-arc.c .

Ok to apply ?

regards
Ravi


ChangeLog:
2004-10-01 Ravi Ramaseshan (ravi.ramaseshan@codito.com)

* gas/config/tc-arc.c :
        tc_gen_reloc : Use symbol_get_bfdsym


Index: tc-arc.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-arc.c,v
retrieving revision 1.27
diff -a -u -r1.27 tc-arc.c
--- tc-arc.c    6 May 2004 11:01:48 -0000       1.27
+++ tc-arc.c    1 Oct 2004 05:40:43 -0000
@@ -1978,8 +1978,9 @@
   arelent *reloc;

   reloc = (arelent *) xmalloc (sizeof (arelent));
+  reloc->sym_ptr_ptr = (asymbol **) xmalloc (sizeof (asymbol *));

-  reloc->sym_ptr_ptr = &fixP->fx_addsy->bsym;
+  *reloc->sym_ptr_ptr = symbol_get_bfdsym(fixP->fx_addsy);
   reloc->address = fixP->fx_frag->fr_address + fixP->fx_where;
   reloc->howto = bfd_reloc_type_lookup (stdoutput, fixP->fx_r_type);
   if (reloc->howto == (reloc_howto_type *) NULL)

Re: Broken Code in gas/symbols.c

[Alan Modra]

On Fri, Oct 01, 2004 at 12:54:32PM +0530, Ravi wrote:
> 2004-10-01 Ravi Ramaseshan (ravi.ramaseshan@codito.com)
> 
> * gas/config/tc-arc.c :
>        tc_gen_reloc : Use symbol_get_bfdsym

Thanks, this is the right patch.  I applied this for you, making a minor
formatting fix.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre