Fix how we handle bit-fields for Objective-C when creating an AST
authorshafik <syaghmour@apple.com>
Mon, 20 Jul 2020 23:09:18 +0000 (16:09 -0700)
committershafik <syaghmour@apple.com>
Mon, 20 Jul 2020 23:12:29 +0000 (16:12 -0700)
Currently expressions dealing with bit-fields in Objective-C objects is pretty broken. When generating debug-info for Objective-C bit-fields DW_AT_data_bit_offset has a different meaning than it does to C and C++.
When we parse the DWARF we validate bit offsets for C and C++ correctly but not for ObjC. For ObjC in some cases we end up incorrectly flagging an error and we don't generate further bit-fields in the AST.
Later on when we do a name lookup we don't find the ObjCIvarDecl in the ObjCInterfaceDecl in some cases since we never added it and then we don't go to the runtime to obtain the offset.

This will fix how we handle bit-fields for the Objective-C case and add tests to verify this fix but also to documents areas that still don't work and will be addressed in follow-up PRs.

Note: we can never correctly calculate offsets statically because of how Objective-C deals with the fragile base class issue. Which means the runtime may need to shift fields over.

Differential Revision: https://reviews.llvm.org/D83433

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/test/API/lang/objc/bitfield_ivars/Makefile [new file with mode: 0644]
lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
lldb/test/API/lang/objc/bitfield_ivars/main.m

index 2d1db66..62b6eee 100644 (file)
@@ -2578,10 +2578,14 @@ void DWARFASTParserClang::ParseSingleMember(
             }
           }
 
-          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 "
diff --git a/lldb/test/API/lang/objc/bitfield_ivars/Makefile b/lldb/test/API/lang/objc/bitfield_ivars/Makefile
new file mode 100644 (file)
index 0000000..a68dad5
--- /dev/null
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS = -framework Foundation
+
+include Makefile.rules
index c0d006e..0cc3cef 100644 (file)
@@ -1,12 +1,39 @@
-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")
+
+        ## FIXME field2 should have a value of 1
+        self.expect("expr chb->hb->field2", matching=False, substrs = ["= 1"]) # this must happen second
+
+        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 1ba1cf0..841464d 100644 (file)
 
 @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
 }