Skip to content

[lldb][ObjC] Don't query objective-c runtime for decls in C++ contexts #95963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Michael137
Copy link
Member

When LLDB isn't able to find a clang::Decl in response to a FindExternalVisibleDeclsByName, it will fall-back to looking into the Objective-C runtime for that decl. This ends up doing a lot of work which isn't necessary when we're debugging a C++ program. This patch makes the ObjC lookup conditional on the language that the ExpressionParser deduced (which can be explicitly set using the expr --language option or is set implicitly if we're stopped in an ObjC frame or a C++ frame without debug-info).

rdar://96236519

When LLDB isn't able to find a `clang::Decl` in response
to a `FindExternalVisibleDeclsByName`, it will fall-back
to looking into the Objective-C runtime for that decl. This
ends up doing a lot of work which isn't necessary when we're
debugging a C++ program. This patch makes the ObjC lookup
conditional on the language that the ExpressionParser deduced
(which can be explicitly set using the `expr --language` option
or is set implicitly if we're stopped in an ObjC frame or a
C++ frame without debug-info).

rdar://96236519
@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2024

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

When LLDB isn't able to find a clang::Decl in response to a FindExternalVisibleDeclsByName, it will fall-back to looking into the Objective-C runtime for that decl. This ends up doing a lot of work which isn't necessary when we're debugging a C++ program. This patch makes the ObjC lookup conditional on the language that the ExpressionParser deduced (which can be explicitly set using the expr --language option or is set implicitly if we're stopped in an ObjC frame or a C++ frame without debug-info).

rdar://96236519


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

3 Files Affected:

  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp (+1-1)
  • (modified) lldb/test/API/lang/objcxx/objc-from-cpp-frames-without-debuginfo/TestObjCFromCppFramesWithoutDebugInfo.py (+5)
  • (added) lldb/test/Shell/Expr/TestObjCInCXXContext.test (+21)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index 82a7a2cc3f1ef..1fdd272dcbece 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -637,7 +637,7 @@ void ClangASTSource::FindExternalVisibleDecls(
     FindDeclInModules(context, name);
   }
 
-  if (!context.m_found_type) {
+  if (!context.m_found_type && m_ast_context->getLangOpts().ObjC) {
     FindDeclInObjCRuntime(context, name);
   }
 }
diff --git a/lldb/test/API/lang/objcxx/objc-from-cpp-frames-without-debuginfo/TestObjCFromCppFramesWithoutDebugInfo.py b/lldb/test/API/lang/objcxx/objc-from-cpp-frames-without-debuginfo/TestObjCFromCppFramesWithoutDebugInfo.py
index ef8d5540fa4ef..2c2e072bba686 100644
--- a/lldb/test/API/lang/objcxx/objc-from-cpp-frames-without-debuginfo/TestObjCFromCppFramesWithoutDebugInfo.py
+++ b/lldb/test/API/lang/objcxx/objc-from-cpp-frames-without-debuginfo/TestObjCFromCppFramesWithoutDebugInfo.py
@@ -15,4 +15,9 @@ def test(self):
         (_, process, _, _) = lldbutil.run_to_name_breakpoint(self, "main")
 
         self.assertState(process.GetState(), lldb.eStateStopped)
+
+        # Tests that we can use builtin Objective-C identifiers.
         self.expect("expr id", error=False)
+
+        # Tests that we can lookup Objective-C decls in the ObjC runtime plugin.
+        self.expect_expr("NSString *c; c == nullptr", result_value="true", result_type="bool")
diff --git a/lldb/test/Shell/Expr/TestObjCInCXXContext.test b/lldb/test/Shell/Expr/TestObjCInCXXContext.test
new file mode 100644
index 0000000000000..8537799bdeb67
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestObjCInCXXContext.test
@@ -0,0 +1,21 @@
+// UNSUPPORTED: system-linux, system-windows
+
+// Tests that we don't consult the the Objective-C runtime
+// plugin when in a purely C++ context.
+//
+// RUN: %clangxx_host %p/Inputs/objc-cast.cpp -g -o %t
+// RUN: %lldb %t \
+// RUN:   -o "b main" -o run \
+// RUN:   -o "expression --language objective-c -- NSString * a; a" \
+// RUN:   -o "expression --language objective-c++ -- NSString * b; b" \
+// RUN:   -o "expression NSString" \
+// RUN:   2>&1 | FileCheck %s
+
+// CHECK:      (lldb) expression --language objective-c -- NSString * a; a
+// CHECK-NEXT: (NSString *){{.*}}= nil
+
+// CHECK:      (lldb) expression --language objective-c++ -- NSString * b; b
+// CHECK-NEXT: (NSString *){{.*}}= nil
+
+// CHECK:      (lldb) expression NSString
+// CHECK-NEXT: error:{{.*}} use of undeclared identifier 'NSString'

Copy link

github-actions bot commented Jun 18, 2024

✅ With the latest revision this PR passed the Python code formatter.

@Michael137 Michael137 merged commit dadf960 into llvm:main Jun 19, 2024
6 checks passed
Michael137 added a commit that referenced this pull request Jun 20, 2024
… contexts (#95963)"

This reverts commit dadf960.

The commit caused `TestEarlyProcessLaunch.py` to fail on the
macOS bots.
Michael137 added a commit that referenced this pull request Jun 21, 2024
… contexts"

This relands #95963. It had to
be reverted because the `TestEarlyProcessLaunch.py` test was failing
on the incremental macOS bots. The test failed because it was relying on
expression log output from the ObjC introspection routines (but was
the expression was called from a C++ context). The relanded patch
simply ensures that the test runs the expressions as `ObjC` expressions.

When LLDB isn't able to find a `clang::Decl` in response
to a `FindExternalVisibleDeclsByName`, it will fall-back
to looking into the Objective-C runtime for that decl. This
ends up doing a lot of work which isn't necessary when we're
debugging a C++ program. This patch makes the ObjC lookup
conditional on the language that the ExpressionParser deduced
(which can be explicitly set using the `expr --language` option
or is set implicitly if we're stopped in an ObjC frame or a
C++ frame without debug-info).

rdar://96236519
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
… contexts (llvm#95963)"

This reverts commit dadf960.

The commit caused `TestEarlyProcessLaunch.py` to fail on the
macOS bots.
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
… contexts"

This relands llvm#95963. It had to
be reverted because the `TestEarlyProcessLaunch.py` test was failing
on the incremental macOS bots. The test failed because it was relying on
expression log output from the ObjC introspection routines (but was
the expression was called from a C++ context). The relanded patch
simply ensures that the test runs the expressions as `ObjC` expressions.

When LLDB isn't able to find a `clang::Decl` in response
to a `FindExternalVisibleDeclsByName`, it will fall-back
to looking into the Objective-C runtime for that decl. This
ends up doing a lot of work which isn't necessary when we're
debugging a C++ program. This patch makes the ObjC lookup
conditional on the language that the ExpressionParser deduced
(which can be explicitly set using the `expr --language` option
or is set implicitly if we're stopped in an ObjC frame or a
C++ frame without debug-info).

rdar://96236519
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants