[PATCH] Fix missed exception spec checks and crashes

[Alp Toker]

This seems to have caused the sanitizer bot to hang with message "
Failed annotate failed run sanitizer tests in gcc build"

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/2311

command timed out: 1200 seconds without output, attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1423.760427

I'm not sure what to make of it* and don't have a sanitizer compiler-rt
bootstrap build handy so will be backing out r192914 shortly unless
someone has spotted the error and can suggest a fix in the meantime.

Alp.

* There's a potential infinite loop where items are added back on the
worklist, that was pre-existing but may have been exercised by the fix
and will be the first place to look.


On 17/10/2013 20:21, Alp Toker wrote:
> 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