[PATCH] Fix missed exception spec checks and crashes

Alp Toker alp at nuanti.com
Thu Oct 17 12:21:14 PDT 2013


Landed in r192914 with suggested simplifications.

With the pop_back() approach the two loops are no longer tied -- in the
off chance this becomes a problem due to the second mutating the first,
we can take a look at using the heavier approach in the original patch
again.

Thanks for the review Richard!

Marshall, let us know if this covers all bases.

Alp.

On 07/10/2013 21:36, Richard Smith wrote:
> On Sun, Oct 6, 2013 at 11:54 AM, Alp Toker <alp at nuanti.com
> <mailto:alp at nuanti.com>> wrote:
>
>     This patch fixes a crash and rejects-valid the libcxx developers were
>     hitting.
>
>     While investigating the defaulted member cases I noticed a similar
>     problem in delayed destructor checking, with checks accumulating but
>     never running or getting cleared if they were instantiated within an
>     exception spec.
>
>     I'll appreciate thoughts on both parts of the fix, particularly on the
>     correctness of moving the destructor checks from
>     ActOnFinishCXXMemberDecls() to ActOnFinishDelayedMemberInitializers()
>     alongside the member checks, which seems to be the only way to get
>     them
>     handled consistently.
>
>
> Both changes look correct to me. Rather than swapping the worklists
> out in each iteration of the loop, can you instead process them in
> reverse order (and pop them as you go)? This should make the code a
> little simpler (and a little more efficient too).
>  
>
>     Marshall, can you think of anything else to excercise in the test
>     case?
>
>     I've attached the patch and crash backtrace. Here's the changelog:
>
>     Delayed exception specification checking for defaulted members and
>     virtual destructors are both susceptible to mutation during
>     iteration so
>     we need to swap and process the worklists iteratively.
>
>     This resolves both accepts-invalid and rejects-valid issues and
>     moreover
>     fixes potential invalid memory access as the contents of the vectors
>     change during iteration and recursive template instantiation.
>
>     This patch also adds two assertions at end of TU to ensure no
>     specs are
>     left unchecked as was happenning before the fix, plus a test case from
>     Marshall Clow for the defaulted member crash extracted from the libcxx
>     headers.
>
>     Alp.
>
>     --
>     http://www.nuanti.com
>     the browser experts
>
>
>     _______________________________________________
>     cfe-commits mailing list
>     cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>

-- 
http://www.nuanti.com
the browser experts




More information about the cfe-commits mailing list