<div dir="ltr"><div>I'm a little confused. isecLoc is initialized in the following statement</div><div><br></div>uint8_t *isecLoc = Out::bufferStart + isec->getParent()->offset + isec->outSecOff;<br><div><br></div><div>, and I don't know how isecLoc can become a nullptr. If Out::bufferStart is a nullptr, the initialization expression should trigger a UBsan.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 10, 2019 at 9:26 PM Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">I guess that you are fixing UB issues that was not noticed before but can now be caught by an improved UBsan? I hoped that you would sent it for review, but if you are trying to fix buildbots, I can see why you wanted to submit this...</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 10, 2019 at 9:20 PM Roman Lebedev via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: lebedevri<br>
Date: Thu Oct 10 05:22:55 2019<br>
New Revision: 374329<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=374329&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=374329&view=rev</a><br>
Log:<br>
[lld] getErrPlace(): don't perform arithmetics on maybe-null pointer<br>
<br>
isecLoc there can be null, but at the same time isec->getSize() may<br>
be non-null. It is UB to offset a nullptr.The most straight-forward fix<br>
here appears to perform casts+normal integral arithmetics.<br>
<br>
FAIL: lld :: ELF/invalid/invalid-relocation-aarch64.test (1158 of 2217)<br>
******************** TEST 'lld :: ELF/invalid/invalid-relocation-aarch64.test' FAILED ********************<br>
Script:<br>
--<br>
: 'RUN: at line 2'; /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/yaml2obj /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/lld/test/ELF/invalid/invalid-relocation-aarch64.test -o /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/tools/lld/test/ELF/invalid/Output/invalid-relocation-aarch64.test.tmp.o<br>
: 'RUN: at line 3'; not /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/ld.lld /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/tools/lld/test/ELF/invalid/Output/invalid-relocation-aarch64.test.tmp.o -o /dev/null 2>&1 | /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/lld/test/ELF/invalid/invalid-relocation-aarch64.test<br>
--<br>
Exit Code: 1<br>
<br>
Command Output (stderr):<br>
--<br>
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/lld/test/ELF/invalid/invalid-relocation-aarch64.test:4:10: error: CHECK: expected string not found in input<br>
# CHECK: error: unknown relocation (1024) against symbol foo<br>
^<br>
<stdin>:1:1: note: scanning from here<br>
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/lld/ELF/Target.cpp:100:41: runtime error: applying non-zero offset 24 to null pointer<br>
^<br>
<stdin>:1:118: note: possible intended match here<br>
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/lld/ELF/Target.cpp:100:41: runtime error: applying non-zero offset 24 to null pointer<br>
^<br>
<br>
--<br>
<br>
********************<br>
Testing: 0.. 10.. 20.. 30.. 40.. 50.<br>
FAIL: lld :: ELF/invalid/invalid-relocation-x64.test (1270 of 2217)<br>
******************** TEST 'lld :: ELF/invalid/invalid-relocation-x64.test' FAILED ********************<br>
Script:<br>
--<br>
: 'RUN: at line 2'; /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/yaml2obj /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/lld/test/ELF/invalid/invalid-relocation-x64.test -o /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/tools/lld/test/ELF/invalid/Output/invalid-relocation-x64.test.tmp1.o<br>
: 'RUN: at line 3'; echo ".global foo; foo:" > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/tools/lld/test/ELF/invalid/Output/invalid-relocation-x64.test.tmp2.s<br>
: 'RUN: at line 4'; /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-mc /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/tools/lld/test/ELF/invalid/Output/invalid-relocation-x64.test.tmp2.s -o /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/tools/lld/test/ELF/invalid/Output/invalid-relocation-x64.test.tmp2.o -filetype=obj -triple x86_64-pc-linux<br>
: 'RUN: at line 5'; not /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/ld.lld /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/tools/lld/test/ELF/invalid/Output/invalid-relocation-x64.test.tmp1.o /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/tools/lld/test/ELF/invalid/Output/invalid-relocation-x64.test.tmp2.o -o /dev/null 2>&1 | /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/lld/test/ELF/invalid/invalid-relocation-x64.test<br>
--<br>
Exit Code: 1<br>
<br>
Command Output (stderr):<br>
--<br>
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/lld/test/ELF/invalid/invalid-relocation-x64.test:6:10: error: CHECK: expected string not found in input<br>
# CHECK: error: unknown relocation (152) against symbol foo<br>
^<br>
<stdin>:1:1: note: scanning from here<br>
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/lld/ELF/Target.cpp:100:41: runtime error: applying non-zero offset 24 to null pointer<br>
^<br>
<stdin>:1:118: note: possible intended match here<br>
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/lld/ELF/Target.cpp:100:41: runtime error: applying non-zero offset 24 to null pointer<br>
^<br>
<br>
--<br>
<br>
********************<br>
Testing: 0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..<br>
Testing Time: 20.73s<br>
********************<br>
Failing Tests (2):<br>
lld :: ELF/invalid/invalid-relocation-aarch64.test<br>
lld :: ELF/invalid/invalid-relocation-x64.test<br>
<br>
Modified:<br>
lld/trunk/ELF/Target.cpp<br>
<br>
Modified: lld/trunk/ELF/Target.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Target.cpp?rev=374329&r1=374328&r2=374329&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Target.cpp?rev=374329&r1=374328&r2=374329&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/Target.cpp (original)<br>
+++ lld/trunk/ELF/Target.cpp Thu Oct 10 05:22:55 2019<br>
@@ -97,7 +97,12 @@ template <class ELFT> static ErrorPlace<br>
continue;<br>
<br>
uint8_t *isecLoc = Out::bufferStart + isec->getParent()->offset + isec->outSecOff;<br>
- if (isecLoc <= loc && loc < isecLoc + isec->getSize())<br>
+ if (isecLoc > loc)<br>
+ continue;<br>
+ // isecLoc might be nullptr here, with isec->getSize() being non-zero.<br>
+ // Adding these two together is not defined in C++.<br>
+ if (loc < reinterpret_cast<uint8_t *>(<br>
+ reinterpret_cast<std::uintptr_t>(isecLoc) + isec->getSize()))<br></blockquote><div><br></div><div>This new expression doesn't make much sense to me. Perhaps we should do `continue` if isecLoc is null.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
return {isec, isec->template getLocation<ELFT>(loc - isecLoc) + ": "};<br>
}<br>
return {};<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>
</blockquote></div>