labath added a comment.

The code mostly fine for me, but this should be reviewed by properly by more 
people, once you're ready to take down the WIP tag.

I am still not happy with the test case.



================
Comment at: lldb/source/Host/common/DebugInfoD.cpp:43
+      buildID.GetBytes().size() ==
+          sizeof(llvm::support::ulittle32_t)) // .gnu_debuglink crc32
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
----------------
jankratochvil wrote:
> If it is done this way (and not in `libdebuginfod.so`) I think there should 
> be `<=8` because LLDB contains:
> ```
>             if (gnu_debuglink_crc) {
>               // Use 4 bytes of crc from the .gnu_debuglink section.
>               u32le data(gnu_debuglink_crc);
>               uuid = UUID::fromData(&data, sizeof(data));
>             } else if (core_notes_crc) {
>               // Use 8 bytes - first 4 bytes for *magic* prefix, mainly to 
> make
>               // it look different form .gnu_debuglink crc followed by 4 bytes
>               // of note segments crc.
>               u32le data[] = {u32le(g_core_uuid_magic), 
> u32le(core_notes_crc)};
>               uuid = UUID::fromData(data, sizeof(data));
>             }
> ```
> 
4 would have probably been fine too, as I don't think a core file "uuid" can 
make its way into here. In either case, we should document what is this working 
around, as 4 or 8 byte uuids are technically valid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to