[PATCH] D128082: [BOLT][AArch64] Handle gold linker veneers

[Rafael Auler via Phabricator via llvm-commits]

rafauler added inline comments.


================
Comment at: bolt/lib/Core/BinaryContext.cpp:1141
+
+bool BinaryContext::tryToHandleAArch64Veneer(uint64_t Address, bool MatchOnly) {
+  BinaryFunction *TargetFunction = getBinaryFunctionContainingAddress(Address);
----------------
It's curious to see that we arrive at disassembly stage without having mapped this address to a given function. but I guess this might happen in this scenario if there are no symbols to cover the veneer region.

This function serves a dual purpose:  both matching a veneer and asserting that there is a veneer at a given address, but also building a new binary function.

I would preferrably avoid functions that have a boolean param and try to break it in two:

1. BC::matchAArch64Veneer()
2. BC::createAArch64VeneerFunction()

And then try to write the code in a way that 2 calls 1, and the BF::disassemble() function calls 1, and RI::disassembleFunctions() calls 2. The "tryToHandle" name prefix here is unnecessary since we can just assume that if we have "bool" as the return type, it might fail because it returns false for some instances, so the naming becomes a bit redundant.

If breaking it up in two functions is too awkward, then I think we can at least rename this to "handleAArch64Veneer", but in that case make the comment in the header file more explicit by saying that this function serves both to match a veneer and, if MatchOnly is false, to create a new function to hold the unmapped veneer instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128082