[Patch mach-o, indirect symbols 1/2] support indirect symbols in mach-o.

[Iain Sandoe]

Hi Tristan,

I've amended and applied (together with its tests) with the additional  
comments as below.

Note that I updated the (c) years in the altered files, but I did NOT  
rotate the ChangeLogs.

Iain

On 2 Jan 2012, at 11:13, Tristan Gingold wrote:
>
> Ok with the suggested changes (mostly stylistic).  Bracketed points  
> are not required to be addressed for the commit.
>> +
>> +      if ((s->n_type & BFD_MACH_O_N_TYPE) == BFD_MACH_O_N_INDR)
>
> [ Maybe we should create a macro to test for indirect symbols ]

done.

>> +	/* A pointer to the referenced symbol will be stored in the udata
>> +	   field.  Use that to find the string index.  */
>> +	s->symbol.value =
>> +	    ((bfd_mach_o_asymbol *)s->symbol.udata.p)->symbol.udata.i;
>
> [ One issue is that we don't follow the BFD convention for indirect  
> symbols.  To be fixed later ]

that's a tricky one - we need the 'normal' symbols ahead of the  
indirects (so we'd have to swap in and out).

> [ One question: does it make sense to output the indirect name here  
> and therefore avoiding taking care of indirect symbols while  
> sorting ? ]

I am not sure I follow the question - we are constrained to do things  
in a particular order by needing certain dependent information.

to construct and order the symtab - we need the sections.
to construct the indirect symbol dependent indices - we need the  
symbols sorted.

to construct the indirect symbol *value* we need the string table  
offset for the dependent symbol (which is only available when the  
symbol table is written).  Note that this is academic when we write a  
dysymtab - since we don't then write 'bare' indirect symbols.  If we  
do not write the dysymtab - then the 'normal' symbols precede the  
indirect ones which means that we get the *value* just in time.

>> +            }
>
> Please don't use the raw buffer buf.  Instead use  
> mach_o_dylib_module[_64]_external.  I know it is boring because of  
> 32/64.

done.

>> +
>> +	  bfd_h_put_32 (abfd, cmd->indirect_syms[i], &raw);
>
> [ Ditto (although might be considered as too trivial) ]

left as is, because there is not an entry in external.h for it...

>> +          bfd_h_put_32 (abfd, v, raw);
>
> [ Ditto ]

likewise.

>> +  if (type == BFD_MACH_O_N_INDR) return 3;
>
> GNU style issue: not on the same line!

sorry, dunno what I was thinking of here...

>> +  qsort (symbols, (size_t) nin, sizeof (void *),  
>> bfd_mach_o_cf_symbols);
>
> [ Unfortunately qsort is not stable! ]

indeed, which is why there's a specific sort decider for each type -  
if you believe I've missed one (or that it's not adequate)  then I  
guess we could switch to mergesort.

===
applied patch attached FTR.

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 12010311-write-dysymtab.txt
URL: <https://sourceware.org/pipermail/binutils/attachments/20120103/6addbf4d/attachment.txt>
-------------- next part --------------