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

[Peter Smith via Phabricator via llvm-commits]

psmith added a comment.

> I asked because I wonder whether a warning is the right approach. GNU ld does not warn. If the projects that are affected is just Chromium, do we still need a warning? It seems @thakis has already fixed the bug.
>  A warning can help users catch bugs, but it can also make a user with a legitimate use case frustrated.

Reasons for a warning

- We are making a change in behavior to match bfd, projects only linked with gold or LLD for some portion of time could suffer runtime errors.
- As thakis points out, Chromium has extensive testing, bisection to find problems, and someone like thakis to report and fix, not all projects will be so well served.
- The author of the problem in Chromium didn't know that they needed to set the type of the symbol to STT_FUNC, I think that this situation is common.
- We can limit the warning to cases where LLD would change behaviour, hence catch problems due to this change.
- All compiled code will be correctly typed, it is only assembly language we need to worry about.

Reasons for not warning

- There are false positives, which in one corner case (bl to section symbol + offset) could be difficult to resolve.
- It makes the code more complicated

At the moment I'd prefer a warning. My suggestion for how to proceed:

- Try with a warning, see if there are false positives reported
- If no then keep it, if yes then write a more complex check using the mapping symbols


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

https://reviews.llvm.org/D73542