jankratochvil added inline comments.

================
Comment at: lldb/source/Host/common/DebugInfoD.cpp:59
+  char *cache_path = nullptr;
+  int rc = debuginfod_find_source(client, buildID.GetBytes().data(),
+                                  buildID.GetBytes().size(), path.c_str(),
----------------
labath wrote:
> fche2 wrote:
> > jankratochvil wrote:
> > > Here it will contact the server even if the binary does not contain any 
> > > build-id - LLDB then generates UUID as 4 bytes long one:
> > > ```
> > >               // Use 4 bytes of crc from the .gnu_debuglink section.
> > >               u32le data(gnu_debuglink_crc);
> > >               uuid = UUID::fromData(&data, sizeof(data));
> > > ```
> > > That is a needless performance regression.
> > > I sure do not like making such decision on the LLDB side. Maybe 
> > > libdebuginfod could rather make such optimization - IMO as Frank Eigler.
> > Could kkleine reject uuid of length 4 in the above test, i.e. something 
> > like:
> > 
> >     if (!uuid.IsValid() || uuid.GetBytes().size() == sizeof(u32le)) // 
> > .gnu_debuglink crc32
> >        continue;
> Ideally, lldb would not use the debug link crc as a uuid (and instead store 
> that elsewhere), but rejecting the short uuids here does not seem _that_ bad.
We were discussing with @kwk that in fact sending anything stored in UUID as 
build-id may not be right. `debuginfod` wants specifically build-id, not any 
other identifier. Or @fche2 - does it? Would `debuginfod` for example accept 
some that Apple UUID for Apple dsym files? Maybe LLDB could store some 
identifier how was the UUID obtained.


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