[clang][AST] fix dereference on class/struct layouts check.

[devnexen]

close #83671.

[llvmbot]

@llvm/pr-subscribers-clang

Author: David CARLIER (devnexen)

Changes

close #83671.

---

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

1 Files Affected:

• (modified) clang/lib/AST/RecordLayoutBuilder.cpp (+3-3)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index a3b7431f7ffd6d..195f17d2e5a42f 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -205,15 +205,15 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
 
   // Check the fields.
   for (const FieldDecl *FD : Class->fields()) {
+    const CXXRecordDecl *MemberDecl;
     const RecordType *RT =
         Context.getBaseElementType(FD->getType())->getAs<RecordType>();
 
-    // We only care about record types.
-    if (!RT)
+    // We only care about members layout.
+    if (!RT || !(MemberDecl = RT->getAsCXXRecordDecl()))
       continue;
 
     CharUnits EmptySize;
-    const CXXRecordDecl *MemberDecl = RT->getAsCXXRecordDecl();
     const ASTRecordLayout &Layout = Context.getASTRecordLayout(MemberDecl);
     if (MemberDecl->isEmpty()) {
       // If the class decl is empty, get its size.

[shafik]

Thank you for the PR.

We need a minimal reproducer here. Looking at the bug report it is not clear to me if this is the correct fix or not. After we have a reproducer we would need a test added to the PR and a release note.

The summary also needs to have more details. This is what usually goes into the git log and it should at least 1) summarize the issue and cause 2) summarize the approach to the fix

[bjrjk]

Hello, this is a mininal reproducer execute with clang++ -Xclang -fdump-record-layouts-complete test.cpp:

template <class _Tp, _Tp __v>
struct integral_constant {
  static constexpr const _Tp value = __v;
  typedef integral_constant type;
};

template <bool _Val>
using _BoolConstant = integral_constant<bool, _Val>;

template <class _Tp, class _Up>
struct is_same : _BoolConstant<__is_same(_Tp, _Up)> {};

[devnexen]

it seems there is an ongoing fix, could you possibly try so we can just close this one.

[Endilll]

I posted a different, even shorter reduction in #83671 (comment).

[Sirraide]

it seems there is an ongoing fix, could you possibly try so we can just close this one.

Yeah, it seems that the pr I opened for #83684 also fixes both the reduced test cases for this issue (and it also doesn’t crash anymore if I try to compile the file that was linked in the issue), though it should be noted that that’s only after I noticed that there was another problem and fixed that too.

I think we should be able to close this one, unless someone comes up with a test case that still crashes anyway.