[LLDB] Run API tests with native PDB too #149305
Open
+32
−1
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
|
✅ With the latest revision this PR passed the Python code formatter. |
|
@DavidSpickett added the PDB tests here - Is the general approach fine? |
|
Yes this is what I was thinking of. Don't go diving into the failures yet, I'll try this on the bot machine first and see if we need other adjustments. |
|
I ran this on Linaro's bot machine and got these results:
So if you are on Windows X64, then it's a good sign we got similar results on Windows on Arm. A couple of those are random timeouts unrelated to PDB, so it's not exactly the same. Unfortunately I don't have time to go digging into them myself, so I am thinking about a sustainable way forward for what you want to do. One idea is to opt-in tests to being run with PDB. This means you can do that for your formatter changes, which keeps reviewers happy. The other stuff we open an issue for the problem that forcing PDB breaks a bunch of existing tests. I like what you're doing with the formatters and don't want to derail you from that. |
|
Opened #149498 so you can refer to it. I think it's a reasonable pitch to say:
|
|
I made it opt-in now. A test can set I'm not sure if it's possible to test both the native parser and the DIA one, since they're selected by an environment variable. |
|
@llvm/pr-subscribers-lldb Author: nerix (Nerixyz) ChangesFrom #148554 (comment) - this adds an option for API tests to be run with the native PDB reader on Windows. As there are a lot of failures with PDB, this is an opt-in per test. Once #51933 makes progress, this could be expanded. To get PDB, #149498 tracks the (currently) failing tests. Full diff: https://github.com/llvm/llvm-project/pull/149305.diff 4 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index ada6f9ff4a54f..2021d348138e6 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -255,6 +255,7 @@ def _getDebugInfoArgs(self, debug_info):
"gmodules": {"MAKE_DSYM": "NO", "MAKE_GMODULES": "YES"},
"debug_names": {"MAKE_DEBUG_NAMES": "YES"},
"dwp": {"MAKE_DSYM": "NO", "MAKE_DWP": "YES"},
+ "pdb": {"DEBUG_INFO_FLAG": "-g"},
}
# Collect all flags, with later options overriding earlier ones
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 63fadb59a82a1..f1b5e38a1c9ec 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1790,6 +1790,11 @@ def no_reason(_):
if can_replicate
]
+ # PDB is off by default, because it has a lot of failures right now.
+ # See llvm.org/pr149498
+ if original_testcase.TEST_WITH_PDB_DEBUG_INFO:
+ categories.append("pdb")
+
xfail_for_debug_info_cat_fn = getattr(
attrvalue, "__xfail_for_debug_info_cat_fn__", no_reason
)
@@ -1877,6 +1882,14 @@ class TestBase(Base, metaclass=LLDBTestCaseFactory):
# test multiple times with various debug info types.
NO_DEBUG_INFO_TESTCASE = False
+ TEST_WITH_PDB_DEBUG_INFO = False
+ """
+ Subclasses can set this to True to test with PDB (native) in addition to
+ the other debug info types. This id off by default because many tests will
+ fail due to missing functionality in PDB.
+ See llvm.org/pr149498.
+ """
+
def generateSource(self, source):
template = source + ".template"
temp = os.path.join(self.getSourceDir(), template)
diff --git a/lldb/packages/Python/lldbsuite/test/test_categories.py b/lldb/packages/Python/lldbsuite/test/test_categories.py
index 1f6e8a78e0c0d..fe475a9f8aef3 100644
--- a/lldb/packages/Python/lldbsuite/test/test_categories.py
+++ b/lldb/packages/Python/lldbsuite/test/test_categories.py
@@ -4,6 +4,7 @@
# System modules
import sys
+import os
# Third-party modules
@@ -12,7 +13,13 @@
# Key: Category name
# Value: should be used in lldbtest's debug-info replication
-debug_info_categories = {"dwarf": True, "dwo": True, "dsym": True, "gmodules": False}
+debug_info_categories = {
+ "dwarf": True,
+ "dwo": True,
+ "dsym": True,
+ "pdb": False,
+ "gmodules": False,
+}
all_categories = {
"basic_process": "Basic process execution sniff tests.",
@@ -34,6 +41,7 @@
"lldb-dap": "Tests for the Debug Adapter Protocol with lldb-dap",
"llgs": "Tests for the gdb-server functionality of lldb-server",
"msvcstl": "Test for MSVC STL data formatters",
+ "pdb": "Tests that can be run with PDB debug information",
"pexpect": "Tests requiring the pexpect library to be available",
"objc": "Tests related to the Objective-C programming language support",
"pyapi": "Tests related to the Python API",
@@ -65,6 +73,13 @@ def is_supported_on_platform(category, platform, compiler_path):
if platform not in ["darwin", "macosx", "ios", "watchos", "tvos", "bridgeos"]:
return False
return gmodules.is_compiler_clang_with_gmodules(compiler_path)
+ elif category == "pdb":
+ if platform == "windows":
+ assert (
+ os.environ.get("LLDB_USE_NATIVE_PDB_READER") == "1"
+ ), "Only the native PDB reader is supported"
+ return True
+ return False
return True
diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index 83713213ce1fe..5bd15a4b8c60b 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -349,6 +349,8 @@ def delete_module_cache(path):
for v in ["SystemDrive"]:
if v in os.environ:
config.environment[v] = os.environ[v]
+ # Always use the native PDB reader
+ config.environment["LLDB_USE_NATIVE_PDB_READER"] = "1"
# Some steps required to initialize the tests dynamically link with python.dll
# and need to know the location of the Python libraries. This ensures that we
|
|
I think as a pre-requisite to this we should remove the non-native PDB parser (and anything associated with it). That way we wouldn't need any special environment variable setting etc. We agreed that the native parser would be the way forward in the last EuroLLVM round-table IIRC (CC @JDevlieghere @labath). I think Jonas had a PR open for it but there were some small test blockers. What was the outcome of that work @JDevlieghere ? |
|
#114906 as well. |
That's right. The problem is that neither implementation is complete, and things only work because there's an automatic fallback. No matter which one you pick, it's going to introduce some regressions. Conceptually, everyone is in agreement that the native parser is the way forward:
Here's the link to that PR: #113647. At the time, removing the DIA implementation caused 72 test failures. Shortly after, @ZequanWu put up some PRs that improved the situation, but I haven't rebased my PR since. Depending on the number of failures, we can reopen the discussion. I totally understand that folks relying on the DIA implementation are hesitant to regress, but I believe it's in the best interest of LLDB. |
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.
From #148554 (comment) - this adds an option for API tests to be run with the native PDB reader on Windows. As there are a lot of failures with PDB, this is an opt-in per test. Once #51933 makes progress, this could be expanded.
To get PDB,
-ghas to be used on Clang. As far as I know, there's no way to specify something like-gpdb.-gcodeviewis the closest, but I don't think it sets the correct linker flags (or something similar) - at least LLDB doesn't have any debug info in that case.#149498 tracks the (currently) failing tests.