[llvm] [Debuginfo] Debuginfod client library.

[noajshu]

Summary

This adds a Debuginfod client library which queries servers specified by the DEBUGINFOD_URLS environment variable for the debuginfo, executable, or a specified source file associated with a given build id.

This diff was split out from D111252.

[noajshu]

Update for refactored HTTP client architecture.

[noajshu]

remove extraneous files related to debuginfod-find tool

[noajshu]

move CURL cmake option update to D112753

[noajshu]

Clean up unrelated changes.

[labath]

llvm/lib/Debuginfod/Debuginfod.cpp

82

This isn't really appropriate as it would use a backslash on windows. I suppose it would be ok if you explicitly specify the posix path style as an argument.

llvm/unittests/Debuginfod/DebuginfodTests.cpp

14

I suppose you could test here that the asset can be retrieved from the cache without hitting the server, but I don't think that's really necessary (if you have such a test in the next patch) -- a lot of library code in llvm is only tested through the relevant tool.

[noajshu]

Fix backslashes on windows.

[labath]

llvm/lib/Debuginfod/CMakeLists.txt

2

This may be the first case, where the very existence of a component library is predicated on the availability of a third party dep. That's not necessarily a bad thing (it'd also be strange to enable it when it is completely non-functional), but it did catch my eye.

[noajshu]

Refactor and add several convenience functions.

[noajshu]

I integrated the client library into llvm-symbolizer (D113717) and found that a lot of boilerplate was required. So I refactored the interface to make it easier to use.
Now all the functionality is in standalone functions. These would be easy to wrap in an object oriented interface similar to the SymbolServer used in LLDB.

llvm/lib/Debuginfod/Debuginfod.cpp

82

Good catch, thanks!

llvm/unittests/Debuginfod/DebuginfodTests.cpp

14

Currently there is no such test in the next patch. There are only lit tests where the cache is emptied each time and we fetch assets from a server. Is it OK to write to the filesystem in a unit test? If so I would like to add one here per your suggestion. If not, I can add a lit test to make sure local caching works properly.

[phosek]

llvm/include/llvm/Debuginfod/Debuginfod.h

41

I'd consider using const ArrayRef<uint8_t> BuildID and do the conversion to string internally (in which case even buildIDToString could be an internal function).

[dblaikie]

llvm/lib/Debuginfod/CMakeLists.txt

2

Yeah, I had mixed feelings about this with the machine learning - if the API is going to always be exposed, maybe the API should be in the library, and it gets compiled into a trivially uninteresting library that always fails the query - rather than having the entry point outside the library, and having that entry point not call into the library if it isn't available/couldn't do something real?

llvm/lib/Debuginfod/Debuginfod.cpp

35

You can drop the top level const here and in similar places.

[noajshu]

Add a typedef ArrayRef<uint8_t> BuildID; and change external API of debuginfod library to require. Also remove unnecessary const qualifiers from args.

llvm/lib/Debuginfod/CMakeLists.txt

2

It should work this way right now. When built without curl, the debuginfod client will still be able to retrieve artifacts from the local cache.

llvm/lib/Debuginfod/Debuginfod.cpp

35

Thanks!

[noajshu]

Introduce BuildIDRef and enable custom cache directory.

[dblaikie]

llvm/lib/Debuginfod/CMakeLists.txt

2

Hmm, I guess I'm not sure what @labath was referring to with "existence of a component library is predicated on the availability of a third party dep" - perhaps the comment is out of date? Which component library did you have in mind, @labath?

[labath]

llvm/lib/Debuginfod/CMakeLists.txt

2

This component. In the earlier versions of this patch the entire file was wrapped in if(LLVM_ENABLE_CURL).

[dblaikie]

llvm/lib/Debuginfod/CMakeLists.txt

2

Ah, OK!

[noajshu]

Update to use DEBUGINFOD_TIMEOUT and DEBUGINFOD_CACHE_PATH env vars and add unit tests for cache hit / miss.

[labath]

llvm/include/llvm/Debuginfod/Debuginfod.h

43

how about returning std::chrono::milliseconds instead?

llvm/lib/Debuginfod/Debuginfod.cpp

33

Now you have both a using llvm declarating and you're putting everything inside the llvm namespace as well. :)

149

since we're passing them by value, I'd say we can also iterate over them in this way...

[dblaikie]

llvm/lib/Debuginfod/Debuginfod.cpp

67

I guess TimeoutMS is actually a timeout in seconds? (or it's in ms and shouldn't be multiplied by 1000)?

[noajshu]

Convert timeout for debuginfod client library to be in std::chrono::milliseconds, miscellaneous typo fixes.

[noajshu]

llvm/include/llvm/Debuginfod/Debuginfod.h

43

Sure, I've updated this. By the way, do you think the HTTPClient.setTimeout should accept std::chrono::milliseconds as well? If so I can update D112751 and D112753. Otherwise, it just converts back to an integer using .count() before passing to setTimeout.

llvm/lib/Debuginfod/Debuginfod.cpp

67

Good catch. I converted to use std::chrono::milliseconds as @labath suggested and also removed the MS suffix here.

149

ok thanks, this makes sense!

[labath]

llvm/include/llvm/Debuginfod/Debuginfod.h

43

Yes, I'd use it in all APIs that we control, and only convert it to int (or whatever) when we're passing it to curl. That way we can avoid questions like https://reviews.llvm.org/D112758#inline-1095701

[noajshu]

Update HTTPClient::setTimeout to accept std::chrono::milliseconds and use empty vector of URLs when DEBUGINFOD_URLS is not set.

[noajshu]

llvm/unittests/Debuginfod/DebuginfodTests.cpp

14

Thanks for the suggestion! I've added this test below.
How about we divide up the testing responsibility like this:
D112758 -- (this diff) -- Unit test a local cache hit and miss (done)
D112759 -- (llvm-debuginfod-find) -- end-to-end test of local cache and server using Python simple http server
D113717 -- (symbolizer integration) -- end-to-end test of local cache only (just to hit the integration point)

The goal would be to get full coverage (local cache, network, client integration point) without too much overlap across the tests.

[dblaikie]

llvm/unittests/Debuginfod/DebuginfodTests.cpp

14

Sounds good to me (I guess it's infeasible to test the network case in this patch (D112758) because it's a bunch more code to have a C++ server compared to the python http server that can be used once it's an actual command line tool, lit tested, etc as with llvm-debuginfod-find)

[noajshu]

Fix moving error returns to address buildbot failure.

[RKSimon]

llvm/unittests/Debuginfod/DebuginfodTests.cpp

41

@noajshu This is failing on windows builds - please can you revert?

E:\llvm\llvm-project\llvm\unittests\Debuginfod\DebuginfodTests.cpp(40): error C3861: 'setenv': identifier not found

[noajshu]

llvm/unittests/Debuginfod/DebuginfodTests.cpp

41

Thank you. Reverted.

[noajshu]

Add _putenv_s workaround for unit testing cache path support on Windows.

[noajshu]

Due to the passing x64 windows check with the setenv fix I'm going to try committing this again and watch for any issues.

[estewart08]

llvm/unittests/Debuginfod/DebuginfodTests.cpp

34

This eventually returns a path to $HOME via a call to cache_directory. I ran into some permission issues on a CI build. Is it safe to assume that $HOME is always writable?

Should this test set XDG_CACHE_HOME instead?

setenv("XDG_CACHE_HOME", "/tmp", /*replace=*/1);

[noajshu]

llvm/unittests/Debuginfod/DebuginfodTests.cpp

34

Hi @estewart08 thanks for pointing this out. I think you're actually hitting that issue for the next test starting on line 42.

44

Needs to set DEBUGINFOD_CACHE_PATH
Does D115813 LGTY @estewart08 ?

[estewart08]

llvm/unittests/Debuginfod/DebuginfodTests.cpp

44

My mistake, yes this is the correct portion of the test. I will go review D115813.

llvm/include/llvm/Debuginfod/Debuginfod.h

+//===-- llvm/Debuginfod/Debuginfod.h - Debuginfod client --------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file contains the declarations of getCachedOrDownloadArtifact and
+/// several convenience functions for specific artifact types:
+/// getCachedOrDownloadSource, getCachedOrDownloadExecutable, and
+/// getCachedOrDownloadDebuginfo. This file also declares
+/// getDefaultDebuginfodUrls and getDefaultDebuginfodCacheDirectory.
+///
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DEBUGINFOD_DEBUGINFOD_H
+#define LLVM_DEBUGINFOD_DEBUGINFOD_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/MemoryBuffer.h"
+
+namespace llvm {
+
+typedef ArrayRef<uint8_t> BuildIDRef;
+
+typedef SmallVector<uint8_t, 10> BuildID;
+
+/// Finds default array of Debuginfod server URLs by checking DEBUGINFOD_URLS
+/// environment variable.
+Expected<SmallVector<StringRef>> getDefaultDebuginfodUrls();
+
+/// Finds a default local file caching directory for the debuginfod client,
+/// first checking DEBUGINFOD_CACHE_PATH.
+Expected<std::string> getDefaultDebuginfodCacheDirectory();
+
+/// Finds a default timeout for debuginfod HTTP requests. Checks
+/// DEBUGINFOD_TIMEOUT environment variable, default is 90 seconds (90000 ms).
+std::chrono::milliseconds getDefaultDebuginfodTimeout();
+
+/// Fetches a specified source file by searching the default local cache
+/// directory and server URLs.
+Expected<std::string> getCachedOrDownloadSource(BuildIDRef ID,
+                                                StringRef SourceFilePath);
+
+/// Fetches an executable by searching the default local cache directory and
+/// server URLs.
+Expected<std::string> getCachedOrDownloadExecutable(BuildIDRef ID);
+
+/// Fetches a debug binary by searching the default local cache directory and
+/// server URLs.
+Expected<std::string> getCachedOrDownloadDebuginfo(BuildIDRef ID);
+
+/// Fetches any debuginfod artifact using the default local cache directory and
+/// server URLs.
+Expected<std::string> getCachedOrDownloadArtifact(StringRef UniqueKey,
+                                                  StringRef UrlPath);
+
+/// Fetches any debuginfod artifact using the specified local cache directory,
+/// server URLs, and request timeout (in milliseconds). If the artifact is
+/// found, uses the UniqueKey for the local cache file.
+Expected<std::string> getCachedOrDownloadArtifact(
+    StringRef UniqueKey, StringRef UrlPath, StringRef CacheDirectoryPath,
+    ArrayRef<StringRef> DebuginfodUrls, std::chrono::milliseconds Timeout);
+
+} // end namespace llvm
+
+#endif

llvm/include/llvm/Support/Caching.h

 using AddBufferFn =
     std::function<void(unsigned Task, std::unique_ptr<MemoryBuffer> MB)>;
 
 /// Create a local file system cache which uses the given cache name, temporary
 /// file prefix, cache directory and file callback. This function also creates
 /// the cache directory if it does not already exist. The cache name appears in
 /// error messages for errors during caching. The temporary file prefix is used
 /// in the temporary file naming scheme used when writing files atomically.
-Expected<FileCache> localCache(Twine CacheNameRef, Twine TempFilePrefixRef,
-                               Twine CacheDirectoryPathRef,
-                               AddBufferFn AddBuffer);
+Expected<FileCache> localCache(
+    Twine CacheNameRef, Twine TempFilePrefixRef, Twine CacheDirectoryPathRef,
+    AddBufferFn AddBuffer = [](size_t Task, std::unique_ptr<MemoryBuffer> MB) {
+    });
 } // namespace llvm
 
 #endif

llvm/lib/CMakeLists.txt

 add_subdirectory(Analysis)
 add_subdirectory(LTO)
 add_subdirectory(MC)
 add_subdirectory(MCA)
 add_subdirectory(Object)
 add_subdirectory(ObjectYAML)
 add_subdirectory(Option)
 add_subdirectory(Remarks)
+add_subdirectory(Debuginfod)
 add_subdirectory(DebugInfo)
 add_subdirectory(DWP)
 add_subdirectory(ExecutionEngine)
 add_subdirectory(Target)
 add_subdirectory(AsmParser)
 add_subdirectory(LineEditor)
 add_subdirectory(ProfileData)
 add_subdirectory(Passes)

llvm/lib/Debuginfod/CMakeLists.txt

+add_llvm_component_library(LLVMDebuginfod
+  Debuginfod.cpp
+
+  ADDITIONAL_HEADER_DIRS
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Debuginfod
+
+  LINK_COMPONENTS
+  Support
+  )

llvm/lib/Debuginfod/Debuginfod.cpp

+//===-- llvm/Debuginfod/Debuginfod.cpp - Debuginfod client library --------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+///
+/// This file defines the fetchInfo function, which retrieves
+/// any of the three supported artifact types: (executable, debuginfo, source
+/// file) associated with a build-id from debuginfod servers. If a source file
+/// is to be fetched, its absolute path must be specified in the Description
+/// argument to fetchInfo.
+///
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/CachePruning.h"
+#include "llvm/Support/Caching.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/HTTPClient.h"
+#include "llvm/Support/xxhash.h"
+
+namespace llvm {
+static std::string uniqueKey(llvm::StringRef S) { return utostr(xxHash64(S)); }
+
+// Returns a binary BuildID as a normalized hex string.
+// Uses lowercase for compatibility with common debuginfod servers.
+static std::string buildIDToString(BuildIDRef ID) {
+  return llvm::toHex(ID, /*LowerCase=*/true);
+}
+
+Expected<SmallVector<StringRef>> getDefaultDebuginfodUrls() {
+  const char *DebuginfodUrlsEnv = std::getenv("DEBUGINFOD_URLS");
+  if (DebuginfodUrlsEnv == NULL)
+    return SmallVector<StringRef>();
+
+  SmallVector<StringRef> DebuginfodUrls;
+  StringRef(DebuginfodUrlsEnv).split(DebuginfodUrls, " ");
+  return DebuginfodUrls;
+}
+
+Expected<std::string> getDefaultDebuginfodCacheDirectory() {
+  if (const char *CacheDirectoryEnv = std::getenv("DEBUGINFOD_CACHE_PATH"))
+    return CacheDirectoryEnv;
+
+  SmallString<64> CacheDirectory;
+  if (!sys::path::cache_directory(CacheDirectory))
+    return createStringError(
+        errc::io_error, "Unable to determine appropriate cache directory.");
+  return std::string(CacheDirectory);
+}
+
+std::chrono::milliseconds getDefaultDebuginfodTimeout() {
+  long Timeout;
+  const char *DebuginfodTimeoutEnv = std::getenv("DEBUGINFOD_TIMEOUT");
+  if (DebuginfodTimeoutEnv &&
+      to_integer(StringRef(DebuginfodTimeoutEnv).trim(), Timeout, 10))
+    return std::chrono::milliseconds(Timeout * 1000);
+
+  return std::chrono::milliseconds(90 * 1000);
+}
+
+/// The following functions fetch a debuginfod artifact to a file in a local
+/// cache and return the cached file path. They first search the local cache,
+/// followed by the debuginfod servers.
+
+Expected<std::string> getCachedOrDownloadSource(BuildIDRef ID,
+                                                StringRef SourceFilePath) {
+  SmallString<64> UrlPath;
+  sys::path::append(UrlPath, sys::path::Style::posix, "buildid",
+                    buildIDToString(ID), "source",
+                    sys::path::convert_to_slash(SourceFilePath));
+  return getCachedOrDownloadArtifact(uniqueKey(UrlPath), UrlPath);
+}
+
+Expected<std::string> getCachedOrDownloadExecutable(BuildIDRef ID) {
+  SmallString<64> UrlPath;
+  sys::path::append(UrlPath, sys::path::Style::posix, "buildid",
+                    buildIDToString(ID), "executable");
+  return getCachedOrDownloadArtifact(uniqueKey(UrlPath), UrlPath);
+}
+
+Expected<std::string> getCachedOrDownloadDebuginfo(BuildIDRef ID) {
+  SmallString<64> UrlPath;
+  sys::path::append(UrlPath, sys::path::Style::posix, "buildid",
+                    buildIDToString(ID), "debuginfo");
+  return getCachedOrDownloadArtifact(uniqueKey(UrlPath), UrlPath);
+}
+
+// General fetching function.
+Expected<std::string> getCachedOrDownloadArtifact(StringRef UniqueKey,
+                                                  StringRef UrlPath) {
+  SmallString<10> CacheDir;
+
+  Expected<std::string> CacheDirOrErr = getDefaultDebuginfodCacheDirectory();
+  if (!CacheDirOrErr)
+    return CacheDirOrErr.takeError();
+  CacheDir = *CacheDirOrErr;
+
+  Expected<SmallVector<StringRef>> DebuginfodUrlsOrErr =
+      getDefaultDebuginfodUrls();
+  if (!DebuginfodUrlsOrErr)
+    return DebuginfodUrlsOrErr.takeError();
+  SmallVector<StringRef> &DebuginfodUrls = *DebuginfodUrlsOrErr;
+  return getCachedOrDownloadArtifact(UniqueKey, UrlPath, CacheDir,
+                                     DebuginfodUrls,
+                                     getDefaultDebuginfodTimeout());
+}
+
+Expected<std::string> getCachedOrDownloadArtifact(
+    StringRef UniqueKey, StringRef UrlPath, StringRef CacheDirectoryPath,
+    ArrayRef<StringRef> DebuginfodUrls, std::chrono::milliseconds Timeout) {
+  SmallString<64> AbsCachedArtifactPath;
+  sys::path::append(AbsCachedArtifactPath, CacheDirectoryPath,
+                    "llvmcache-" + UniqueKey);
+
+  Expected<FileCache> CacheOrErr =
+      localCache("Debuginfod-client", ".debuginfod-client", CacheDirectoryPath);
+  if (!CacheOrErr)
+    return CacheOrErr.takeError();
+
+  FileCache Cache = *CacheOrErr;
+  // We choose an arbitrary Task parameter as we do not make use of it.
+  unsigned Task = 0;
+  Expected<AddStreamFn> CacheAddStreamOrErr = Cache(Task, UniqueKey);
+  if (!CacheAddStreamOrErr)
+    return CacheAddStreamOrErr.takeError();
+  AddStreamFn &CacheAddStream = *CacheAddStreamOrErr;
+  if (!CacheAddStream)
+    return std::string(AbsCachedArtifactPath);
+  // The artifact was not found in the local cache, query the debuginfod
+  // servers.
+  if (!HTTPClient::isAvailable())
+    return createStringError(errc::io_error,
+                             "No working HTTP client is available.");
+
+  HTTPClient Client;
+  Client.setTimeout(Timeout);
+  for (StringRef ServerUrl : DebuginfodUrls) {
+    SmallString<64> ArtifactUrl;
+    sys::path::append(ArtifactUrl, sys::path::Style::posix, ServerUrl, UrlPath);
+
+    Expected<HTTPResponseBuffer> ResponseOrErr = Client.get(ArtifactUrl);
+    if (!ResponseOrErr)
+      return ResponseOrErr.takeError();
+
+    HTTPResponseBuffer &Response = *ResponseOrErr;
+    if (Response.Code != 200)
+      continue;
+
+    // We have retrieved the artifact from this server, and now add it to the
+    // file cache.
+    Expected<std::unique_ptr<CachedFileStream>> FileStreamOrErr =
+        CacheAddStream(Task);
+    if (FileStreamOrErr)
+      return FileStreamOrErr.takeError();
+    std::unique_ptr<CachedFileStream> &FileStream = *FileStreamOrErr;
+    if (!Response.Body)
+      return createStringError(
+          errc::io_error, "Unallocated MemoryBuffer in HTTPResponseBuffer.");
+
+    *FileStream->OS << StringRef(Response.Body->getBufferStart(),
+                                 Response.Body->getBufferSize());
+
+    // Return the path to the artifact on disk.
+    return std::string(AbsCachedArtifactPath);
+  }
+
+  return createStringError(errc::argument_out_of_domain, "build id not found");
+}
+} // namespace llvm

llvm/unittests/CMakeLists.txt

 add_subdirectory(ADT)
 add_subdirectory(Analysis)
 add_subdirectory(AsmParser)
 add_subdirectory(BinaryFormat)
 add_subdirectory(Bitcode)
 add_subdirectory(Bitstream)
 add_subdirectory(CodeGen)
 add_subdirectory(DebugInfo)
+add_subdirectory(Debuginfod)
 add_subdirectory(Demangle)
 add_subdirectory(ExecutionEngine)
 add_subdirectory(FileCheck)
 add_subdirectory(Frontend)
 add_subdirectory(FuzzMutate)
 add_subdirectory(InterfaceStub)
 add_subdirectory(IR)
 add_subdirectory(LineEditor)

llvm/unittests/Debuginfod/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  Debuginfod
+  )
+
+add_llvm_unittest(DebuginfodTests
+  DebuginfodTests.cpp
+  )
+
+target_link_libraries(DebuginfodTests PRIVATE LLVMTestingSupport)

llvm/unittests/Debuginfod/DebuginfodTests.cpp

+//===-- llvm/unittest/Support/DebuginfodTests.cpp - unit tests --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/HTTPClient.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+#ifdef _WIN32
+#define setenv(name, var, ignore) _putenv_s(name, var)
+#endif
+
+using namespace llvm;
+
+// Check that the Debuginfod client can find locally cached artifacts.
+TEST(DebuginfodClient, CacheHit) {
+  int FD;
+  SmallString<64> CachedFilePath;
+  sys::fs::createTemporaryFile("llvmcache-key", "temp", FD, CachedFilePath);
+  StringRef CacheDir = sys::path::parent_path(CachedFilePath);
+  StringRef UniqueKey = sys::path::filename(CachedFilePath);
+  EXPECT_TRUE(UniqueKey.consume_front("llvmcache-"));
+  raw_fd_ostream OF(FD, true, /*unbuffered=*/true);
+  OF << "contents\n";
+  OF << CacheDir << "\n";
+  OF.close();
+  Expected<std::string> PathOrErr = getCachedOrDownloadArtifact(
+      UniqueKey, /*UrlPath=*/"/null", CacheDir,
+      /*DebuginfodUrls=*/{}, /*Timeout=*/std::chrono::milliseconds(1));
+  EXPECT_THAT_EXPECTED(PathOrErr, HasValue(CachedFilePath));
+}
+
+// Check that the Debuginfod client returns an Error when it fails to find an
+// artifact.
+TEST(DebuginfodClient, CacheMiss) {
+  // Ensure there are no urls to guarantee a cache miss.
+  setenv("DEBUGINFOD_URLS", "", /*replace=*/1);
+  HTTPClient::initialize();
+  Expected<std::string> PathOrErr = getCachedOrDownloadArtifact(
+      /*UniqueKey=*/"nonexistent-key", /*UrlPath=*/"/null");
+  EXPECT_THAT_EXPECTED(PathOrErr, Failed<StringError>());
+}