[PATCH] Avoid a "diagnostic already in flight" assertion in diagnoseOdrViolations()

[Richard Smith]

On Mon, Aug 4, 2014 at 3:03 PM, Ben Langmuir <blangmuir at apple.com> wrote:

> Hi Richard,
>
> We have several lazy builtin Decls (for ObjC decls, va_list, etc.) that
> might get filled in when we desugar a type for the ODR diagnostics, and
> these may deserialize more content from a module when they lookup an
> IdentifierInfo, leading to trying to emit a diagnostic while a diagnostic
> is already in flight.  My patch fixes the specific issue I ran into with
> the ODR diagnostics, but it seems like there may be a more general problem
> that filling in these lazy builtins may not be safe during diagnostic
> printing.  Any thoughts?


Your patch LGTM.

I don't think this is specific to filling in lazy builtins: any AST
deserialization triggered by diagnoseOdrViolations seems to hit this issue.
This used to be protected from this particular issue by occurring within a
'Deserializing' RAII region; I moved it out of that region because it
depends indirectly on AST invariants being maintained, so it seemed better
to keep it outside the 'danger zone' of finishPendingActions.

We could perhaps make this more robust by adding a flag indicating whether
we're already doing diagnoseOdrViolations, and skipping it if it
reentrantly triggers itself, but your change looks like it would have the
same effect in practice.

FWIW, the PassInterestingDeclsToConsumer call in FinishedDeserializing has
a similar, but worse, issue: it calls back into CodeGen, and may be
triggered by CodeGen inspecting the AST.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140804/1509b355/attachment.html>