The first attempt at getting a relocation type added.
So far nothing special but should set the architecture going forward for the rest of the relocations
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This looks great!
Style issues: There are a number of single line statements with braces around them -- those braces should be moved to match the style guide. You should also clang-format the patch if you haven't already. Otherwise this is good to land.
llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp | ||
---|---|---|
196–200 | No braces for single-line statements. :) Side note regarding the TODO: Long term I think it would make sense to do some sort of up-front validation of the input file, but for now this is the right place to check this. | |
237 | For what error condition? Empty sections? Does your parser ever produce them? |
Clang-format was run but doesn't seem to catch this case, ill clean it up later this afternoon.
llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp | ||
---|---|---|
237 | No. If a JITSection is found, it would be next to impossible not to have a block. So if i push the error check on jitSection that should ensure a block is present |
Oh, wait: this still needs a test case before it lands. The llvm/test/ExecutionEngine/JITLink/X86/ELF_x86-64_relocations.s test should be updated to include a check for the new relocation.
This should fix up the review comments, and the clang-tidy issue
Will add the tests later tonight
a6deaeec370ec5e34f9e5aa3fad3bc73770d4895 adds basic support for llvm-jitlink -check tests for ELF. That should allow you to test this.
No braces for single-line statements. :)
Side note regarding the TODO: Long term I think it would make sense to do some sort of up-front validation of the input file, but for now this is the right place to check this.