[PATCH] D41215: Fix crash on invalid

[Peter Collingbourne via llvm-commits]

On Thu, Dec 14, 2017 at 3:34 PM, Rui Ueyama <ruiu at google.com> wrote:

> I tried to test the behavior of BFD linker, and looks like the GNU linker
> handles the symbol as a private one. The following program doesn't link
> with the .so file.
>
>   int _ZN8tcmalloc11ThreadCache17threadlocal_heap_E();
>   int main() { return _ZN8tcmalloc11ThreadCache17threadlocal_heap_E(); }
>
> When I change the function name (e.g. GetCdmVersion which is exported by
> the .so), it worked. So I think I'm sure the GNU linker handles it as a
> global symbol.
>

Assuming that you get the same result if you declare the symbol as a TLS
symbol, that seems like what I would expect. We probably want to skip the
symbol then.

Peter

>
> On Thu, Dec 14, 2017 at 1:48 PM, Peter Collingbourne <peter at pcc.me.uk>
> wrote:
>
>>
>>
>> On Thu, Dec 14, 2017 at 1:29 PM, Rui Ueyama <ruiu at google.com> wrote:
>>
>>> On Thu, Dec 14, 2017 at 1:22 PM, Peter Collingbourne <peter at pcc.me.uk>
>>> wrote:
>>>
>>>> On Thu, Dec 14, 2017 at 10:27 AM, Rafael Avila de Espindola via
>>>> llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Is libwidevinecdm.so available somewhere?
>>>>>
>>>>
>>>> If you have a recent version of Chrome installed on your machine, you
>>>> should be able to find a copy in your installation directory.
>>>>
>>>> We can try to make it a warning, but what is the correct thing to do
>>>>> about _ZN8tcmalloc11ThreadCache17threadlocal_heap_E? Should we skip
>>>>> it?
>>>>> Handle it as if it was global?
>>>>>
>>>>
>>>> I think we should skip it. It looks like the symbol itself is supposed
>>>> to be local.
>>>> https://cs.chromium.org/chromium/src/third_party/tcmalloc/ve
>>>> ndor/src/thread_cache.h?q=threadlocal_heap&sq=package:chromium&l=255
>>>>
>>>
>>> Is it? It's a static member of some class and not a file local symbol.
>>>
>>
>> Oh right, that wouldn't have caused the symbol to become STB_LOCAL then.
>> It could have been made local as a result of the library being built with
>> -fvisibility=hidden though. It could then have ended up in .dynsym as a
>> result of one of the bugs mentioned here:
>> https://sourceware.org/ml/binutils/2016-05/msg00076.html
>>
>> Peter
>>
>>>
>>>
>>>> Peter
>>>>
>>>>
>>>>> Cheers,
>>>>> Rafael
>>>>>
>>>>> Hans Wennborg via Phabricator <reviews at reviews.llvm.org> writes:
>>>>>
>>>>> > hans added a comment.
>>>>> >
>>>>> > This fixes the assert, now we error on it, which breaks our build
>>>>> all the same. It's a pre-built .so, which I'm not sure exactly where it
>>>>> comes from:
>>>>> >
>>>>> > FAILED: libwidevinecdmadapter.so
>>>>> > ../../../../../../../../work/llvm/build.goma/bin/clang++ -shared
>>>>> libwidevinecdm.so -Wl,--fatal-warnings -Wl,--build-id=sha1 -fPIC
>>>>> -Wl,-z,noexecstack -Wl,-z,now -Wl,-z,relro -Wl,-z,defs -Wl,--no-as-needed
>>>>> -lpthread -Wl,--as-needed -fuse-ld=lld -Wl,--icf=all -flto=thin
>>>>> -Wl,--thinlto-jobs=8 -Wl,--thinlto-cache-dir=thinlto-cache
>>>>> -Wl,--thinlto-cache-policy,cache_size=10\% -Wl,--lto-O0
>>>>> -fwhole-program-vtables -m64 -Werror -Wl,-O1 -Wl,--gc-sections
>>>>> -Wl,--gdb-index -nostdlib++ --sysroot=../../build/linux/debian_stretch_amd64-sysroot
>>>>> -L../../build/linux/debian_stretch_amd64-sysroot/lib/x86_64-linux-gnu
>>>>> -Wl,-rpath-link=../../build/linux/debian_stretch_amd64-sysroot/lib/x86_64-linux-gnu
>>>>> -L../../build/linux/debian_stretch_amd64-sysroot/usr/lib/x86_64-linux-gnu
>>>>> -Wl,-rpath-link=../../build/linux/debian_stretch_amd64-sysroot/usr/lib/x86_64-linux-gnu
>>>>> -fsanitize=cfi-vcall -Wl,-rpath=\$ORIGIN/. -Wl,-rpath-link=. -o
>>>>> "./libwidevinecdmadapter.so" -Wl,-soname="libwidevinecdmadapter.so"
>>>>> @"./libwidevinecdmadapter.so.rsp"
>>>>> > /usr/local/google/work/chromium/src/out/release/../../../../
>>>>> ../../../../work/llvm/build.goma/bin/ld.lld: error: Found local
>>>>> symbol '_ZN8tcmalloc11ThreadCache17threadlocal_heap_E' in global part
>>>>> of symbol table in file libwidevinecdm.so
>>>>> > clang-6.0: error: linker command failed with exit code 1 (use -v to
>>>>> see invocation)
>>>>> > ninja: build stopped: subcommand failed.
>>>>> >
>>>>> > Maybe the erorr could be downgraded to a warning? (We'd need to not
>>>>> treat warnings as errors for this target on our side I suppose)
>>>>> >
>>>>> >
>>>>> > https://reviews.llvm.org/D41215
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> --
>>>> Peter
>>>>
>>>
>>>
>>
>>
>> --
>> --
>> Peter
>>
>
>


-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171214/18184962/attachment.html>