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 * +@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 * +@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 * +@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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
