[cfe-commits] [PATCH] Fix random crasher

[Robinson, Paul]

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: metafoo at gmail.com [metafoo at gmail.com] on behalf of Richard Smith [richard at metafoo.co.uk]
Sent: Wednesday, July 18, 2012 5:40 PM
To: Robinson, Paul
Cc: cfe-commits at cs.uiuc.edu
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 <Paul.Robinson at am.sony.com<mailto:Paul.Robinson at am.sony.com>> 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: metafoo at gmail.com<mailto:metafoo at gmail.com> [metafoo at gmail.com<mailto:metafoo at gmail.com>] on behalf of Richard Smith [richard at metafoo.co.uk<mailto:richard at metafoo.co.uk>]
Sent: Wednesday, July 18, 2012 4:54 PM
To: Robinson, Paul
Cc: cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
Subject: Re: [cfe-commits] [PATCH] Fix random crasher

On Wed, Jul 18, 2012 at 4:51 PM, Richard Smith <richard at metafoo.co.uk<mailto:richard at metafoo.co.uk>> wrote:
On Wed, Jul 18, 2012 at 3:55 PM, Robinson, Paul <Paul.Robinson at am.sony.com<mailto:Paul.Robinson at am.sony.com>> 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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120719/152ccc96/attachment.html>