Skip to content

[lldb][test] Add test-coverage for DW_AT_APPLE_objc_complete_type parsing #120279

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
merged 2 commits into from
Dec 20, 2024

Conversation

Michael137
Copy link
Member

@Michael137 Michael137 commented Dec 17, 2024

When given a DIE for an Objective-C interface (which doesn't have a DW_AT_APPLE_objc_complete_type), the DWARFASTParserClang will try to find the DIE which corresponds to the implementation to complete the interface DIE. The code is here:

if ((attrs.class_language == eLanguageTypeObjC ||
attrs.class_language == eLanguageTypeObjC_plus_plus) &&
!attrs.is_complete_objc_class &&
die.Supports_DW_AT_APPLE_objc_complete_type()) {
// We have a valid eSymbolTypeObjCClass class symbol whose name
// matches the current objective C class that we are trying to find
// and this DIE isn't the complete definition (we checked
// is_complete_objc_class above and know it is false), so the real
// definition is in here somewhere
TypeSP type_sp =
dwarf->FindCompleteObjCDefinitionTypeForDIE(die, attrs.name, true);
if (!type_sp) {
SymbolFileDWARFDebugMap *debug_map_symfile = dwarf->GetDebugMapSymfile();
if (debug_map_symfile) {
// We weren't able to find a full declaration in this DWARF,
// see if we have a declaration anywhere else...
type_sp = debug_map_symfile->FindCompleteObjCDefinitionTypeForDIE(
die, attrs.name, true);
}
}

However, this was currently not exercised in our test-suite (removing the code above didn't fail any LLDB test).

This patch adds a test which exercises this codepath (it will fail if we don't fetch the implementation DIE in the DWARFASTParserClang).

Something that's not currently clear to me is why frame var *f succeeds even without the DW_AT_APPLE_objc_complete_type infrastructure. If it's using the ObjC runtime, we should make expr do the same, in which case we can remove this code from DWARFASTParserClang.

…sing

When given a DIE for an Objective-C interface (which doesn't have
a `DW_AT_APPLE_objc_complete_type`), the `DWARFASTParserClang` will
try to find the DIE which corresponds to the implementation to complete
the interface DIE. The code is here:
https://github.com/llvm/llvm-project/blob/d2e7ee77d33e8b3be3b1d4e9bc5bc4c60b62b554/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L1718-L1738

However, this was currently not exercised in our test-suite (removing
the code above didn't fail any LLDB test).

This patch adds a test which exercises this codepath (it will fail
if we don't fetch the implementation DIE in the `DWARFASTParserClang`).

Something that's not currently clear to me is why `frame var *f`
succeeds even without the `DW_AT_APPLE_objc_complete_type`
infrastructure. If it's using the ObjC runtime, which should make
`expr` do the same, in which case we can remove this code from
`DWARFASTParserClang`.
@llvmbot
Copy link
Member

llvmbot commented Dec 17, 2024

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

When given a DIE for an Objective-C interface (which doesn't have a DW_AT_APPLE_objc_complete_type), the DWARFASTParserClang will try to find the DIE which corresponds to the implementation to complete the interface DIE. The code is here:

if ((attrs.class_language == eLanguageTypeObjC ||
attrs.class_language == eLanguageTypeObjC_plus_plus) &&
!attrs.is_complete_objc_class &&
die.Supports_DW_AT_APPLE_objc_complete_type()) {
// We have a valid eSymbolTypeObjCClass class symbol whose name
// matches the current objective C class that we are trying to find
// and this DIE isn't the complete definition (we checked
// is_complete_objc_class above and know it is false), so the real
// definition is in here somewhere
TypeSP type_sp =
dwarf->FindCompleteObjCDefinitionTypeForDIE(die, attrs.name, true);
if (!type_sp) {
SymbolFileDWARFDebugMap *debug_map_symfile = dwarf->GetDebugMapSymfile();
if (debug_map_symfile) {
// We weren't able to find a full declaration in this DWARF,
// see if we have a declaration anywhere else...
type_sp = debug_map_symfile->FindCompleteObjCDefinitionTypeForDIE(
die, attrs.name, true);
}
}

However, this was currently not exercised in our test-suite (removing the code above didn't fail any LLDB test).

This patch adds a test which exercises this codepath (it will fail if we don't fetch the implementation DIE in the DWARFASTParserClang).

Something that's not currently clear to me is why frame var *f succeeds even without the DW_AT_APPLE_objc_complete_type infrastructure. If it's using the ObjC runtime, which should make expr do the same, in which case we can remove this code from DWARFASTParserClang.


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

1 Files Affected:

  • (added) lldb/test/Shell/Expr/TestObjCHiddenIvars.test (+63)
diff --git a/lldb/test/Shell/Expr/TestObjCHiddenIvars.test b/lldb/test/Shell/Expr/TestObjCHiddenIvars.test
new file mode 100644
index 00000000000000..724a17fb7e2b8c
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestObjCHiddenIvars.test
@@ -0,0 +1,63 @@
+# Tests that LLDB correctly finds the implementation
+# DIE (with a `DW_AT_APPLE_objc_complete_type`)
+# given an interface DIE (without said attribute).
+#
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/lib.m  -c -g -o %t/lib.o
+# RUN: %clangxx_host %t/main.m -c -g -o %t/main.o
+# RUN: %clangxx_host %t/main.o %t/lib.o -o %t/a.out -framework Foundation
+#
+# RUN: %lldb %t/a.out \
+# RUN:       -o "breakpoint set -p 'return' -X main" \
+# RUN:       -o run \
+# RUN:       -o "expression *f" \
+# RUN:       -o exit | FileCheck %s
+
+# CHECK:      (lldb) expression *f
+# CHECK:      (Foo) ${{[0-9]+}} = {
+# CHECK:         y = 2 
+# CHECK-NEXT:    i = 1 
+
+#--- main.m
+#import <Foundation/Foundation.h>
+#import "lib.h"
+
+extern Foo * func();
+
+int main() {
+    Foo * f = func();
+    return 0;
+}
+
+#--- lib.m
+#import <Foundation/Foundation.h>
+#import "lib.h"
+
+@implementation Foo {
+int i;
+}
+
+- (id)init {
+  self->i = 1;
+  self->y = 2;
+
+  return self;
+}
+@end
+
+Foo * func() {
+  return [[Foo alloc] init];
+}
+
+#--- lib.h
+#ifndef LIB_H_IN
+#define LIB_H_IN
+
+#import <Foundation/Foundation.h>
+
+@interface Foo : NSObject {
+int y;
+}
+@end
+
+#endif  // _H_IN

@Michael137
Copy link
Member Author

TestHiddenIvars.py looks like it intended to test the same codepath, but it passes cleanly with and without the calls to FindCompleteObjCDefinitionTypeForDIE

Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

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

Something that's not currently clear to me is why frame var *f succeeds even without the DW_AT_APPLE_objc_complete_type infrastructure. If it's using the ObjC runtime, which should make expr do the same, in which case we can remove this code from DWARFASTParserClang.

I don't know, but I'm all for deleting code :D

# RUN: split-file %s %t
# RUN: %clangxx_host %t/lib.m -c -g -o %t/lib.o
# RUN: %clangxx_host %t/main.m -c -g -o %t/main.o
# RUN: %clangxx_host %t/main.o %t/lib.o -o %t/a.out -framework Foundation
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this needs a REQUIRES: system-darwin.

For science, I also tried to make a version of this test which could run on other systems, but all I managed is to hit an (lldb)assert:

$ cat a.m 
__attribute__((objc_root_class)) @interface  Foo  {
int y;
}
@end

#ifdef DEF
@implementation Foo {
    int i;
}

- (id)init {
  self->i = 1;
  self->y = 2;

  return self;
}
@end
#endif

Foo *foo;
$ clang -c a.m -g -DDEF --target=aarch64-apple-macosx
$ lldb a.o -o "target variable foo[0]"
(lldb) target create "a.o"
Current executable set to '/tmp/a.o' (arm64).
(lldb) target variable foo[0]
warning: trying to determine the size of type @interface Foo{
    int y;
    int i;
}
@end
without a valid ExecutionContext. this is not reliable. please file a bug against LLDB.
backtrace:
 #0 0x00007f117b0ab29e llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) llvm/lib/Support/Unix/Signals.inc:723:22
 #1 0x00007f117af11e90 lldb_private::TypeSystemClang::GetObjCBitSize(clang::QualType, lldb_private::ExecutionContextScope*) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:4780:17

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for trying! Yea I'll probably add the Darwin requirement then

@Michael137 Michael137 merged commit 000febd into llvm:main Dec 20, 2024
7 checks passed
@Michael137 Michael137 deleted the lldb/test-hidden-ivars branch December 20, 2024 12:16
Michael137 added a commit that referenced this pull request Dec 23, 2024
…type parsing (#120279)"

This reverts commit 000febd.

This is failing on the macOS public buildbots. It's unclear
to me why (I can't reproduce the failure on my local M1 machine).
I suspect the test might be relying on some non-deterministic
linker properties (such as order of entries in the debug-map
or something like that). The failure is as follows:
```
CHECK-NEXT: expected string not found in input
              ^
<stdin>:25:7: note: scanning from here
 y = 2
      ^
<stdin>:27:4: note: possible intended match here
(lldb) exit
   ^

Input file: <stdin>
Check file: /Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/test/Shell/Expr/TestObjCHiddenIvars.test

-dump-input=help explains the following input dump.

Input was:
<<<<<<
           .
           .
           .
          20: (lldb) expression *f
          21: (Foo) $0 = {
          22:  NSObject = {
          23:  isa = Foo
          24:  }
          25:  y = 2
next:21'0           X error: no match found
          26: }
next:21'0     ~~
          27: (lldb) exit
next:21'0     ~~~~~~~~~~~~
next:21'1        ?         possible intended match
>>>>>>
```
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