teemperor added a comment.
So for the test case that this fixes I get:
x i 8
f1 b15 12 (DWARF 0x0)
f2 b4 13 (DWARF 0x7)
f3 b4 14 (DWARF 0x3)
For the 2/4/4 bitfield case I get:
x i 8
f1 b2 12 (DWARF 0x0)
f2 b4 12 (DWARF 0x2)
f3 b4 12 (DWARF 0x6)
Anyway, Adrian's and Jim's point seem right. DWARF contains the bit offset from
the start of the 'container' (it seems ObjC just groups bitfields together by
some algorithm). If I look at the code for printing bitfields it seems like we
expect to get the *bit* offset from the layout we store and we get the *byte*
offset from the runtime. And then we combine those two into the actual offset
we have to read. The values we get from the example above seem to match that
and get the expected result, so I think that's right.
Regarding the second test: Running the test (You need to add the `test_` prefix
first) I get a pretty long warning that we try to get the size of the interface
without an execution context:
warning: trying to determine the size of type @interface HasBitfield2 :
NSObject{
unsigned int x;
unsigned int field1;
unsigned int field2;
unsigned int field3;
}
@end
without a valid ExecutionContext. this is not reliable. please file a bug
against LLDB.
backtrace:
0 liblldb.11.0.0git.dylib 0x00000001070146b5
llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
1 liblldb.11.0.0git.dylib 0x0000000106f16a1b
lldb_private::TypeSystemClang::GetBitSize(void*,
lldb_private::ExecutionContextScope*) + 827
2 liblldb.11.0.0git.dylib 0x0000000106ba0421
lldb_private::CompilerType::GetByteSize(lldb_private::ExecutionContextScope*)
const + 33
3 liblldb.11.0.0git.dylib 0x00000001079bcd68
IRForTarget::CreateResultVariable(llvm::Function&) + 2760
4 liblldb.11.0.0git.dylib 0x00000001079c30d0
IRForTarget::runOnModule(llvm::Module&) + 928
5 liblldb.11.0.0git.dylib 0x00000001079a15a4
lldb_private::ClangExpressionParser::PrepareForExecution(unsigned long long&,
unsigned long long&, std::__1::shared_ptr<lldb_private::IRExecutionUnit>&,
lldb_private::ExecutionContext&, bool&, lldb_private::ExecutionPolicy) + 1572
6 liblldb.11.0.0git.dylib 0x00000001079b723a
lldb_private::ClangUserExpression::Parse(lldb_private::DiagnosticManager&,
lldb_private::ExecutionContext&, lldb_private::ExecutionPolicy, bool, bool) +
1002
7 liblldb.11.0.0git.dylib 0x0000000106b31e77
lldb_private::UserExpression::Evaluate(lldb_private::ExecutionContext&,
lldb_private::EvaluateExpressionOptions const&, llvm::StringRef,
llvm::StringRef, std::__1::shared_ptr<lldb_private::ValueObject>&,
lldb_private::Status&, std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> >*,
lldb_private::ValueObject*) + 2375
8 liblldb.11.0.0git.dylib 0x0000000106c358e5
lldb_private::Target::EvaluateExpression(llvm::StringRef,
lldb_private::ExecutionContextScope*,
std::__1::shared_ptr<lldb_private::ValueObject>&,
lldb_private::EvaluateExpressionOptions const&, std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> >*,
lldb_private::ValueObject*) + 885
In general the semantics here seem kinda off. The result of the expression is
`HasBitfield2` which I don't think is something we really have in Obj-C as it
seems like a stack-allocated `HasBitfields2` instance. Changing the expression
to return `HasBitfield2 *` instead and then forcing that we dereference Obj-C
pointers fixes the second case for me. (FWIW, doing `frame var *hb2` is also
working and is doing essentially the same thing as it just prints the children
of the pointer). Fixing that can just be a follow up PR.
So I would say removing the sanity check here seems fine. We maybe want to have
a specific Obj-C sanity check but I'm not even sure how this would look like.
Maybe figuring out what's the maximum 'container size' we can have in Obj-C and
checking that offset + size is below that makes sense, but otherwise I don't
think we can do a lot. So I think my previous comment that this is ready to go
beside some minor changes is IMHO the way to go.
================
Comment at: lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py:24
+ @expectedFailureAll()
+ def ExpressionOnObject(self):
+ self.build()
----------------
This method isn't a test at the moment as it doesn't have a `test` prefix in
the name. E.g. `test_ExpressionOnObject` is the way to make this a test that
actually runs. Currently it's just a normal method that is expected to be run
by other `test_` methods.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83433/new/
https://reviews.llvm.org/D83433
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
