[PATCH] D80613: Getting R_X86_64_PC32 working as our first relocation
Lang Hames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 27 15:16:32 PDT 2020
lhames accepted this revision.
lhames added a comment.
This revision is now accepted and ready to land.
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.
================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp:196-200
+ // TODO can the elf obj file do this for me?
+ if (SecRef.sh_type == ELF::SHT_REL) {
+ return make_error<llvm::StringError>("Shouldn't have REL in x64",
+ llvm::inconvertibleErrorCode());
+ }
----------------
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.
================
Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp:237
+ }
+ // TODO error handling?
+ auto BlockToFix = *JITSection->blocks().begin();
----------------
For what error condition? Empty sections? Does your parser ever produce them?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80613/new/
https://reviews.llvm.org/D80613
More information about the llvm-commits
mailing list