[PATCH] D59553: [WIP][LLD][ELF][DebugInfo] Use predefined value to mark debug address ranges pointing to deleted code.

[Alexey Lapshin via Phabricator via llvm-commits]

avl marked 2 inline comments as done.
avl added a comment.

would address all comments and rewrite test case with yaml.



================
Comment at: lld/ELF/InputSection.cpp:852-853
+
+  if (config->is64 && config->emachine == EM_X86_64 && type == R_X86_64_32)
+    res &= 0x00000000ffffffffULL;
+
----------------
jhenderson wrote:
> I might well have missed something, but why this special casing specifically for X86_64? Do other 64-bit targets not have the same issue (I'm assuming the masking is required to prevent some sort of "too big to fit" error from the relocation processing).
> 
> It also looks like some of this if statement is untested (specifically the bit about EM_X86_64).
> I might well have missed something, but why this special casing specifically for X86_64? Do other 64-bit targets not have the same issue (I'm assuming the masking is required to prevent some sort of "too big to fit" error from the relocation processing).

right. lld reported "too big to fit" error. X86_64::relocate() function has following check:

checkUInt(loc, val, 32, rel);

It leads to this error for ILP32 case:

relocation R_X86_64_32 out of range: 18446744073709551615 is not in [0, 4294967295]; consider recompiling with -fdebug-types-section to reduce size of debug sections.

I checked these targets and they seem do not have above problem:

aarch64
aarch64_32
arm64
arm64_32
riscv64

But , I missed sparcv9 target. It has similar problem.

>It also looks like some of this if statement is untested (specifically the bit about EM_X86_64).

It assumed to be tested with following line from test file:

# RUN:   | llvm-mc --filetype=obj -triple=x86_64-unknown-linux32 - -o %t
# RUN: ld.lld %t -o %t2 --gc-sections

specifying x86_64 inside triple should lead to config->emachine = EM_X86_64, as far as I understand.


================
Comment at: lld/test/ELF/gc-sections-debuginfo.s:3
+
+# RUN: echo '.section .text,"a"; .byte 0;  .section .debug_foo,"", at progbits; .quad .text; .section .debug_loc,"", at progbits; .quad .text' \
+# RUN:   | llvm-mc --filetype=obj -triple=x86_64-unknown-linux - -o %t
----------------
jhenderson wrote:
> Is there a reason you are echoing this content, rather than having it written in the test file like most tests (I assume it's the need for two different blocks of assembly)? If there is, then the test file should end with .test, rather than .s since this file currently contains no assembly.
Originally I wrote this test as two files with assembly. But there was request to put it`s content into "echo". Would rewrite it with yaml, as requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59553