> On Aug 4, 2014, at 4:17 PM, Richard Smith <[email protected]> wrote:
>
> On Mon, Aug 4, 2014 at 3:03 PM, Ben Langmuir <[email protected]
> <mailto:[email protected]>> 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.
Would it make more sense for me to just hoist+rename the guard for
PassInterestingDeclsToConsumer up and protect every function that we call here?
I.e.
if (NumCurrentElementsDeserializing == 0) {
//==> move recursion guard here
diagnoseOdrViolations();
// We are not in recursive loading, so it's safe to pass the "interesting"
// decls to the consumer.
if (Consumer)
PassInterestingDeclsToConsumer();
}
I don’t see any reason to allow diagnosing ODR violations while we are passing
interesting decls to the consumer, or vice-versa._______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
