[PATCH] D113912: [JITLink] Fix splitBlock if there are symbols span across the boundary

[Steven Wu via Phabricator via llvm-commits]

steven_wu added inline comments.


================
Comment at: llvm/lib/ExecutionEngine/JITLink/JITLink.cpp:217-225
+      auto *Sym = BlockSymbols.back();
+      if (Sym->getOffset() + Sym->getSize() > SplitIndex) {
+        // This is an anonymous symbol extends across the split boundary, remove
+        // it because this is likely a symbol to mark the block.
+        if(Sym->hasName())
+          return make_error<JITLinkError>(
+              "named symbol span across the split boundary");
----------------
dexonsmith wrote:
> This doesn't seem safe in general, since anonymous symbols aren't necessarily safe to split just because they're anonymous.
> 
> I wonder if, rather than return `Error`s, it'd be better to trust the caller and split blocks anyway (as the code was), but truncate `Symbol::size()` as necessary to avoid it going past the end of the block it points at.
> 
> Two other ideas:
> - Change the LinkGraph parser to set the symbol size to 0 for symbols that are generated just to allow edges to a block.
> - Add a bit to Symbol that says "safe to split", which LinkGraph would set for symbols it generates just to allow edges to a block.
> 
> 
> @lhames, WDYT?
So the symbol in question is the one created here:
```
MachOLinkGraphBuilder::addSectionStartSymAndBlock
```

I don't know how this one is being used but the one marked `eh_frame` will become redundant after eh_frame splitting. I guess we can shrink the size of section start sym to have size 0 then we can just always split the block and not worry about that. Then any symbols span across the split boundary will be Error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113912