[lld] r374329 - [lld] getErrPlace(): don't perform arithmetics on maybe-null pointer

Thu Oct 10 05:29:12 PDT 2019

I'm a little confused. isecLoc is initialized in the following statement

uint8_t *isecLoc = Out::bufferStart + isec->getParent()->offset +
isec->outSecOff;

, and I don't know how isecLoc can become a nullptr. If Out::bufferStart is
a nullptr, the initialization expression should trigger a UBsan.

On Thu, Oct 10, 2019 at 9:26 PM Rui Ueyama <ruiu at google.com> wrote:

> 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...
>
> On Thu, Oct 10, 2019 at 9:20 PM Roman Lebedev via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: lebedevri
>> Date: Thu Oct 10 05:22:55 2019
>> New Revision: 374329
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=374329&view=rev
>> Log:
>> [lld] getErrPlace(): don't perform arithmetics on maybe-null pointer
>>
>> isecLoc there can be null, but at the same time isec->getSize() may
>> be non-null. It is UB to offset a nullptr.The most straight-forward fix
>> here appears to perform casts+normal integral arithmetics.
>>
>> FAIL: lld :: ELF/invalid/invalid-relocation-aarch64.test (1158 of 2217)
>> ******************** TEST 'lld ::
>> ELF/invalid/invalid-relocation-aarch64.test' FAILED ********************
>> Script:
>> --
>> : '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
>> : '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
>> --
>> Exit Code: 1
>>
>> Command Output (stderr):
>> --
>> /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
>> # CHECK: error: unknown relocation (1024) against symbol foo
>>          ^
>> <stdin>:1:1: note: scanning from here
>> /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
>> ^
>> <stdin>:1:118: note: possible intended match here
>> /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
>>
>>                                            ^
>>
>> --
>>
>> ********************
>> Testing:  0.. 10.. 20.. 30.. 40.. 50.
>> FAIL: lld :: ELF/invalid/invalid-relocation-x64.test (1270 of 2217)
>> ******************** TEST 'lld ::
>> ELF/invalid/invalid-relocation-x64.test' FAILED ********************
>> Script:
>> --
>> : '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
>> : '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
>> : '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
>> : '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
>> --
>> Exit Code: 1
>>
>> Command Output (stderr):
>> --
>> /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
>> # CHECK: error: unknown relocation (152) against symbol foo
>>          ^
>> <stdin>:1:1: note: scanning from here
>> /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
>> ^
>> <stdin>:1:118: note: possible intended match here
>> /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
>>
>>                                            ^
>>
>> --
>>
>> ********************
>> Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
>> Testing Time: 20.73s
>> ********************
>> Failing Tests (2):
>>     lld :: ELF/invalid/invalid-relocation-aarch64.test
>>     lld :: ELF/invalid/invalid-relocation-x64.test
>>
>> Modified:
>>     lld/trunk/ELF/Target.cpp
>>
>> Modified: lld/trunk/ELF/Target.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Target.cpp?rev=374329&r1=374328&r2=374329&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/Target.cpp (original)
>> +++ lld/trunk/ELF/Target.cpp Thu Oct 10 05:22:55 2019
>> @@ -97,7 +97,12 @@ template <class ELFT> static ErrorPlace
>>        continue;
>>
>>      uint8_t *isecLoc = Out::bufferStart + isec->getParent()->offset +
>> isec->outSecOff;
>> -    if (isecLoc <= loc && loc < isecLoc + isec->getSize())
>> +    if (isecLoc > loc)
>> +      continue;
>> +    // isecLoc might be nullptr here, with isec->getSize() being
>> non-zero.
>> +    // Adding these two together is not defined in C++.
>> +    if (loc < reinterpret_cast<uint8_t *>(
>> +                  reinterpret_cast<std::uintptr_t>(isecLoc) +
>> isec->getSize()))
>>
>
> This new expression doesn't make much sense to me. Perhaps we should do
> `continue` if isecLoc is null.
>
>        return {isec, isec->template getLocation<ELFT>(loc - isecLoc) + ":
>> "};
>>    }
>>    return {};
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191010/e15452a3/attachment.html>