Re: ARM mapping symbols

[Bruno De Bus <bdebus at elis dot ugent dot be>]

On Mon, 8 Mar 2004, Nick Clifton wrote:

> Hi Bruno,
> > > Thank you very much for submitting this patch.  There is one initial
> > > problem however: do you have a copyright assignment on file with the
> > > FSF ?  Without such an assignment we could not accept the patch :-(
> > 
> > Is this really necessary? The patch is very small (I add a parameter to 
> > the mapping_state function, change all call sites (11 lines) and 
> > change one check in mapping state to check the extra parameter (1 line)), 
> > so I believe this would fall under "not legally significant for 
> > copyright".
> 
> Well it is a judgement call really.  Providing that the patch does not
> get too big then it can be considered to be "obvious" and so not need
> a copyright assignment.  I do like to encourage people to obtain
> copyright assignments however as we always hope that they contribute
> further patches in the future.
> 
> > If it is, please give me some more info on this copyright assignment.  
> 
> If you are interested, please fill out and send off the attached form
> to start the ball rolling.

I will look into it...
> 
> >>   * Why is the forced argument needed for the mapping_state() function
> >>     ?  Surely if the last mapping symbol emitted was a data symbol,
> >>     then there is no need to emit a new one ?
> >
> > That would be the case if the object was assembled from top to bottom, 
> > which is not the case for the literal pools. As the literal pools 
> > need to be appended after a function return or an unconditional jump they 
> > cannot be assembled until the rest of the code is assembled. Although this 
> > could probably be handled differently, at the moment the creation of the 
> > table is even deferred until all sections are assembled. 
> 
> Actually not quite true.  It is the assembly code writer's
> responsibility to insert .ltorg directives into their code at suitable
> points so that the pools can be generated.  The code in arm_cleanup()
> is that to catch the case where a programmer forgets to insert these
> directives.  It makes sure that any left over pools are dumped before
> the assembler starts to resolve references to their contents.

Ok, I did not know that... Thought the assembler figured this out on his 
own. (We do this automated in our link-time optimizer).

> 
> > So, what is the problem:
> >
> > The last assembled section is often a data section, so the mapping
> > symbol is $d. Then arm_cleanup is called and the literal pool(s)
> > is(/are) written. As the last symbol already was $d, no new symbol
> > is added... 
> 
> Ok - but this is easily fixed by calling arm_elf_change_section()
> after calling subseg_set() in arm_cleanup().  Then the correct-for-
> that-section mapping symbol will be emitted, which can then be changed
> by your patch to s_ltorg().

I might be wrong again, but I believe this will add both a $d and a 
$a symbol? 
 
> [It occurs to me though that the correct thing to do is to change the
>  mapping symbol code so that the current symbol is maintained on a
>  *pre-section* basis and not just globally.  Since otherwise section
>  changes in the assembler source could introduce spurious mapping
>  symbols].
> 
> >> Which files ?  [Clean fixes are always preferred to quick hacks :-)]
> >
> > It would also be possible to change subseg_set to find out when we jump 
> > from one part of the file to another. This would probably be cleaner....
> 
> Agreed.  At first I thought that subset_set() did this, but upon
> checking the code I found out that you were correct.

> Still I think a per-section mapping symbol state maintained in the ARM
> backend would also work and it would be less intrusive into the
> generic GAS code.  (You could define TC_SEGMENT_INFO_TYPE and hang the
> information off there).

That is probably the best solution... I you want I can look into this, as 
this information is very important for us, but I'm not familiar with 
GAS/binutils code, so it might take me a while.... 

Thanks,

Bruno

> Cheers
>         Nick
> 
>