Skip to content

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Sep 3, 2025

Backport a862225

Requested by: @Michael137

…types (llvm#156681)

This upstreams swiftlang#10313.

If we detect a situation where a forward declaration is C++ and the
definition DIE is Objective-C, then just don't try to complete the type
(it would crash otherwise). In the long term we might want to add
support for completing such types.

We've seen real world crashes when debugging WebKit and wxWidgets
because of this. Both projects forward declare ObjC++ decls in the way
shown in the test.

rdar://145959981
(cherry picked from commit a862225)
@llvmbot
Copy link
Member Author

llvmbot commented Sep 3, 2025

@labath What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Sep 3, 2025

@llvm/pr-subscribers-lldb

Author: None (llvmbot)

Changes

Backport a862225

Requested by: @Michael137


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

2 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+12)
  • (added) lldb/test/Shell/SymbolFile/DWARF/objcxx-forward-decls.test (+70)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index c76d67b47b336..8916c58beec0f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2201,6 +2201,18 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
       for (DelayedAddObjCClassProperty &property : delayed_properties)
         property.Finalize();
     }
+  } else if (Language::LanguageIsObjC(
+                 static_cast<LanguageType>(die.GetAttributeValueAsUnsigned(
+                     DW_AT_APPLE_runtime_class, eLanguageTypeUnknown)))) {
+    /// The forward declaration was C++ but the definition is Objective-C.
+    /// We currently don't handle such situations. In such cases, keep the
+    /// forward declaration without a definition to avoid violating Clang AST
+    /// invariants.
+    LLDB_LOG(GetLog(LLDBLog::Expressions),
+             "WARNING: Type completion aborted because forward declaration for "
+             "'{0}' is C++ while definition is Objective-C.",
+             llvm::StringRef(die.GetName()));
+    return {};
   }
 
   if (!bases.empty()) {
diff --git a/lldb/test/Shell/SymbolFile/DWARF/objcxx-forward-decls.test b/lldb/test/Shell/SymbolFile/DWARF/objcxx-forward-decls.test
new file mode 100644
index 0000000000000..30109c2943c9b
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/objcxx-forward-decls.test
@@ -0,0 +1,70 @@
+# REQUIRES: system-darwin
+
+# In this test we have two CUs with conflicting forward declaration
+# depending on the CU language (one is C++ and the other is Objective-C++).
+# We are then stopped in the C++ CU and try to print the type, at which
+# point LLDB will try to make it into an Clang AST node. If LLDB were to
+# interpret the type as C++ instead of Objective-C, we'd violate Clang
+# invariants and crash.
+#
+# RUN: split-file %s %t
+# RUN: %clangxx_host -c -g -x objective-c++ %t/request.m -o %t/request_objc.o
+# RUN: %clangxx_host -c -g %t/main.cpp -o %t/main.o
+# RUN: %clangxx_host %t/main.o %t/request_objc.o -framework Foundation -o %t/a.out
+#
+# RUN: %lldb %t/a.out \
+# RUN:    -o "breakpoint set -p return -X main" \
+# RUN:    -o run \
+# RUN:    -o "frame variable r" \
+# RUN:    -o exit | FileCheck %s
+#
+# RUN: dsymutil %t/a.out
+#
+# RUN: %lldb %t/a.out \
+# RUN:    -o "breakpoint set -p return -X main" \
+# RUN:    -o run \
+# RUN:    -o "frame variable r" \
+# RUN:    -o exit | FileCheck %s --check-prefix=CHECK-DSYM
+
+# CHECK:      (lldb) frame variable r
+# CHECK-NEXT: (Request) ::r = (m_request = "Hello, World!")
+
+# CHECK-DSYM:      (lldb) frame variable r
+# CHECK-DSYM-NEXT: (Request) ::r = (m_request = "Hello, World!")
+
+#--- lib.h
+#ifndef LIB_H_IN
+#define LIB_H_IN
+
+#ifdef __OBJC__
+@class NSString;                                               
+#else
+class NSString;
+#endif
+
+struct Request {
+  NSString * m_request = nullptr;
+};
+
+#endif // _H_IN
+
+#--- main.cpp
+#include "lib.h"
+
+void process(Request *);
+
+Request r;
+
+int main() {
+    process(&r);
+    return 0;
+}
+
+#--- request.m
+#import <Foundation/Foundation.h>
+
+#include "lib.h"
+
+void process(Request * r) {
+  r->m_request = @"Hello, World!";
+}

@Michael137
Copy link
Member

@tru any idea why automation hasn't moved this to Needs Merge? Or did the job just not run yet or something like that?

@tru
Copy link
Collaborator

tru commented Sep 5, 2025

@Michael137 we are waiting for someone to approve the PR, then it moves to "needs merge". We usually want someone else than the requester to approve the PR. In this case @labath was suggested above.

@tru tru moved this from Needs Triage to Needs Review in LLVM Release Status Sep 5, 2025
@Michael137
Copy link
Member

@Michael137 we are waiting for someone to approve the PR, then it moves to "needs merge". We usually want someone else than the requester to approve the PR. In this case @labath was suggested above.

Oh whoops, I thought @labath hit the review button. Makes sense!

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

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

Pavel may be OOO. His opinion should take precedence over mine, but in his absence this LGTM.

@github-project-automation github-project-automation bot moved this from Needs Review to Needs Merge in LLVM Release Status Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Needs Merge
Development

Successfully merging this pull request may close these issues.

4 participants