-
Notifications
You must be signed in to change notification settings - Fork 15k
[lldb][ObjC] Consult Objective-C runtime decl vendor when completing type #164011
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
Conversation
…type (Note, this upstreams code that has been deployed on Apple's Swift LLDB for many years at this point). When a `ValueObject` computes its "complete type" (`MaybeCalculateCompleteType`), it gives the language runtimes a chance to override the type known to it. The current implementation of `ObjCLanguageRuntime::GetRuntimeType`, however, didn't consult the `AppleObjCDeclVendor` to look for types. As demonstrated in the attached test, when we don't have debug-info for a base class type (most commonly happens when inheriting from system framework types) we would not be able to deduce ivars of that type. However, the runtime knows about the ivars, so we should be able to retrieve them. There's still a couple of caveats for future follow-up/investigation: 1. `frame var` isn't able to access such backing ivars explicitly (even if they do exist) 2. When compiling with `-gmodules`, LLDB gets confused about what is correct source of information for these decls is. rdar://162069497
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes(Note, this upstreams code that has been deployed on Apple's Swift LLDB for many years at this point). When a As demonstrated in the attached test, when we don't have debug-info for a base class type (most commonly happens when inheriting from system framework types) we would not be able to deduce ivars of that type. However, the runtime knows about the ivars, so we should be able to retrieve them. There's still a couple of caveats for future follow-up/investigation:
rdar://162069497 Full diff: https://github.com/llvm/llvm-project/pull/164011.diff 7 Files Affected:
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
index c33760eccd127..2b2ca080c7f55 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
@@ -423,6 +423,46 @@ Status ObjCLanguageRuntime::ObjCExceptionPrecondition::ConfigurePrecondition(
return error;
}
+CompilerType ObjCLanguageRuntime::LookupInModulesVendor(ConstString class_name,
+ Target &target) {
+ assert(class_name);
+
+ auto *persistent_state = llvm::cast<ClangPersistentVariables>(
+ target.GetPersistentExpressionStateForLanguage(lldb::eLanguageTypeC));
+ if (!persistent_state)
+ return {};
+
+ auto clang_modules_decl_vendor_sp =
+ persistent_state->GetClangModulesDeclVendor();
+ if (!clang_modules_decl_vendor_sp)
+ return {};
+
+ auto types = clang_modules_decl_vendor_sp->FindTypes(
+ class_name, /*max_matches*/ UINT32_MAX);
+ if (types.empty())
+ return {};
+
+ return types.front();
+}
+
+CompilerType ObjCLanguageRuntime::LookupInRuntime(ConstString class_name) {
+ auto *runtime_vendor = GetDeclVendor();
+ if (!runtime_vendor)
+ return {};
+
+ std::vector<CompilerDecl> compiler_decls;
+ runtime_vendor->FindDecls(class_name, false, UINT32_MAX, compiler_decls);
+ if (compiler_decls.empty())
+ return {};
+
+ auto *ctx =
+ llvm::dyn_cast<TypeSystemClang>(compiler_decls[0].GetTypeSystem());
+ if (!ctx)
+ return {};
+
+ return ctx->GetTypeForDecl(compiler_decls[0].GetOpaqueDecl());
+}
+
std::optional<CompilerType>
ObjCLanguageRuntime::GetRuntimeType(CompilerType base_type) {
CompilerType class_type;
@@ -442,18 +482,21 @@ ObjCLanguageRuntime::GetRuntimeType(CompilerType base_type) {
if (!class_name)
return std::nullopt;
- TypeSP complete_objc_class_type_sp = LookupInCompleteClassCache(class_name);
- if (!complete_objc_class_type_sp)
- return std::nullopt;
-
- CompilerType complete_class(
- complete_objc_class_type_sp->GetFullCompilerType());
- if (complete_class.GetCompleteType()) {
- if (is_pointer_type)
- return complete_class.GetPointerType();
- else
- return complete_class;
+ if (TypeSP complete_objc_class_type_sp =
+ LookupInCompleteClassCache(class_name)) {
+ if (CompilerType complete_class =
+ complete_objc_class_type_sp->GetFullCompilerType();
+ complete_class.GetCompleteType())
+ return is_pointer_type ? complete_class.GetPointerType() : complete_class;
}
+ assert(m_process);
+ if (CompilerType found =
+ LookupInModulesVendor(class_name, m_process->GetTarget()))
+ return is_pointer_type ? found.GetPointerType() : found;
+
+ if (CompilerType found = LookupInRuntime(class_name))
+ return is_pointer_type ? found.GetPointerType() : found;
+
return std::nullopt;
}
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
index 45de098c15f51..cc8281e9d1a02 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -465,6 +465,10 @@ class ObjCLanguageRuntime : public LanguageRuntime {
ObjCLanguageRuntime(const ObjCLanguageRuntime &) = delete;
const ObjCLanguageRuntime &operator=(const ObjCLanguageRuntime &) = delete;
+
+private:
+ CompilerType LookupInRuntime(ConstString class_name);
+ CompilerType LookupInModulesVendor(ConstString class_name, Target &process);
};
} // namespace lldb_private
diff --git a/lldb/test/API/lang/objc/ivar-in-framework-base/Makefile b/lldb/test/API/lang/objc/ivar-in-framework-base/Makefile
new file mode 100644
index 0000000000000..c7947fc138c18
--- /dev/null
+++ b/lldb/test/API/lang/objc/ivar-in-framework-base/Makefile
@@ -0,0 +1,6 @@
+OBJC_SOURCES := main.m lib.m
+LD_EXTRAS = -framework Foundation
+
+include Makefile.rules
+
+lib.o: CFLAGS = $(CFLAGS_NO_DEBUG)
diff --git a/lldb/test/API/lang/objc/ivar-in-framework-base/TestIvarInFrameworkBase.py b/lldb/test/API/lang/objc/ivar-in-framework-base/TestIvarInFrameworkBase.py
new file mode 100644
index 0000000000000..40fc6b7a68995
--- /dev/null
+++ b/lldb/test/API/lang/objc/ivar-in-framework-base/TestIvarInFrameworkBase.py
@@ -0,0 +1,39 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestIvarInFrameworkBase(TestBase):
+ """
+ Tests whether LLDB's data inspection commands can correctly retrieve
+ information about ivars from the Objective-C runtime.
+ In this test-case we have a base class type for which we don't have access
+ to the debug-info of the implementation (mimicking the scenario of subclassing
+ a type from a system framework). LLDB won't be able to see the backing ivar for
+ 'fooProp' from just debug-info, but it will fall back on the runtime to get the
+ necessary information.
+ """
+
+ def test_frame_var(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.m"))
+ self.expect("frame variable *bar", substrs=["_fooProp = 10", "_barProp = 15"])
+
+ def test_expr(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.m"))
+ self.expect_expr(
+ "*bar",
+ result_type="Bar",
+ result_children=[
+ ValueCheck(
+ name="Foo",
+ children=[
+ ValueCheck(name="NSObject"),
+ ValueCheck(name="_fooProp", value="10"),
+ ],
+ ),
+ ValueCheck(name="_barProp", value="15"),
+ ],
+ )
diff --git a/lldb/test/API/lang/objc/ivar-in-framework-base/lib.h b/lldb/test/API/lang/objc/ivar-in-framework-base/lib.h
new file mode 100644
index 0000000000000..31ceb53dc6885
--- /dev/null
+++ b/lldb/test/API/lang/objc/ivar-in-framework-base/lib.h
@@ -0,0 +1,6 @@
+#import <Foundation/Foundation.h>
+
+@interface Foo : NSObject
+@property int fooProp;
+- (id)init;
+@end
diff --git a/lldb/test/API/lang/objc/ivar-in-framework-base/lib.m b/lldb/test/API/lang/objc/ivar-in-framework-base/lib.m
new file mode 100644
index 0000000000000..e1bf80ac4bd4c
--- /dev/null
+++ b/lldb/test/API/lang/objc/ivar-in-framework-base/lib.m
@@ -0,0 +1,8 @@
+#import "lib.h"
+
+@implementation Foo
+- (id)init {
+ self.fooProp = 10;
+ return self;
+}
+@end
diff --git a/lldb/test/API/lang/objc/ivar-in-framework-base/main.m b/lldb/test/API/lang/objc/ivar-in-framework-base/main.m
new file mode 100644
index 0000000000000..1fd352ec92c54
--- /dev/null
+++ b/lldb/test/API/lang/objc/ivar-in-framework-base/main.m
@@ -0,0 +1,22 @@
+#import "lib.h"
+#include <stdio.h>
+
+@interface Bar : Foo
+@property int barProp;
+- (id)init;
+@end
+
+@implementation Bar
+
+- (id)init {
+ self = [super init];
+ self.barProp = 15;
+ return self;
+}
+@end
+
+int main() {
+ Bar *bar = [Bar new];
+ puts("break here");
+ return 0;
+}
|
| return {}; | ||
|
|
||
| auto types = clang_modules_decl_vendor_sp->FindTypes( | ||
| class_name, /*max_matches*/ UINT32_MAX); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why search for UINT32_MAX types if we only return the first one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now just wanted to upstream the version we had. But yea I agree using a UINT32_MAX is not ideal. Can change it in a follow-up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it's the search for UINT32_MAX types that is the problem, but rather the not handling the case where you got more than one type back. If there's more than one, how do you know the first one is the right one?
It's fine to do this as a followup, since this code has been working on the swift fork for a while now. But that's the thing I'd fix, not preemptively ignoring multiple definitions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM too.
(Note, this upstreams code that has been deployed on Apple's Swift LLDB for many years at this point).
When a
ValueObjectcomputes its "complete type" (MaybeCalculateCompleteType), it gives the language runtimes a chance to override the type known to it. The current implementation ofObjCLanguageRuntime::GetRuntimeType, however, didn't consult theAppleObjCDeclVendorto look for types.As demonstrated in the attached test, when we don't have debug-info for a base class type (most commonly happens when inheriting from system framework types) we would not be able to deduce ivars of that type. However, the runtime knows about the ivars, so we should be able to retrieve them.
There's still a couple of caveats for future follow-up/investigation:
frame varisn't able to access such backing ivars explicitly (even if they do exist)-gmodules, LLDB gets confused about what is correct source of information for these decls is.rdar://162069497