zturner marked an inline comment as done.
zturner added inline comments.
================
Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:304-314
- union {
- struct {
- uint64_t _mutations;
- };
- struct {
- uint32_t _muts;
- uint32_t _used:25;
----------------
zturner wrote:
> jingham wrote:
> > I don't think this is right. The data formatter is reading a target side
> > struct onto a host struct to read it in. For instance, later on the code
> > does:
> >
> > m_data_64 = new DataDescriptor_64();
> > process_sp->ReadMemory(data_location, m_data_64,
> > sizeof(DataDescriptor_64),
> > error);
> >
> > So if you remove mutations you won't read in enough bytes and they won't be
> > correctly aligned.
> >
> > That's not safe if the host and target have different endian-ness, so this
> > doesn't seem a great way to get this data into lldb. It would be more
> > correct to make a DataExtractor for this read, and then pull the fields out
> > of it by size and type.
> >
> > But as it stands, you can't just delete these fields.
> That is indeed how I assumed this works, but if you look carefully, it's a
> union of a) a 64-bit struct (struct with one uint64 member), and b) a struct
> with a uint32 followed by another uint32 bitfield. So also 64-bits. So
> deleting the first struct (which is never referenced) allows us to remove the
> union entirely, and should have no impact on the size of the final struct.
Just to be sure, I added this:
```
struct DataDescriptor_64 {
uint64_t _buffer;
uint32_t _muts;
uint32_t _used : 25;
uint32_t _kvo : 1;
uint32_t _szidx : 6;
uint64_t GetSize() {
return (_szidx) >= NSDictionaryNumSizeBuckets ?
0 : NSDictionaryCapacities[_szidx];
}
};
struct DataDescriptor_64_2 {
uint64_t _buffer;
union {
struct {
uint64_t _mutations;
};
struct {
uint32_t _muts;
uint32_t _used : 25;
uint32_t _kvo : 1;
uint32_t _szidx : 6;
};
};
};
static_assert(sizeof(DataDescriptor_64_2) == sizeof(DataDescriptor_64),
"mismatched size!");
```
And the static assert passes. I wouldn't want to leave that in the final code,
but the point is that I it's still NFC even in the face of deleting the fields.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57413/new/
https://reviews.llvm.org/D57413
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
