[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

1

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

1

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

1

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

1

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

1

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

1

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

35

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

35

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

45

Needs to set DEBUGINFOD_CACHE_PATH
Does D115813 LGTY @estewart08 ?

[estewart08]

llvm/unittests/Debuginfod/DebuginfodTests.cpp

45

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 the debuginfod::fetchInfo
+/// function and the debuginfod::AssetType enum class
+///
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_SUPPORT_DEBUGINFOD_H
+#define LLVM_SUPPORT_DEBUGINFOD_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/MemoryBuffer.h"
+
+namespace llvm {
+
+/// DebuginfodAsset - This enum class specifies the types of
+/// debugging information that can be requested from a debuginfod
+/// server.
+enum class DebuginfodAssetType { Executable, Debuginfo, Source };
+
+/// Fetch a debuginfod asset to a file in a local cache and return the cached
+/// file path. First queries the local cache in the given directory path,
+/// followed by the debuginfod servers at the given urls for the specified
+/// type of information about the given build ID. Source files are specified
+/// by their absolute path provided in the Description. A callback function
+/// may optionally be provided for further processing of the fetched asset.
+Expected<SmallString<64>> fetchDebuginfo(
+    StringRef CacheDirectoryPath, ArrayRef<StringRef> DebuginfodUrls,
+    StringRef BuildID, DebuginfodAssetType Type, StringRef Description = "",
+    std::function<void(size_t, std::unique_ptr<MemoryBuffer>)> AddBuffer =
+        [](size_t Task, std::unique_ptr<MemoryBuffer> MB) {});
+
+} // end namespace llvm
+
+#endif

llvm/lib/CMakeLists.txt

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

llvm/lib/Debuginfod/CMakeLists.txt

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

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 asset 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/CurlHTTPClient.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/HTTPClient.h"
+#include "llvm/Support/xxhash.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "DEBUGINFOD"
+
+Expected<SmallString<64>> llvm::fetchDebuginfo(
+    StringRef CacheDirectoryPath, ArrayRef<StringRef> DebuginfodUrls,
+    StringRef BuildID, DebuginfodAssetType Type, StringRef Description,
+    std::function<void(size_t, std::unique_ptr<MemoryBuffer>)> AddBuffer) {
+  LLVM_DEBUG(dbgs() << "fetching info, debuginfod urls size = "
+                    << DebuginfodUrls.size() << "\n";);
+  std::string Suffix;
+  switch (Type) {
+  case DebuginfodAssetType::Executable:
+    Suffix = "executable";
+    break;
+  case DebuginfodAssetType::Debuginfo:
+    Suffix = "debuginfo";
+    break;
+  case DebuginfodAssetType::Source:
+    // Description is the absolute source path
+    Suffix = "source" + Description.str();
+    break;
+  }
+
+  std::string UniqueKey = utostr(xxHash64((BuildID + Suffix).str()));
+
+  Expected<NativeObjectCache> CacheOrErr = localCache(
+      "Debuginfod-client", ".debuginfod-client", CacheDirectoryPath, AddBuffer);
+  if (Error Err = CacheOrErr.takeError())
+    return Err;
+
+  NativeObjectCache &Cache = *CacheOrErr;
+
+  // We choose an arbitrary Task parameter as we do not make use of it.
+  unsigned Task = 0;
+  AddStreamFn CacheAddStream = Cache(Task, UniqueKey);
+
+  SmallString<64> AbsCachedAssetPath;
+  sys::path::append(AbsCachedAssetPath, CacheDirectoryPath,
+                    "llvmcache-" + UniqueKey);
+
+  if (!CacheAddStream) {
+    LLVM_DEBUG(dbgs() << "cache hit\n";);
+    return AbsCachedAssetPath;
+  }
+
+  LLVM_DEBUG(dbgs() << "cache miss, UniqueKey = " << UniqueKey << "\n";);
+
+  // The asset was not found in the local cache, so we must query
+  // the debuginfod servers.
+  for (const StringRef &ServerUrl : DebuginfodUrls) {
+    SmallString<64> AssetUrl;
+    sys::path::append(AssetUrl, sys::path::Style::posix, ServerUrl, "buildid",
+                      BuildID, Suffix);
+
+    Expected<HTTPResponseBuffer> BufferOrErr = CurlHTTPClient().get(AssetUrl);
+    if (Error Err = BufferOrErr.takeError())
+      return Err;
+    HTTPResponseBuffer &Buf = *BufferOrErr;
+    if (Buf.Code != 200)
+      continue;
+
+    // We have retrieved the asset from this server and now add it to the file
+    // cache.
+    auto Stream = CacheAddStream(Task);
+    assert(Buf.Body && "Unallocated MemoryBuffer in HTTPResponseBuffer.");
+    *Stream->OS << StringRef(Buf.Body->getBufferStart(),
+                             Buf.Body->getBufferSize());
+
+    // Return the path to the asset on disk.
+    return AbsCachedAssetPath;
+  }
+
+  return createStringError(errc::argument_out_of_domain, "build id not found");
+}

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

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

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/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+TEST(DebuginfodTests, noDebuginfodUrlsFetchInfoTest) {
+  EXPECT_THAT_EXPECTED(fetchDebuginfo("./", {}, "fakeBuildId",
+                                      llvm::DebuginfodAssetType::Executable,
+                                      ""),
+                       llvm::Failed<llvm::StringError>());
+}