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

Mon Jun 30 01:22:49 PDT 2014

I was about to do the same :) But one thing I wanted to ask is, why change
Entry into a pointer? Why not just call operator[] twice? As Tobias said
we'll need to cast to CXXRecordDecl. And we'll have to change Entry.get to
Entry->get if Entry needs to be a pointer.

On Mon, Jun 30, 2014 at 5:42 PM, Tobias Hahn <tobias.hahn at ableton.com>
wrote:

>  Hey Richard,
>
> I'll check your patch in a minute, but except for a missing const_cast I
> believe it should be fine just by comparing it with the patch I came up
> with (attached below).
>
> Best,
> Tobias
>   Am 27.06.2014 um 18:24 schrieb Richard Smith <richard at metafoo.co.uk>:
>
>
> > 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
> >> >>
> >>
> >>
> >>
> >
>
> 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/20140630/fe68a9ac/attachment.html>