-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LLDB][PDB] Warn if DIA plugin is requested but not available #160067
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-lldb Author: nerix (Nerixyz) ChangesIf LLDB was built without the DIA SDK and the DIA reader is explicitly requested (through This PR adds the warning and a test when LLDB is not built with the SDK on Windows. I don't think any builder runs this configuration, as there are still five failing tests. I tested this locally with and without the SDK. Full diff: https://github.com/llvm/llvm-project/pull/160067.diff 3 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index 0e2ca1784e7e9..68e229b697116 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -14,6 +14,7 @@
#include "clang/Lex/Lexer.h"
#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+#include "lldb/Core/Debugger.h"
#include "lldb/Core/Mangled.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/PluginManager.h"
@@ -105,7 +106,6 @@ enum {
#include "SymbolFilePDBPropertiesEnum.inc"
};
-#if LLVM_ENABLE_DIA_SDK && defined(_WIN32)
bool ShouldUseNativeReaderByDefault() {
static bool g_use_native_by_default = true;
@@ -117,11 +117,16 @@ bool ShouldUseNativeReaderByDefault() {
!env_value.equals_insensitive("1") &&
!env_value.equals_insensitive("true"))
g_use_native_by_default = false;
+
+#if !LLVM_ENABLE_DIA_SDK || !defined(_WIN32)
+ // if the environment value is unset, the native reader is requested
+ if (env_value.empty())
+ g_use_native_by_default = true;
+#endif
});
return g_use_native_by_default;
}
-#endif
class PluginProperties : public Properties {
public:
@@ -136,6 +141,21 @@ class PluginProperties : public Properties {
bool UseNativeReader() const {
#if LLVM_ENABLE_DIA_SDK && defined(_WIN32)
+ return IsNativeReaderRequested();
+#else
+ if (!IsNativeReaderRequested()) {
+ static std::once_flag g_warning_shown;
+ Debugger::ReportWarning(
+ "The DIA PDB reader was explicitly requested, but LLDB was built "
+ "without the DIA SDK. The native reader will be used instead.",
+ {}, &g_warning_shown);
+ }
+ return true;
+#endif
+ }
+
+private:
+ bool IsNativeReaderRequested() const {
auto value =
GetPropertyAtIndexAs<PDBReader>(ePropertyReader, ePDBReaderDefault);
switch (value) {
@@ -147,9 +167,6 @@ class PluginProperties : public Properties {
case ePDBReaderDefault:
return ShouldUseNativeReaderByDefault();
}
-#else
- return true;
-#endif
}
};
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/native-setting.cpp b/lldb/test/Shell/SymbolFile/NativePDB/native-setting.cpp
new file mode 100644
index 0000000000000..fc0ee218dd0d2
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/native-setting.cpp
@@ -0,0 +1,65 @@
+// REQUIRES: !diasdk, target-windows
+
+// Test plugin.symbol-file.pdb.reader setting without the DIA SDK
+// RUN: %build -o %t.exe -- %s
+// RUN: env -u LLDB_USE_NATIVE_PDB_READER %lldb %t.exe -o 'target modules dump symfile' 2>&1 | FileCheck --check-prefix=NO-ENV %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER= %lldb %t.exe -o 'target modules dump symfile' 2>&1 | FileCheck --check-prefix=NO-ENV %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=0 %lldb %t.exe -o 'target modules dump symfile' 2>&1 | FileCheck --check-prefix=ENV0 %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb %t.exe -o 'target modules dump symfile' 2>&1 | 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: 2>&1 | 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: 2>&1 | 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: 2>&1 | 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: 2>&1 | FileCheck --check-prefix=ENV1-SET-NATIVE %s
+
+// NO-ENV-NOT: warning:
+// NO-ENV: (lldb) target modules dump symfile
+// NO-ENV: Dumping debug symbols for 1 modules.
+// NO-ENV: SymbolFile native-pdb
+
+// ENV0: warning: The DIA PDB reader was explicitly requested, but LLDB was built without the DIA SDK. The native reader will be used instead.
+// ENV0: (lldb) target modules dump symfile
+// ENV0: Dumping debug symbols for 1 modules.
+// ENV0: SymbolFile native-pdb
+
+// ENV1-NOT: warning:
+// ENV1: (lldb) target modules dump symfile
+// ENV1: Dumping debug symbols for 1 modules.
+// ENV1: SymbolFile native-pdb
+
+// ENV0-SET-DIA: warning: The DIA PDB reader was explicitly requested, but LLDB was built without the DIA SDK. The native reader will be used instead.
+// ENV0-SET-DIA: (lldb) target modules dump symfile
+// ENV0-SET-DIA: Dumping debug symbols for 1 modules.
+// ENV0-SET-DIA: SymbolFile native-pdb
+
+// ENV1-SET-DIA: warning: The DIA PDB reader was explicitly requested, but LLDB was built without the DIA SDK. The native reader will be used instead.
+// ENV1-SET-DIA: (lldb) target modules dump symfile
+// ENV1-SET-DIA: Dumping debug symbols for 1 modules.
+// ENV1-SET-DIA: SymbolFile native-pdb
+
+// ENV1-SET-NATIVE-NOT: warning:
+// 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-NOT: warning:
+// 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() {}
diff --git a/lldb/test/Shell/SymbolFile/PDB/native-setting.cpp b/lldb/test/Shell/SymbolFile/PDB/native-setting.cpp
index a3077252f08f1..750a61666912d 100644
--- a/lldb/test/Shell/SymbolFile/PDB/native-setting.cpp
+++ b/lldb/test/Shell/SymbolFile/PDB/native-setting.cpp
@@ -2,49 +2,62 @@
// 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 -u LLDB_USE_NATIVE_PDB_READER %lldb %t.exe -o 'target modules dump symfile' 2>&1 | FileCheck --check-prefix=NO-ENV %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER= %lldb %t.exe -o 'target modules dump symfile' 2>&1 | FileCheck --check-prefix=NO-ENV %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=0 %lldb %t.exe -o 'target modules dump symfile' 2>&1 | FileCheck --check-prefix=ENV0 %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb %t.exe -o 'target modules dump symfile' 2>&1 | 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: 2>&1 | 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: 2>&1 | 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: 2>&1 | 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
+// RUN: 2>&1 | FileCheck --check-prefix=ENV1-SET-NATIVE %s
+// NO-ENV-NOT: warning:
+// NO-ENV: (lldb) target modules dump symfile
+// NO-ENV: Dumping debug symbols for 1 modules.
+// NO-ENV: SymbolFile pdb
+
+// ENV0-NOT: warning:
// ENV0: (lldb) target modules dump symfile
// ENV0: Dumping debug symbols for 1 modules.
// ENV0: SymbolFile pdb
+// ENV1-NOT: warning:
// ENV1: (lldb) target modules dump symfile
// ENV1: Dumping debug symbols for 1 modules.
// ENV1: SymbolFile native-pdb
+// ENV0-SET-DIA-NOT: warning:
// 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-NOT: warning:
// 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-NOT: warning:
// 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-NOT: warning:
// ENV1-SET-NATIVE: (lldb) target modules dump symfile
// ENV1-SET-NATIVE: Dumping debug symbols for 1 modules.
// ENV1-SET-NATIVE: SymbolFile native-pdb
|
If `case ePDBReaderDefault` and `default` is used, Clang and GCC complain. If only `case ePDBReaderDefault` is used, MSVC complains.
Co-authored-by: Michael Buch <[email protected]>
| "The DIA PDB reader was explicitly requested, but LLDB was built " | ||
| "without the DIA SDK. The native reader will be used instead.", | ||
| {}, &g_warning_shown); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Errors and warnings should start the first sentence with a lowercase letter, and finish the last sentence without a period, if it would end in one otherwise.
| "The DIA PDB reader was explicitly requested, but LLDB was built " | |
| "without the DIA SDK. The native reader will be used instead.", | |
| {}, &g_warning_shown); | |
| "the DIA PDB reader was explicitly requested, but LLDB was built " | |
| "without the DIA SDK. The native reader will be used instead", | |
| {}, &g_warning_shown); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I forgot that. Opened #160398.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries, thanks for the quick fix!
Makes the warning message conform to the [LLVM error and warning style](https://llvm.org/docs/CodingStandards.html#error-and-warning-messages) as mentioned in #160067 (comment).
Makes the warning message conform to the [LLVM error and warning style](https://llvm.org/docs/CodingStandards.html#error-and-warning-messages) as mentioned in llvm/llvm-project#160067 (comment).
If LLDB was built without the DIA SDK and the DIA reader is explicitly requested (through
LLDB_USE_NATIVE_PDB_READER=0orsettings set plugin.symbol-file.pdb.reader dia), LLDB should print a warning, because it will use the native reader in any case (#159769 (comment)).This PR adds the warning and a test when LLDB is not built with the SDK on Windows. I don't think any builder runs this configuration, as there are still five failing tests. I tested this locally with and without the SDK.