[LLDB] Add setting for native PDB reader (#151490)

Initially suggested in
#149305 (comment)
- this PR adds the setting `plugin.symbol-file.pdb.use-native-reader`.
It doesn't remove support for `LLDB_USE_NATIVE_PDB_READER` to allow some
backwards compatibility. This was the suggested way to use the native
reader - changing that would mean users who set this, now use the DIA
reader. The setting has priority over the environment variable, though.
If the default gets flipped on Windows, the environment variable could
probably be removed as well.

This would make it possible to test both native PDB and DIA PDB in the
API tests (see linked PR).

Author: Nerixyz

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

@@ -11,6 +11,7 @@
 #include "Plugins/ExpressionParser/Clang/ClangUtil.h"
 #include "Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h"
 #include "Plugins/ObjectFile/PDB/ObjectFilePDB.h"
+#include "Plugins/SymbolFile/PDB/SymbolFilePDB.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
@@ -298,6 +299,9 @@ llvm::StringRef SymbolFileNativePDB::GetPluginDescriptionStatic() {
 }
 
 SymbolFile *SymbolFileNativePDB::CreateInstance(ObjectFileSP objfile_sp) {
+  if (!SymbolFilePDB::UseNativePDB())
+    return nullptr;
+
   return new SymbolFileNativePDB(std::move(objfile_sp));
 }
 

lldb/source/Plugins/SymbolFile/PDB/CMakeLists.txt

@@ -1,3 +1,11 @@
+lldb_tablegen(SymbolFilePDBProperties.inc -gen-lldb-property-defs
+  SOURCE SymbolFilePDBProperties.td
+  TARGET LLDBPluginSymbolFilePDBPropertiesGen)
+
+lldb_tablegen(SymbolFilePDBPropertiesEnum.inc -gen-lldb-property-enum-defs
+  SOURCE SymbolFilePDBProperties.td
+  TARGET LLDBPluginSymbolFilePDBPropertiesEnumGen)
+
 add_lldb_library(lldbPluginSymbolFilePDB PLUGIN
   PDBASTParser.cpp
   PDBLocationToDWARFExpression.cpp
@@ -16,3 +24,7 @@ add_lldb_library(lldbPluginSymbolFilePDB PLUGIN
     clangAST
     clangLex
   )
+
+add_dependencies(lldbPluginSymbolFilePDB
+  LLDBPluginSymbolFilePDBPropertiesGen
+  LLDBPluginSymbolFilePDBPropertiesEnumGen)

lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

@@ -71,6 +71,93 @@ LLDB_PLUGIN_DEFINE(SymbolFilePDB)
 char SymbolFilePDB::ID;
 
 namespace {
+
+enum PDBReader {
+  ePDBReaderDefault,
+  ePDBReaderDIA,
+  ePDBReaderNative,
+};
+
+constexpr OptionEnumValueElement g_pdb_reader_enums[] = {
+    {
+        ePDBReaderDefault,
+        "default",
+        "Use DIA PDB reader unless LLDB_USE_NATIVE_PDB_READER environment "
+        "variable is set",
+    },
+    {
+        ePDBReaderDIA,
+        "dia",
+        "Use DIA PDB reader",
+    },
+    {
+        ePDBReaderNative,
+        "native",
+        "Use native PDB reader",
+    },
+};
+
+#define LLDB_PROPERTIES_symbolfilepdb
+#include "SymbolFilePDBProperties.inc"
+
+enum {
+#define LLDB_PROPERTIES_symbolfilepdb
+#include "SymbolFilePDBPropertiesEnum.inc"
+};
+
+#if LLVM_ENABLE_DIA_SDK && defined(_WIN32)
+bool ShouldUseNativeReaderByDefault() {
+  static bool g_use_native_by_default = true;
+
+  static llvm::once_flag g_initialize;
+  llvm::call_once(g_initialize, [] {
+    llvm::StringRef env_value = ::getenv("LLDB_USE_NATIVE_PDB_READER");
+    if (!env_value.equals_insensitive("on") &&
+        !env_value.equals_insensitive("yes") &&
+        !env_value.equals_insensitive("1") &&
+        !env_value.equals_insensitive("true"))
+      g_use_native_by_default = false;
+  });
+
+  return g_use_native_by_default;
+}
+#endif
+
+class PluginProperties : public Properties {
+public:
+  static llvm::StringRef GetSettingName() {
+    return SymbolFilePDB::GetPluginNameStatic();
+  }
+
+  PluginProperties() {
+    m_collection_sp = std::make_shared<OptionValueProperties>(GetSettingName());
+    m_collection_sp->Initialize(g_symbolfilepdb_properties);
+  }
+
+  bool UseNativeReader() const {
+#if LLVM_ENABLE_DIA_SDK && defined(_WIN32)
+    auto value =
+        GetPropertyAtIndexAs<PDBReader>(ePropertyReader, ePDBReaderDefault);
+    switch (value) {
+    case ePDBReaderNative:
+      return true;
+    case ePDBReaderDIA:
+      return false;
+    default:
+    case ePDBReaderDefault:
+      return ShouldUseNativeReaderByDefault();
+    }
+#else
+    return true;
+#endif
+  }
+};
+
+PluginProperties &GetGlobalPluginProperties() {
+  static PluginProperties g_settings;
+  return g_settings;
+}
+
 lldb::LanguageType TranslateLanguage(PDB_Lang lang) {
   switch (lang) {
   case PDB_Lang::Cpp:
@@ -97,46 +184,43 @@ bool ShouldAddLine(uint32_t requested_line, uint32_t actual_line,
 }
 } // namespace
 
-static bool ShouldUseNativeReader() {
-#if defined(_WIN32)
-#if LLVM_ENABLE_DIA_SDK
-  llvm::StringRef use_native = ::getenv("LLDB_USE_NATIVE_PDB_READER");
-  if (!use_native.equals_insensitive("on") &&
-      !use_native.equals_insensitive("yes") &&
-      !use_native.equals_insensitive("1") &&
-      !use_native.equals_insensitive("true"))
-    return false;
-#endif
-#endif
-  return true;
-}
-
 void SymbolFilePDB::Initialize() {
-  if (ShouldUseNativeReader()) {
-    npdb::SymbolFileNativePDB::Initialize();
-  } else {
-    PluginManager::RegisterPlugin(GetPluginNameStatic(),
-                                  GetPluginDescriptionStatic(), CreateInstance,
-                                  DebuggerInitialize);
-  }
+  // Initialize both but check in CreateInstance for the desired plugin
+  npdb::SymbolFileNativePDB::Initialize();
+
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+                                GetPluginDescriptionStatic(), CreateInstance,
+                                DebuggerInitialize);
 }
 
 void SymbolFilePDB::Terminate() {
-  if (ShouldUseNativeReader()) {
-    npdb::SymbolFileNativePDB::Terminate();
-  } else {
-    PluginManager::UnregisterPlugin(CreateInstance);
-  }
+  npdb::SymbolFileNativePDB::Terminate();
+
+  PluginManager::UnregisterPlugin(CreateInstance);
+}
+
+bool SymbolFilePDB::UseNativePDB() {
+  return GetGlobalPluginProperties().UseNativeReader();
 }
 
-void SymbolFilePDB::DebuggerInitialize(lldb_private::Debugger &debugger) {}
+void SymbolFilePDB::DebuggerInitialize(lldb_private::Debugger &debugger) {
+  if (!PluginManager::GetSettingForSymbolFilePlugin(
+          debugger, PluginProperties::GetSettingName())) {
+    PluginManager::CreateSettingForSymbolFilePlugin(
+        debugger, GetGlobalPluginProperties().GetValueProperties(),
+        "Properties for the PDB symbol-file plug-in.", true);
+  }
+}
 
 llvm::StringRef SymbolFilePDB::GetPluginDescriptionStatic() {
   return "Microsoft PDB debug symbol file reader.";
 }
 
 lldb_private::SymbolFile *
 SymbolFilePDB::CreateInstance(ObjectFileSP objfile_sp) {
+  if (UseNativePDB())
+    return nullptr;
+
   return new SymbolFilePDB(std::move(objfile_sp));
 }
 

lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h

@@ -49,6 +49,8 @@ class SymbolFilePDB : public lldb_private::SymbolFileCommon {
   static lldb_private::SymbolFile *
   CreateInstance(lldb::ObjectFileSP objfile_sp);
 
+  static bool UseNativePDB();
+
   // Constructors and Destructors
   SymbolFilePDB(lldb::ObjectFileSP objfile_sp);
 

lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDBProperties.td

@@ -0,0 +1,14 @@
+include "../../../../include/lldb/Core/PropertiesBase.td"
+
+let Definition = "symbolfilepdb" in {
+  def Reader: Property<"reader", "Enum">,
+    Global,
+    DefaultEnumValue<"ePDBReaderDefault">,
+    EnumValues<"OptionEnumValues(g_pdb_reader_enums)">,
+    Desc<"Selects the reader for PDB symbol files. "
+         "The native PDB reader that uses LLVM's PDB support is always available (value: 'native'). "
+         "Secondly, the DIA PDB reader is only available if LLVM was comppiled with Microsoft's DIA SDK on Windows (value: 'DIA'). "
+         "By default, the DIA PDB reader is used if available. "
+         "The LLDB_USE_NATIVE_PDB_READER environment variable can be used to switch to the native reader when this setting has the default value. "
+         "Otherwise, the setting always has priority.">;
+}

lldb/test/Shell/SymbolFile/PDB/native-setting.cpp

@@ -0,0 +1,52 @@
+// REQUIRES: target-windows
+
+// Test plugin.symbol-file.pdb.reader setting
+// RUN: %build -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=0 %lldb %t.exe -o 'target modules dump symfile' | FileCheck --check-prefix=ENV0 %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb %t.exe -o 'target modules dump symfile' | FileCheck --check-prefix=ENV1 %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=0 %lldb \
+// RUN:     -o 'settings set plugin.symbol-file.pdb.reader dia' \
+// RUN:     -o 'target create %t.exe' \
+// RUN:     -o 'target modules dump symfile' \
+// RUN:     | FileCheck --check-prefix=ENV0-SET-DIA %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb \
+// RUN:     -o 'settings set plugin.symbol-file.pdb.reader dia' \
+// RUN:     -o 'target create %t.exe' \
+// RUN:     -o 'target modules dump symfile' \
+// RUN:     | FileCheck --check-prefix=ENV1-SET-DIA %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=0 %lldb \
+// RUN:     -o 'settings set plugin.symbol-file.pdb.reader native' \
+// RUN:     -o 'target create %t.exe' \
+// RUN:     -o 'target modules dump symfile' \
+// RUN:     | FileCheck --check-prefix=ENV0-SET-NATIVE %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb \
+// RUN:     -o 'settings set plugin.symbol-file.pdb.reader native' \
+// RUN:     -o 'target create %t.exe' \
+// RUN:     -o 'target modules dump symfile' \
+// RUN:     | FileCheck --check-prefix=ENV1-SET-NATIVE %s
+
+// ENV0: (lldb) target modules dump symfile
+// ENV0: Dumping debug symbols for 1 modules.
+// ENV0: SymbolFile pdb
+
+// ENV1: (lldb) target modules dump symfile
+// ENV1: Dumping debug symbols for 1 modules.
+// ENV1: SymbolFile native-pdb
+
+// ENV0-SET-DIA: (lldb) target modules dump symfile
+// ENV0-SET-DIA: Dumping debug symbols for 1 modules.
+// ENV0-SET-DIA: SymbolFile pdb
+
+// ENV1-SET-DIA: (lldb) target modules dump symfile
+// ENV1-SET-DIA: Dumping debug symbols for 1 modules.
+// ENV1-SET-DIA: SymbolFile pdb
+
+// ENV0-SET-NATIVE: (lldb) target modules dump symfile
+// ENV0-SET-NATIVE: Dumping debug symbols for 1 modules.
+// ENV0-SET-NATIVE: SymbolFile native-pdb
+
+// ENV1-SET-NATIVE: (lldb) target modules dump symfile
+// ENV1-SET-NATIVE: Dumping debug symbols for 1 modules.
+// ENV1-SET-NATIVE: SymbolFile native-pdb
+
+int main() {}