Merge pull request ClickHouse#80545 from ClickHouse/sind

Fix build id lookup in SymbolIndex

Author: al13n321

src/Common/StackTrace.cpp

@@ -369,7 +369,10 @@ StackTrace::StackTrace(const ucontext_t & signal_context)
         /// Skip excessive stack frames that we have created while finding stack trace.
         for (size_t i = 0; i < size; ++i)
         {
-            if (frame_pointers[i] == caller_address)
+            if (frame_pointers[i] == caller_address ||
+                /// This compensates for a hack in libunwind, see the "+ 1" in
+                /// UnwindCursor<A, R>::stepThroughSigReturn.
+                frame_pointers[i] == reinterpret_cast<void *>(reinterpret_cast<char *>(caller_address) + 1))
             {
                 offset = i;
                 break;

src/Common/SymbolIndex.cpp

@@ -238,7 +238,9 @@ String getBuildIDFromProgramHeaders(dl_phdr_info * info)
 
         std::string_view view(reinterpret_cast<const char *>(info->dlpi_addr + phdr.p_vaddr), phdr.p_memsz);
         __msan_unpoison(view.data(), view.size());
-        return Elf::getBuildID(view.data(), view.size());
+        String build_id = Elf::getBuildID(view.data(), view.size());
+        if (!build_id.empty()) // there may be multiple PT_NOTE segments
+            return build_id;
     }
     return {};
 }
@@ -316,37 +318,38 @@ void collectSymbolsFromELF(
     dl_phdr_info * info,
     std::vector<SymbolIndex::Symbol> & symbols,
     std::vector<SymbolIndex::Object> & objects,
-    String & build_id)
+    String & self_build_id)
 {
     String object_name;
-    String our_build_id;
+    String build_id;
 
 #if defined (USE_MUSL)
     object_name = "/proc/self/exe";
-    our_build_id = Elf(object_name).getBuildID();
-    build_id = our_build_id;
+    build_id = Elf(object_name).getBuildID();
+    self_build_id = build_id;
 #else
     /// MSan does not know that the program segments in memory are initialized.
     __msan_unpoison(info, sizeof(*info));
     __msan_unpoison_string(info->dlpi_name);
 
     object_name = info->dlpi_name;
-    our_build_id = getBuildIDFromProgramHeaders(info);
+    build_id = getBuildIDFromProgramHeaders(info);
 
     /// If the name is empty and there is a non-empty build-id - it's main executable.
     /// Find a elf file for the main executable and set the build-id.
     if (object_name.empty())
     {
         object_name = "/proc/self/exe";
 
-        if (our_build_id.empty())
-            our_build_id = Elf(object_name).getBuildID();
-
         if (build_id.empty())
-            build_id = our_build_id;
+            build_id = Elf(object_name).getBuildID();
+
+        if (self_build_id.empty())
+            self_build_id = build_id;
     }
 #endif
 
+    /// Note: we load ELF from file; this doesn't work for vdso because it's only present in memory.
     std::error_code ec;
     std::filesystem::path canonical_path = std::filesystem::canonical(object_name, ec);
 
@@ -405,17 +408,18 @@ void collectSymbolsFromELF(
 
     String file_build_id = object.elf->getBuildID();
 
-    if (our_build_id != file_build_id)
+    if (build_id != file_build_id)
     {
-        /// If debug info doesn't correspond to our binary, fallback to the info in our binary.
+        /// If the separate debuginfo binary doesn't correspond to the loaded binary, fallback to
+        /// the info in the loaded binary.
         if (object_name != canonical_path)
         {
             object_name = canonical_path;
             object.elf = std::make_unique<Elf>(object_name);
 
             /// But it can still be outdated, for example, if executable file was deleted from filesystem and replaced by another file.
             file_build_id = object.elf->getBuildID();
-            if (our_build_id != file_build_id)
+            if (build_id != file_build_id)
                 return;
         }
         else
@@ -445,7 +449,7 @@ int collectSymbols(dl_phdr_info * info, size_t, void * data_ptr)
     SymbolIndex::Data & data = *reinterpret_cast<SymbolIndex::Data *>(data_ptr);
 
     collectSymbolsFromProgramHeaders(info, data.symbols);
-    collectSymbolsFromELF(info, data.symbols, data.objects, data.build_id);
+    collectSymbolsFromELF(info, data.symbols, data.objects, data.self_build_id);
 
     /* Continue iterations */
     return 0;
@@ -499,10 +503,10 @@ const SymbolIndex::Object * SymbolIndex::findObject(const void * address) const
 String SymbolIndex::getBuildIDHex() const
 {
     String build_id_hex;
-    build_id_hex.resize(data.build_id.size() * 2);
+    build_id_hex.resize(data.self_build_id.size() * 2);
 
     char * pos = build_id_hex.data();
-    for (auto c : data.build_id)
+    for (auto c : data.self_build_id)
     {
         writeHexByteUppercase(c, pos);
         pos += 2;

src/Common/SymbolIndex.h

@@ -46,14 +46,15 @@ class SymbolIndex : private boost::noncopyable
     const std::vector<Object> & objects() const { return data.objects; }
 
     /// The BuildID that is generated by compiler.
-    String getBuildID() const { return data.build_id; }
+    String getBuildID() const { return data.self_build_id; }
     String getBuildIDHex() const;
 
     struct Data
     {
         std::vector<Symbol> symbols;
         std::vector<Object> objects;
-        String build_id;
+        /// BuildID from the Object corresponding to main executable (as opposed to dynamic libraries).
+        String self_build_id;
     };
 private:
     Data data;