[lld] [LLD] Tombstone LocalTU entry in .debug_names (PR #70701)

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 1 11:12:49 PDT 2023


================
@@ -918,7 +917,8 @@ void InputSection::relocateNonAlloc(uint8_t *buf, ArrayRef<RelTy> rels) {
       continue;
 
     if (tombstone ||
-        (isDebug && (type == target.symbolicRel || expr == R_DTPREL))) {
+        ((isDebug && (type == target.symbolicRel || expr == R_DTPREL))) ||
----------------
dwblaikie wrote:

Yeah, mechanically I understand what the check does. 

What I mean is: Why is this check necessary? What bad things happen (some tests fail, but I haven't looked closely at what they're testing or what the bad behavior looks like) when this check fails/isn't checked? And why is it OK to ignore this check in the `.debug_names` case? And could we look at that reason and generalize the check so it's not specifically about `.debug_names`, but about whatever general property is true in the `.debug_names` case and would be true in other cases?

Basically, it seems the `type == target.symbolicRel` check is overly restrictive (we have at least one counter example - the `.debug_names` `R_X86_64_32` situation) and I'd like to know ideally how to generically relax this check to account for our situation, but also others that may come up in the future/just not to have a weird `.debug_names` special case that we don't really understand/can't explain why it's acceptable here, but not there - and layering special cases makes the code harder to understand and maintain.

https://github.com/llvm/llvm-project/pull/70701


More information about the llvm-commits mailing list