The original problem occurred in our Windows-hosted compiler, which was built with Visual Studio 2010. It turns out VS'10 was not correctly value-initializing the temporary object created by OverloadCandidate() which led to the problem.
So, this was a build-compiler bug, not a Clang bug. Thanks, --paulr ________________________________ From: [email protected] [[email protected]] on behalf of Robinson, Paul [[email protected]] Sent: Thursday, July 19, 2012 12:11 PM To: Richard Smith Cc: [email protected] Subject: Re: [cfe-commits] [PATCH] Fix random crasher A colleague had already tried valgrind; no joy there because the crash came about due to trying to destroy a DeductionFailure instance, which is a normal member of OverloadCandidate and not dynamically allocated on its own. After further research I agree that it is unexpected for FailureKind to be nonzero when Viable is true; the constructor ought to have zeroed it. I am looking for a smaller test case but no promises. --paulr ________________________________ From: [email protected] [[email protected]] on behalf of Richard Smith [[email protected]] Sent: Wednesday, July 18, 2012 5:40 PM To: Robinson, Paul Cc: [email protected] Subject: Re: [cfe-commits] [PATCH] Fix random crasher Yes, a reduced testcase would be very useful. On Wed, Jul 18, 2012 at 5:27 PM, Robinson, Paul <[email protected]<mailto:[email protected]>> wrote: Thanks! I am not familiar with valgrind, but I suppose I could learn, if there is still interest in a test case that triggers it. --paulr ________________________________ From: [email protected]<mailto:[email protected]> [[email protected]<mailto:[email protected]>] on behalf of Richard Smith [[email protected]<mailto:[email protected]>] Sent: Wednesday, July 18, 2012 4:54 PM To: Robinson, Paul Cc: [email protected]<mailto:[email protected]> Subject: Re: [cfe-commits] [PATCH] Fix random crasher On Wed, Jul 18, 2012 at 4:51 PM, Richard Smith <[email protected]<mailto:[email protected]>> wrote: On Wed, Jul 18, 2012 at 3:55 PM, Robinson, Paul <[email protected]<mailto:[email protected]>> wrote: Guard use of a possibly uninitialized field. This was causing very unpredictable compiler crashes. I have not provided a test because even our most reliable reproducer still failed less than 10% of the time. I really really really don't like sometimes-uninitialized fields guarded by flags. It is not a robust practice and took us a couple of weeks of poking at it to find the root cause. But it is how the rest of SemaOverload handles this field, so we fixed it using the prevailing practice in the module. Do you know where the uninitialized OverloadCandidate is coming from? The only place I can see one being created is in OverloadCandidateSet::addCandidate, which says: Candidates.push_back(OverloadCandidate()); This zero-initializes the OverloadCandidate object. I've checked in a variant on your change in r160470: it seems correct and appropriate even if FailureKind is always initialized, since we were previously implicitly and accidentally relying on ovl_fail_bad_deduction being nonzero.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
