Author: David Peixotto Date: 2025-04-01T12:55:41-07:00 New Revision: 782e0cef762c1346396eb7dd75462f842be350e3
URL: https://github.com/llvm/llvm-project/commit/782e0cef762c1346396eb7dd75462f842be350e3 DIFF: https://github.com/llvm/llvm-project/commit/782e0cef762c1346396eb7dd75462f842be350e3.diff LOG: [lldb] Fix intel trace plugin tests (#133826) 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. Added: lldb/unittests/Process/ProcessTraceTest.cpp Modified: lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py lldb/source/Target/ProcessTrace.cpp lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py lldb/test/API/commands/trace/TestTraceEvents.py lldb/test/API/commands/trace/TestTraceSave.py lldb/test/API/commands/trace/TestTraceStartStop.py lldb/test/API/commands/trace/TestTraceTSC.py lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py lldb/unittests/Process/CMakeLists.txt Removed: ################################################################################ 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..6e1d7e38f3a0f 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 skipIfNoIntelPT(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..ebfda6226eef0 100644 --- a/lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py +++ b/lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py @@ -133,6 +133,7 @@ def testFunctionCallsWithErrors(self): ], ) + @skipIfNoIntelPT def testInlineFunctionCalls(self): self.expect( "file " + os.path.join(self.getSourceDir(), "inline-function", "a.out") @@ -194,6 +195,7 @@ def testInlineFunctionCalls(self): ], ) + @skipIfNoIntelPT 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..af23907bb6019 100644 --- a/lldb/test/API/commands/trace/TestTraceEvents.py +++ b/lldb/test/API/commands/trace/TestTraceEvents.py @@ -45,6 +45,7 @@ def testCPUEvents(self): ], ) + @skipIfNoIntelPT @testSBAPIAndCommands def testPauseEvents(self): """ diff --git a/lldb/test/API/commands/trace/TestTraceSave.py b/lldb/test/API/commands/trace/TestTraceSave.py index af38669cb4fce..4e3c70695bcee 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 ) + @skipIfNoIntelPT 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) + @skipIfNoIntelPT 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..9450f8b0961a8 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 * +@skipIfNoIntelPT 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..b20ba5255549c 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 * +@skipIfNoIntelPT 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..017f5c845c7c6 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 * +@skipIfNoIntelPT 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); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
