[cfe-dev] Clang produces corrupt pch leading to crash

[Richard Smith]

I think this may be one of those changes for which we need to accept that a
unit test would not be useful. Does the change I suggested upthread fix the
issue for you?

On Fri, Jun 27, 2014 at 9:15 AM, Tobias Hahn <tobias.hahn at ableton.com>
wrote:

>  The last test case I sent around is as minimal as I could get it, but
> still over a hundred loc. It only crashes reliably when clang is built with
> asan (valgrind would probably be another option). I've tried a patch very
> similar to yours; and it fixes the issue.
>
>  Maybe one could construct a simpler test case with the knowledge that it
> has to force the KeyFunctions DenseMap to re-allocate?
>
>  I've also thought about how to unit-test this, but it seems impossible
> to write a unit test without exposing the KeyFunctions cache in ASTContext.
> At this point I stopped because I have no idea what your policy is for such
> changes and in what general directions this code is meant to evolve.
>
>  Best,
> Tobias
>
> Am 27.06.2014 um 17:24 schrieb "Richard Smith" <richard at metafoo.co.uk>:
>
>    On Sun, Jun 22, 2014 at 5:45 PM, Nikola Smiljanic <popizdeh at gmail.com>
> wrote:
>
>> Awesome work Tobias, very interesting sequence of events. Adding Richard
>> as he'll know how to best reshuffle this code :D
>>
>>  The precompiled header is good, the trouble starts in
>> ASTContext::getCurrentKeyFunctions:
>>
>>  LazyDeclPtr &Entry = KeyFunctions[RD];
>>   if (!Entry)
>>     Entry = const_cast<CXXMethodDecl*>(computeKeyFunction(*this, RD));
>>
>
>  That's a classic :)
>
>  It should be straightforward to fix this, something like:
>
>  LazyDeclPtr *Entry = &KeyFunctions[RD];
> if (!*Entry) {
>   auto *KeyFunction = computeKeyFunction(*this, RD);
>   // Entry might have been invalidated by computeKeyFunction.
>   Entry = &KeyFunctions[RD];
>   *Entry = KeyFunction;
> }
> // do stuff with *Entry
>
>  Do we have a reduced testcase? I suspect that such a test may be fragile
> to the point of being useless, so maybe we should go without =(
>
>   KeyFunctions DenseMap will undergo reallocation inside
>> computeKeyFunctions (see two markers in the attached callstack). Upon
>> return Entry will point to freed memory. Good lord it took me a while to
>> figure this out, I suck at debugging on Linux...
>>
>>
>> On Sat, Jun 21, 2014 at 6:12 AM, Tobias Hahn <tobias.hahn at ableton.com>
>> wrote:
>>
>>>  Hi Nikola,
>>>
>>> I can reproduce the bug on linux if I tell clang to cross-compile for
>>> osx. Attached is a minimal example. You need to build clang with asan to
>>> reproduce the crash:
>>>
>>> ../llvm/configure --enable-libcpp CFLAGS="-fsanitize=address"
>>> CXXFLAGS="-fsanitize=address"
>>>
>>> Thanks again for looking into this crash!
>>>
>>> Best,
>>> Tobias
>>>
>>> Am 17.06.2014 um 00:16 schrieb Nikola Smiljanic <popizdeh at gmail.com>:
>>>
>>>
>>> > It might make sense to file this with apple if you're using clang
>>> shipped with XCode as they ship their own releases. What version of clang
>>> are you using? Is the isystem flag important? What about #include <list>?
>>> I've tried to reproduce this but it's not so straightforward because I'm on
>>> linux and some of the stuff in that bash script assumes mac os... I was
>>> wondering if it's possible to reduce this to something that's reproducible
>>> everywhere or if this is a mac specific issue.
>>> >
>>> >
>>> > On Mon, Jun 16, 2014 at 11:55 PM, Nikola Smiljanic <popizdeh at gmail.com>
>>> wrote:
>>> > Thanks for the detailed report! The only thing you can do more is try
>>> and debug this yourself ;)
>>> >
>>> >
>>> > On Mon, Jun 16, 2014 at 8:33 PM, Tobias Hahn <tobias.hahn at ableton.com>
>>> wrote:
>>> > Hi all,
>>> >
>>> > I've run into (what I believe is) a memory bug with clang while
>>> producing a precompiled header. In short, under certain circumstances,
>>> clang will write a pch that causes a crash when trying to use this pch in a
>>> later compilation unit.
>>> >
>>> > Occasionally, while clang is compiling the pch, malloc complains that
>>> one of its checksums has been overwritten; while at other times, clang
>>> throws an error that a definition has a different exception specification
>>> than the declaration two lines above it (when both have no exception
>>> specification). Both these symptoms lead me to believe that somewhere clang
>>> overwrites memory.
>>> >
>>> > I have stripped the code that reliably causes this crash down to a few
>>> hundred lines and have created a little script to reproduce the bug
>>> (details at http://llvm.org/bugs/show_bug.cgi?id=20026). I'm not sure,
>>> however, about your process for handling such bugs, which is why I'm
>>> cross-posting here. My main question is if there is anything else I could
>>> provide you with to help fixing this issue.
>>> >
>>> > Thank you very much in advance!
>>> >
>>> > Best,
>>> > Tobias
>>> >
>>> >
>>> > Ableton AG, Schoenhauser Allee 6-7, 10119 Berlin, Germany
>>> > Sitz (Registered Office) Berlin, Amtsgericht Berlin-Charlottenburg,
>>> HRB 72838
>>> > Vorstand (Management Board): Gerhard Behles, Jan Bohl
>>> > Vorsitzender des Aufsichtsrats (Chair of the Supervisory Board): Uwe
>>> Struck
>>> >
>>> >
>>> >
>>> > _______________________________________________
>>> > cfe-dev mailing list
>>> > cfe-dev at cs.uiuc.edu
>>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>> >
>>> >
>>>
>>> Ableton AG, Schoenhauser Allee 6-7, 10119 Berlin, Germany
>>> Sitz (Registered Office) Berlin, Amtsgericht Berlin-Charlottenburg, HRB
>>> 72838
>>> Vorstand (Management Board): Gerhard Behles, Jan Bohl
>>> Vorsitzender des Aufsichtsrats (Chair of the Supervisory Board): Uwe
>>> Struck
>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140627/3a333db4/attachment.html>