[Lldb-commits] [PATCH] D57413: Fix some warnings with gcc on Linux

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

I don't think the data formatter changes are right.  Looks like whoever wrote these data formatters was directly reading target memory onto a host sized struct, expecting everything to line up.  So you can't take fields out of it.



================
Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:304-314
-    union {
-      struct {
-        uint64_t _mutations;
-      };
-      struct {
-        uint32_t _muts;
-        uint32_t _used:25;
----------------
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57413/new/

https://reviews.llvm.org/D57413