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

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 10 05:45:28 PDT 2019 

I think I now understand what is the problem. Submitted a patch on top of
this as r374332.

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

> 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/a06d7d4a/attachment.html>

More information about the llvm-commits mailing list