[lldb-dev] LLDB does not support the default 8 byte build ID generated by LLD

Thu Jun 21 10:01:19 PDT 2018

I didn't know about llvm::SmallVector, but it seems a good match indeed,
thanks Greg.

On Thu, Jun 21, 2018 at 9:49 AM, Greg Clayton <clayborg at gmail.com> wrote:

>
>
> On Jun 21, 2018, at 9:46 AM, Leonard Mosescu <mosescu at google.com> wrote:
>
> Leonard, I'm not going to use your patch, as it's a bit un-llvm-y
>> (uses std::ostream and such). However, I wanted to check whether 20
>> bytes will be enough for your use cases (uuids in minidumps)?
>>
>
> For minidumps we normally use either 16 or 20 byte UUIDs, so I don't see
> any immediate problems. Are you planning to make 20 a hard limit or have
> the 20 bytes "inlined" and dynamically allocate if larger?
>
>
> We could use a llvm::SmallVector<uint8_t, 20> to have up to 20 bytes
> before going larger and allocating on the heap.
>
>
> On Thu, Jun 21, 2018 at 8:18 AM, Pavel Labath <labath at google.com> wrote:
>
>> That sounds like a plan. I have started cleaning up the class a bit
>> (removing manual uuid string formatting in various places and such),
>> and then I'll send a patch which implements that.
>>
>> Leonard, I'm not going to use your patch, as it's a bit un-llvm-y
>> (uses std::ostream and such). However, I wanted to check whether 20
>> bytes will be enough for your use cases (uuids in minidumps)?
>> On Thu, 21 Jun 2018 at 16:03, Greg Clayton <clayborg at gmail.com> wrote:
>> >
>> > I am fine if we go with any number of bytes. We should have the
>> lldb_private::UUID class have an array of bytes that is in the class that
>> is to to 20 bytes. We can increase it later if needed. I would rather not
>> have a dynamically allocated buffer.
>> >
>> > That being said a few points:
>> > - Length can be set to zero to indicate invalid UUID. Better that than
>> filling in all zeroes and having to check for that IMHO. I know there were
>> some problems with the last patch around this.
>> > - Don't set length to a valid value and have UUID contain zeros unless
>> that is a true UUID that was calculated. LLDB does a lot of things by
>> matching UUID values so we can't have multiple modules claiming to have a
>> UUID that is filled with zeroes, otherwise many matches will occur that we
>> don't want
>> > - 32 bit GNU debug info CRCs from ELF notes could be filled in as 4
>> byte UUIDs
>> > - Comparing two UUIDs can start with the length field first the if they
>> match proceed to compare the bytes (which is hopefully what is already
>> happening)
>> >
>> >
>> > On Jun 20, 2018, at 11:01 AM, Leonard Mosescu via lldb-dev <
>> lldb-dev at lists.llvm.org> wrote:
>> >
>> > Here's a snapshot of the old changes I had:
>> https://reviews.llvm.org/D48381
>> > (hopefully it helps a bit but caveat emptor: this is a quick merge from
>> an old patch, so it's for illustrative purposes only)
>> >
>> >
>> > On Wed, Jun 20, 2018 at 10:26 AM, Pavel Labath <labath at google.com>
>> wrote:
>> >>
>> >> From the looks of it, the patch stalled on the part whether we can
>> >> consider all-zero UUIDs as valid or not. I've dug around the code a
>> >> bit now, and I've found this comment in ObjectFileMachO.cpp.
>> >>
>> >>            // "main bin spec" (main binary specification) data payload
>> is
>> >>            // formatted:
>> >>            //    uint32_t version       [currently 1]
>> >>            //    uint32_t type          [0 == unspecified, 1 ==
>> >> kernel, 2 == user process]
>> >>            //    uint64_t address       [ UINT64_MAX if address not
>> specified ]
>> >>            //    uuid_t   uuid          [ all zero's if uuid not
>> specified ]
>> >>            //    uint32_t log2_pagesize [ process page size in log
>> >> base 2, e.g. 4k pages are 12.  0 for unspecified ]
>> >>
>> >>
>> >> So it looks like there are situations where we consider all-zero UUIDs
>> >> as invalid.
>> >>
>> >> I guess that means we either have to keep IsValid() definition as-is,
>> >> or make ObjectFileMachO check the all-zero case itself. (Some middle
>> >> ground may be where we have two SetFromStringRef functions, one which
>> >> treats all-zero case specially (sets m_num_uuid_bytes to 0), and one
>> >> which doesn't). Then clients can pick which semantics they want.
>> >>
>> >>
>> >> > 1. A variable-length UUID likely incurs an extra heap allocation.
>> >> Not really. If you're happy with the current <=20 limit, then you can
>> >> just use the existing data structure. Otherwise, you could use a
>> >> SmallVector<uint8_t, 20>.
>> >>
>> >> > 2. Formatting arbitrary length UUIDs as string is a bit inconvenient
>> as you noted as well.
>> >> For the string representation, I would say we should just use the
>> >> existing layout of dashes (after 4, 6, 8, 10 and 16 bytes) and just
>> >> cut it short when we have less bytes. The implementation of that
>> >> should be about a dozen lines of code.
>> >>
>> >> The fact that these new UUIDs would not be real UUIDs could be solved
>> >> by renaming this class to something else, if anyone can think of a
>> >> good name for it (I can't). Then the "real" UUIDs will be just a
>> >> special case of the new object.
>> >>
>> >> pl
>> >
>> >
>> > _______________________________________________
>> > lldb-dev mailing list
>> > lldb-dev at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>> >
>> >
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20180621/c8919fcd/attachment.html>