Re: Fix assembly of Thumb pcrel LDRs against global symbols

[Dave Martin <dave.martin@linaro.org>]

On Wed, Mar 2, 2011 at 4:45 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>
> On Wed, 2011-03-02 at 16:37 +0000, Dave Martin wrote:
>> On Wed, Mar 2, 2011 at 3:18 PM, Richard Earnshaw <rearnsha@arm.com> wrote:
>> >
>> > On Mon, 2011-02-28 at 12:59 +0000, Richard Sandiford wrote:
>> >> The PC-relative LDR instructions have no associated relocation,
>> >> so even LDRs for global symbols should be resolved by the assembler.
>> >> We currently handle this correctly for single-register ARM loads,
>> >> but we're missing the associated relocation types for LDRD and Thumb.
>> >> This leads to errors like:
>> >
>> > I'm not sure I agree with this.  If I write
>> >
>> > .global foo
>> >
>> > ...
>> >        ldr r0, foo
>> >
>> > ...
>> >
>> > foo:
>> >        ...
>> >
>> > but then at link/load time pre-empt foo with some other definition, that
>> > will silently leave me with the wrong answer.
>> >
>> > It's clearly OK to do this if foo is protected or private, but not
>> > otherwise.
>> >
>> > The existing error message is not very informative as to what the
>> > problem is, but I'm not sure I agree with your solution...
>> >
>> > R.
>>
>> That's a reasonable argument, but similarly to the ADR case, such code
>> referencing global symbols is already allowed through for ARM _with no
>> relocation emitted_  - the Thumb treatment is inconsistent and causes
>> existing code which builds for ARM (but where symbol preemption may
>> silently fail to work).  This raises non-trivial questions about which
>> behaviour should be considered correct.
>>
>> Unless there are some tricks I haven't thought of, it seems that ADR
>> and PC-relative LDR and friends simply can't be generally preemptible,
>> period -- because the relocation ranges these instructions support
>> just aren't large enough to guarantee that a relocation on these
>> instructions can ever be resolved except in a trivial subset of cases.
>>
>> Solving this inconsistency requires a conclusion regarding whether the
>> tools should trust the programmer to write correct code (i.e., fix up
>> pc-relative LDR and ADR in the assembler and emit no relocation) or
>> not (i.e., treat these instructions as an error when they reference
>> global symbols).
>>
>> I would regard the latter option as incorrect, since it's quite
>> legitimate to use these instructions in this way in static
>> environments, and the assembler doesn't have knowledge of whether this
>> is what's intended.  There is also a possible middle way of allowing
>> the instructions but always emitting a relocation, which allows a tool
>> further down the line to check whether the preemption is needed and
>> throw an error if so.  Since the Linux kernel doesn't do symbol
>> preemption it can safely ignore these relocations with a tiny amount
>> of extra code in the module loader.
>>
>> In general, we can't expect to prevent absolutely the programmer from
>> doing something that generates the wrong answer, such as writing
>> non-PIC code and the linking it into a PIC environment; though any
>> diagnostics which help avoid this happening by accident are still
>> useful.
>>
>>
>> To give the flipside to this, there is a case when the assembler knows
>> that preemption absolutely must be supported, and that is when a
>> symbol is declared weak.  So:
>>
>> .type g, %function
>> g:    ldr     r0, d
>>
>>       .weak d
>> d:    .long 0
>>
>> $ arm-linux-gnueabi-as --version
>> GNU assembler (GNU Binutils for Ubuntu) 2.20.51.20100908
>> [...]
>> $ arm-linux-gnueabi-as -o tst.o tst.s && arm-linux-gnueabi-objdump -drt tst.o
>> tst.s: Assembler messages:
>> tst.s:2: Error: internal_relocation (type: OFFSET_IMM) not fixed up
>> $ arm-linux-gnueabi-as -mthumb -o tst.o tst.s &&
>> arm-linux-gnueabi-objdump -drt tst.o
>> tst.s: Assembler messages:
>> tst.s:2: Error: invalid offset, value too big (0xFFFFFFFFFFFFFFFC)
>>
>> i.e., cryptic error messages aside, the assembler fails sensibly for
>> ARM and Thumb alike.  This is appropriate, because ".weak" declares
>> explicitly the need for preemptibility, and the assembler knows that
>> can't be guaranteed with these instruction forms.  ".globl" does not,
>> because where the symbol is not weak it's impossible for the assembler
>> to know what the programmer intends, and so throwing an error for such
>> cases seems too zealous from my point of view.
>>
>> For comparison, changing ".weak" to ".globl" in the above example gives:
>> $ arm-linux-gnueabi-as -o tst.o tst.s && arm-linux-gnueabi-objdump -drt tst.o
>> [...]
>> 00000004 g       .text        00000000 d
>> [...]
>> 00000000 <f>:
>>    0: e51f0004        ldr     r0, [pc, #-4]   ; 4 <d>
>> 00000004 <d>:
>>    4: 00000000        .word   0x00000000
>>
>> $ arm-linux-gnueabi-as -mthumb -o tst.o tst.s &&
>> arm-linux-gnueabi-objdump -drt tst.o
>> tst.s: Assembler messages:
>> tst.s:2: Error: invalid offset, value too big (0xFFFFFFFFFFFFFFFC)
>>
>> Notice that the ARM code refernce to d is not preemptible.
>>
>
> It's easy for the user to express their intention, simply use
> a .visibility (protected/hidden) directive (at least in ELF, and that's
> what I really care about...).

Can non-weak symbol references which resolve within the static partion
of the link (i.e., the main program in dynamic environments) be
preempted?  I thought the answer was "no", hence the need for things
like R_ARM_COPY.  If so that suggests that making all symbols in the
static portion hidden shouldn't be needed, and using the affected
instruction forms on global symbols should be legitimate -- but the
suggested behaviour contradicts this.

My assumption may be wrong here, though.

>
> Then the linker will (can be safely made to) permit the early resolution
> of the symbol do the right thing, and will also do the right thing if
> the user then accidentally tries to override the symbol later on.
>
> The tools *can* correctly support the behaviours being asked for.  The
> implementation is currently wrong.  The fact that there is broken code
> out there that relies on the currently broken implementation is not
> really an excuse for keeping the status quo.

Fair point.  Just to be clear, the current implementation may
therefore be considered broken in either one or two ways: take your
pick from:

a) For ARM code, non-preemptible ADR and LDRs to preemptible symbols
are fixed up locally and emitted as-is, with no relocation.
b) For Thumb code, ADR and LDR to preemptible symbols both cause the
assembler to throw an error, even if the symbol is explicitly declared
non-preemptible (e.g., .hidden or similar).

Cheers
---Dave