[LLDB] Add setting for native PDB reader #151490
Open
+195
−27
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: nerix (Nerixyz) ChangesInitially suggested in #149305 (comment) - this PR adds the setting This would make it possible to test both native PDB and DIA PDB in the API tests (see linked PR). Full diff: https://github.com/llvm/llvm-project/pull/151490.diff 6 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 20d8c1acf9c42..2afe4dd12f7d5 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/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));
}
diff --git a/lldb/source/Plugins/SymbolFile/PDB/CMakeLists.txt b/lldb/source/Plugins/SymbolFile/PDB/CMakeLists.txt
index 64590d7977b1f..c1cc3cd13a96f 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/CMakeLists.txt
+++ b/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)
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index 96519ae0b3157..954e92c9a4460 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -71,6 +71,93 @@ LLDB_PLUGIN_DEFINE(SymbolFilePDB)
char SymbolFilePDB::ID;
namespace {
+
+enum UseNativePDBReader {
+ eUseNativePDBReaderDefault,
+ eUseNativePDBReaderOn,
+ eUseNativePDBReaderOff,
+};
+
+constexpr OptionEnumValueElement g_native_pdb_reader_enums[] = {
+ {
+ eUseNativePDBReaderDefault,
+ "default",
+ "Use DIA PDB reader unless LLDB_USE_NATIVE_PDB_READER environment "
+ "variable is set",
+ },
+ {
+ eUseNativePDBReaderOn,
+ "on",
+ "Use native PDB reader",
+ },
+ {
+ eUseNativePDBReaderOff,
+ "off",
+ "Use DIA 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<UseNativePDBReader>(
+ ePropertyUseNativeReader, eUseNativePDBReaderDefault);
+ switch (value) {
+ case eUseNativePDBReaderOn:
+ return true;
+ case eUseNativePDBReaderOff:
+ return false;
+ default:
+ case eUseNativePDBReaderDefault:
+ 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,39 +184,33 @@ 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.";
@@ -137,6 +218,9 @@ llvm::StringRef SymbolFilePDB::GetPluginDescriptionStatic() {
lldb_private::SymbolFile *
SymbolFilePDB::CreateInstance(ObjectFileSP objfile_sp) {
+ if (UseNativePDB())
+ return nullptr;
+
return new SymbolFilePDB(std::move(objfile_sp));
}
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
index c0b25b6ee4055..e6560813ce75e 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ b/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);
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDBProperties.td b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDBProperties.td
new file mode 100644
index 0000000000000..a6d902d49abdb
--- /dev/null
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDBProperties.td
@@ -0,0 +1,13 @@
+include "../../../../include/lldb/Core/PropertiesBase.td"
+
+let Definition = "symbolfilepdb" in {
+ def UseNativeReader: Property<"use-native-reader", "Enum">,
+ Global,
+ DefaultEnumValue<"eUseNativePDBReaderDefault">,
+ EnumValues<"OptionEnumValues(g_native_pdb_reader_enums)">,
+ Desc<"When 'on', use the native PDB reader based on LLVM's PDB support as opposed to the reader using Microsoft's DIA SDK. "
+ "On Windows, the default is controlled by the LLDB_USE_NATIVE_PDB_READER environment variable. "
+ "If this is set, then the native reader is used. "
+ "Note that the setting value will always have priority and that it needs to be set before a target is created. "
+ "On other platforms, the native reader is always used.">;
+}
diff --git a/lldb/test/Shell/SymbolFile/PDB/native-setting.cpp b/lldb/test/Shell/SymbolFile/PDB/native-setting.cpp
new file mode 100644
index 0000000000000..830daffbfd6bf
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/PDB/native-setting.cpp
@@ -0,0 +1,52 @@
+// REQUIRES: target-windows
+
+// Test plugin.symbol-file.pdb.use-native-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.use-native-reader off' \
+// RUN: -o 'target create %t.exe' \
+// RUN: -o 'target modules dump symfile' \
+// RUN: | FileCheck --check-prefix=ENV0-SET0 %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb \
+// RUN: -o 'settings set plugin.symbol-file.pdb.use-native-reader off' \
+// RUN: -o 'target create %t.exe' \
+// RUN: -o 'target modules dump symfile' \
+// RUN: | FileCheck --check-prefix=ENV1-SET0 %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=0 %lldb \
+// RUN: -o 'settings set plugin.symbol-file.pdb.use-native-reader on' \
+// RUN: -o 'target create %t.exe' \
+// RUN: -o 'target modules dump symfile' \
+// RUN: | FileCheck --check-prefix=ENV0-SET1 %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb \
+// RUN: -o 'settings set plugin.symbol-file.pdb.use-native-reader on' \
+// RUN: -o 'target create %t.exe' \
+// RUN: -o 'target modules dump symfile' \
+// RUN: | FileCheck --check-prefix=ENV1-SET1 %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-SET0: (lldb) target modules dump symfile
+// ENV0-SET0: Dumping debug symbols for 1 modules.
+// ENV0-SET0: SymbolFile pdb
+
+// ENV1-SET0: (lldb) target modules dump symfile
+// ENV1-SET0: Dumping debug symbols for 1 modules.
+// ENV1-SET0: SymbolFile pdb
+
+// ENV0-SET1: (lldb) target modules dump symfile
+// ENV0-SET1: Dumping debug symbols for 1 modules.
+// ENV0-SET1: SymbolFile native-pdb
+
+// ENV1-SET1: (lldb) target modules dump symfile
+// ENV1-SET1: Dumping debug symbols for 1 modules.
+// ENV1-SET1: SymbolFile native-pdb
+
+int main(){}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Nerixyz
force-pushed
the
feat/lldb-native-pdb-option
branch
from
d1aefd0 to
5f1bd9e
Compare
There was a problem hiding this comment.
Thanks for turning this into a proper setting. I find the current situation somewhat confusing, but IIRC, when you built without DIA, you only get the native PDB parser (e.g. on a non-Windows platform). If you built with DIA, by default, it uses DIA first and if that can't resolve a type, then it falls back to the native one, unless you have the environment variable set, then it only uses the native one.
Since we're already using a setting, can we encode these different behaviors directly in the setting?
eNative: only use the native PDB parsereDIA: only use the DIA PDB parser (is this even possible?)eDIAWithFallbackToNative: use the DIA PDB parser and fall back to the native oneeDefault: keep the current behavior, which meanseDIAWithFallbackToNativeif compiled with DIA, otherwiseeNativeifLLDB_USE_NATIVE_PDB_READERis set or if compiled without DIA.
Is this the current behavior? In my understanding, both plugins are mostly independent. The DIA PDB parser only brings in the native PDB parser to initialize it. It doesn't instantiate it to resolve a type. Regardless, having an option like you proposed seems more reasonable than the boolean-ish one. |
It's possible I'm misremembering this. In the PR that removes DIA support, @mstorsjo said that without DIA a bunch of NativePDB files started failing, which would have meant that we're falling back to DIA (the inverse of what I said earlier). Anyway, for the setting we can consider that an implementation detail. |
|
It's also kind of weird that we have |
JDevlieghere
approved these changes
Aug 1, 2025
Hmm, I can't reproduce that when compiling with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Initially suggested in #149305 (comment) - this PR adds the setting
plugin.symbol-file.pdb.use-native-reader. It doesn't remove support forLLDB_USE_NATIVE_PDB_READERto 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).