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

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 13 23:49:33 PST 2020


grimar added inline comments.


================
Comment at: lld/ELF/Target.cpp:98
     auto *isec = cast<InputSection>(d);
     if (!isec->getParent())
       continue;
----------------
MaskRay wrote:
> arichardson wrote:
> > I could also add the `|| (isec->type & SHT_NOBITS)` to this if? Is that preferable?
> Agree. `|| (isec->type & SHT_NOBITS)` here is preferable.
+1.


================
Comment at: lld/test/ELF/mips-jalr-non-functions.s:10
+## 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
----------------
nit: I do not think we mention function names in tests. They might change.
I think just saying that LLD asserted with this test case because of .bss is enough probably.
No need to specify the exact code place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72567





More information about the llvm-commits mailing list