This is an archive of the discontinued LLVM Phabricator instance.

[llvm] [Debuginfod] LLVM debuginfod server.
ClosedPublic

Authored by noajshu on Nov 30 2021, 10:26 PM.

Details

Summary

This implements a debuginfod server in llvm using the DebuginfodCollection and DebuginfodServer classes. This is tested with lit tests against the debuginfod-find client.

The server scans 0 or more local directories for artifacts. It serves the debuginfod protocol over HTTP. Only the executable and debuginfo endpoints are supported (no /source endpoint).
The server also uses the debuginfod client as a fallback, so it can hit the local debuginfod cache or federate to other known debuginfod servers.
The client behavior is controllable through the standard environment variables (DEBUGINFOD_URLS, DEBUGINFOD_CACHE_PATH, DEBUGINFOD_TIMEOUT)

The server implements on-demand collection updates as follows:
If the build-id is not found by a local lookup, rescan immediately and look up the build-id again before returning 404. To protect against DoS attacks, do not rescan more frequently than once per N seconds (specified by -m).

Lit tests are provided which test the llvm-debuginfod-find client against the llvm-debuginfod server.

Diff Detail

Event Timeline

noajshu created this revision.Nov 30 2021, 10:26 PM
noajshu requested review of this revision.Nov 30 2021, 10:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 30 2021, 10:27 PM
phosek added a subscriber: phosek.Dec 2 2021, 12:08 PM
phosek added inline comments.Dec 2 2021, 12:08 PM
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
122

This should be controllable through a command line argument, elfutils' debuginfod supports -t SECONDS and defaults to 300 seconds.

noajshu updated this revision to Diff 394933.Dec 16 2021, 11:15 AM
noajshu marked an inline comment as done.

Add -c concurrency limit and -t scan interval command-line opts. Refactor almost all server logic out of llvm-debuginfod.cpp.

noajshu edited the summary of this revision. (Show Details)Dec 16 2021, 11:16 AM
noajshu updated this revision to Diff 394999.Dec 16 2021, 2:35 PM

Remove extra copy of the LLVM_ENABLE_HTTPLIB option

noajshu updated this revision to Diff 403040.Jan 25 2022, 2:42 PM

Remove extraneous #includes from llvm-debuginfod.cpp and suppress unrelated cl options.

noajshu updated this revision to Diff 403097.Jan 25 2022, 6:16 PM

Set llvm-debuginfod to use the hardware concurrency by default or if -c 0 is passed.

noajshu updated this revision to Diff 406296.Feb 6 2022, 2:59 PM
noajshu retitled this revision from [llvm] [DebugInfo] LLVM debuginfod server. (WIP) to [llvm] [DebugInfo] LLVM debuginfod server..
noajshu edited the summary of this revision. (Show Details)
noajshu added reviewers: phosek, dblaikie.

Add end-to-end tests.

noajshu retitled this revision from [llvm] [DebugInfo] LLVM debuginfod server. to [llvm] [Debuginfod] LLVM debuginfod server..Feb 6 2022, 3:02 PM
noajshu updated this revision to Diff 411711.Feb 27 2022, 4:52 PM
noajshu edited the summary of this revision. (Show Details)

rebase

pcc added a subscriber: pcc.Mar 8 2022, 5:13 PM
pcc added inline comments.
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
11

Looks like this comment should be updated to describe llvm-debuginfod instead of llvm-debuginfod-find.

Herald added a project: Restricted Project. · View Herald TranscriptMar 8 2022, 5:13 PM
pcc added inline comments.Mar 8 2022, 5:43 PM
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
70

Could we make the bind interface configurable please? This will allow running a local symbol server without exposing it to other machines on the network.

noajshu updated this revision to Diff 419604.Mar 31 2022, 7:20 PM

Incorporate DebuginfodLog. Fix tests by adding binary files and enabling concurrency. Make threading a requirement for the tests.

noajshu marked an inline comment as done.Mar 31 2022, 7:44 PM
noajshu added inline comments.
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
70

Great idea, this has been added as the -i command line option.

noajshu updated this revision to Diff 420362.Apr 4 2022, 6:22 PM
noajshu marked an inline comment as done.

Add -v flag to enable verbose logging, suppress verbosity by default.

noajshu updated this revision to Diff 426564.May 2 2022, 8:09 PM
noajshu marked an inline comment as done.
noajshu edited the summary of this revision. (Show Details)

Updated comments in llvm-debuginfod.cpp.

llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
11

Thank you for catching this!

phosek added inline comments.May 25 2022, 10:23 AM
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
44

10000s is a long time between scans, elfutils defaults to 300s which I think would be better.

pcc added inline comments.May 25 2022, 10:37 AM
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
44

If the server implements the following behavior, do we need a scan interval?

  • If the build-id is not found by a local lookup, rescan immediately and look up the build-id again before returning 404.
  • To protect against DoS attacks, do not rescan more frequently than once per N seconds (e.g. N = 30).

debuginfod does not have this behavior and it was somewhat of a pain point when using it for local development.

noajshu added inline comments.May 25 2022, 10:56 PM
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
44

10000s is a long time between scans, elfutils defaults to 300s which I think would be better.

Thanks, will update this.

If the server implements the following behavior, do we need a scan interval?

Thanks for this suggestion, this would be really nice for local development. I've just updated D114845 (which implements DebuginfodCollection) to support this behavior via updateIfStale(), though we still need to add the option here. It's not much trouble to support both periodic and on-demand update triggers, so perhaps we could allow the when-to-update behavior to be customized by the cl::opts.

44

10000s is a long time between scans, elfutils defaults to 300s which I think would be better.

mysterymath added inline comments.May 31 2022, 4:45 PM
llvm/test/tools/llvm-debuginfod/llvm-debuginfod.test
25–27 ↗(On Diff #426564)

It doesn't seem like the python script does anything for these two invocations; the DEBUGINFOD_CACHE_PATH is set manually, and there's nothing to wait for. It'd be clearer just to use llvm-debuginfod-find directly. This should also help remove conditional logic from the Python script.

51 ↗(On Diff #426564)

Consider just env=os.environ, since env is otherwise unused.

54 ↗(On Diff #426564)
58 ↗(On Diff #426564)
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
44

It looks like this hasn't yet been updated as of the most recent patch.

98

Consider adding an llvm_unreachable() and a comment after this for documentation purposes; it was surprising to discover that the structure of the pool means it will never finish, but that there's still a wait call.

noajshu updated this revision to Diff 433647.Jun 1 2022, 9:14 PM

Reduce default directory scan wait period to 300s, incorporate on-demand update using timer.

noajshu edited the summary of this revision. (Show Details)Jun 1 2022, 9:16 PM
noajshu marked 8 inline comments as done.
mysterymath accepted this revision.Jun 2 2022, 11:59 AM
mysterymath added inline comments.
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
102

std::chrono::seconds? It looks like the implicit conversion should automatically convert the durations.

This revision is now accepted and ready to land.Jun 2 2022, 11:59 AM
noajshu updated this revision to Diff 437764.Jun 16 2022, 6:13 PM
noajshu marked an inline comment as done.

Update the -t delay parameter to require integer delay time in seconds between scans.

noajshu added inline comments.Jun 21 2022, 1:51 PM
llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
102

The goal of this was to allow someone to specify non-integer arguments, which would be rounded to the nearest millisecond. However I think it is more canonical to require an integer, so I've updated the argument and used your suggested conversion. Thanks!

noajshu updated this revision to Diff 438828.Jun 21 2022, 2:04 PM

Rebase against main

noajshu updated this revision to Diff 442988.Jul 7 2022, 10:48 AM

rebase against main

This revision was landed with ongoing or failed builds.Jul 7 2022, 11:33 AM
This revision was automatically updated to reflect the committed changes.

Hi @fche2 thanks for the ping.

D75750 is a great starting point that looked up source files with elfutils' debuginfod.

Integrating llvm-debuginfod into lldb may be pretty simple as the library comes for free with llvm if built with libcurl.

I am curious if others are already reviving this effort? I could work on it if not.

Hi @fche2 thanks for the ping.

D75750 is a great starting point that looked up source files with elfutils' debuginfod.

Integrating llvm-debuginfod into lldb may be pretty simple as the library comes for free with llvm if built with libcurl.

I am curious if others are already reviving this effort? I could work on it if not.

I'm planning to pick this up in the vaguely nearish future (if noone else does), but I don't have a clear ETA for when I'll get around to it.