[PATCH] D72567: [ELF] Avoid false-positive assert in getErrPlace()

[Alexander Richardson via Phabricator via llvm-commits]

arichardson created this revision.
arichardson added reviewers: MaskRay, grimar, ruiu.
Herald added subscribers: llvm-commits, atanasyan, jrtc27, krytarowski, sdardis, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

This assertion was added as part of D70659 <https://reviews.llvm.org/D70659> but did not account for .bss
input sections. I noticed that this assert was incorrectly triggering
while building FreeBSD for MIPS64. Fixed by relaxing the assert to also
account for SHT_NOBITS input sections and adjust the test
mips-jalr-non-function.s to link a file with a .bss section first.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72567

Files:
  lld/ELF/Target.cpp
  lld/test/ELF/mips-jalr-non-functions.s


Index: lld/test/ELF/mips-jalr-non-functions.s
===================================================================
--- lld/test/ELF/mips-jalr-non-functions.s
+++ lld/test/ELF/mips-jalr-non-functions.s
@@ -6,7 +6,12 @@
 ## relocations to avoid generating binaries that crash when executed.
 
 # RUN: llvm-mc -filetype=obj -triple=mips64-unknown-linux %s -o %t.o
-# RUN: ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s -check-prefix WARNING-MESSAGE
+## Link in another object file with a .bss as a regression test:
+## Previously getErrPlace() would assert when skipping over .bss sections.
+## By adding another file with a .bss section before the actual %t.o we can
+## reproduce this case.
+# RUN: llvm-mc -filetype=obj -triple=mips64-unknown-linux %S/Inputs/common.s -o %t-common.o
+# RUN: ld.lld -shared %t-common.o %t.o -o %t.so 2>&1 | FileCheck %s -check-prefix WARNING-MESSAGE
 # RUN: llvm-objdump --no-show-raw-insn --no-leading-addr -d %t.so | FileCheck %s
 
 .set	noreorder
@@ -23,12 +28,20 @@
   nop
 # WARNING-MESSAGE: warning: {{.+}}.tmp.o:(.text+0x8): found R_MIPS_JALR relocation against non-function symbol reg_obj. This is invalid and most likely a compiler bug.
 
-  .reloc .Ltmp3, R_MIPS_JALR, untyped
+
+  .reloc .Ltmp3, R_MIPS_JALR, common_sym
 .Ltmp3:
   jr  $t9
   nop
+# WARNING-MESSAGE: warning: {{.+}}.tmp.o:(.text+0x10): found R_MIPS_JALR relocation against non-function symbol common_sym. This is invalid and most likely a compiler bug.
+
 
 ## However, we do perform the optimization for untyped symbols:
+  .reloc .Ltmp4, R_MIPS_JALR, untyped
+.Ltmp4:
+  jr  $t9
+  nop
+
 untyped:
   nop
 
@@ -37,6 +50,11 @@
 tls_obj:
   .word 0
 
+
+.type  common_sym, at object
+.bss
+.comm common_sym,4,4
+
   .type  reg_obj, at object
   .data
 reg_obj:
@@ -49,5 +67,7 @@
 # CHECK-NEXT: nop
 # CHECK-NEXT: jr	$25
 # CHECK-NEXT: nop
+# CHECK-NEXT: jr	$25
+# CHECK-NEXT: nop
 # CHECK-NEXT: b	8 <untyped>
 # CHECK-NEXT: nop
Index: lld/ELF/Target.cpp
===================================================================
--- lld/ELF/Target.cpp
+++ lld/ELF/Target.cpp
@@ -103,7 +103,8 @@
             ? (Out::bufferStart + isec->getParent()->offset + isec->outSecOff)
             : isec->data().data();
     if (isecLoc == nullptr) {
-      assert(isa<SyntheticSection>(isec) && "No data but not synthetic?");
+      assert((isa<SyntheticSection>(isec) || (isec->type & SHT_NOBITS)) &&
+             "No data but not synthetic and not SHT_NOBITS?");
       continue;
     }
     if (isecLoc <= loc && loc < isecLoc + isec->getSize())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72567.237534.patch
Type: text/x-patch
Size: 2540 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200112/c049809c/attachment.bin>