[PATCH] D88867: Fix JITLink for ELF X86 so that it accepts a .bss section by adding a call to createZeroFillBlock

Lang Hames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 5 21:17:01 PDT 2020


lhames added a comment.

Looks great -- thanks @drmeister.

All of Lint's comments about formatting are valid. If you want an easy way to avoid formatting comments in the future you can use the `git-clang-format` utility. Just make sure clang is in your path, then run:

  /path/to/clang/tools/clang-format/git-clang-format <commit>

e.g.

  /path/to/clang/tools/clang-format/git-clang-format HEAD

That will reformat your patch to match clang's formatting guides. (In this patch you'll also need to fix the case on one of your variables: I don't believe clang-format fixes case for you).

This also needs an llvm-jitlink regression test. I think the following change to llvm/test/ExecutionEngine/JITLink/X86/ELF_x86-64_relocations.s should work:

  diff --git a/llvm/test/ExecutionEngine/JITLink/X86/ELF_x86-64_relocations.s b/llvm/test/ExecutionEngine/JITLink/X86/ELF_x86-64_relocations.s
  index ca9c926b32d..0eef1111026 100644
  --- a/llvm/test/ExecutionEngine/JITLink/X86/ELF_x86-64_relocations.s
  +++ b/llvm/test/ExecutionEngine/JITLink/X86/ELF_x86-64_relocations.s
  @@ -46,6 +46,17 @@ named_data:
           .long   42
           .size   named_data, 4
   
  +# Test BSS / zero-fill section handling.
  +# llvm-jitlink: *{4}bss_variable = 0
  +
  +       .type   bss_variable, at object
  +       .bss
  +       .globl  bss_variable
  +       .p2align        2
  +bss_variable:
  +       .long   0
  +       .size   bss_variable, 4
  +
           .ident  "clang version 10.0.0-4ubuntu1 "
           .section        ".note.GNU-stack","", at progbits

I have verified that it fails without your patch -- if it runs with it then this is a viable test.


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

https://reviews.llvm.org/D88867



More information about the llvm-commits mailing list