[clang] Fix crash when compiling error with invalid decl

[jcsxky]

APValue::LValueBase::LValueBase constructs ValueDecl with its canonicalDecl, even though it's invalid. And when obtain its type, it also check all redecls and ignore checking if it's valid. This will cause crash in invalid code. This patch fixes this issue by checking its validation and skip invalid decls.
Fix issue

[llvmbot]

@llvm/pr-subscribers-clang

Author: Qizhi Hu (jcsxky)

Changes

APValue::LValueBase::LValueBase constructs ValueDecl with its canonicalDecl, even though it's invalid. And when obtain its type, it also check all redecls and ignore checking if it's valid. This will cause crash in invalid code. This patch fixes this issue by checking its validation and skip invalid decls.
Fix issue

---

Full diff: https://github.com/llvm/llvm-project/pull/77893.diff

2 Files Affected:

• (modified) clang/lib/AST/APValue.cpp (+6-2)
• (added) clang/test/AST/invalid-decl-no-crash.cpp (+12)

diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp index 4eae308ef5b34c..b1fd93b23f38cc 100644 --- a/clang/lib/AST/APValue.cpp +++ b/clang/lib/AST/APValue.cpp @@ -40,7 +40,11 @@ static_assert( "Type is insufficiently aligned"); APValue::LValueBase::LValueBase(const ValueDecl *P, unsigned I, unsigned V) - : Ptr(P ? cast<ValueDecl>(P->getCanonicalDecl()) : nullptr), Local{I, V} {} + : Ptr(P ? cast<ValueDecl>(P->getCanonicalDecl()->isInvalidDecl() + ? P + : P->getCanonicalDecl()) + : nullptr), + Local{I, V} {} APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V) : Ptr(P), Local{I, V} {} @@ -73,7 +77,7 @@ QualType APValue::LValueBase::getType() const { for (auto *Redecl = cast<ValueDecl>(D->getMostRecentDecl()); Redecl; Redecl = cast_or_null<ValueDecl>(Redecl->getPreviousDecl())) { QualType T = Redecl->getType(); - if (!T->isIncompleteArrayType()) + if (!T->isIncompleteArrayType() && !Redecl->isInvalidDecl()) return T; } return D->getType(); diff --git a/clang/test/AST/invalid-decl-no-crash.cpp b/clang/test/AST/invalid-decl-no-crash.cpp new file mode 100644 index 00000000000000..2b35ef702ae553 --- /dev/null +++ b/clang/test/AST/invalid-decl-no-crash.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -fsyntax-only %s -verify + +a[i] = b[i]; // expected-error {{use of undeclared identifier 'i'}} \ + expected-error {{a type specifier is required for all declarations}} \ + expected-error {{use of undeclared identifier 'b'}} \ + expected-error {{use of undeclared identifier 'i'}} +extern char b[]; +extern char a[]; + +void foo(int j) { + a[j] = b[j]; +} 

[shafik]

Not clear to me why we check isInvalidDecl() here

[erichkeane]

This part of the change doesn't really make sense to me either, it isn't clear why the invalid decl is assumed to have a not-invalid-canonical decl, thats an incorrect assumption.