Re: [PATCH 4.0/4 v2] MIPS/GAS: Propagate symbol attributes

[Richard Sandiford <rdsandiford at googlemail dot com>]

"Maciej W. Rozycki" <macro@codesourcery.com> writes:
>  There are several cases where ELF symbol attributes are not correctly set 
> for symbols leading to all kinds of odd side effects for MIPS16 code (and 
> with the upcoming change also for microMIPS code).
>
>  For example this program:
>
> 	.text
> foo:
> 	xor	$16, $17
> 	.set	fnord, . + 2
> 	addu	$2, $3, $4
> 	xor	$5, $6
> bar:
> 	subu	$7, $16
>
> assembles to this:
>
> Disassembly of section .text:
>
> 00000000 <foo>:
>    0:	e82e     	xor	s0,s1
>    2:	e389     	addu	v0,v1,a0
>
> 00000004 <fnord>:
>    4:	edcee71f 	swc3	$14,-6369(t6)
>
> 00000006 <bar>:
>    6:	e71f     	subu	a3,s0
>
> Notice how fnord's ELF attributes have not been set to indicate a MIPS16 
> symbol and the resulting confusion (this symbol, of course, if used as a 
> jump target will not set the ISA bit correctly).  Similar symptoms are 
> seen with equated symbols (defined with .eqv) although the attributes are 
> lost at a different stage of assembly.
>
>  Here's a fix for these problems and a test case covering hopefully most 
> of them as well as those fixed by patches submitted previously in this series.
>
>  There are two new functions defined:
>
> - mips_elf_copy_symbol_attributes() -- used for symbols defined with .set.  
>   These, if calculated from a label that has been defined for the current 
>   location (the "dot" special symbol being a prominent example, but any 
>   label will actually do) will not have their ELF attributes set, because 
>   the original label will only get them set once an instruction has been 
>   emitted.  A solution is to place the newly defined symbol on the list of 
>   labels too.
>
> - mips_elf_propagate_symbol_attributes() -- used for symbols defined with 
>   .eqv.  They are processed late and currently attributes are copied only 
>   for exact copies of other symbols, i.e.:
>
> 	.eqv	foo, bar
>
>   but not:
>
> 	.eqv	foo, bar + 2
>
>   In the case of MIPS16 (and microMIPS) ELF attribute we want it to be 
>   propagated even for offsetted symbols, hence the new hook in 
>   resolve_symbol_value().
>
>  Finally I realised the uniqueness of the -mmips:16 option to `objdump' 
> that causes all the disassembled to be treated as MIPS16 code, regardless 
> of symbol annotations found.  This actually covers the bugs addressed 
> here, hence I'm removing it.  The implication is all MIPS16 tests need to 
> have a label at the beginning to set the ISA mode of the disassembler 
> correctly.
>
>  Perhaps the behaviour of the -mmips:16 option should be changed instead 
> so that it respects symbol annotations.  I think it would make sense -- 
> the option should normally only be needed for binary objects with no 
> sufficient symbol information.  OTOH, the current behaviour is good in 
> case `objdump' gets confused because of a bug or a broken binary, hence I 
> have no strong preference actually towards making the change.
>
>  If we agree to make it after all, then the option can be added back.  I 
> believe -mmips:micromips behaves the same.  Additionally odd text symbols 
> with no ELF annotation are unconditionally treated as MIPS16 code.  This 
> should probably be changed.
>
>  Compared to the original version, this change has only been trivially 
> updated to take the new treatment of symbols equated to an expression 
> involving "." into account.  The update is limited to the test case 
> according to the comment I'll just repeat here: "Move the location counter 
> away from the end of code to avoid the final values of symbols equated to 
> expressions involving the counter interfering with disassembly."  Without 
> this change MIPS16 code would be disassembled incorrectly as the equated 
> symbols have the MIPS16 attribute clear (quite correctly, because the 
> final value of "." does not point to MIPS16 code).  Dumps have been 
> adjusted accordingly.

Sorry for not really getting to this patch when you posted the original
series.  Do you actually have a "real world" use case for this though?
Why wouldn't you just put "fnord:" in the appropriate place?

I'm not certain that, in general, we can say that "mips16 symbol + offset"
should always be a mips16 symbol.

Richard