[Lldb-commits] [lldb] [lldb] Fix intel trace plugin tests (PR #133826)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 31 16:45:54 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: David Peixotto (dmpots)
<details>
<summary>Changes</summary>
The tests for the
[intel-pt](https://github.com/llvm/llvm-project/blob/348374028970c956f2e49ab7553b495d7408ccd9/lldb/docs/use/intel_pt.rst) trace plugin were failing for multiple reasons.
On machines where tracing is supported many of the tests were crashing because of a nullptr dereference. It looks like the `core_file` parameter in `ProcessTrace::CreateInstance` was once ignored, but was changed to always being dereferenced. This caused the tests to fail even when tracing was supported.
On machines where tracing is not supported we would still run tests that attempt to take a trace. These would obviously fail because the required hardware is not present. Note that some of the tests simply read serialized json as trace files which does not require any special hardware.
This PR fixes these two issues by guarding the pointer dereference and then skipping unsupported tests on machines. With these changes the trace tests pass on both types of machines.
We also add a new unit test to validate that a process can be created with a nullptr core_file through the generic process trace plugin path.
---
Full diff: https://github.com/llvm/llvm-project/pull/133826.diff
10 Files Affected:
- (modified) lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py (+7)
- (modified) lldb/source/Target/ProcessTrace.cpp (+2-1)
- (modified) lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py (+2)
- (modified) lldb/test/API/commands/trace/TestTraceEvents.py (+1)
- (modified) lldb/test/API/commands/trace/TestTraceSave.py (+2)
- (modified) lldb/test/API/commands/trace/TestTraceStartStop.py (+1)
- (modified) lldb/test/API/commands/trace/TestTraceTSC.py (+1)
- (modified) lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py (+1)
- (modified) lldb/unittests/Process/CMakeLists.txt (+3-1)
- (added) lldb/unittests/Process/ProcessTraceTest.cpp (+63)
``````````diff
diff --git a/lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
index f1b7d7c33bf07..176dce6a435f7 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
@@ -18,6 +18,13 @@ def wrapper(*args, **kwargs):
return wrapper
+def skipIfCpuDoesNotSupportIntelPT(func):
+ """Skip tests if the system does not support tracing."""
+
+ supported = os.path.exists("/sys/bus/event_source/devices/intel_pt/type")
+ return unittest.skipIf(not supported, "intel-pt tracing is unsupported")(func)
+
+
# Class that should be used by all python Intel PT tests.
#
# It has a handy check that skips the test if the intel-pt plugin is not enabled.
diff --git a/lldb/source/Target/ProcessTrace.cpp b/lldb/source/Target/ProcessTrace.cpp
index f131339905474..02272b1651da5 100644
--- a/lldb/source/Target/ProcessTrace.cpp
+++ b/lldb/source/Target/ProcessTrace.cpp
@@ -36,7 +36,8 @@ ProcessSP ProcessTrace::CreateInstance(TargetSP target_sp,
bool can_connect) {
if (can_connect)
return nullptr;
- return std::make_shared<ProcessTrace>(target_sp, listener_sp, *crash_file);
+ return std::make_shared<ProcessTrace>(target_sp, listener_sp,
+ crash_file ? *crash_file : FileSpec());
}
bool ProcessTrace::CanDebug(TargetSP target_sp, bool plugin_specified_by_name) {
diff --git a/lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py b/lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
index 761c262ae4de0..8b30414953d7d 100644
--- a/lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
+++ b/lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
@@ -133,6 +133,7 @@ def testFunctionCallsWithErrors(self):
],
)
+ @skipIfCpuDoesNotSupportIntelPT
def testInlineFunctionCalls(self):
self.expect(
"file " + os.path.join(self.getSourceDir(), "inline-function", "a.out")
@@ -194,6 +195,7 @@ def testInlineFunctionCalls(self):
],
)
+ @skipIfCpuDoesNotSupportIntelPT
def testIncompleteInlineFunctionCalls(self):
self.expect(
"file " + os.path.join(self.getSourceDir(), "inline-function", "a.out")
diff --git a/lldb/test/API/commands/trace/TestTraceEvents.py b/lldb/test/API/commands/trace/TestTraceEvents.py
index c20bcc247105b..7e6e07db0c1dd 100644
--- a/lldb/test/API/commands/trace/TestTraceEvents.py
+++ b/lldb/test/API/commands/trace/TestTraceEvents.py
@@ -45,6 +45,7 @@ def testCPUEvents(self):
],
)
+ @skipIfCpuDoesNotSupportIntelPT
@testSBAPIAndCommands
def testPauseEvents(self):
"""
diff --git a/lldb/test/API/commands/trace/TestTraceSave.py b/lldb/test/API/commands/trace/TestTraceSave.py
index af38669cb4fce..d8f39a1f3fa2a 100644
--- a/lldb/test/API/commands/trace/TestTraceSave.py
+++ b/lldb/test/API/commands/trace/TestTraceSave.py
@@ -43,6 +43,7 @@ def testErrorMessages(self):
"trace save", substrs=["error: Process is not being traced"], error=True
)
+ @skipIfCpuDoesNotSupportIntelPT
def testSaveToInvalidDir(self):
self.expect(
"target create "
@@ -165,6 +166,7 @@ def checkSessionBundle(session_file_path):
copied_cpu = find(lambda cor: cor["id"] == cpu["id"], copy["cpus"])
self.assertIsNotNone(copied_cpu)
+ @skipIfCpuDoesNotSupportIntelPT
def testSaveTrace(self):
self.expect(
"target create "
diff --git a/lldb/test/API/commands/trace/TestTraceStartStop.py b/lldb/test/API/commands/trace/TestTraceStartStop.py
index 5add321b4c83f..94b460bf1623a 100644
--- a/lldb/test/API/commands/trace/TestTraceStartStop.py
+++ b/lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -5,6 +5,7 @@
from lldbsuite.test.decorators import *
+ at skipIfCpuDoesNotSupportIntelPT
class TestTraceStartStop(TraceIntelPTTestCaseBase):
def expectGenericHelpMessageForStartCommand(self):
self.expect(
diff --git a/lldb/test/API/commands/trace/TestTraceTSC.py b/lldb/test/API/commands/trace/TestTraceTSC.py
index 4a19065e60c2b..2a412b197b704 100644
--- a/lldb/test/API/commands/trace/TestTraceTSC.py
+++ b/lldb/test/API/commands/trace/TestTraceTSC.py
@@ -5,6 +5,7 @@
from lldbsuite.test.decorators import *
+ at skipIfCpuDoesNotSupportIntelPT
class TestTraceTimestampCounters(TraceIntelPTTestCaseBase):
@testSBAPIAndCommands
@skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
diff --git a/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py b/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
index 12f99f07c78a8..658a62d894cc8 100644
--- a/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ b/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -6,6 +6,7 @@
from lldbsuite.test.decorators import *
+ at skipIfCpuDoesNotSupportIntelPT
class TestTraceStartStopMultipleThreads(TraceIntelPTTestCaseBase):
@skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
@testSBAPIAndCommands
diff --git a/lldb/unittests/Process/CMakeLists.txt b/lldb/unittests/Process/CMakeLists.txt
index d3b37e006fd89..a240d773c3f30 100644
--- a/lldb/unittests/Process/CMakeLists.txt
+++ b/lldb/unittests/Process/CMakeLists.txt
@@ -7,8 +7,9 @@ endif()
add_subdirectory(Utility)
add_subdirectory(minidump)
-add_lldb_unittest(ProcessEventDataTests
+add_lldb_unittest(ProcessTests
ProcessEventDataTest.cpp
+ ProcessTraceTest.cpp
LINK_LIBS
lldbCore
@@ -18,5 +19,6 @@ add_lldb_unittest(ProcessEventDataTests
lldbUtility
lldbUtilityHelpers
lldbInterpreter
+ lldbPluginPlatformLinux
lldbPluginPlatformMacOSX
)
diff --git a/lldb/unittests/Process/ProcessTraceTest.cpp b/lldb/unittests/Process/ProcessTraceTest.cpp
new file mode 100644
index 0000000000000..fc6b92e868248
--- /dev/null
+++ b/lldb/unittests/Process/ProcessTraceTest.cpp
@@ -0,0 +1,63 @@
+//===-- ProcessEventDataTest.cpp ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Target/ProcessTrace.h"
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/HostInfo.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb;
+using namespace platform_linux;
+
+// This is needed for the tests that create a trace process.
+class ProcessTraceTest : public ::testing::Test {
+public:
+ void SetUp() override {
+ ProcessTrace::Initialize();
+ FileSystem::Initialize();
+ HostInfo::Initialize();
+ PlatformLinux::Initialize();
+ }
+ void TearDown() override {
+ PlatformLinux::Initialize();
+ HostInfo::Terminate();
+ FileSystem::Terminate();
+ ProcessTrace::Terminate();
+ }
+};
+
+TargetSP CreateTarget(DebuggerSP &debugger_sp, const ArchSpec &arch) {
+ PlatformSP platform_sp;
+ TargetSP target_sp;
+ debugger_sp->GetTargetList().CreateTarget(
+ *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp);
+ return target_sp;
+}
+
+// Test that we can create a process trace with a nullptr core file.
+TEST_F(ProcessTraceTest, ConstructorWithNullptrCoreFile) {
+ ArchSpec arch("i386-pc-linux");
+
+ Platform::SetHostPlatform(PlatformLinux::CreateInstance(true, &arch));
+ ASSERT_NE(Platform::GetHostPlatform(), nullptr);
+
+ DebuggerSP debugger_sp = Debugger::CreateInstance();
+ ASSERT_TRUE(debugger_sp);
+
+ TargetSP target_sp = CreateTarget(debugger_sp, arch);
+ ASSERT_TRUE(target_sp);
+
+ ProcessSP process_sp = target_sp->CreateProcess(
+ /*listener*/ nullptr, "trace",
+ /*crash_file*/ nullptr,
+ /*can_connect*/ false);
+
+ ASSERT_NE(process_sp, nullptr);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/133826
More information about the lldb-commits
mailing list