[PATCH] D90722: [RTDYLD] support absolute relocations where needed

[Lang Hames via Phabricator via llvm-commits]

lhames accepted this revision.
lhames added a comment.

Hi Jameson, Valentin,

Valentin -- Sorry I didn't get to your initial review earlier!

Jameson -- Thanks for catching the DWARF / sentinel value issue. I would have missed that.

This looks great to me. I think the original problem was misapplied relocations in the presence of empty symbols, right? In that case the correct way to test would be to write an assembly file that triggers generation of one empty symbol, plus a relocation that's expected to fail in the presence of that empty symbol. Then you can use llvm-rtdyld in -verify mode to verify that the relocation was handled correctly. See https://github.com/llvm/llvm-project/blob/master/llvm/test/ExecutionEngine/RuntimeDyld/X86/ELF-large-pic-relocations.s for an example of this style of test.

I don't know exactly which relocations were failing for you in the presence of empty symbols, but say it was plain R_X86_64_64s. You could write a program like this to generate one:

  void bad_target(void) {}
  
  void good_target(void) {}
  
  void test_relocation(void) {
    good_target();
  }

Convert it to assembly with:

`% clang -mcmodel=large --target=x86_64-unknown-linux -fno-asynchronous-unwind-tables -S -o ELF_empty_symbol_name.s input.c`

Then add an `# rtdyld-check` line to verify that the relocation applies correctly.

  # RUN: rm -rf %t && mkdir -p %t
  # RUN: llvm-mc -triple=x86_64-unknown-freebsd -filetype=obj -o %t/ELF_empty_symbol_name.o %s
  # RUN: llvm-rtdyld -triple=x86_64-unknown-linux -verify -check=%s %t/ELF_empty_symbol_name.o
  
  	.text
  	.globl	bad_target
  	.p2align	4, 0x90
  	.type	bad_target, at function
  bad_target:
  	retq
  .Lfunc_end0:
  	.size	bad_target, .Lfunc_end0-bad_target
  
  	.globl	good_target
  	.p2align	4, 0x90
  	.type	good_target, at function
  good_target:
  	retq
  .Lfunc_end1:
  	.size	good_target, .Lfunc_end1-good_target
  
  	.globl	test_relocation
  	.p2align	4, 0x90
  	.type	test_relocation, at function
  
  # rtdyld-check: decode-operand(test_relocation) = good_target
  test_relocation:
  	movabsq	$good_target, %rax
  	callq	*%rax
  	retq
  .Lfunc_end2:
  	.size	test_relocation, .Lfunc_end2-test_relocation
  
  	.ident	"Apple clang version 12.0.0 (clang-1200.0.32.27)"
  	.section	".note.GNU-stack","", at progbits
  	.addrsig
  	.addrsig_sym good_target

Then add whatever assembly / DWARF you need to trigger the bug and verify that the test fails. Then apply your patch and verify that it passes again.

If it's too fiddly I don't mind this going in without a test case right now -- I think extra cycles would be just as well spent bringing up / testing JITLink for ELF. We should make sure we don't replicate this issue by using empty strings as sentinels there.

I'll be on the road a lot the next few days: If you have any follow-up questions in the short term discord will be the best way to get hold of me. Otherwise I'll check back in here in a week or so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90722