On Tue, Jun 30, 2015 at 4:57 PM, Justin Bogner <m...@justinbogner.com>
wrote:
> ping
>
> Justin Bogner <m...@justinbogner.com> writes:
> > If we hit an error already, we may have set Name to nullptr, which
> > means calling isAcceptableTagRedeclaration hits UB. Avoid this like we
> > do elsewhere in the function by checking Name first.
> >
> > We could also fix this by passing OrigName instead, since the name is
> > only used for diagnostics anyway. Should I do that instead, or is this
> > good to commit?
> >
> > commit 203946970eafb48a120b29dfac1612b8762b9115
> > Author: Justin Bogner <m...@justinbogner.com>
> > Date: Mon Jun 22 00:05:05 2015 -0700
> >
> > Sema: Fix some undefined behaviour when acting on redeclarations
> >
> > If we hit an error already, we may have set Name to nullptr, which
> > means calling isAcceptableTagRedeclaration hits UB. Avoid this like
> we
> > do elsewhere in the function by checking Name first.
> >
> > diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
> > index ce89d99..8c94460 100644
> > --- a/lib/Sema/SemaDecl.cpp
> > +++ b/lib/Sema/SemaDecl.cpp
> > @@ -11747,9 +11747,9 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec,
> TagUseKind TUK,
> > SS.isNotEmpty() || isExplicitSpecialization)) {
> > // Make sure that this wasn't declared as an enum and now used
> as a
> > // struct or something similar.
> > - if (!isAcceptableTagRedeclaration(PrevTagDecl, Kind,
> > - TUK == TUK_Definition, KWLoc,
> > - *Name)) {
> > + if (Name && !isAcceptableTagRedeclaration(PrevTagDecl, Kind,
> > + TUK ==
> TUK_Definition, KWLoc,
> > + *Name)) {
>
So, this is tricky: if SafeToContinue is false, we don't want to carry on
because we'd make an EnumDecl a redeclaration of a RecordDecl or vicer
versa, which would violate the AST invariants. I'm not sure how we get here
from error recovery: both places that set Name to nullptr also jump past
this code. I think the only way to reach this is if SkipBody->Previous is
non-null, in which case this is not error recovery and we should perform
this check.
Can you instead change isAcceptableTagRedeclaration to take a const
IdentifierInfo*?
Also, do you have a testcase? Is this already caught by our testsuite +
ubsan?
> bool SafeToContinue
> > = (PrevTagDecl->getTagKind() != TTK_Enum &&
> > Kind != TTK_Enum);
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
