Skip to content

[lldb][DWARFASTParserClang][ObjC] Remove workaround for old ObjC DWARF #120218

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

@Michael137 Michael137 commented Dec 17, 2024

With all the recent versions of Clang that I tested, ObjC forward declarations like

@class ForwardObjcClass;

don't emit the kind of DWARF that this workaround was put in place for.

Also, zero-sized structures are valid in C (and thus Objective-C), so this workaround makes things confusing to reason about when mixing the two languages.

This workaround has been in place for at least a decade, and given that recent compilers don't produce this anymore, we think it's a good time to remove it.

With all the recent versions of Clang that I tested, ObjC forward
declarations like
```
@Class ForwardObjcClass;
```
don't emit the kind of DWARF that this workaround was put in place for.

Also, zero-sized structures are valid in C (and thus Objective-C),
so this workaround makes things confusing to reason about when mixing
the two languages.

This workaround has been in place for at least a decade, and given
that recent compilers to produce this anymore, we think it's a good
time to remove it.
@llvmbot
Copy link
Member

llvmbot commented Dec 17, 2024

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

With all the recent versions of Clang that I tested, ObjC forward declarations like

@<!-- -->class ForwardObjcClass;

don't emit the kind of DWARF that this workaround was put in place for.

Also, zero-sized structures are valid in C (and thus Objective-C), so this workaround makes things confusing to reason about when mixing the two languages.

This workaround has been in place for at least a decade, and given that recent compilers don't produce this anymore, we think it's a good time to remove it.


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

1 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (-14)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 58f7b805abe2fd..3f49ad25710a9f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1657,20 +1657,6 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
   ConstString unique_typename(attrs.name);
   Declaration unique_decl(attrs.decl);
   uint64_t byte_size = attrs.byte_size.value_or(0);
-  if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
-      !die.HasChildren() && cu_language == eLanguageTypeObjC) {
-    // Work around an issue with clang at the moment where forward
-    // declarations for objective C classes are emitted as:
-    //  DW_TAG_structure_type [2]
-    //  DW_AT_name( "ForwardObjcClass" )
-    //  DW_AT_byte_size( 0x00 )
-    //  DW_AT_decl_file( "..." )
-    //  DW_AT_decl_line( 1 )
-    //
-    // Note that there is no DW_AT_declaration and there are no children,
-    // and the byte size is zero.
-    attrs.is_forward_declaration = true;
-  }
 
   if (attrs.name) {
     GetUniqueTypeNameAndDeclaration(die, cu_language, unique_typename,

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.

Woohoo.

@Michael137 Michael137 merged commit 794cd81 into llvm:main Dec 17, 2024
9 checks passed
@Michael137 Michael137 deleted the lldb/objc-remove-old-compiler-assumption branch December 17, 2024 12:35
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Dec 23, 2024
llvm#120218)

With all the recent versions of Clang that I tested, ObjC forward
declarations like
```
@Class ForwardObjcClass;
```
don't emit the kind of DWARF that this workaround was put in place for.

Also, zero-sized structures are valid in C (and thus Objective-C), so
this workaround makes things confusing to reason about when mixing the
two languages.

This workaround has been in place for at least a decade, and given that
recent compilers don't produce this anymore, we think it's a good time
to remove it.

(cherry picked from commit 794cd81)
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