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

[Rafael Auler via Phabricator via llvm-commits]

rafauler added a comment.

I'm currently investigating why this diff causes BOLT to output different eh_frame sections. It's very important for BOLT to correctly support these. I don't have all the answers yet, but I did notice a few weird things I'll comment in the code, so you can help me understand what's happening.



================
Comment at: bolt/lib/Rewrite/JITLinkLinker.cpp:90
+                         jitlink::PassConfiguration &Config) override {
+    Config.PrePrunePasses.push_back(markSectionsLive);
+    return Error::success();
----------------
First thing I noticed is that the size of our eh_frame is missing a few bytes. I thought that maybe we're missing the NULL terminator.

Shouldn't we add something like:

  Config.PrePrunePasses.push_back(jitlink::EHFrameNullTerminator(".eh_frame"));

here? But then I did that and noticed that the terminators we originally had were.. ugh... maybe illegal (in the middle of the eh_frame). So we can probably live without this, but bear in mind that if there is no terminator here, eh_frame will miscompare (versus the previous version).

This version currently has a final terminator in the end of the eh_frame so that should be fine.


================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:1661
+    auto *Sym = BC->getOrCreateGlobalSymbol(Value, "EHat");
+    RelocatedEHFrameSection->addRelocation(Offset, Sym, RelType, Value);
   };
----------------
Second thing I noticed was this. This just looks wrong because here we're adding a reloc against a new symbol Sym, but then we're adding its own Value as an addend too. That Value probably should be zero, instead:

  RelocatedEHFrameSection->addRelocation(Offset, Sym, RelType, 0);

But then I did that and .eh_frame still looks completely different (the addresses). I'm not sure what's happening.

To see that, you need to build a test case with exceptions with two versions of BOLT, with and without your change, and then compare the eh_frame sections in them. To dump the eh_frame contents you can use a utility such as  "eu-readelf -e binary".

Finally, to check that your JITLink enabled BOLT is generating the same output of the old BOLT and thus, proving that this diff is safe, I suggest using this utility: https://github.com/llvm/llvm-project/blob/main/bolt/utils/llvm-bolt-wrapper.py

This utility basically replaces llvm-bolt in your binary folder, and when you run "check-bolt", it will trigger two BOLT runs, with and without your cahnge, and then compare the binaries produced by both BOLTs.  That's essentially part of the testing that we do when we have to approve a diff on BOLT, we take a look if the diff is not unintentionally changing some parts of the binary that it shouldn't, otherwise it's not safe to go to production.

Essentially, your diff is blocked atm because it is generating different eh_frame contents in x86 in a way that doesn't look right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147544