[Lldb-commits] [PATCH] D40211: Add comments to DWARFCompileUnit length fields/methods

[Zachary Turner via lldb-commits]

You can make structs that are host and byte-order independent, LLVM is
filled with stuff like this.  And while you might end up processing the
information off in a way that it can be stored in a single compile unit
without such a struct, it still can be useful when you're actually *doing*
the parsing.

For one, it serves as documentation of the format.  It sounds like in order
to know what a DWARF64 header looks like currently, I have to go read the
code that actually parses one, which might require me to understand several
functions and follow some pointer arithmetic and various other stuff.

Second, it can converts code like this:
```
if (m_dwarf64) {
  // read the first field
  // adjust a pointer
  // read the second field
  // adjust a pointer
  // read the third field
  // adjust a pointer
  ...

  // store the fields in the CompileUnit
} else {
  // read the first field
  // adjust a pointer
  // read the second field
  // adjust a pointer
  // read the third field
  // adjust a pointer
  ...
  // store the fields in the CompileUnit
}
```
into this:
```
if (m_dwarf64) {
  DWARF64_HEADER h;
  readStructure(h);
  // store the fields in the CompileUnit
} else {
  DWARF32_HEADER h;
  readStructure(h);
  // store the fields in the CompileUnit
}
```

Anyway, you answered my question, which is that we don't have one

On Mon, Nov 20, 2017 at 8:07 AM Greg Clayton <clayborg at gmail.com> wrote:

> sizeof(struct) tends to include system level padding for the current host.
> But to answer your question, no there isn't a structure defined like this
> and we wouldn't use them anyway as we want to fill out one compile unit
> struct that works for both.
>
> On Nov 20, 2017, at 8:01 AM, Zachary Turner <zturner at google.com> wrote:
>
> Right but isn’t there a DWARF64_HEADER and DEARF32_HEADER struct
> somewhere? This way you could just say
>
> return m_isdwarf64 ? sizeof(DWARF64_HEADER) : sizeof(DWARF32_HEADER);
> On Mon, Nov 20, 2017 at 7:50 AM Greg Clayton <clayborg at gmail.com> wrote:
>
>>
>> On Nov 19, 2017, at 4:56 PM, Zachary Turner <zturner at google.com> wrote:
>>
>>
>>
>> On Sun, Nov 19, 2017 at 6:35 AM Jan Kratochvil via Phabricator via
>> lldb-commits <lldb-commits at lists.llvm.org> wrote:
>>
>>> This revision was automatically updated to reflect the committed changes.
>>> Closed by commit rL318626: Add comments to DWARFCompileUnit length
>>> fields/methods (authored by jankratochvil).
>>>
>>> Changed prior to commit:
>>>   https://reviews.llvm.org/D40211?vs=123472&id=123498#toc
>>>
>>> Repository:
>>>   rL LLVM
>>>
>>> https://reviews.llvm.org/D40211
>>>
>>> Files:
>>>   lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
>>>
>>>
>>> Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
>>> ===================================================================
>>> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
>>> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
>>> @@ -41,26 +41,24 @@
>>>    void Clear();
>>>    bool Verify(lldb_private::Stream *s) const;
>>>    void Dump(lldb_private::Stream *s) const;
>>> +  // Offset of the initial length field.
>>>    dw_offset_t GetOffset() const { return m_offset; }
>>>    lldb::user_id_t GetID() const;
>>> -  uint32_t Size() const {
>>> -    return m_is_dwarf64 ? 23
>>> -                        : 11; /* Size in bytes of the compile unit
>>> header */
>>> -  }
>>> +  // Size in bytes of the initial length + compile unit header.
>>> +  uint32_t Size() const { return m_is_dwarf64 ? 23 : 11; }
>>>
>>
>> This is pretty gross.  Don't we have a structure somewhere that
>> represents a compile unit header?  That we can just call sizeof on?  Same
>> goes for the rest of the patch
>>
>>
>> It varies depending on data on how the length unit is represented in the
>> data stream. If the length starts with UINT32_MAX, it is followed by a 64
>> bit length. If the length isn't UINT32_MAX it is just a 32 bit length.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20171120/c9b5ca25/attachment-0001.html>