Skip to content

Conversation

@frederick-vs-ja
Copy link
Contributor

Since P2280R4 Unknown references and pointers was implemented, HandleLValueBase now has to deal with referneces:

D.MostDerivedType->getAsCXXRecordDecl()

will return a nullptr if D.MostDerivedType is a ReferenceType. The fix is to use getNonReferenceType() to obtain the Pointee Type if we have a reference.

(cherry picked from commit 136f2ba)

@frederick-vs-ja frederick-vs-ja added this to the LLVM 20.X Release milestone May 16, 2025
@frederick-vs-ja frederick-vs-ja added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 16, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status May 16, 2025
@llvmbot
Copy link
Member

llvmbot commented May 16, 2025

@llvm/pr-subscribers-clang

Author: A. Jiang (frederick-vs-ja)

Changes

Since P2280R4 Unknown references and pointers was implemented, HandleLValueBase now has to deal with referneces:

D.MostDerivedType->getAsCXXRecordDecl()

will return a nullptr if D.MostDerivedType is a ReferenceType. The fix is to use getNonReferenceType() to obtain the Pointee Type if we have a reference.

(cherry picked from commit 136f2ba)


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/lib/AST/ExprConstant.cpp (+5-1)
  • (modified) clang/test/SemaCXX/constant-expression-p2280r4.cpp (+21)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 47ef2f80ac3f2..2f43dc4021fd8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -909,6 +909,8 @@ Bug Fixes in This Version
   being deleted has a potentially throwing destructor (#GH118660).
 - Clang now outputs correct values when #embed data contains bytes with negative
   signed char values (#GH102798).
+- Fix crash due to unknown references and pointer implementation and handling of
+  base classes. (GH139452)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 23602362eaa79..e0746f4532245 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -3311,7 +3311,11 @@ static bool HandleLValueBase(EvalInfo &Info, const Expr *E, LValue &Obj,
     return false;
 
   // Extract most-derived object and corresponding type.
-  DerivedDecl = D.MostDerivedType->getAsCXXRecordDecl();
+  // FIXME: After implementing P2280R4 it became possible to get references
+  // here. We do MostDerivedType->getAsCXXRecordDecl() in several other
+  // locations and if we see crashes in those locations in the future
+  // it may make more sense to move this fix into Lvalue::set.
+  DerivedDecl = D.MostDerivedType.getNonReferenceType()->getAsCXXRecordDecl();
   if (!CastToDerivedClass(Info, E, Obj, DerivedDecl, D.MostDerivedPathLength))
     return false;
 
diff --git a/clang/test/SemaCXX/constant-expression-p2280r4.cpp b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
index 6c9a87267109c..87beeb4d3dc84 100644
--- a/clang/test/SemaCXX/constant-expression-p2280r4.cpp
+++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
@@ -179,3 +179,24 @@ namespace extern_reference_used_as_unknown {
   int y;
   constinit int& g = (x,y); // expected-warning {{left operand of comma operator has no effect}}
 }
+
+namespace GH139452 {
+struct Dummy {
+  explicit operator bool() const noexcept { return true; }
+};
+
+struct Base { int error; };
+struct Derived : virtual Base { };
+
+template <class R>
+constexpr R get_value() {
+    const auto& derived_val = Derived{};
+    if (derived_val.error != 0)
+        /* nothing */;
+    return R{};
+}
+
+int f() {
+    return !get_value<Dummy>(); // contextually convert the function call result to bool
+}
+}

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; seems like a conservative fix for the crash, should be safe for the branch.

@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Merge in LLVM Release Status May 16, 2025
@efriedma-quic
Copy link
Collaborator

How do we do release notes for things that are fixed on a release branch after the release? Do we put a release note for both 20 and 21?

Since P2280R4 Unknown references and pointers was implemented,
HandleLValueBase now has to deal with referneces:

D.MostDerivedType->getAsCXXRecordDecl()

will return a nullptr if D.MostDerivedType is a ReferenceType. The fix
is to use getNonReferenceType() to obtain the Pointee Type if we have a
reference.

Fixes: llvm#139452
(cherry picked from commit 136f2ba)

# Conflicts:
#	clang/docs/ReleaseNotes.rst
@tstellar tstellar merged commit 5befd1f into llvm:release/20.x May 16, 2025
8 of 11 checks passed
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status May 16, 2025
@github-actions
Copy link

@frederick-vs-ja (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

Development

Successfully merging this pull request may close these issues.

5 participants