[llvm-debuginfo-analyzer] LLVM 16.0.0-rc1 Failing test on osx-64.

[CarlosAlbertoEnciso]

Summary

[llvm-debuginfo-analyzer] LLVM 16.0.0-rc1 Failing test on osx-64.

As describe in: https://github.com/llvm/llvm-project/issues/60363

the following DebugInfo LogicalView Tests unit tests failed:

• ELFReader
• SelectElements

The tests fail only on the OSX-64 platform with the CMake options:

-DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_LINK_LLVM_DYLIB=ON

Using the same options on a Linux platform all the tests pass:
https://lab.llvm.org/buildbot/#/builders/196 (llvm-x86_64-debian-dylib)

Basically it is a dynamic library initialization affecting a static instance for the string pool (LVStringPool).

That string pool instance is accessed by all the logical elements to store/retrieve any associated string during the creation of the logical view.

For a logical view comparison, both logical readers (Reference and Target) use retrieved indexes when comparing their strings.

Moved the static instance to LVSupport module (unnamed namespace).

[jmorse]

If I understand correctly, this is moving the tool-global string pool from a header-function into something with one-definition -- definitely the right direction to take. I think Orlando might have said that a context-object-with-string-pool is probably the best overall solution, but this is a good improvement for now.

llvm/include/llvm/DebugInfo/LogicalView/Core/LVStringPool.h

39–40

Best not to include unrelated NFC changes as it can be a review burden,

[CarlosAlbertoEnciso]

llvm/include/llvm/DebugInfo/LogicalView/Core/LVStringPool.h

39–40

Good point. I will remove it from the patch.

[h-vetinari]

Tested this patch in https://github.com/conda-forge/llvmdev-feedstock/pull/198, can confirm it works. Thanks a lot!

[CarlosAlbertoEnciso]

In D143716#4121360, @h-vetinari wrote:

Tested this patch in https://github.com/conda-forge/llvmdev-feedstock/pull/198, can confirm it works. Thanks a lot!

@h-vetinari Thanks for testing the patch.

[CarlosAlbertoEnciso]

@jmorse Thanks for your review.

llvm/include/llvm/DebugInfo/LogicalView/Core/LVElement.h

 // information element.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_DEBUGINFO_LOGICALVIEW_CORE_LVELEMENT_H
 #define LLVM_DEBUGINFO_LOGICALVIEW_CORE_LVELEMENT_H
 
 #include "llvm/DebugInfo/LogicalView/Core/LVObject.h"
-#include "llvm/DebugInfo/LogicalView/Core/LVStringPool.h"
 #include "llvm/Support/Casting.h"
 #include <map>
 #include <set>
 #include <vector>
 
 namespace llvm {
 namespace logicalview {
 

llvm/include/llvm/DebugInfo/LogicalView/Core/LVStringPool.h

class LVStringPool {
   using ValueType = TableType::value_type;
   BumpPtrAllocator Allocator;
   TableType StringTable;
   std::vector<ValueType *> Entries;
 
 public:
   LVStringPool() { getIndex(""); }
   LVStringPool(LVStringPool const &other) = delete;
   LVStringPool(LVStringPool &&other) = delete;
   ~LVStringPool() = default;
 
   bool isValidIndex(size_t Index) const { return Index != BadIndex; }
 
   // Return number of strings in the pool. The empty string is allocated
   // at the slot zero. We substract 1 to indicate the number of non empty
   // strings.
   size_t getSize() const { return Entries.size() - 1; }
 
size_t getIndex(StringRef Key) {
     return Value;
   }
 
   // Given the index, return its corresponding string.
   StringRef getString(size_t Index) const {
     return (Index >= Entries.size()) ? StringRef() : Entries[Index]->getKey();
   }
 
-  static LVStringPool &getInstance() {
-    static LVStringPool Instance;
-    return Instance;
-  }
-
   void print(raw_ostream &OS) const {
     if (!Entries.empty()) {
       OS << "\nString Pool:\n";
       for (const ValueType *Entry : Entries)
         OS << "Index: " << Entry->getValue() << ", "
            << "Key: '" << Entry->getKey() << "'\n";
     }
   }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   void dump() const { print(dbgs()); }
 #endif
 };
 
-inline LVStringPool &getStringPool() { return LVStringPool::getInstance(); }
-
 } // namespace logicalview
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_LOGICALVIEW_CORE_LVSTRINGPOOL_H

llvm/include/llvm/DebugInfo/LogicalView/Core/LVSupport.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_DEBUGINFO_LOGICALVIEW_CORE_LVSUPPORT_H
 #define LLVM_DEBUGINFO_LOGICALVIEW_CORE_LVSUPPORT_H
 
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/DebugInfo/LogicalView/Core/LVStringPool.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cctype>
 #include <map>
 #include <sstream>
 
 namespace llvm {
 namespace logicalview {
 
+// Returns the unique string pool instance.
+LVStringPool &getStringPool();
+
 template <typename T>
 using TypeIsValid = std::bool_constant<std::is_pointer<T>::value>;
 
 // Utility class to help memory management and perform an automatic cleaning.
 template <typename T, unsigned N = 8>
 class LVAutoSmallVector : public SmallVector<T, N> {
   static_assert(TypeIsValid<T>::value, "T must be a pointer type");
 

llvm/lib/DebugInfo/LogicalView/Core/LVSupport.cpp

 #include "llvm/Support/FormatVariadic.h"
 #include <iomanip>
 
 using namespace llvm;
 using namespace llvm::logicalview;
 
 #define DEBUG_TYPE "Support"
 
+namespace {
+// Unique string pool instance used by all logical readers.
+LVStringPool StringPool;
+} // namespace
+LVStringPool &llvm::logicalview::getStringPool() { return StringPool; }
+
 // Perform the following transformations to the given 'Path':
 // - all characters to lowercase.
 // - '\\' into '/' (Platform independent).
 // - '//' into '/'
 std::string llvm::logicalview::transformPath(StringRef Path) {
   std::string Name(Path);
   std::transform(Name.begin(), Name.end(), Name.begin(), tolower);
   std::replace(Name.begin(), Name.end(), '\\', '/');