[PATCH v1] arm: relocate: Replace ADR instruction with non-pseudo-instruction

[Jesse Taube]

On 7/11/22 08:57, Andre Przywara wrote:
> On Sun, 10 Jul 2022 03:09:53 -0400
> Jesse Taube <mr.bossman075 at gmail.com> wrote:
> 
> Hi Jesse,
> 
>> In Binutils 2.37 the ADR instruction has changed
>> use alternate instructions.
> 
> Can you elaborate on this? What has changed exactly, and why? Looking at
> the commit you mention below I don't see an immediate problem that would
> require code changes? Also it speaks of forward references, but this one
> is not one?
> And I didn't spot any difference between 2.38 and 2.35, at least not in my
> isolated test (but I didn't bother to compile a whole stage 1 GCC with
> newer binutils yet).
> 
>> The change causes armv7-m to not boot.
> 
> What does "causes armv7-m to not boot" mean? It compiles fine, but hangs
> or crashes?
well when we reallocate it doesn't copy the whole instruction
so its an invalid instruction and boot loops.
> Can you show the relevant disassembly from both binutils versions?
> 
> And from trying to reproduce this minimally, do we need a ".syntax unified"
> in the .S file? 
Yes its required. sorry i didn't post the test code here...
The test code is as follows
```
.syntax unified
.global bug;
.align 4
bug:

         adr     r3, bug
.size bug, .-bug
.type bug 2; // This changes offset from 4 to 3 in
include/linux/linkage.h:ENDPROC
//arm-linux-gnueabi-as  -march=armv7-m -c -o bug.o bug.S &&
arm-linux-gnueabi-objdump --disassemble=bug bug.o
```
> 
>> Signed-off-by: Jesse Taube <Mr.Bossman075 at gmail.com>
>> ---
>>  arch/arm/lib/relocate.S | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S
>> index 14b7f61c1a..22c419534f 100644
>> --- a/arch/arm/lib/relocate.S
>> +++ b/arch/arm/lib/relocate.S
>> @@ -78,7 +78,13 @@ ENDPROC(relocate_vectors)
>>   */
>>  
>>  ENTRY(relocate_code)
>> -	adr	r3, relocate_code
>> +/*
>> + * Binutils doesn't comply with the arm docs for adr in thumb2
>> + * from commit d3e52e120b68bf19552743fbc078e0a759f48cb7 onward
>> + * to remove ambiguity explicitly define the pseudo-instruction
>> + */
>> +	mov r3, pc
>> +	subs r3, #4
> 
> But this will break ARM, won't it? Because it would require to subtract #8?
> I mean there is a reason for this adr instruction, because this offset
> calculation is best left to the assembler. Not to speak of the fragility
> of assuming that the relocate_code label is pointing to the very first
> instruction. The ENTRY macro could also insert instructions.
> 
> Cheers,
> Andre
> 
>>  	ldr	r1, _image_copy_start_ofs
>>  	add	r1, r3			/* r1 <- Run &__image_copy_start */
>>  	subs	r4, r0, r1		/* r4 <- Run to copy offset      */
>