Skip to content

Commit

Permalink
[Clang][Sema] Handle class member access expressions with valid neste…
Browse files Browse the repository at this point in the history
…d-name-specifiers that become invalid after lookup (llvm#98167)

The following code causes an assert in `SemaExprMember.cpp` on line 981
to fail:
```
struct A { };
struct B;

void f(A *x) {
  x->B::y; // crash here
}
```
This happens because we only return early from
`BuildMemberReferenceExpr` when the `CXXScopeSpecifier` is invalid
_before_ the lookup is performed. Since the lookup may invalidate the
`CXXScopeSpecifier` (e.g. if the _nested-name-specifier_ is incomplete),
this results in the second `BuildMemberReferenceExpr` overload being
called with an invalid `CXXScopeSpecifier`, which causes the assert to
fail. This patch moves the early return for invalid `CXXScopeSpecifiers`
to occur _after_ lookup is performed. This fixes llvm#92972.

I also removed the `if (SS.isSet() && SS.isInvalid())` check in
`ActOnMemberAccessExpr` because the condition can never be true (`isSet`
returns `getScopeRep() != nullptr` and `isInvalid` returns
`Range.isValid() && getScopeRep() == nullptr`).
  • Loading branch information
sdkrystian authored Jul 9, 2024
1 parent 10f3f06 commit 9e1f1cf
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 10 deletions.
17 changes: 7 additions & 10 deletions clang/lib/Sema/SemaExprMember.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -789,9 +789,6 @@ ExprResult Sema::BuildMemberReferenceExpr(
ActOnMemberAccessExtraArgs *ExtraArgs) {
LookupResult R(*this, NameInfo, LookupMemberName);

if (SS.isInvalid())
return ExprError();

// Implicit member accesses.
if (!Base) {
TypoExpr *TE = nullptr;
Expand Down Expand Up @@ -826,6 +823,11 @@ ExprResult Sema::BuildMemberReferenceExpr(
BaseType = Base->getType();
}

// BuildMemberReferenceExpr expects the nested-name-specifier, if any, to be
// valid.
if (SS.isInvalid())
return ExprError();

return BuildMemberReferenceExpr(Base, BaseType,
OpLoc, IsArrow, SS, TemplateKWLoc,
FirstQualifierInScope, R, TemplateArgs, S,
Expand Down Expand Up @@ -1745,14 +1747,9 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,

ExprResult Sema::ActOnMemberAccessExpr(Scope *S, Expr *Base,
SourceLocation OpLoc,
tok::TokenKind OpKind,
CXXScopeSpec &SS,
tok::TokenKind OpKind, CXXScopeSpec &SS,
SourceLocation TemplateKWLoc,
UnqualifiedId &Id,
Decl *ObjCImpDecl) {
if (SS.isSet() && SS.isInvalid())
return ExprError();

UnqualifiedId &Id, Decl *ObjCImpDecl) {
// Warn about the explicit constructor calls Microsoft extension.
if (getLangOpts().MicrosoftExt &&
Id.getKind() == UnqualifiedIdKind::IK_ConstructorName)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// RUN: %clang_cc1 -verify -Wno-unused %s

struct A {
int y;
};

struct B; // expected-note 4{{forward declaration of 'B'}}

void f(A *a, B *b) {
a->B::x; // expected-error {{incomplete type 'B' named in nested name specifier}}
a->A::x; // expected-error {{no member named 'x' in 'A'}}
a->A::y;
b->B::x; // expected-error {{member access into incomplete type 'B'}}
b->A::x; // expected-error {{member access into incomplete type 'B'}}
b->A::y; // expected-error {{member access into incomplete type 'B'}}
}

0 comments on commit 9e1f1cf

Please sign in to comment.