[Lldb-commits] [lldb] [lldb] Add caching and _NT_SYMBOL_PATH parsing in SymbolLocatorSymStore (PR #191782)

[Stefan Gränitz via lldb-commits]

================
@@ -223,16 +293,86 @@ std::optional<FileSpec> FindFileInLocalSymStore(llvm::StringRef root_dir,
   return spec;
 }
 
-std::optional<FileSpec> LocateSymStoreEntry(llvm::StringRef base_url,
+std::optional<FileSpec> MoveToLocalSymStore(llvm::StringRef cache,
                                             llvm::StringRef key,
-                                            llvm::StringRef pdb_name) {
-  if (base_url.starts_with("http://") || base_url.starts_with("https://"))
-    return RequestFileFromSymStoreServerHTTP(base_url, key, pdb_name);
+                                            llvm::StringRef pdb_name,
+                                            FileSpec tmp_file) {
+  // Caches have SymStore directory structure: cache/pdb_name/key/pdb_name
+  llvm::SmallString<256> dest_dir;
+  llvm::sys::path::append(dest_dir, cache, pdb_name, key);
+  if (std::error_code ec = llvm::sys::fs::create_directories(dest_dir)) {
+    Debugger::ReportWarning(
+        llvm::formatv("failed to create SymStore cache directory '{0}': {1}",
+                      dest_dir, ec.message()));
+    return tmp_file;
+  }
+
+  llvm::SmallString<256> dest;
+  llvm::sys::path::append(dest, dest_dir, pdb_name);
+  std::error_code ec = llvm::sys::fs::rename(tmp_file.GetPath(), dest);
+
+  // Fall back to copy+delete if we move to a different volume.
+  if (ec == std::errc::cross_device_link) {
+    ec = llvm::sys::fs::copy_file(tmp_file.GetPath(), dest);
+    if (!ec)
+      llvm::sys::fs::remove(tmp_file.GetPath());
+  }
+  if (ec) {
+    Debugger::ReportWarning(
+        llvm::formatv("failed to move '{0}' to SymStore cache '{1}': {2}",
+                      tmp_file.GetPath(), dest, ec.message()));
+    return tmp_file;
+  }
+
+  return FileSpec(dest.str());
+}
+
+std::optional<FileSpec>
+LocateSymStoreEntry(const SymbolLocatorSymStore::LookupEntry &entry,
+                    llvm::StringRef key, llvm::StringRef pdb_name) {
+  Log *log = GetLog(LLDBLog::Symbols);
+  std::string default_cache = GetGlobalPluginProperties().GetCachePath();
+
+  llvm::StringRef url = entry.source;
+  if (url.starts_with("http://") || url.starts_with("https://")) {
+    // Check cache first.
+    if (entry.cache) {
+      if (auto spec = FindFileInLocalSymStore(*entry.cache, key, pdb_name)) {
+        LLDB_LOG_VERBOSE(log, "Found {0} in SymStore cache {1}", pdb_name,
+                         *entry.cache);
+        return *spec;
+      }
+    } else {
+      // Check LLDB default cache to avoid duplicate downloads in the same
+      // session.
+      if (auto spec = FindFileInLocalSymStore(default_cache, key, pdb_name)) {
+        LLDB_LOG_VERBOSE(log, "Found {0} in SymStore cache {1}", pdb_name,
+                         default_cache);
+        return *spec;
+      }
+    }
+
+    // Download and move to cache.
+    if (auto spec = RequestFileFromSymStoreServerHTTP(url, key, pdb_name)) {
+      LLDB_LOG_VERBOSE(log, "Downloaded {0} from SymStore {1}", pdb_name, url);
----------------
weliveindetail wrote:

Right, for some reason I started with the verbose one and never questioned that. Let me land that in a separate PR from https://github.com/weliveindetail/llvm-project/commit/lldb-symbollocator-symstore-logs

https://github.com/llvm/llvm-project/pull/191782