[PATCH] D73542: [LLD][ELF][ARM] Do not substitute BL/BLX for non STT_FUNC symbols.

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

In D73542#1860211 <https://reviews.llvm.org/D73542#1860211>, @thakis wrote:

> Hello, this causes a bunch of chromium binaries to crash during startup: https://bugs.chromium.org/p/chromium/issues/detail?id=1047531
>
> Looks like most of them contain assembly code. Do you have any idea from this fuzzy description what might be up? What's data that I can collect that's useful for debugging?


The crash will be an illegal instruction because there was a state change from Arm to Thumb or vice-versa expected. This change makes LLD stricter when it comes to complying with the ABI for the Arm architecture. For debugging what we'd need to know is the assembly compliant with the ABI and I've got this patch wrong. Or is the assembly non-compliant and this increase in strictness triggered a problem.

The specific change is that the linker should only "interwork" which in this patch means changing a BL to a BLX when source and target are in different state for symbols with type STT_FUNC, and a BLX to bl if source and target are the same state.

To give an example:

          .syntax unified
          .global arm1
          .global arm2
          .global thumb1
          .global thumb2
  
          // uncomment these to get blx from Arm to Thumb and vice versa
          // .type arm1, %function
          // .type arm2, %function
          // .type thumb1, %function
          // .type thumb2, %function
          
          .section .text.01, "ax", %progbits
          .arm
  arm1:
          bl arm2
          bl thumb1
  
          .section .text.02, "ax", %progbits
          .arm
  arm2:   bx lr
  
          .section .text.03, "ax", %progbits
          .thumb
  thumb1:
          bl arm1
          bl thumb2
          bx lr
          .section .text.04, "ax", %progbits
          .thumb
  thumb2: 
          bx lr

In this example all the symbols have type STT_NOTYPE so the linker does not interwork and does not change bl to blx. When the .type lines are uncommented two of the bl instructions (thumb to arm, and arm to thumb) are changed to blx.

At least in this example ld.bfd and (with this patch) ld.lld agree.

What I'd suggest is:
The crash will be an illegal instruction, this instruction will be the target of a BL and is highly likely that in a non-stripped binary there will be a symbol at that address. What is the type of that symbol? If it is not STT_FUNC then the assembly code needs to add .type <symbol name>, %function.

I had hoped that as I was bringing LLD inline with the GNU linker there wouldn't be a problem in tightening up LLD to the specification. There is a chance that code that has only been linked with LLD, and unfortunately, it looks like ld.gold, will expect interworking for all symbol types. If this is going to be a giant legacy problem, then I think we may need a strict switch or potentially a warning.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73542/new/

https://reviews.llvm.org/D73542