[PATCH] D147544: [BOLT] Move from RuntimeDyld to JITLink

[Job Noorman via Phabricator via llvm-commits]

jobnoorman added inline comments.


================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:1665
+    // symbol with value 0x0 and set addend to the absolute value.
+    auto *ZeroSym = BC->getOrCreateGlobalSymbol(0, "ZEROat");
+    RelocatedEHFrameSection->addRelocation(Offset, ZeroSym, RelType, Value);
----------------
rafauler wrote:
> I was digging a bit into the problem of bad eh_frames for HHVM and discovered this is the problem. HHVM has some symbols that are initially set to zero but later BOLT will relocate them. These are special hot_start/hot_end markers that delimit the location of hot code in case your application really wants to know the location of hot functions (older versions of HHVM were doing self huge page allocation based on that) . So these symbols are registered at address zero in BinaryContext, but we will move them later.  When you create a reloc here that points to a MCSymbol returned by BinaryContext::getOrCreateGlobalSymbol(), BinaryContext checks its internal maps, realizes it already has a symbol registered at address zero, and then returns to you "hot_end", which is unfortunately a special symbol that will not evaluate to zero after binary rewriting is complete.
> 
> If you really want a symbol that always evaluates to zero and will never be relocated, I guess you will need to create a special MCSymbol whenever we initialize BinaryContext and return that in a special getter, and then adapt our symbol resolver to, whenever the linker wants the address of the special zero marker, set it to zero.  Maybe there are better solutions, but that's all I can think of at the moment without digging deeper, unless you can make the absolute relocs work in JITLink.
I'm not entirely sure what changed in JITLink but this workaround doesn't seem necessary anymore so I removed it. Could you check if this solves your HHVM issue?

Note that for some reason, some sections changed order when using RuntimeDyld so I had to update the simulation patch to make sure `llvm-bolt-wrapper.py` passes on all tests: {F27527270}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147544