From: Richard Sandiford <rdsandiford@googlemail.com>
To: "Maciej W. Rozycki" <macro@codesourcery.com>
Cc: Catherine Moore <clm@codesourcery.com>, binutils@sourceware.org
Subject: Re: [PATCH 4.0/4 v2] MIPS/GAS: Propagate symbol attributes
Date: Sat, 30 Oct 2010 10:37:00 -0000 [thread overview]
Message-ID: <87y69gm0ex.fsf@firetop.home> (raw)
In-Reply-To: <alpine.DEB.1.10.1010291526390.25860@tp.orcam.me.uk> (Maciej W. Rozycki's message of "Fri, 29 Oct 2010 15:39:53 +0100 (BST)")
"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
next prev parent reply other threads:[~2010-10-30 10:37 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-26 10:48 [PATCH 4/4] " Maciej W. Rozycki
2010-10-29 14:40 ` [PATCH 4.0/4 v2] " Maciej W. Rozycki
2010-10-30 10:37 ` Richard Sandiford [this message]
2010-12-08 19:24 ` [PATCH 4.0/4 v3] " Maciej W. Rozycki
2010-12-09 17:25 ` Richard Sandiford
2010-12-09 18:02 ` Maciej W. Rozycki
2010-12-11 0:09 ` [PATCH 4.0/4 v4] MIPS/GAS/testsuite: Branch to self/label tests Maciej W. Rozycki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y69gm0ex.fsf@firetop.home \
--to=rdsandiford@googlemail.com \
--cc=binutils@sourceware.org \
--cc=clm@codesourcery.com \
--cc=macro@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).