This is an archive of the discontinued LLVM Phabricator instance.

[lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms
ClosedPublic

Authored by sgraenitz on Mar 15 2023, 10:34 AM.

Details

Summary

This is the next patch after D146058. We can now parse expressions to print instance variables from ObjC classes. Until now the expression parser would bail out with an error like this:

error: expression failed to parse:
error: Error [IRForTarget]: Couldn't find Objective-C indirect ivar symbol OBJC_IVAR_$_TestObj._int

Diff Detail

Event Timeline

sgraenitz created this revision.Mar 15 2023, 10:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2023, 10:34 AM
sgraenitz requested review of this revision.Mar 15 2023, 10:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2023, 10:34 AM
theraven added inline comments.Mar 15 2023, 10:44 AM
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
43

I'm not familiar with lldb internals, will this exclude ObjC++?

48

I never finished the v2 ABI support for Mach-O, but v1 works on Apple platforms and v2 will eventually. We definitely shouldn't be the default here, but it would be nice not to prevent it working on macOS.

132

I'm not sure how this is meant to work. I'd expect this to call one of the runtime's introspection functions. I believe the Apple version has variants of these that run without acquiring locks, which can be used in the debugger. We can add these quite easily if necessary.

lldb/test/Shell/Expr/objc-gnustep-print.m
84

It's not clear from this test if it's correctly computing the offsets. I suspect that it isn't, since I don't see any reference to the ivar offset variables.

sgraenitz added inline comments.Mar 15 2023, 10:44 AM
lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
253

This was always declared in the header, but never implemented. Instead, AppleObjCRuntime had its own implementation -- I could make a follow-up patch to reuse it there as well:
https://github.com/llvm/llvm-project/blob/release/16.x/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp#L609-L622

lldb/test/Shell/Expr/objc-gnustep-print.m
72–78

This is missing the newly added members. I will fix it in the course of this review.

sgraenitz marked 3 inline comments as done.Mar 15 2023, 11:06 AM

Hi David, thanks for your notes. Please find responses inline.

lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
43

Good question. There is also eLanguageTypeObjC_plus_plus, but the plugin scanner e.g. seems to use eLanguageTypeObjC for both. I'd like to keep it like that until I actually start testing ObjC++.

48

For the moment it would be nice to stay away from potential conflicts, since they won't make reviews easier. We can still add this later on if there is demand.

132

The Apple V2 runtime has two ways to do it: https://github.com/llvm/llvm-project/blob/release/16.x/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp#L1179 I experimented with the one that doesn't require void *gdb_class_getClass(void*) from the runtime library, but the JIT kept on crashing. So, for this first version I opted for a minimal implementation that doesn't do any selector checks. Let's look at it in another review.

lldb/test/Shell/Expr/objc-gnustep-print.m
84

What exactly would you expect to see in the output?

aprantl accepted this revision.Mar 15 2023, 12:32 PM

In general I don't think I have a problem with adding this, especially since Clang also supports the runtime as a compilation target, and LLDB is tightly integrated with Clang.

This revision is now accepted and ready to land.Mar 15 2023, 12:32 PM
aprantl requested changes to this revision.Mar 15 2023, 12:36 PM

One thing I just realized — we need to make sure that we don't accidentally create a GNUstep ObjC runtime in a Swift process that was built without ObjC support on Linux. How can we ensure this works for both cases?

This revision now requires changes to proceed.Mar 15 2023, 12:36 PM

I.e., can you detect based on the presence of a symbol or shared object that an GNUstep runtime is present?

theraven added inline comments.Mar 16 2023, 2:20 AM
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
132

Can you open bugs against the runtime for the gdb_* symbols that we need? I will add them.

The other method will crash on small objects because they don't have an isa pointer, their isa is in the low 1-3 bits of the object pointer, indexing into a table in the runtime.

lldb/test/Shell/Expr/objc-gnustep-print.m
84

If the offsets are all wrong, this will still work if it happens to land within zeroed memory, which is quite likely. It would be good to have a method that set all of the ivars to something and then check for those values explicitly. Or check for (char*)t->{ivar} - (char*)t and make sure that this is the right value (this will change between 32/64/128-bit platforms).

sgraenitz marked 3 inline comments as done.Mar 16 2023, 2:22 AM

One thing I just realized — we need to make sure that we don't accidentally create a GNUstep ObjC runtime in a Swift process that was built without ObjC support on Linux.

Yes, thanks for bringing this up. The goal definitely is to avoid any accidental conflicts with existing use cases that don't need or expect a GNUstep runtime. I really want to get my focus to the Windows side and PDB parsing. It's useful to have Linux working as well, so that we have a testable feature set to match. Otherwise, we don't want to invest a lot of effort here yet.

How can we ensure this works for both cases?

Shouldn't the Swift processes report eLanguageTypeSwift? Then GNUstepObjCRuntime::CreateInstance() rejects it reliably.

I.e., can you detect based on the presence of a symbol or shared object that an GNUstep runtime is present?

Are there existing cases that follow such an approach? Looking at the order of events here, it appears that we have to wait for ModulesDidLoad() to report the shared library before we can inspect its symbols. How would we proceed if we want to create the language runtime first? I.e. here https://github.com/llvm/llvm-project/blob/release/16.x/lldb/source/Target/Process.cpp#L5727-L5732

The shared library has a GNUstep-specific EH personality for example, would that do?

> llvm-nm libobjc2/build/libobjc.so | grep gnustep_objc
00000000000264c0 T __gnustep_objc_personality_v0
0000000000026500 T __gnustep_objcxx_personality_v0
sgraenitz updated this revision to Diff 505760.Mar 16 2023, 3:40 AM
sgraenitz marked 3 inline comments as done.

Extend the test to check both, zero initialization and assigned ivars

lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
132
lldb/test/Shell/Expr/objc-gnustep-print.m
84

Ok, updated the test. I only checked on 64-bit, but it should support arbitrary pointer widths. Can we skip the test for ivar offset distance? I'd like to avoid testing the actual memory layout here.

The test looks fine to me, I'm quite surprised that it passes.

One thing I just realized — we need to make sure that we don't accidentally create a GNUstep ObjC runtime in a Swift process that was built without ObjC support on Linux.

Yes, thanks for bringing this up. The goal definitely is to avoid any accidental conflicts with existing use cases that don't need or expect a GNUstep runtime. I really want to get my focus to the Windows side and PDB parsing. It's useful to have Linux working as well, so that we have a testable feature set to match. Otherwise, we don't want to invest a lot of effort here yet.

How can we ensure this works for both cases?

Shouldn't the Swift processes report eLanguageTypeSwift? Then GNUstepObjCRuntime::CreateInstance() rejects it reliably.

I'm not sure where eLanguageType being passed in here comes from. Generally for Swift processes with (Apple) Objective-C interoperability enabled, it is the expected case to have both a Swift and an Objective-C runtime in the same process.

I.e., can you detect based on the presence of a symbol or shared object that an GNUstep runtime is present?

Are there existing cases that follow such an approach? Looking at the order of events here, it appears that we have to wait for ModulesDidLoad() to report the shared library before we can inspect its symbols. How would we proceed if we want to create the language runtime first? I.e. here https://github.com/llvm/llvm-project/blob/release/16.x/lldb/source/Target/Process.cpp#L5727-L5732

The shared library has a GNUstep-specific EH personality for example, would that do?

> llvm-nm libobjc2/build/libobjc.so | grep gnustep_objc
00000000000264c0 T __gnustep_objc_personality_v0
0000000000026500 T __gnustep_objcxx_personality_v0

I think that would be a great way to guard against false positives!

Here is an example of how the Swift Runtime plugin detects both itself and whether ObjC interop is enabled. Basically all I'm asking is to not accidentally break this mechanism.
https://github.com/apple/llvm-project/blob/7750ffa35111df6d38a5c73ab7e78ccebc9b43c3/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp#L124
If you want to make 100% sure, you could checkout the Swift compiler on Linux (or I suppose Windows) from github.com/apple/swift run swift/utils/update-checkout --clone apply your patch to llvm-project and run swift/utils/build-script --lldb --test -- --skip-build-benchmarks --skip-test-cmark --skip-test-swift and make sure the lldb tests still pass after applying you patch. But if you check for the symbol that would be sufficient for me.

sgraenitz updated this revision to Diff 506989.Mar 21 2023, 8:15 AM

Check for GNUstep EH personality in ELF targets and __objc_load on native Windows

Here is an example of how the Swift Runtime plugin detects both itself and whether ObjC interop is enabled. Basically all I'm asking is to not accidentally break this mechanism.

Thanks for the link! I think Swift is doing the exact right thing here: it requests the Apple ObjC V2 runtime explicitly in https://github.com/apple/llvm-project/blob/fcf8f57c74defcc0c422331cb90a3b9dd8d7b476/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp#L102-L106

Anyway, I updated my patch to do the symbol lookup, because the pure presence of an ObjC runtime makes for some interesting regressions in the test suite. E.g. here we enable ObjC in C++ code if the runtime exists:

lang_opts.ObjC =
    process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC) != nullptr;
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
63

There is no EH personality on Windows so I am checking for a core ObjC runtime symbol. Hope that's ok?
I put a list with all DLL symbols here: https://gist.github.com/weliveindetail/0c57135741b314d2952236bcffae0ab1

aprantl accepted this revision.Mar 21 2023, 9:44 AM

Thanks for adding the check!

lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
186

Do you have access to the target triple here? Might want to only check for the strings based on the the OS?

This revision is now accepted and ready to land.Mar 21 2023, 9:44 AM
sgraenitz marked an inline comment as done.

Make shared library name checks OS-dependent

Thanks for reviewing! I will land this as soon as the previous review is ready.