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

Wed Feb 5 13:03:30 PST 2020

psmith added a comment.

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

> https://bugs.chromium.org/p/chromium/issues/detail?id=1047531#c19 anc #c20 suggest that no assembly is involved and that this instead related to jumps to __attribute__((weak)) functions.

Thanks for the details. I've just got home, sorry it is getting late in UK. It looks like the linker was outputting BL and is now outputting BLX. If I'm reading the diff right then - is bad and + is good

  -  11d3e2:	f12c fe8d 	bl	24a100 <linker_script_end_of_text+0x828>
  +  11d3e2:	f12c ee8e 	blx	24a100 <linker_script_end_of_text+0x828>

Given the PLT is always in ARM state then BLX is what I would expect.
The key line seems to be:

  bool interwork = (rel.sym && rel.sym->isFunc()) || rel.type == R_PLT_PC;
  if (interwork ? (val & 1) == 0 : isBlx) {

It looks like interwork is coming out false for weak symbols.

Just seen your latest message, thanks for the reproducer, it seems to match. I will try and fix as soon as possible. Given my schedule over the next few days (catching a flight tomorrow afternoon) I suggest reverting. It is likely a quick fix, but I'd prefer not to it late at night. I'll submit a new patch for review as soon as I can.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73542