shafik created this revision.
shafik added reviewers: teemperor, martong.
Herald added a subscriber: rnkovacs.
Once we start the definition of an `ObjCInterfaceDecl` we won't attempt to
`ImportDeclContext` later on. Unlike `RecordDecl` case which uses
`DefinitionCompleter` to force `completeDefinition` we don't seem to have a
similar mechanism for `ObjCInterfaceDecl`.
This fix was needed due to a bug we see in LLDB expression parsing where an
initial expression cause an `ObjCInterfaceDecl` to be defined and subsequent
expressions during import do not call `ImportDeclContext` and we can end up in
a situation where ivars are imported out of order and not all ivars are
imported e.g.
(lldb) expr chb1->hb->field2
ObjCInterfaceDecl 0x7f9457495fe0 <<invalid sloc>> <invalid sloc>
<undeserialized declarations> HasBitfield1
|-super ObjCInterface 0x7f9457495e78 'NSObject'
`-ObjCIvarDecl 0x7f945749d478 <<invalid sloc>> <invalid sloc> field2
'unsigned int' public
`-IntegerLiteral 0x7f945749d458 <<invalid sloc>> 'int' 1
We have `field2` which is the second ivar but we are missing `field1`.
In this particular case `ASTContext::lookupFieldBitOffset(...)` assumes we have
all ivars and they are in order to obtain the index of the ivar and we break
this assumption. This leads eventually to bad IRGen since it has the wrong
field offset.
https://reviews.llvm.org/D83972
Files:
clang/lib/AST/ASTImporter.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/test/API/lang/objc/bitfield_ivars/Makefile
lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
lldb/test/API/lang/objc/bitfield_ivars/main.m
Index: lldb/test/API/lang/objc/bitfield_ivars/main.m
===================================================================
--- lldb/test/API/lang/objc/bitfield_ivars/main.m
+++ lldb/test/API/lang/objc/bitfield_ivars/main.m
@@ -34,10 +34,31 @@
@end
+@interface HasBitfield2 : NSObject {
+@public
+ unsigned int x;
+
+ unsigned field1 : 15;
+ unsigned field2 : 4;
+ unsigned field3 : 4;
+}
+@end
+
+@implementation HasBitfield2
+- (id)init {
+ return (self = [super init]);
+}
+@end
+
int main(int argc, const char * argv[]) {
ContainsAHasBitfield *chb = [[ContainsAHasBitfield alloc] init];
- printf("%d\n", chb->hb->field2); //% self.expect("expression -- chb->hb->field1", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["= 0"])
- //% self.expect("expression -- chb->hb->field2", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["= 1"]) # this must happen second
- return 0;
+ HasBitfield2 *hb2 = [[HasBitfield2 alloc] init];
+
+ hb2->x = 100;
+ hb2->field1 = 10;
+ hb2->field2 = 3;
+ hb2->field3 = 4;
+
+ return 0; // break here
}
Index: lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
===================================================================
--- lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
+++ lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
@@ -1,12 +1,37 @@
-from lldbsuite.test import lldbinline
-from lldbsuite.test import decorators
-
-lldbinline.MakeInlineTest(
- __file__,
- globals(),
- [
- # This is a Darwin-only failure related to incorrect expression-
- # evaluation for single-bit ObjC bitfields.
- decorators.skipUnlessDarwin,
- decorators.expectedFailureAll(
- bugnumber="rdar://problem/17990991")])
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestBitfieldIvars(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+
+ @skipUnlessDarwin
+ def test(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.m"))
+
+ self.expect_expr("chb->hb->field1", result_type="unsigned int", result_value="0")
+ self.expect_expr("chb->hb->field2", result_type="unsigned int", result_value="1")
+
+ self.expect_expr("hb2->field1", result_type="unsigned int", result_value="10")
+ self.expect_expr("hb2->field2", result_type="unsigned int", result_value="3")
+ self.expect_expr("hb2->field3", result_type="unsigned int", result_value="4")
+
+ self.expect("frame var *hb2", substrs = [ 'x =', '100',
+ 'field1 =', '10',
+ 'field2 =', '3',
+ 'field3 =', '4'])
+
+ @expectedFailureAll()
+ def testExprWholeObject(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.m"))
+
+ ## FIXME expression with individual bit-fields obtains correct values but not with the whole object
+ self.expect("expr *hb2", substrs = [ 'x =', '100',
+ 'field1 =', '10',
+ 'field2 =', '3',
+ 'field3 =', '4'])
Index: lldb/test/API/lang/objc/bitfield_ivars/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/lang/objc/bitfield_ivars/Makefile
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS = -framework Foundation
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2609,10 +2609,14 @@
}
}
- if ((this_field_info.bit_offset >= parent_bit_size) ||
- (last_field_info.IsBitfield() &&
- !last_field_info.NextBitfieldOffsetIsValid(
- this_field_info.bit_offset))) {
+ // The ObjC runtime knows the byte offset but we still need to provide
+ // the bit-offset in the layout. It just means something different then
+ // what it does in C and C++. So we skip this check for ObjC types.
+ if (!TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type) &&
+ ((this_field_info.bit_offset >= parent_bit_size) ||
+ (last_field_info.IsBitfield() &&
+ !last_field_info.NextBitfieldOffsetIsValid(
+ this_field_info.bit_offset)))) {
ObjectFile *objfile = die.GetDWARF()->GetObjectFile();
objfile->GetModule()->ReportWarning(
"0x%8.8" PRIx64 ": %s bitfield named \"%s\" has invalid "
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -4698,11 +4698,10 @@
return ToImplOrErr.takeError();
}
- if (shouldForceImportDeclContext(Kind)) {
- // Import all of the members of this class.
- if (Error Err = ImportDeclContext(From, /*ForceImport=*/true))
- return Err;
- }
+ // Import all of the members of this class.
+ if (Error Err = ImportDeclContext(From, /*ForceImport=*/true))
+ return Err;
+
return Error::success();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
