[Lldb-commits] [lldb] [lldb] Fixed the test TestPlatformProcessLaunch (PR #91923)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/91923 Transfer `stdio.log` from the remote target if necessary. >From 47839ef62cea9d1c79c97a70c4714f365eeb4e02 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 10:56:36 +0400 Subject: [PATCH] [lldb] Fixed the test TestPlatformProcessLaunch Transfer `stdio.log` from the remote target if necessary. --- .../launch/TestPlatformProcessLaunch.py | 91 ++- 1 file changed, 47 insertions(+), 44 deletions(-) diff --git a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py index 3fb7d00c93d29..21c12bcb9369f 100644 --- a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py +++ b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py @@ -2,7 +2,9 @@ Test platform process launch. """ +import lldb from textwrap import dedent +from lldbsuite.test import lldbutil from lldbsuite.test.lldbtest import TestBase @@ -11,9 +13,10 @@ class ProcessLaunchTestCase(TestBase): def setup(self): self.build() -exe = self.getBuildArtifact("a.out") -self.runCmd("file " + exe) -return (exe, self.getBuildArtifact("stdio.log")) +self.runCmd("file " + self.getBuildArtifact("a.out")) +exe = lldbutil.append_to_process_working_directory(self, "a.out") +outfile = lldbutil.append_to_process_working_directory(self, "stdio.log") +return (exe, outfile) def test_process_launch_no_args(self): # When there are no extra arguments we just have 0, the program name. @@ -21,18 +24,18 @@ def test_process_launch_no_args(self): self.runCmd("platform process launch --stdout {} -s".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 1 argument(s). -[0]: {} -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 1 argument(s). +[0]: {} +""".format( +exe +) +), +stdio_log, +) def test_process_launch_command_args(self): exe, outfile = self.setup() @@ -41,21 +44,21 @@ def test_process_launch_command_args(self): self.runCmd("platform process launch --stdout {} -s -- A B C".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 4 argument(s). -[0]: {} -[1]: A -[2]: B -[3]: C -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 4 argument(s). +[0]: {} +[1]: A +[2]: B +[3]: C +""".format( +exe +) +), +stdio_log, +) def test_process_launch_target_args(self): exe, outfile = self.setup() @@ -64,17 +67,17 @@ def test_process_launch_target_args(self): self.runCmd("platform process launch --stdout {} -s".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 3 argument(s). -[0]: {} -[1]: D -[2]: E -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 3 argument(s). +[0]: {} +[1]: D +[2]: E +""".format( +exe +) +), +stdio_log, +) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestPlatformProcessLaunch (PR #91923)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Dmitry Vasilyev (slydiman) Changes Transfer `stdio.log` from the remote target if necessary. --- Full diff: https://github.com/llvm/llvm-project/pull/91923.diff 1 Files Affected: - (modified) lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py (+47-44) ``diff diff --git a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py index 3fb7d00c93d29..21c12bcb9369f 100644 --- a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py +++ b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py @@ -2,7 +2,9 @@ Test platform process launch. """ +import lldb from textwrap import dedent +from lldbsuite.test import lldbutil from lldbsuite.test.lldbtest import TestBase @@ -11,9 +13,10 @@ class ProcessLaunchTestCase(TestBase): def setup(self): self.build() -exe = self.getBuildArtifact("a.out") -self.runCmd("file " + exe) -return (exe, self.getBuildArtifact("stdio.log")) +self.runCmd("file " + self.getBuildArtifact("a.out")) +exe = lldbutil.append_to_process_working_directory(self, "a.out") +outfile = lldbutil.append_to_process_working_directory(self, "stdio.log") +return (exe, outfile) def test_process_launch_no_args(self): # When there are no extra arguments we just have 0, the program name. @@ -21,18 +24,18 @@ def test_process_launch_no_args(self): self.runCmd("platform process launch --stdout {} -s".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 1 argument(s). -[0]: {} -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 1 argument(s). +[0]: {} +""".format( +exe +) +), +stdio_log, +) def test_process_launch_command_args(self): exe, outfile = self.setup() @@ -41,21 +44,21 @@ def test_process_launch_command_args(self): self.runCmd("platform process launch --stdout {} -s -- A B C".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 4 argument(s). -[0]: {} -[1]: A -[2]: B -[3]: C -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 4 argument(s). +[0]: {} +[1]: A +[2]: B +[3]: C +""".format( +exe +) +), +stdio_log, +) def test_process_launch_target_args(self): exe, outfile = self.setup() @@ -64,17 +67,17 @@ def test_process_launch_target_args(self): self.runCmd("platform process launch --stdout {} -s".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 3 argument(s). -[0]: {} -[1]: D -[2]: E -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 3 argument(s). +[0]: {} +[1]: D +[2]: E +""".format( +exe +) +), +stdio_log, +) `` https://github.com/llvm/llvm-project/pull/91923 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestGdbRemoteLaunch (PR #91931)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/91931 Install `a.out` to the remote target (after handshake) if necessary and use the remote path to call `vRun`. >From cbc183bd78c26ca3777cc4a81797d57ab3fa945d Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 11:26:03 +0400 Subject: [PATCH] [lldb] Fixed the test TestGdbRemoteLaunch Install `a.out` to the remote target (after handshake) if necessary and use the remote path to call vRun. --- .../tools/lldb-server/TestGdbRemoteLaunch.py | 56 --- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py b/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py index 78a4d326c12d1..530f5aa57ea48 100644 --- a/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py +++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py @@ -8,17 +8,32 @@ class GdbRemoteLaunchTestCase(gdbremote_testcase.GdbRemoteTestCaseBase): +def get_exe_path(self): +exe_path = self.getBuildArtifact("a.out") +if lldb.remote_platform: +remote_path = lldbutil.append_to_process_working_directory(self, "a.out") +err = lldb.remote_platform.Install( +lldb.SBFileSpec(exe_path, True), lldb.SBFileSpec(remote_path, False) +) +if err.Fail(): +raise Exception( +"remote_platform.Install('%s', '%s') failed: %s" +% (exe_path, remote_path, err) +) +exe_path = remote_path +return exe_path + @skipIfWindows # No pty support to test any inferior output @add_test_categories(["llgs"]) def test_launch_via_A(self): self.build() -exe_path = self.getBuildArtifact("a.out") -args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"] -hex_args = [seven.hexlify(x) for x in args] - server = self.connect_to_debug_monitor() self.assertIsNotNone(server) self.do_handshake() +exe_path = self.get_exe_path() +args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"] +hex_args = [seven.hexlify(x) for x in args] + # NB: strictly speaking we should use %x here but this packet # is deprecated, so no point in changing lldb-server's expectations self.test_sequence.add_log_lines( @@ -38,13 +53,13 @@ def test_launch_via_A(self): @add_test_categories(["llgs"]) def test_launch_via_vRun(self): self.build() -exe_path = self.getBuildArtifact("a.out") -args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"] -hex_args = [seven.hexlify(x) for x in args] - server = self.connect_to_debug_monitor() self.assertIsNotNone(server) self.do_handshake() +exe_path = self.get_exe_path() +args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"] +hex_args = [seven.hexlify(x) for x in args] + self.test_sequence.add_log_lines( [ "read packet: $vRun;%s;%s;%s;%s#00" % tuple(hex_args), @@ -60,12 +75,12 @@ def test_launch_via_vRun(self): @add_test_categories(["llgs"]) def test_launch_via_vRun_no_args(self): self.build() -exe_path = self.getBuildArtifact("a.out") -hex_path = seven.hexlify(exe_path) - server = self.connect_to_debug_monitor() self.assertIsNotNone(server) self.do_handshake() +exe_path = self.get_exe_path() +hex_path = seven.hexlify(exe_path) + self.test_sequence.add_log_lines( [ "read packet: $vRun;%s#00" % (hex_path,), @@ -78,6 +93,7 @@ def test_launch_via_vRun_no_args(self): self.expect_gdbremote_sequence() @add_test_categories(["llgs"]) +@skipIfRemote def test_launch_failure_via_vRun(self): self.build() exe_path = self.getBuildArtifact("a.out") @@ -110,14 +126,13 @@ def test_launch_failure_via_vRun(self): @add_test_categories(["llgs"]) def test_QEnvironment(self): self.build() -exe_path = self.getBuildArtifact("a.out") -env = {"FOO": "test", "BAR": "a=z"} -args = [exe_path, "print-env:FOO", "print-env:BAR"] -hex_args = [seven.hexlify(x) for x in args] - server = self.connect_to_debug_monitor() self.assertIsNotNone(server) self.do_handshake() +exe_path = self.get_exe_path() +env = {"FOO": "test", "BAR": "a=z"} +args = [exe_path, "print-env:FOO", "print-env:BAR"] +hex_args = [seven.hexlify(x) for x in args] for key, value in env.items(): self.test_sequence.add_log_lines( @@ -143,14 +158,13 @@ def test_QEnvironment(self): @add_test_categories(["llgs"]) def test_QEnvironmentHexEncoded(self): self.build() -exe_path = self.getBuildArtifact("a.out")
[Lldb-commits] [lldb] [lldb] Fixed the test TestGdbRemoteLaunch (PR #91931)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Dmitry Vasilyev (slydiman) Changes Install `a.out` to the remote target (after handshake) if necessary and use the remote path to call `vRun`. --- Full diff: https://github.com/llvm/llvm-project/pull/91931.diff 1 Files Affected: - (modified) lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py (+35-21) ``diff diff --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py b/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py index 78a4d326c12d1..530f5aa57ea48 100644 --- a/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py +++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py @@ -8,17 +8,32 @@ class GdbRemoteLaunchTestCase(gdbremote_testcase.GdbRemoteTestCaseBase): +def get_exe_path(self): +exe_path = self.getBuildArtifact("a.out") +if lldb.remote_platform: +remote_path = lldbutil.append_to_process_working_directory(self, "a.out") +err = lldb.remote_platform.Install( +lldb.SBFileSpec(exe_path, True), lldb.SBFileSpec(remote_path, False) +) +if err.Fail(): +raise Exception( +"remote_platform.Install('%s', '%s') failed: %s" +% (exe_path, remote_path, err) +) +exe_path = remote_path +return exe_path + @skipIfWindows # No pty support to test any inferior output @add_test_categories(["llgs"]) def test_launch_via_A(self): self.build() -exe_path = self.getBuildArtifact("a.out") -args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"] -hex_args = [seven.hexlify(x) for x in args] - server = self.connect_to_debug_monitor() self.assertIsNotNone(server) self.do_handshake() +exe_path = self.get_exe_path() +args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"] +hex_args = [seven.hexlify(x) for x in args] + # NB: strictly speaking we should use %x here but this packet # is deprecated, so no point in changing lldb-server's expectations self.test_sequence.add_log_lines( @@ -38,13 +53,13 @@ def test_launch_via_A(self): @add_test_categories(["llgs"]) def test_launch_via_vRun(self): self.build() -exe_path = self.getBuildArtifact("a.out") -args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"] -hex_args = [seven.hexlify(x) for x in args] - server = self.connect_to_debug_monitor() self.assertIsNotNone(server) self.do_handshake() +exe_path = self.get_exe_path() +args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"] +hex_args = [seven.hexlify(x) for x in args] + self.test_sequence.add_log_lines( [ "read packet: $vRun;%s;%s;%s;%s#00" % tuple(hex_args), @@ -60,12 +75,12 @@ def test_launch_via_vRun(self): @add_test_categories(["llgs"]) def test_launch_via_vRun_no_args(self): self.build() -exe_path = self.getBuildArtifact("a.out") -hex_path = seven.hexlify(exe_path) - server = self.connect_to_debug_monitor() self.assertIsNotNone(server) self.do_handshake() +exe_path = self.get_exe_path() +hex_path = seven.hexlify(exe_path) + self.test_sequence.add_log_lines( [ "read packet: $vRun;%s#00" % (hex_path,), @@ -78,6 +93,7 @@ def test_launch_via_vRun_no_args(self): self.expect_gdbremote_sequence() @add_test_categories(["llgs"]) +@skipIfRemote def test_launch_failure_via_vRun(self): self.build() exe_path = self.getBuildArtifact("a.out") @@ -110,14 +126,13 @@ def test_launch_failure_via_vRun(self): @add_test_categories(["llgs"]) def test_QEnvironment(self): self.build() -exe_path = self.getBuildArtifact("a.out") -env = {"FOO": "test", "BAR": "a=z"} -args = [exe_path, "print-env:FOO", "print-env:BAR"] -hex_args = [seven.hexlify(x) for x in args] - server = self.connect_to_debug_monitor() self.assertIsNotNone(server) self.do_handshake() +exe_path = self.get_exe_path() +env = {"FOO": "test", "BAR": "a=z"} +args = [exe_path, "print-env:FOO", "print-env:BAR"] +hex_args = [seven.hexlify(x) for x in args] for key, value in env.items(): self.test_sequence.add_log_lines( @@ -143,14 +158,13 @@ def test_QEnvironment(self): @add_test_categories(["llgs"]) def test_QEnvironmentHexEncoded(self): self.build() -exe_path = self.getBuildArtifact("a.out") -env = {"FOO": "test", "BAR": "a=z", "BAZ": "a*}#z"} -args = [exe_path, "print-env:FOO", "print-env:BAR", "print-env:BAZ"] -hex_args = [seven.hexlify(x) for x in args] - server = self.connect_to_debug_moni
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
omjavaid wrote: @labath this seems to have broken lldb-aarch64-windows bot with TestInterruptBacktrace.py failing on num_frames = thread.GetNumFrames() https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix redundant condition in Target.cpp (PR #91882)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/91882 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix redundant condition in Target.cpp (PR #91882)
@@ -841,12 +841,14 @@ static bool CheckIfWatchpointsSupported(Target *target, Status &error) { if (!num_supported_hardware_watchpoints) return true; - if (num_supported_hardware_watchpoints == 0) { -error.SetErrorStringWithFormat( -"Target supports (%u) hardware watchpoint slots.\n", -*num_supported_hardware_watchpoints); -return false; - } + // If num_supported_hardware_watchpoints is zero, set an + //error message and return false. DavidSpickett wrote: I think this function can go one of three ways: * We can't detect h/w watch, so we assume they are supported. * We can detect them and there are > 0, return true. * We can detect them but there are 0 of them, set error message and return false. The code as you've got it here will set the error message if either of the final 2 are true. What you could do to keep the intended behaviour is simply change the if to: ``` if (*num_supported_hardware_watchpoints == 0) ``` Then there is no redundant conditon, the first if checks whether the optional is valid, the second one checks the value contained in it. https://github.com/llvm/llvm-project/pull/91882 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix redundant condition in Target.cpp (PR #91882)
https://github.com/DavidSpickett requested changes to this pull request. I think this function got a bit confused when we changed the return type to an optional, thanks for taking a look at it. https://github.com/llvm/llvm-project/pull/91882 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [lldb] [llvm] [mlir] [openmp] [polly] fix(python): fix comparison to None (PR #91857)
DavidSpickett wrote: LLVM uses a GitHub setting where the PR is squashed, and the commit message is taken from the PR description rather than the commits themselves. So all you have to do here is copy the commit message you updated into the PR's description. https://github.com/llvm/llvm-project/pull/91857 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [lldb] [llvm] [mlir] [openmp] [polly] fix(python): fix comparison to None (PR #91857)
https://github.com/DavidSpickett approved this pull request. LLDB changes look good to me. https://github.com/llvm/llvm-project/pull/91857 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestSettings (PR #91915)
https://github.com/DavidSpickett approved this pull request. Makes sense to me. LLDB checks that the folder exists locally, which a remote folder won't, so it's not in the JSON read back. And this test isn't doing anything with the module cache itself, so any existing local path will do. https://github.com/llvm/llvm-project/pull/91915 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestSettings when run with a remote target (PR #91915)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/91915 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestQuoting (PR #91886)
https://github.com/DavidSpickett approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/91886 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add ability to show enum as name and value at the same time (PR #90059)
DavidSpickett wrote: >So each ValueObject has the ability to show its value as an enumeration if its >format is set to eFormatEnum. If the format is set to eFormatHex, >eFormatUnsigned, or eFormatSigned, then we show the numeric value. Sure, the problem I have is that often with registers you'd want to see not just the name but the value. It's not 100% crucial for the feature but I found it useful in a previous debugger I used. >Can you show some sample output for this? I only have examples for registers right now, for example AArch64's mte_ctrl will look like (where TCF is an enum): ``` = (TAGS = 65535, TCF = TCF_SYNC (1), TAGGED_ADDR_ENABLE = 1) ``` So that users know what a value means (no looking in the manuals) and the value (in case they need to add to or confirm the number they see in their source code). When I've added tests for the existing options, I'll update this PR with test cases that are not register specific. https://github.com/llvm/llvm-project/pull/90059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestPlatformProcessLaunch running on a remote target (PR #91923)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/91923 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestPlatformProcessLaunch running on a remote target (PR #91923)
@@ -2,7 +2,9 @@ Test platform process launch. """ +import lldb DavidSpickett wrote: Is this required? https://github.com/llvm/llvm-project/pull/91923 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestGdbRemoteLaunch (PR #91931)
@@ -78,6 +93,7 @@ def test_launch_via_vRun_no_args(self): self.expect_gdbremote_sequence() @add_test_categories(["llgs"]) +@skipIfRemote DavidSpickett wrote: Why is only this test skipped? https://github.com/llvm/llvm-project/pull/91931 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestGdbRemoteLaunch (PR #91931)
@@ -78,6 +93,7 @@ def test_launch_via_vRun_no_args(self): self.expect_gdbremote_sequence() @add_test_categories(["llgs"]) +@skipIfRemote DavidSpickett wrote: Ah, because we need to open the file while we attempt to run it, and doing that on the remote is difficult. We could run some shell command, but it'd be a different one per remote platform, and failing that, Python. Assuming the remote even has an OS. So yeah, skipIfRemote is logical here :) https://github.com/llvm/llvm-project/pull/91931 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestGdbRemoteLaunch (PR #91931)
https://github.com/DavidSpickett approved this pull request. LGTM Surprised we haven't needed this before, but I didn't see any existing function for it. https://github.com/llvm/llvm-project/pull/91931 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix the test TestGdbRemotePlatformFile (PR #91918)
@@ -108,6 +109,20 @@ def test_platform_file_wronly_fail(self): ) self.expect_gdbremote_sequence() +def remote_install(self, path, filename="test"): +if lldb.remote_platform: +remote_path = lldbutil.append_to_process_working_directory(self, filename) +err = lldb.remote_platform.Install( +lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) +) +if err.Fail(): +raise Exception( +"remote_platform.Install('%s', '%s') failed: %s" +% (path, remote_path, err) +) +path = remote_path +return path DavidSpickett wrote: This looks almost the same as the code in https://github.com/llvm/llvm-project/pull/91931, can this be de-duplicated? https://github.com/llvm/llvm-project/pull/91918 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Disable macOS specific test for Windows host instead of Windows target (PR #91942)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/91942 This test failed in case of Windows host and Linux target. >From ae6c04bc568c88e41d348a1658fb44cff327ad6e Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 14:10:55 +0400 Subject: [PATCH] [lldb][Windows] Disable macOS specific test for Windows host instead of Windows target This test failed in case of Windows host and Linux target. --- .../macosx/duplicate-archive-members/TestDuplicateMembers.py| 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/macosx/duplicate-archive-members/TestDuplicateMembers.py b/lldb/test/API/macosx/duplicate-archive-members/TestDuplicateMembers.py index 1746b2d3a47b1..e9d13bdcf3f12 100644 --- a/lldb/test/API/macosx/duplicate-archive-members/TestDuplicateMembers.py +++ b/lldb/test/API/macosx/duplicate-archive-members/TestDuplicateMembers.py @@ -9,7 +9,7 @@ class BSDArchivesTestCase(TestBase): @expectedFailureAll( -oslist=["windows"], +hostoslist=["windows"], bugnumber="llvm.org/pr24527. Makefile.rules doesn't know how to build static libs on Windows", ) def test(self): ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Disable macOS specific test for Windows host instead of Windows target (PR #91942)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Dmitry Vasilyev (slydiman) Changes This test failed in case of Windows host and Linux target. --- Full diff: https://github.com/llvm/llvm-project/pull/91942.diff 1 Files Affected: - (modified) lldb/test/API/macosx/duplicate-archive-members/TestDuplicateMembers.py (+1-1) ``diff diff --git a/lldb/test/API/macosx/duplicate-archive-members/TestDuplicateMembers.py b/lldb/test/API/macosx/duplicate-archive-members/TestDuplicateMembers.py index 1746b2d3a47b1..e9d13bdcf3f12 100644 --- a/lldb/test/API/macosx/duplicate-archive-members/TestDuplicateMembers.py +++ b/lldb/test/API/macosx/duplicate-archive-members/TestDuplicateMembers.py @@ -9,7 +9,7 @@ class BSDArchivesTestCase(TestBase): @expectedFailureAll( -oslist=["windows"], +hostoslist=["windows"], bugnumber="llvm.org/pr24527. Makefile.rules doesn't know how to build static libs on Windows", ) def test(self): `` https://github.com/llvm/llvm-project/pull/91942 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestPlatformProcessLaunch running on a remote target (PR #91923)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/91923 >From 47839ef62cea9d1c79c97a70c4714f365eeb4e02 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 10:56:36 +0400 Subject: [PATCH 1/2] [lldb] Fixed the test TestPlatformProcessLaunch Transfer `stdio.log` from the remote target if necessary. --- .../launch/TestPlatformProcessLaunch.py | 91 ++- 1 file changed, 47 insertions(+), 44 deletions(-) diff --git a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py index 3fb7d00c93d29..21c12bcb9369f 100644 --- a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py +++ b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py @@ -2,7 +2,9 @@ Test platform process launch. """ +import lldb from textwrap import dedent +from lldbsuite.test import lldbutil from lldbsuite.test.lldbtest import TestBase @@ -11,9 +13,10 @@ class ProcessLaunchTestCase(TestBase): def setup(self): self.build() -exe = self.getBuildArtifact("a.out") -self.runCmd("file " + exe) -return (exe, self.getBuildArtifact("stdio.log")) +self.runCmd("file " + self.getBuildArtifact("a.out")) +exe = lldbutil.append_to_process_working_directory(self, "a.out") +outfile = lldbutil.append_to_process_working_directory(self, "stdio.log") +return (exe, outfile) def test_process_launch_no_args(self): # When there are no extra arguments we just have 0, the program name. @@ -21,18 +24,18 @@ def test_process_launch_no_args(self): self.runCmd("platform process launch --stdout {} -s".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 1 argument(s). -[0]: {} -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 1 argument(s). +[0]: {} +""".format( +exe +) +), +stdio_log, +) def test_process_launch_command_args(self): exe, outfile = self.setup() @@ -41,21 +44,21 @@ def test_process_launch_command_args(self): self.runCmd("platform process launch --stdout {} -s -- A B C".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 4 argument(s). -[0]: {} -[1]: A -[2]: B -[3]: C -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 4 argument(s). +[0]: {} +[1]: A +[2]: B +[3]: C +""".format( +exe +) +), +stdio_log, +) def test_process_launch_target_args(self): exe, outfile = self.setup() @@ -64,17 +67,17 @@ def test_process_launch_target_args(self): self.runCmd("platform process launch --stdout {} -s".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 3 argument(s). -[0]: {} -[1]: D -[2]: E -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 3 argument(s). +[0]: {} +[1]: D +[2]: E +""".format( +exe +) +), +stdio_log, +) >From 966de73bf351ece5968145bbdec8021762133391 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 14:21:09 +0400 Subject: [PATCH 2/2] Cleanup --- .../platform/process/launch/TestPlatformProcessLaunch.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py index 21c12bcb9369f..7cbad03ad 100644 --- a/lldb/test/API/commands/platform/pr
[Lldb-commits] [lldb] [lldb] Fixed the test TestPlatformProcessLaunch running on a remote target (PR #91923)
@@ -2,7 +2,9 @@ Test platform process launch. """ +import lldb slydiman wrote: This is a debug artifact. Removed now. Thanks. https://github.com/llvm/llvm-project/pull/91923 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Enforce exec permission using Platform::Install() from Windows host (PR #91887)
labath wrote: The `ifdef` in the generic code is not exactly ideal, and I'm wondering if we should just do the same thing as Target::Install does (i.e., set the execute flag unconditionally). Looking at the history, it appears that the Target::Install code was [introduced](https://reviews.llvm.org/D9492) to fix the same problem you are having, and it's quite possible this code path was not noticed because we were still in the very early stages of cross-debugging bringup (or the function did even not exist back then). In fact, if I'm reading this correctly, you should be able to remove the Target::Install workaround if you put the code here. WDYT? https://github.com/llvm/llvm-project/pull/91887 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/91944 It can be used in tests #91918, #91931 and such. >From de7135a8a4a40b5aa5ac1f44e58d62874c96448b Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 14:45:33 +0400 Subject: [PATCH] [lldb] Add lldbutil.target_install() helper It can be used in tests #91918, #91931 and such. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 16 1 file changed, 16 insertions(+) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 58eb37fd742d7..e67b68b727b80 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,6 +1654,22 @@ def find_library_callable(test): ) +def target_install(test, filename): +path = test.getBuildArtifact(filename) +if lldb.remote_platform: +remote_path = lldbutil.append_to_process_working_directory(test, filename) +err = lldb.remote_platform.Install( +lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) +) +if err.Fail(): +raise Exception( +"remote_platform.Install('%s', '%s') failed: %s" +% (path, remote_path, err) +) +path = remote_path +return path + + def read_file_on_target(test, remote): if lldb.remote_platform: local = test.getBuildArtifact("file_from_target") ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix the test TestGdbRemotePlatformFile (PR #91918)
@@ -108,6 +109,20 @@ def test_platform_file_wronly_fail(self): ) self.expect_gdbremote_sequence() +def remote_install(self, path, filename="test"): +if lldb.remote_platform: +remote_path = lldbutil.append_to_process_working_directory(self, filename) +err = lldb.remote_platform.Install( +lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) +) +if err.Fail(): +raise Exception( +"remote_platform.Install('%s', '%s') failed: %s" +% (path, remote_path, err) +) +path = remote_path +return path slydiman wrote: Sure. Please look at #91944. https://github.com/llvm/llvm-project/pull/91918 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Dmitry Vasilyev (slydiman) Changes It can be used in tests #91918, #91931 and such. --- Full diff: https://github.com/llvm/llvm-project/pull/91944.diff 1 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/lldbutil.py (+16) ``diff diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 58eb37fd742d7..e67b68b727b80 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,6 +1654,22 @@ def find_library_callable(test): ) +def target_install(test, filename): +path = test.getBuildArtifact(filename) +if lldb.remote_platform: +remote_path = lldbutil.append_to_process_working_directory(test, filename) +err = lldb.remote_platform.Install( +lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) +) +if err.Fail(): +raise Exception( +"remote_platform.Install('%s', '%s') failed: %s" +% (path, remote_path, err) +) +path = remote_path +return path + + def read_file_on_target(test, remote): if lldb.remote_platform: local = test.getBuildArtifact("file_from_target") `` https://github.com/llvm/llvm-project/pull/91944 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestGdbRemoteLaunch (PR #91931)
slydiman wrote: @DavidSpickett Please look at #91944. https://github.com/llvm/llvm-project/pull/91931 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
labath wrote: > Ah, I misunderstood what the nature of the failure was. I tried running the > shell test, and it's failing for different reasons. I almost never touch > shell tests, I find them really hard to debug so I'm not sure what the > problem is. If I run it by hand, > > ``` > (lldb) settings set platform.plugin.darwin.ignored-exceptions > EXC_BAD_INSTRUCTION > (lldb) b sigill_handler > Breakpoint 1: where = a.out`sigill_handler + 20 at > signal-in-leaf-function-aarch64.c:10:34, address = 0x00013f4c > (lldb) r > Process 25854 launched: '/tmp/a.out' (arm64) > Process 25854 stopped > * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGILL > frame #0: 0x00013f2c a.out`signal_generating_add at > signal-in-leaf-function-aarch64.c:5:3 >2 #include >3 >4 int __attribute__((naked)) signal_generating_add(int a, int b) { > -> 5asm("add w0, w1, w0\n\t" >6"udf #0xdead\n\t" >7"ret"); >8 } > (lldb) c > Process 25854 resuming > Process 25854 stopped > * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 > frame #0: 0x00013f4c a.out`sigill_handler(signo=4) at > signal-in-leaf-function-aarch64.c:10:34 >7"ret"); >8 } >9 > -> 10 void sigill_handler(int signo) { _exit(0); } >11 >12 int main() { >13 signal(SIGILL, sigill_handler); > (lldb) bt > * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 > * frame #0: 0x00013f4c a.out`sigill_handler(signo=4) at > signal-in-leaf-function-aarch64.c:10:34 > frame #1: 0x000197f93584 libsystem_platform.dylib`_sigtramp + 56 > frame #2: 0x00013f7c a.out`main at > signal-in-leaf-function-aarch64.c:14:3 > frame #3: 0x000197bda0e0 dyld`start + 2360 > (lldb) > ``` > > which all looks good to me, but it the shell test fails with It's not completely correct -- the frame for `signal_generating_add` is missing. That's what the error message is complaining about. The test stops the process before and after injecting a signal and verifies that the relevant parts of backtrace (i.e., calls to `signal_generating_add` and `main`) are present (and have the same PC) in both places. > > ``` > /Volumes/work/llvm/llvm-project/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test:26:10: > error: CHECK: expected string not found in input > # CHECK: frame #{{[0-9]+}}: [[ADD]] {{.*}}`signal_generating_add ^ This is the most important part of the message. Btw, at the top of the failure message lit will print the commands executed as a part of this test. For most tests (this one included), you can just yank the lldb command out of there and run it in a terminal to debug. https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestSettings when run with a remote target (PR #91915)
https://github.com/labath approved this pull request. Makes sense, although I think that slapping `@skipIfRemote` on the test would work equally well. AFAICT, this test does not actually care about the remote (or any other) platform, so there's very little value in running it in these configurations. https://github.com/llvm/llvm-project/pull/91915 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestQuoting (PR #91886)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/91886 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Disable macOS specific test for Windows host instead of Windows target (PR #91942)
labath wrote: This doesn't sound right. The archive format is a property of the target system, not the host. It should still be possible to create these kinds of linux archives, even when running on windows hosts (and vice versa: if we're running on a linux host, then we won't be able to create windows-targeted bsd-archives -- not that anyone cares about this combination). What was the error you were seeing? Does the Makefile contain unix-specific commands? https://github.com/llvm/llvm-project/pull/91942 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 0a6103e - Revert "[lldb] Attempt to fix signal-in-leaf-function-aarch64 on darwin"
Author: Muhammad Omair Javaid Date: 2024-05-13T16:05:02+05:00 New Revision: 0a6103eaeb7f22c009f9add87c84780b6f7f293a URL: https://github.com/llvm/llvm-project/commit/0a6103eaeb7f22c009f9add87c84780b6f7f293a DIFF: https://github.com/llvm/llvm-project/commit/0a6103eaeb7f22c009f9add87c84780b6f7f293a.diff LOG: Revert "[lldb] Attempt to fix signal-in-leaf-function-aarch64 on darwin" This reverts commit b903badd73a2467fdd4e363231f2bf9b0704b546. TestInterruptBacktrace was broken on AArch64/Windows as a result of this change. see lldb-aarch64-windows buildbot here: https://lab.llvm.org/buildbot/#/builders/219/builds/11261 Added: Modified: lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test Removed: diff --git a/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c b/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c index fe020affcad0f..9a751330623f4 100644 --- a/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c +++ b/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c @@ -7,7 +7,7 @@ int __attribute__((naked)) signal_generating_add(int a, int b) { "ret"); } -void sigill_handler(int signo) { _exit(0); } +void sigill_handler(int) { _exit(0); } int main() { signal(SIGILL, sigill_handler); diff --git a/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test b/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test index 09f17c174bbfd..0580d0cf734ae 100644 --- a/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test +++ b/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test @@ -4,9 +4,6 @@ # RUN: %clang_host %S/Inputs/signal-in-leaf-function-aarch64.c -o %t # RUN: %lldb -s %s -o exit %t | FileCheck %s -# Convert EXC_BAD_INSTRUCTION to SIGILL on darwin -settings set platform.plugin.darwin.ignored-exceptions EXC_BAD_INSTRUCTION - breakpoint set -n sigill_handler # CHECK: Breakpoint 1: where = {{.*}}`sigill_handler ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 4b44502 - Revert "[lldb/aarch64] Fix unwinding when signal interrupts a leaf function (#91321)"
Author: Muhammad Omair Javaid Date: 2024-05-13T16:05:02+05:00 New Revision: 4b44502ac81259630b422e791a82e0252e6478c3 URL: https://github.com/llvm/llvm-project/commit/4b44502ac81259630b422e791a82e0252e6478c3 DIFF: https://github.com/llvm/llvm-project/commit/4b44502ac81259630b422e791a82e0252e6478c3.diff LOG: Revert "[lldb/aarch64] Fix unwinding when signal interrupts a leaf function (#91321)" This reverts commit fd1bd53ba5a06f344698a55578f6a5d79c457e30. TestInterruptBacktrace was broken on AArch64/Windows as a result of this change. See lldb-aarch64-windows buildbot here: https://lab.llvm.org/buildbot/#/builders/219/builds/11261 Added: Modified: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp lldb/source/Target/RegisterContextUnwind.cpp lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp Removed: lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test diff --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp index 62ecac3e0831d..6ca4fb052457e 100644 --- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp +++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp @@ -444,8 +444,6 @@ bool EmulateInstructionARM64::CreateFunctionEntryUnwind( // Our previous Call Frame Address is the stack pointer row->GetCFAValue().SetIsRegisterPlusOffset(gpr_sp_arm64, 0); - row->SetRegisterLocationToSame(gpr_lr_arm64, /*must_replace=*/false); - row->SetRegisterLocationToSame(gpr_fp_arm64, /*must_replace=*/false); unwind_plan.AppendRow(row); unwind_plan.SetSourceName("EmulateInstructionARM64"); diff --git a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp index 49edd40544e32..c4a171ec7d01b 100644 --- a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp +++ b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp @@ -424,6 +424,8 @@ size_t UnwindAssemblyInstEmulation::WriteMemory( log->PutString(strm.GetString()); } + const bool cant_replace = false; + switch (context.type) { default: case EmulateInstruction::eContextInvalid: @@ -465,7 +467,7 @@ size_t UnwindAssemblyInstEmulation::WriteMemory( m_pushed_regs[reg_num] = addr; const int32_t offset = addr - m_initial_sp; m_curr_row->SetRegisterLocationToAtCFAPlusOffset(reg_num, offset, - /*can_replace=*/true); + cant_replace); m_curr_row_modified = true; } } diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp index e2d712cb72eae..13e101413a477 100644 --- a/lldb/source/Target/RegisterContextUnwind.cpp +++ b/lldb/source/Target/RegisterContextUnwind.cpp @@ -1555,12 +1555,12 @@ RegisterContextUnwind::SavedLocationForRegister( } if (unwindplan_regloc.IsSame()) { -if (!m_all_registers_available && +if (!IsFrameZero() && (regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_PC || regnum.GetAsKind(eRegisterKindGeneric) == LLDB_REGNUM_GENERIC_RA)) { UnwindLogMsg("register %s (%d) is marked as 'IsSame' - it is a pc or " - "return address reg on a frame which does not have all " - "registers available -- treat as if we have no information", + "return address reg on a non-zero frame -- treat as if we " + "have no information", regnum.GetName(), regnum.GetAsKind(eRegisterKindLLDB)); return UnwindLLDB::RegisterSearchResult::eRegisterNotFound; } else { diff --git a/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c b/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c deleted file mode 100644 index 9a751330623f4..0 --- a/lldb/test/Shell/Unwind/Inputs/signal-in-leaf-function-aarch64.c +++ /dev/null @@ -1,15 +0,0 @@ -#include -#include - -int __attribute__((naked)) signal_generating_add(int a, int b) { - asm("add w0, w1, w0\n\t" - "udf #0xdead\n\t" - "ret"); -} - -void sigill_handler(int) { _exit(0); } - -int main() { - signal(SIGILL, sigill_handler); - return signal_generating_add(42, 47); -} diff --git a/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test b/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test deleted file mode 100644 index 0580d0cf734ae..0 --- a/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test +
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
omjavaid wrote: LLDB became unresponsive on windows when a `thread backtrace` command was issued after hitting the exception i have temporarily reverted the change to make buildbot green. https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
labath wrote: > LLDB became unresponsive on windows when a `thread backtrace` command was > issued after hitting the exception i have temporarily reverted the change to > make buildbot green. Could you please give me some more information about the problem? I don't have access to a windows arm machine, and the failure message doesn't give me much to go on. https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestPlatformProcessLaunch running on a remote target (PR #91923)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/91923 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestGdbRemoteLaunch (PR #91931)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/91931 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/91944 >From de7135a8a4a40b5aa5ac1f44e58d62874c96448b Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 14:45:33 +0400 Subject: [PATCH 1/2] [lldb] Add lldbutil.target_install() helper It can be used in tests #91918, #91931 and such. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 16 1 file changed, 16 insertions(+) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 58eb37fd742d7..e67b68b727b80 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,6 +1654,22 @@ def find_library_callable(test): ) +def target_install(test, filename): +path = test.getBuildArtifact(filename) +if lldb.remote_platform: +remote_path = lldbutil.append_to_process_working_directory(test, filename) +err = lldb.remote_platform.Install( +lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) +) +if err.Fail(): +raise Exception( +"remote_platform.Install('%s', '%s') failed: %s" +% (path, remote_path, err) +) +path = remote_path +return path + + def read_file_on_target(test, remote): if lldb.remote_platform: local = test.getBuildArtifact("file_from_target") >From b4f5798258b29996b6805037bed70a819a671618 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 15:51:36 +0400 Subject: [PATCH 2/2] Added the full local path as optional parameter. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index e67b68b727b80..4290fe94590a1 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,10 +1654,14 @@ def find_library_callable(test): ) -def target_install(test, filename): -path = test.getBuildArtifact(filename) +def target_install(test, filename=None, path=None): +test.assertTrue(filename or path, "filename or path must be specified.") +if filename is None: +filename = os.path.basename(path) +if path is None: +path = test.getBuildArtifact(filename) if lldb.remote_platform: -remote_path = lldbutil.append_to_process_working_directory(test, filename) +remote_path = append_to_process_working_directory(test, filename) err = lldb.remote_platform.Install( lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) ) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] bc17361 - [lldb][DWARF] Fix delayed-definition-die-searching.test for Windows
Author: Muhammad Omair Javaid Date: 2024-05-13T16:53:15+05:00 New Revision: bc17361c2baa0351f7f19b716fbe76bc9f99e903 URL: https://github.com/llvm/llvm-project/commit/bc17361c2baa0351f7f19b716fbe76bc9f99e903 DIFF: https://github.com/llvm/llvm-project/commit/bc17361c2baa0351f7f19b716fbe76bc9f99e903.diff LOG: [lldb][DWARF] Fix delayed-definition-die-searching.test for Windows This is follow up fix on top of 9a7262c2601874e5aa64c5db19746770212d4b44 This fixes delayed-definition-die-searching.test to use -gdwarf. This is required to explicitly select DWARF instead of PDB on windows. Fixe LLDB build lldb-aarch64-windows: https://lab.llvm.org/buildbot/#/builders/219/builds/11303 Added: Modified: lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test Removed: diff --git a/lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test b/lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test index a2b36dadedd21..836fcd7b587bc 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test +++ b/lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test @@ -1,7 +1,7 @@ # Test definition DIE searching is delayed until complete type is required. # RUN: split-file %s %t -# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -g -o %t.out +# RUN: %clangxx_host %t/main.cpp %t/t1_def.cpp -gdwarf -o %t.out # RUN: %lldb -b %t.out -s %t/lldb.cmd | FileCheck %s # CHECK: (lldb) p v1 @@ -31,4 +31,4 @@ int main() { struct t1 { // this CU contains definition DIE for t1. int x; }; -t1 v2; \ No newline at end of file +t1 v2; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 710d95d - [lldb] Fixed the test TestSettings when run with a remote target (#91915)
Author: Dmitry Vasilyev Date: 2024-05-13T15:55:20+04:00 New Revision: 710d95d1ecb4b6d69507cb910274ef3077ddc9c9 URL: https://github.com/llvm/llvm-project/commit/710d95d1ecb4b6d69507cb910274ef3077ddc9c9 DIFF: https://github.com/llvm/llvm-project/commit/710d95d1ecb4b6d69507cb910274ef3077ddc9c9.diff LOG: [lldb] Fixed the test TestSettings when run with a remote target (#91915) The setting `platform.module-cache-directory` is a local path on the host. It cannot be set to a working directory from the remote target. This test failed in case of Windows host and Linux target because of the incompatible path. Use the local build dir instead. Added: Modified: lldb/test/API/commands/settings/TestSettings.py Removed: diff --git a/lldb/test/API/commands/settings/TestSettings.py b/lldb/test/API/commands/settings/TestSettings.py index 104a9f09788c3..385acceb7a8b5 100644 --- a/lldb/test/API/commands/settings/TestSettings.py +++ b/lldb/test/API/commands/settings/TestSettings.py @@ -953,7 +953,7 @@ def test_settings_api(self): # Test OptionValueFileSpec self.verify_setting_value_json( -"platform.module-cache-directory", self.get_process_working_directory() +"platform.module-cache-directory", self.getBuildDir() ) # Test OptionValueArray ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestSettings when run with a remote target (PR #91915)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/91915 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 7ce3dd4 - [lldb] Fixed the test TestQuoting (#91886)
Author: Dmitry Vasilyev Date: 2024-05-13T15:58:39+04:00 New Revision: 7ce3dd49eb80816e3af52022ba2521b28a068c7b URL: https://github.com/llvm/llvm-project/commit/7ce3dd49eb80816e3af52022ba2521b28a068c7b DIFF: https://github.com/llvm/llvm-project/commit/7ce3dd49eb80816e3af52022ba2521b28a068c7b.diff LOG: [lldb] Fixed the test TestQuoting (#91886) os.path.join() uses the path separator of the host OS by default. outfile_arg will be incorrect in case of Windows host and Linux target. Use lldbutil.append_to_process_working_directory() instead. Added: Modified: lldb/test/API/commands/settings/quoting/TestQuoting.py Removed: diff --git a/lldb/test/API/commands/settings/quoting/TestQuoting.py b/lldb/test/API/commands/settings/quoting/TestQuoting.py index 393f4be3c8242..60ad4e0a4 100644 --- a/lldb/test/API/commands/settings/quoting/TestQuoting.py +++ b/lldb/test/API/commands/settings/quoting/TestQuoting.py @@ -51,9 +51,7 @@ def expect_args(self, args_in, args_out): outfile = self.getBuildArtifact(filename) if lldb.remote_platform: -outfile_arg = os.path.join( -lldb.remote_platform.GetWorkingDirectory(), filename -) +outfile_arg = lldbutil.append_to_process_working_directory(self, filename) else: outfile_arg = outfile ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestQuoting (PR #91886)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/91886 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] [polly] fix(python): fix invalid escape sequences (PR #91856)
zufuliu wrote: > Why do we need to make this change? https://docs.python.org/3/reference/lexical_analysis.html#escape-sequences > Changed in version 3.12: Unrecognized escape sequences produce a > [SyntaxWarning](https://docs.python.org/3/library/exceptions.html#SyntaxWarning). > In a future Python version they will be eventually a > [SyntaxError](https://docs.python.org/3/library/exceptions.html#SyntaxError). doubling backlash inside regex string is poor than adding raw string prefix `r`. https://github.com/llvm/llvm-project/pull/91856 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Enforce exec permission using Platform::Install() from Windows host (PR #91887)
slydiman wrote: Target::Install() and Platform::Install() are used indirectly in many cases. For example look at the test `lldb/test/API/python_api/hello_world/TestHelloWorld.py`. target.LaunchSimple() uses Target::Install() and there is no problem with the exec permission. spawnSubprocess() uses the class _RemoteProcess and finally Platform::Install(). spawnSubprocess() is used in 27 test files and they are failed is case of Windows host and Linux target. > Target::Install does (i.e., set the execute flag unconditionally) Target::Install() checks is_main_executable enumerating all modules. But Target::Install()'s logic is not applicable in most cases where Platform::Install() is used. I think `Target::Install` is not a workaround and we cannot remove this code. I'd say this patch is a workaround for the case host=Windows and target!=Windows. We can even add a comment FIXME:... if someone will have an idea how to fix it better way. https://github.com/llvm/llvm-project/pull/91887 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Disable macOS specific test for Windows host instead of Windows target (PR #91942)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/91942 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Disable macOS specific test for Windows host instead of Windows target (PR #91942)
slydiman wrote: Agreed. It seems the real problem is that `ar` is missing on the Windows host. We will try to use cross `llvm-ar` instead. I will close this MR. Thanks. https://github.com/llvm/llvm-project/pull/91942 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1a25b72 - [lldb] Fixed the test TestPlatformProcessLaunch running on a remote target (#91923)
Author: Dmitry Vasilyev Date: 2024-05-13T17:47:03+04:00 New Revision: 1a25b723628e439d62dfb28ca5fa52e4b2a78e5a URL: https://github.com/llvm/llvm-project/commit/1a25b723628e439d62dfb28ca5fa52e4b2a78e5a DIFF: https://github.com/llvm/llvm-project/commit/1a25b723628e439d62dfb28ca5fa52e4b2a78e5a.diff LOG: [lldb] Fixed the test TestPlatformProcessLaunch running on a remote target (#91923) Transfer `stdio.log` from the remote target if necessary. Added: Modified: lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py Removed: diff --git a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py index 3fb7d00c93d29..7cbad03ad 100644 --- a/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py +++ b/lldb/test/API/commands/platform/process/launch/TestPlatformProcessLaunch.py @@ -3,6 +3,7 @@ """ from textwrap import dedent +from lldbsuite.test import lldbutil from lldbsuite.test.lldbtest import TestBase @@ -11,9 +12,10 @@ class ProcessLaunchTestCase(TestBase): def setup(self): self.build() -exe = self.getBuildArtifact("a.out") -self.runCmd("file " + exe) -return (exe, self.getBuildArtifact("stdio.log")) +self.runCmd("file " + self.getBuildArtifact("a.out")) +exe = lldbutil.append_to_process_working_directory(self, "a.out") +outfile = lldbutil.append_to_process_working_directory(self, "stdio.log") +return (exe, outfile) def test_process_launch_no_args(self): # When there are no extra arguments we just have 0, the program name. @@ -21,18 +23,18 @@ def test_process_launch_no_args(self): self.runCmd("platform process launch --stdout {} -s".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 1 argument(s). -[0]: {} -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 1 argument(s). +[0]: {} +""".format( +exe +) +), +stdio_log, +) def test_process_launch_command_args(self): exe, outfile = self.setup() @@ -41,21 +43,21 @@ def test_process_launch_command_args(self): self.runCmd("platform process launch --stdout {} -s -- A B C".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 4 argument(s). -[0]: {} -[1]: A -[2]: B -[3]: C -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 4 argument(s). +[0]: {} +[1]: A +[2]: B +[3]: C +""".format( +exe +) +), +stdio_log, +) def test_process_launch_target_args(self): exe, outfile = self.setup() @@ -64,17 +66,17 @@ def test_process_launch_target_args(self): self.runCmd("platform process launch --stdout {} -s".format(outfile)) self.runCmd("continue") -with open(outfile) as f: -self.assertEqual( -dedent( -"""\ -Got 3 argument(s). -[0]: {} -[1]: D -[2]: E -""".format( -exe -) -), -f.read(), -) +stdio_log = lldbutil.read_file_on_target(self, outfile) +self.assertEqual( +dedent( +"""\ +Got 3 argument(s). +[0]: {} +[1]: D +[2]: E +""".format( +exe +) +), +stdio_log, +) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the test TestPlatformProcessLaunch running on a remote target (PR #91923)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/91923 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
@@ -1654,6 +1654,26 @@ def find_library_callable(test): ) +def target_install(test, filename=None, path=None): +test.assertTrue(filename or path, "filename or path must be specified.") +if filename is None: +filename = os.path.basename(path) +if path is None: +path = test.getBuildArtifact(filename) labath wrote: This seems a bit... magical. Can we just have the caller specify a full path to the thing it wants to install? `self.getBuildArtifact` is not that much extra typing. https://github.com/llvm/llvm-project/pull/91944 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
@@ -1654,6 +1654,26 @@ def find_library_callable(test): ) +def target_install(test, filename=None, path=None): labath wrote: This is optional, but I think something like `install_to_target` would be a better match for the naming style of the surrounding functions. https://github.com/llvm/llvm-project/pull/91944 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Disable macOS specific test for Windows host instead of Windows target (PR #91942)
labath wrote: SGTM. llvm-objcopy is already set in test/API/CMakeLists.txt:88. It should be straightforward to add llvm-ar as well. https://github.com/llvm/llvm-project/pull/91942 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Enforce exec permission using Platform::Install() from Windows host (PR #91887)
labath wrote: I'm not sure I'm following your reasoning. I'm not saying we should replace/remove Target::Install. I'm saying we should make everything installed by Platform::Install executable by default (which, by extension, includes everything installed by Target::Install, because it delegates to this function. And you're saying that's not a good idea, because what? That not everything installed through Platform::Install is an actual executable? https://github.com/llvm/llvm-project/pull/91887 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix redundant condition in Target.cpp (PR #91882)
https://github.com/aabhinavg updated https://github.com/llvm/llvm-project/pull/91882 >From 9b4160975efe059f39a842689b1f750a10453203 Mon Sep 17 00:00:00 2001 From: aabhinavg Date: Sun, 12 May 2024 12:42:59 +0530 Subject: [PATCH] Fix redundant condition in Target.cpp This commit addresses issue #87244, where a redundant condition was found in the Target.cpp file. Static analyzer cppcheck flagged the issue in the Target.cpp file --- lldb/source/Target/Target.cpp | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 82f3040e539a3..fe87728a33dc8 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -841,12 +841,14 @@ static bool CheckIfWatchpointsSupported(Target *target, Status &error) { if (!num_supported_hardware_watchpoints) return true; - if (num_supported_hardware_watchpoints == 0) { -error.SetErrorStringWithFormat( -"Target supports (%u) hardware watchpoint slots.\n", -*num_supported_hardware_watchpoints); -return false; - } + // If num_supported_hardware_watchpoints is zero, set an + //error message and return false. + + error.SetErrorStringWithFormat( + "Target supports (%u) hardware watchpoint slots.\n", + *num_supported_hardware_watchpoints); + return false; + return true; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
@@ -1654,6 +1654,26 @@ def find_library_callable(test): ) +def target_install(test, filename=None, path=None): +test.assertTrue(filename or path, "filename or path must be specified.") +if filename is None: +filename = os.path.basename(path) +if path is None: +path = test.getBuildArtifact(filename) slydiman wrote: I expected the following usage: 1: Use filename ``` target_path = lldbutil.install_to_target(self, "a.out") ``` 2: Use path ``` local_path = self.getBuildArtifact("some_file") # write to local_path here ... # avoid typing "some_file" constant twice, just extract the filename from the local_path target_path = lldbutil.install_to_target(self, path=local_path) ``` 3: Use both ``` # "temp" is the filename on the remote target target_path = lldbutil.install_to_target(self, "temp", local_temp_path_outside_build_dir) ``` https://github.com/llvm/llvm-project/pull/91944 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix redundant condition in Target.cpp (PR #91882)
https://github.com/aabhinavg updated https://github.com/llvm/llvm-project/pull/91882 >From 9b4160975efe059f39a842689b1f750a10453203 Mon Sep 17 00:00:00 2001 From: aabhinavg Date: Sun, 12 May 2024 12:42:59 +0530 Subject: [PATCH 1/2] Fix redundant condition in Target.cpp This commit addresses issue #87244, where a redundant condition was found in the Target.cpp file. Static analyzer cppcheck flagged the issue in the Target.cpp file --- lldb/source/Target/Target.cpp | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 82f3040e539a3..fe87728a33dc8 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -841,12 +841,14 @@ static bool CheckIfWatchpointsSupported(Target *target, Status &error) { if (!num_supported_hardware_watchpoints) return true; - if (num_supported_hardware_watchpoints == 0) { -error.SetErrorStringWithFormat( -"Target supports (%u) hardware watchpoint slots.\n", -*num_supported_hardware_watchpoints); -return false; - } + // If num_supported_hardware_watchpoints is zero, set an + //error message and return false. + + error.SetErrorStringWithFormat( + "Target supports (%u) hardware watchpoint slots.\n", + *num_supported_hardware_watchpoints); + return false; + return true; } >From 421b4eed4c8bbcc2777fcbee1bf6c27786a393de Mon Sep 17 00:00:00 2001 From: aabhinavg Date: Mon, 13 May 2024 21:25:58 +0530 Subject: [PATCH 2/2] Revert "Fix redundant condition in Target.cpp" This reverts commit 9b4160975efe059f39a842689b1f750a10453203. The CheckIfWatchpointsSupported function is refactored to maintain the intended behavior as described below: 1. If we can't detect hardware watchpoints, assume they are supported. 2. If we can detect hardware watchpoints and there are more than 0, return true. 3. If we can detect hardware watchpoints but there are 0 of them, set an error message and return false. fix #87244 --- lldb/source/Target/Target.cpp | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index fe87728a33dc8..77731167995e1 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -841,14 +841,12 @@ static bool CheckIfWatchpointsSupported(Target *target, Status &error) { if (!num_supported_hardware_watchpoints) return true; - // If num_supported_hardware_watchpoints is zero, set an - //error message and return false. - - error.SetErrorStringWithFormat( - "Target supports (%u) hardware watchpoint slots.\n", - *num_supported_hardware_watchpoints); - return false; - + if (*num_supported_hardware_watchpoints == 0) { +error.SetErrorStringWithFormat( +"Target supports (%u) hardware watchpoint slots.\n", +*num_supported_hardware_watchpoints); +return false; + } return true; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/91944 >From de7135a8a4a40b5aa5ac1f44e58d62874c96448b Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 14:45:33 +0400 Subject: [PATCH 1/3] [lldb] Add lldbutil.target_install() helper It can be used in tests #91918, #91931 and such. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 16 1 file changed, 16 insertions(+) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 58eb37fd742d7..e67b68b727b80 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,6 +1654,22 @@ def find_library_callable(test): ) +def target_install(test, filename): +path = test.getBuildArtifact(filename) +if lldb.remote_platform: +remote_path = lldbutil.append_to_process_working_directory(test, filename) +err = lldb.remote_platform.Install( +lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) +) +if err.Fail(): +raise Exception( +"remote_platform.Install('%s', '%s') failed: %s" +% (path, remote_path, err) +) +path = remote_path +return path + + def read_file_on_target(test, remote): if lldb.remote_platform: local = test.getBuildArtifact("file_from_target") >From b4f5798258b29996b6805037bed70a819a671618 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 15:51:36 +0400 Subject: [PATCH 2/3] Added the full local path as optional parameter. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index e67b68b727b80..4290fe94590a1 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,10 +1654,14 @@ def find_library_callable(test): ) -def target_install(test, filename): -path = test.getBuildArtifact(filename) +def target_install(test, filename=None, path=None): +test.assertTrue(filename or path, "filename or path must be specified.") +if filename is None: +filename = os.path.basename(path) +if path is None: +path = test.getBuildArtifact(filename) if lldb.remote_platform: -remote_path = lldbutil.append_to_process_working_directory(test, filename) +remote_path = append_to_process_working_directory(test, filename) err = lldb.remote_platform.Install( lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) ) >From 79831a4c0e276caf4952b4d5748d5d7ba9e59365 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 20:02:41 +0400 Subject: [PATCH 3/3] Renamed to `install_to_target`. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 4290fe94590a1..e1aa8d63b9b72 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,7 +1654,7 @@ def find_library_callable(test): ) -def target_install(test, filename=None, path=None): +def install_to_target(test, filename=None, path=None): test.assertTrue(filename or path, "filename or path must be specified.") if filename is None: filename = os.path.basename(path) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem][NFCI] Log creation of new TypeSystem instances to expression log (PR #91985)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/91985 We emit `ASTContext` and `TypeSystem` pointers into the `expr` log but there is no easy way (that I know of) to correlate the pointer value back to an easily readible form. This patch simply logs the name of the `TypeSystem` and the associated `ASTContext` into the `expr` channel whenever we create a new `TypeSystemClang`. The following is an example of the new log entries: ``` $ grep Created /tmp/lldb.log Created new TypeSystem for (ASTContext*)0x000101a2e200 'ASTContext for '/Users/michaelbuch/a.out'' Created new TypeSystem for (ASTContext*)0x000102512a00 'scratch ASTContext' Created new TypeSystem for (ASTContext*)0x000102116a00 'ClangModulesDeclVendor ASTContext' Created new TypeSystem for (ASTContext*)0x0001022e8c00 'Expression ASTContext for ''' Created new TypeSystem for (ASTContext*)0x0001103e7200 'AppleObjCTypeEncodingParser ASTContext' Created new TypeSystem for (ASTContext*)0x0001103f7000 'AppleObjCDeclVendor AST' Created new TypeSystem for (ASTContext*)0x0001104bfe00 'Expression ASTContext for ''' Created new TypeSystem for (ASTContext*)0x000101f01000 'Expression ASTContext for ''' Created new TypeSystem for (ASTContext*)0x0001025d3c00 'Expression ASTContext for ''' Created new TypeSystem for (ASTContext*)0x000110422400 'Expression ASTContext for ''' Created new TypeSystem for (ASTContext*)0x00011602c200 'Expression ASTContext for ''' Created new TypeSystem for (ASTContext*)0x000110641600 'Expression ASTContext for ''' Created new TypeSystem for (ASTContext*)0x000110617400 'Expression ASTContext for ''' ``` >From 1ccf935f97b21e0bd87955f3313cb47261d11379 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 13 May 2024 15:34:24 +0100 Subject: [PATCH] [lldb][TypeSystem][NFCI] Log creation of new TypeSystem instances to expression log We emit `ASTContext` and `TypeSystem` pointers into the `expr` log but there is no easy way (that I know of) to correlate the pointer value back to an easily readible form. This patch simply logs the name of the `TypeSystem` and the associated `ASTContext` into the `expr` channel whenever we create a new `TypeSystemClang`. --- .../TypeSystem/Clang/TypeSystemClang.cpp | 21 +++ .../TypeSystem/Clang/TypeSystemClang.h| 4 +++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index d0033fcd9cdfc..a7b5c55098de2 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -501,6 +501,8 @@ TypeSystemClang::TypeSystemClang(llvm::StringRef name, // The caller didn't pass an ASTContext so create a new one for this // TypeSystemClang. CreateASTContext(); + + LogCreation(); } TypeSystemClang::TypeSystemClang(llvm::StringRef name, @@ -510,6 +512,8 @@ TypeSystemClang::TypeSystemClang(llvm::StringRef name, m_ast_up.reset(&existing_ctxt); GetASTMap().Insert(&existing_ctxt, this); + + LogCreation(); } // Destructor @@ -544,13 +548,16 @@ lldb::TypeSystemSP TypeSystemClang::CreateInstance(lldb::LanguageType language, } } + lldb::TypeSystemSP instance; + if (module) { std::string ast_name = "ASTContext for '" + module->GetFileSpec().GetPath() + "'"; -return std::make_shared(ast_name, triple); +instance = std::make_shared(ast_name, triple); } else if (target && target->IsValid()) -return std::make_shared(*target, triple); - return lldb::TypeSystemSP(); +instance = std::make_shared(*target, triple); + + return instance; } LanguageSet TypeSystemClang::GetSupportedLanguagesForTypes() { @@ -630,7 +637,7 @@ void TypeSystemClang::SetExternalSource( ast.setExternalSource(ast_source_up); } -ASTContext &TypeSystemClang::getASTContext() { +ASTContext &TypeSystemClang::getASTContext() const { assert(m_ast_up); return *m_ast_up; } @@ -9750,3 +9757,9 @@ bool TypeSystemClang::SetDeclIsForcefullyCompleted(const clang::TagDecl *td) { metadata->SetIsForcefullyCompleted(); return true; } + +void TypeSystemClang::LogCreation() const { + if (auto *log = GetLog(LLDBLog::Expressions)) +LLDB_LOG(log, "Created new TypeSystem for (ASTContext*){0:x} '{1}'", + &getASTContext(), getDisplayName()); +} diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 59ca69622d9e8..6ba2c44c36584 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -162,7 +162,7 @@ class TypeSystemClang : public TypeSystem { llvm::StringRef getDisplayName() const { return m_display_name; } /// Returns the clang::ASTContext instance managed by this TypeSystemCla
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
@@ -1654,6 +1654,26 @@ def find_library_callable(test): ) +def target_install(test, filename=None, path=None): slydiman wrote: Updated. Thanks. https://github.com/llvm/llvm-project/pull/91944 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem][NFCI] Log creation of new TypeSystem instances to expression log (PR #91985)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes We emit `ASTContext` and `TypeSystem` pointers into the `expr` log but there is no easy way (that I know of) to correlate the pointer value back to an easily readible form. This patch simply logs the name of the `TypeSystem` and the associated `ASTContext` into the `expr` channel whenever we create a new `TypeSystemClang`. The following is an example of the new log entries: ``` $ grep Created /tmp/lldb.log Created new TypeSystem for (ASTContext*)0x000101a2e200 'ASTContext for '/Users/michaelbuch/a.out'' Created new TypeSystem for (ASTContext*)0x000102512a00 'scratch ASTContext' Created new TypeSystem for (ASTContext*)0x000102116a00 'ClangModulesDeclVendor ASTContext' Created new TypeSystem for (ASTContext*)0x0001022e8c00 'Expression ASTContext for ''' Created new TypeSystem for (ASTContext*)0x0001103e7200 'AppleObjCTypeEncodingParser ASTContext' Created new TypeSystem for (ASTContext*)0x0001103f7000 'AppleObjCDeclVendor AST' Created new TypeSystem for (ASTContext*)0x0001104bfe00 'Expression ASTContext for ' '' Created new TypeSystem for (ASTContext*)0x000101f01000 'Expression ASTContext for ' '' Created new TypeSystem for (ASTContext*)0x0001025d3c00 'Expression ASTContext for ' '' Created new TypeSystem for (ASTContext*)0x000110422400 'Expression ASTContext for ' '' Created new TypeSystem for (ASTContext*)0x00011602c200 'Expression ASTContext for ' '' Created new TypeSystem for (ASTContext*)0x000110641600 'Expression ASTContext for ' '' Created new TypeSystem for (ASTContext*)0x000110617400 'Expression ASTContext for ' '' ``` --- Full diff: https://github.com/llvm/llvm-project/pull/91985.diff 2 Files Affected: - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+17-4) - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+3-1) ``diff diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index d0033fcd9cdfc..a7b5c55098de2 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -501,6 +501,8 @@ TypeSystemClang::TypeSystemClang(llvm::StringRef name, // The caller didn't pass an ASTContext so create a new one for this // TypeSystemClang. CreateASTContext(); + + LogCreation(); } TypeSystemClang::TypeSystemClang(llvm::StringRef name, @@ -510,6 +512,8 @@ TypeSystemClang::TypeSystemClang(llvm::StringRef name, m_ast_up.reset(&existing_ctxt); GetASTMap().Insert(&existing_ctxt, this); + + LogCreation(); } // Destructor @@ -544,13 +548,16 @@ lldb::TypeSystemSP TypeSystemClang::CreateInstance(lldb::LanguageType language, } } + lldb::TypeSystemSP instance; + if (module) { std::string ast_name = "ASTContext for '" + module->GetFileSpec().GetPath() + "'"; -return std::make_shared(ast_name, triple); +instance = std::make_shared(ast_name, triple); } else if (target && target->IsValid()) -return std::make_shared(*target, triple); - return lldb::TypeSystemSP(); +instance = std::make_shared(*target, triple); + + return instance; } LanguageSet TypeSystemClang::GetSupportedLanguagesForTypes() { @@ -630,7 +637,7 @@ void TypeSystemClang::SetExternalSource( ast.setExternalSource(ast_source_up); } -ASTContext &TypeSystemClang::getASTContext() { +ASTContext &TypeSystemClang::getASTContext() const { assert(m_ast_up); return *m_ast_up; } @@ -9750,3 +9757,9 @@ bool TypeSystemClang::SetDeclIsForcefullyCompleted(const clang::TagDecl *td) { metadata->SetIsForcefullyCompleted(); return true; } + +void TypeSystemClang::LogCreation() const { + if (auto *log = GetLog(LLDBLog::Expressions)) +LLDB_LOG(log, "Created new TypeSystem for (ASTContext*){0:x} '{1}'", + &getASTContext(), getDisplayName()); +} diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 59ca69622d9e8..6ba2c44c36584 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -162,7 +162,7 @@ class TypeSystemClang : public TypeSystem { llvm::StringRef getDisplayName() const { return m_display_name; } /// Returns the clang::ASTContext instance managed by this TypeSystemClang. - clang::ASTContext &getASTContext(); + clang::ASTContext &getASTContext() const; clang::MangleContext *getMangleContext(); @@ -1166,6 +1166,8 @@ class TypeSystemClang : public TypeSystem { bool IsTypeImpl(lldb::opaque_compiler_type_t type, llvm::function_ref predicate) const; + void LogCreatio
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/91944 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Log pointers as hex (PR #91989)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/91989 This ensures that we log pointers as lower-case hex. E.g., instead of: ``` LayoutRecordType on (ASTContext*)0x00010E78D600 'scratch ASTContext' for (RecordDecl*)0x00010E797 ``` we now log: ``` LayoutRecordType on (ASTContext*)0x00010e78d600 'scratch ASTContext' for (RecordDecl*)0x00010e797 ``` Which is consistent with how the AST dump gets emitted into the log, making it easier to correlate pointers we log from LLDB and pointers that are part of any AST dumps in the same `expr` log. >From 1fba045db76f51a2dbd9a2a3cb9879858b5b653e Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 13 May 2024 14:50:07 +0100 Subject: [PATCH] [lldb][ExpressionParser][NFCI] Log pointers as hex This ensures that we log pointers as lower-case hex. E.g., instead of: ``` LayoutRecordType on (ASTContext*)0x00010E78D600 'scratch ASTContext' for (RecordDecl*)0x00010E797 ``` we now log: ``` ``` --- .../Clang/ClangASTImporter.cpp| 48 +-- .../ExpressionParser/Clang/ClangASTSource.cpp | 44 - 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 30b50df79da90..2889bef26c3bc 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -313,8 +313,8 @@ CompilerType ClangASTImporter::DeportType(TypeSystemClang &dst, return {}; LLDB_LOG(log, - "[ClangASTImporter] DeportType called on ({0}Type*){1} " - "from (ASTContext*){2} to (ASTContext*){3}", + "[ClangASTImporter] DeportType called on ({0}Type*){1:x} " + "from (ASTContext*){2:x} to (ASTContext*){3:x}", src_type.GetTypeName(), src_type.GetOpaqueQualType(), &src_ctxt->getASTContext(), &dst.getASTContext()); @@ -334,8 +334,8 @@ clang::Decl *ClangASTImporter::DeportDecl(clang::ASTContext *dst_ctx, clang::ASTContext *src_ctx = &decl->getASTContext(); LLDB_LOG(log, - "[ClangASTImporter] DeportDecl called on ({0}Decl*){1} from " - "(ASTContext*){2} to (ASTContext*){3}", + "[ClangASTImporter] DeportDecl called on ({0}Decl*){1:x} from " + "(ASTContext*){2:x} to (ASTContext*){3:x}", decl->getDeclKindName(), decl, src_ctx, dst_ctx); DeclContextOverride decl_context_override; @@ -352,8 +352,8 @@ clang::Decl *ClangASTImporter::DeportDecl(clang::ASTContext *dst_ctx, return nullptr; LLDB_LOG(log, - "[ClangASTImporter] DeportDecl deported ({0}Decl*){1} to " - "({2}Decl*){3}", + "[ClangASTImporter] DeportDecl deported ({0}Decl*){1:x} to " + "({2}Decl*){3:x}", decl->getDeclKindName(), decl, result->getDeclKindName(), result); return result; @@ -637,8 +637,8 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( clang::ASTContext &dest_ctx = record->getASTContext(); LLDB_LOG(log, - "LayoutRecordType on (ASTContext*){0} '{1}' for (RecordDecl*)" - "{2} [name = '{3}']", + "LayoutRecordType on (ASTContext*){0:x} '{1}' for (RecordDecl*)" + "{2:x} [name = '{3}']", &dest_ctx, TypeSystemClang::GetASTContext(&dest_ctx)->getDisplayName(), record, record->getName()); @@ -703,7 +703,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( if (log) { LLDB_LOG(log, "LRT returned:"); -LLDB_LOG(log, "LRT Original = (RecordDecl*){0}", +LLDB_LOG(log, "LRT Original = (RecordDecl*){0:x}", static_cast(origin_record.decl)); LLDB_LOG(log, "LRT Size = {0}", size); LLDB_LOG(log, "LRT Alignment = {0}", alignment); @@ -712,7 +712,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( fe = record->field_end(); fi != fe; ++fi) { LLDB_LOG(log, - "LRT (FieldDecl*){0}, Name = '{1}', Type = '{2}', Offset = " + "LRT (FieldDecl*){0:x}, Name = '{1}', Type = '{2}', Offset = " "{3} bits", *fi, fi->getName(), fi->getType().getAsString(), field_offsets[*fi]); @@ -734,7 +734,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( DynCast(base_record); LLDB_LOG(log, - "LRT {0}(CXXRecordDecl*){1}, Name = '{2}', Offset = " + "LRT {0}(CXXRecordDecl*){1:x}, Name = '{2}', Offset = " "{3} chars", (is_virtual ? "Virtual " : ""), base_cxx_record.decl, base_cxx_record.decl->getName(), @@ -1025,7 +1025,7 @@ void ClangASTImporter::ForgetDestination(clang::ASTContext *dst_ast) { Log *log = GetLog(LLDBLog::Expressions); LLD
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
https://github.com/feg208 updated https://github.com/llvm/llvm-project/pull/91544 >From d01efd10f8a4b7d908acaa3237541bf6a83b4c7c Mon Sep 17 00:00:00 2001 From: Fred Grim Date: Wed, 8 May 2024 15:36:16 -0700 Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo To implement SaveCore for elf binaries we need to populate some additional fields in the prpsinfo struct. Those fields are the nice value of the process whose core is to be taken as well as a boolean flag indicating whether or not that process is a zombie. This commit adds those as well as tests to ensure that the values are consistent with expectations --- lldb/include/lldb/Utility/ProcessInfo.h | 94 +++-- lldb/source/Host/linux/Host.cpp | 32 ++--- lldb/source/Utility/ProcessInfo.cpp | 11 ++- lldb/unittests/Host/linux/HostTest.cpp | 21 ++ 4 files changed, 109 insertions(+), 49 deletions(-) diff --git a/lldb/include/lldb/Utility/ProcessInfo.h b/lldb/include/lldb/Utility/ProcessInfo.h index 54ac000dc7fc2..995873a6869e0 100644 --- a/lldb/include/lldb/Utility/ProcessInfo.h +++ b/lldb/include/lldb/Utility/ProcessInfo.h @@ -15,6 +15,7 @@ #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/NameMatches.h" #include "lldb/Utility/StructuredData.h" +#include #include namespace lldb_private { @@ -147,72 +148,71 @@ class ProcessInstanceInfo : public ProcessInfo { ProcessInstanceInfo() = default; ProcessInstanceInfo(const char *name, const ArchSpec &arch, lldb::pid_t pid) - : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX), -m_parent_pid(LLDB_INVALID_PROCESS_ID) {} + : ProcessInfo(name, arch, pid) {} void Clear() { ProcessInfo::Clear(); -m_euid = UINT32_MAX; -m_egid = UINT32_MAX; -m_parent_pid = LLDB_INVALID_PROCESS_ID; +m_euid = std::nullopt; +m_egid = std::nullopt; +m_parent_pid = std::nullopt; } - uint32_t GetEffectiveUserID() const { return m_euid; } + uint32_t GetEffectiveUserID() const { return m_euid.value(); } - uint32_t GetEffectiveGroupID() const { return m_egid; } + uint32_t GetEffectiveGroupID() const { return m_egid.value(); } - bool EffectiveUserIDIsValid() const { return m_euid != UINT32_MAX; } + bool EffectiveUserIDIsValid() const { return m_euid.has_value(); } - bool EffectiveGroupIDIsValid() const { return m_egid != UINT32_MAX; } + bool EffectiveGroupIDIsValid() const { return m_egid.has_value(); } void SetEffectiveUserID(uint32_t uid) { m_euid = uid; } void SetEffectiveGroupID(uint32_t gid) { m_egid = gid; } - lldb::pid_t GetParentProcessID() const { return m_parent_pid; } + lldb::pid_t GetParentProcessID() const { return m_parent_pid.value(); } void SetParentProcessID(lldb::pid_t pid) { m_parent_pid = pid; } - bool ParentProcessIDIsValid() const { -return m_parent_pid != LLDB_INVALID_PROCESS_ID; - } + bool ParentProcessIDIsValid() const { return m_parent_pid.has_value(); } - lldb::pid_t GetProcessGroupID() const { return m_process_group_id; } + lldb::pid_t GetProcessGroupID() const { return m_process_group_id.value(); } void SetProcessGroupID(lldb::pid_t pgrp) { m_process_group_id = pgrp; } - bool ProcessGroupIDIsValid() const { -return m_process_group_id != LLDB_INVALID_PROCESS_ID; - } + bool ProcessGroupIDIsValid() const { return m_process_group_id.has_value(); } - lldb::pid_t GetProcessSessionID() const { return m_process_session_id; } + lldb::pid_t GetProcessSessionID() const { +return m_process_session_id.value(); + } void SetProcessSessionID(lldb::pid_t session) { m_process_session_id = session; } bool ProcessSessionIDIsValid() const { -return m_process_session_id != LLDB_INVALID_PROCESS_ID; +return m_process_session_id.has_value(); } - struct timespec GetUserTime() const { return m_user_time; } + struct timespec GetUserTime() const { return m_user_time.value(); } void SetUserTime(struct timespec utime) { m_user_time = utime; } bool UserTimeIsValid() const { -return m_user_time.tv_sec > 0 || m_user_time.tv_usec > 0; +return m_user_time.has_value() && + (m_user_time->tv_sec > 0 || m_user_time->tv_usec > 0); } - struct timespec GetSystemTime() const { return m_system_time; } + struct timespec GetSystemTime() const { return m_system_time.value(); } void SetSystemTime(struct timespec stime) { m_system_time = stime; } bool SystemTimeIsValid() const { -return m_system_time.tv_sec > 0 || m_system_time.tv_usec > 0; +return m_system_time.has_value() && + (m_system_time->tv_sec > 0 || m_system_time->tv_usec > 0); } struct timespec GetCumulativeUserTime() const { -return m_cumulative_user_time; +return m_cumulative_user_time.value(); } void SetCumulativeUserTime(struct timespec cutime) { @@ -220,12 +220,13 @@ class ProcessInstanceInfo : public ProcessInfo { } bool CumulativeUserTimeIsValid() cons
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Log pointers as hex (PR #91989)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/91989 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -86,4 +89,22 @@ TEST_F(HostTest, GetProcessInfo) { ProcessInstanceInfo::timespec next_user_time = Info.GetUserTime(); ASSERT_TRUE(user_time.tv_sec <= next_user_time.tv_sec || user_time.tv_usec <= next_user_time.tv_usec); + + struct rlimit rlim; + EXPECT_EQ(getrlimit(RLIMIT_NICE, &rlim), 0); + // getpriority can return -1 so we zero errno first + errno = 0; + int prio = getpriority(PRIO_PROCESS, PRIO_PROCESS); + ASSERT_TRUE((prio < 0 && errno == 0) || prio >= 0); + ASSERT_EQ(Info.GetPriorityValue(), prio); + // If we can't raise our nice level then this test can't be performed feg208 wrote: done https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Log pointers as hex (PR #91989)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes This ensures that we log pointers as lower-case hex. E.g., instead of: ``` LayoutRecordType on (ASTContext*)0x00010E78D600 'scratch ASTContext' for (RecordDecl*)0x00010E797 ``` we now log: ``` LayoutRecordType on (ASTContext*)0x00010e78d600 'scratch ASTContext' for (RecordDecl*)0x00010e797 ``` Which is consistent with how the AST dump gets emitted into the log. This makes it easier to correlate pointers we log from LLDB and pointers that are part of any AST dumps in the same `expr` log. --- Full diff: https://github.com/llvm/llvm-project/pull/91989.diff 2 Files Affected: - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp (+24-24) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp (+22-22) ``diff diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 30b50df79da90..2889bef26c3bc 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -313,8 +313,8 @@ CompilerType ClangASTImporter::DeportType(TypeSystemClang &dst, return {}; LLDB_LOG(log, - "[ClangASTImporter] DeportType called on ({0}Type*){1} " - "from (ASTContext*){2} to (ASTContext*){3}", + "[ClangASTImporter] DeportType called on ({0}Type*){1:x} " + "from (ASTContext*){2:x} to (ASTContext*){3:x}", src_type.GetTypeName(), src_type.GetOpaqueQualType(), &src_ctxt->getASTContext(), &dst.getASTContext()); @@ -334,8 +334,8 @@ clang::Decl *ClangASTImporter::DeportDecl(clang::ASTContext *dst_ctx, clang::ASTContext *src_ctx = &decl->getASTContext(); LLDB_LOG(log, - "[ClangASTImporter] DeportDecl called on ({0}Decl*){1} from " - "(ASTContext*){2} to (ASTContext*){3}", + "[ClangASTImporter] DeportDecl called on ({0}Decl*){1:x} from " + "(ASTContext*){2:x} to (ASTContext*){3:x}", decl->getDeclKindName(), decl, src_ctx, dst_ctx); DeclContextOverride decl_context_override; @@ -352,8 +352,8 @@ clang::Decl *ClangASTImporter::DeportDecl(clang::ASTContext *dst_ctx, return nullptr; LLDB_LOG(log, - "[ClangASTImporter] DeportDecl deported ({0}Decl*){1} to " - "({2}Decl*){3}", + "[ClangASTImporter] DeportDecl deported ({0}Decl*){1:x} to " + "({2}Decl*){3:x}", decl->getDeclKindName(), decl, result->getDeclKindName(), result); return result; @@ -637,8 +637,8 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( clang::ASTContext &dest_ctx = record->getASTContext(); LLDB_LOG(log, - "LayoutRecordType on (ASTContext*){0} '{1}' for (RecordDecl*)" - "{2} [name = '{3}']", + "LayoutRecordType on (ASTContext*){0:x} '{1}' for (RecordDecl*)" + "{2:x} [name = '{3}']", &dest_ctx, TypeSystemClang::GetASTContext(&dest_ctx)->getDisplayName(), record, record->getName()); @@ -703,7 +703,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( if (log) { LLDB_LOG(log, "LRT returned:"); -LLDB_LOG(log, "LRT Original = (RecordDecl*){0}", +LLDB_LOG(log, "LRT Original = (RecordDecl*){0:x}", static_cast(origin_record.decl)); LLDB_LOG(log, "LRT Size = {0}", size); LLDB_LOG(log, "LRT Alignment = {0}", alignment); @@ -712,7 +712,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( fe = record->field_end(); fi != fe; ++fi) { LLDB_LOG(log, - "LRT (FieldDecl*){0}, Name = '{1}', Type = '{2}', Offset = " + "LRT (FieldDecl*){0:x}, Name = '{1}', Type = '{2}', Offset = " "{3} bits", *fi, fi->getName(), fi->getType().getAsString(), field_offsets[*fi]); @@ -734,7 +734,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( DynCast(base_record); LLDB_LOG(log, - "LRT {0}(CXXRecordDecl*){1}, Name = '{2}', Offset = " + "LRT {0}(CXXRecordDecl*){1:x}, Name = '{2}', Offset = " "{3} chars", (is_virtual ? "Virtual " : ""), base_cxx_record.decl, base_cxx_record.decl->getName(), @@ -1025,7 +1025,7 @@ void ClangASTImporter::ForgetDestination(clang::ASTContext *dst_ast) { Log *log = GetLog(LLDBLog::Expressions); LLDB_LOG(log, - "[ClangASTImporter] Forgetting destination (ASTContext*){0}", + "[ClangASTImporter] Forgetting destination (ASTContext*){0:x}", dst_ast); m_metadata_map.erase(dst_ast); @@ -1039,7 +1039,7 @@ void ClangASTImporter::ForgetSource(clang::ASTContext *dst_ast,
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -115,13 +124,19 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo &ProcessInfo, return ts; }; + // priority (nice) values run from 19 to -20 inclusive (in linux). In the + // prpsinfo struct pr_nice is a char feg208 wrote: done https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -115,13 +124,19 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo &ProcessInfo, return ts; }; + // priority (nice) values run from 19 to -20 inclusive (in linux). In the feg208 wrote: done https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -70,6 +71,12 @@ struct StatFields { long unsigned stime; long cutime; long cstime; + // in proc_pid_stat(5) this field is specified as priority + // but documented as realtime priority. To keep with the adopted + // nomenclature in ProcessInstanceInfo we adopt the documented + // naming here feg208 wrote: done https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -70,6 +71,12 @@ struct StatFields { long unsigned stime; long cutime; long cstime; + // in proc_pid_stat(5) this field is specified as priority + // but documented as realtime priority. To keep with the adopted + // nomenclature in ProcessInstanceInfo we adopt the documented feg208 wrote: done https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -70,6 +71,12 @@ struct StatFields { long unsigned stime; long cutime; long cstime; + // in proc_pid_stat(5) this field is specified as priority feg208 wrote: done https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
feg208 wrote: > Sorry for the nits! I am not smart enough to contribute on the technical > merits, so I am trying to find some way to help! Ehhh grammar is not where I shine. I rolled them up https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Log pointers as hex (PR #91989)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/91989 >From 1fba045db76f51a2dbd9a2a3cb9879858b5b653e Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 13 May 2024 14:50:07 +0100 Subject: [PATCH 1/2] [lldb][ExpressionParser][NFCI] Log pointers as hex This ensures that we log pointers as lower-case hex. E.g., instead of: ``` LayoutRecordType on (ASTContext*)0x00010E78D600 'scratch ASTContext' for (RecordDecl*)0x00010E797 ``` we now log: ``` ``` --- .../Clang/ClangASTImporter.cpp| 48 +-- .../ExpressionParser/Clang/ClangASTSource.cpp | 44 - 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 30b50df79da90..2889bef26c3bc 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -313,8 +313,8 @@ CompilerType ClangASTImporter::DeportType(TypeSystemClang &dst, return {}; LLDB_LOG(log, - "[ClangASTImporter] DeportType called on ({0}Type*){1} " - "from (ASTContext*){2} to (ASTContext*){3}", + "[ClangASTImporter] DeportType called on ({0}Type*){1:x} " + "from (ASTContext*){2:x} to (ASTContext*){3:x}", src_type.GetTypeName(), src_type.GetOpaqueQualType(), &src_ctxt->getASTContext(), &dst.getASTContext()); @@ -334,8 +334,8 @@ clang::Decl *ClangASTImporter::DeportDecl(clang::ASTContext *dst_ctx, clang::ASTContext *src_ctx = &decl->getASTContext(); LLDB_LOG(log, - "[ClangASTImporter] DeportDecl called on ({0}Decl*){1} from " - "(ASTContext*){2} to (ASTContext*){3}", + "[ClangASTImporter] DeportDecl called on ({0}Decl*){1:x} from " + "(ASTContext*){2:x} to (ASTContext*){3:x}", decl->getDeclKindName(), decl, src_ctx, dst_ctx); DeclContextOverride decl_context_override; @@ -352,8 +352,8 @@ clang::Decl *ClangASTImporter::DeportDecl(clang::ASTContext *dst_ctx, return nullptr; LLDB_LOG(log, - "[ClangASTImporter] DeportDecl deported ({0}Decl*){1} to " - "({2}Decl*){3}", + "[ClangASTImporter] DeportDecl deported ({0}Decl*){1:x} to " + "({2}Decl*){3:x}", decl->getDeclKindName(), decl, result->getDeclKindName(), result); return result; @@ -637,8 +637,8 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( clang::ASTContext &dest_ctx = record->getASTContext(); LLDB_LOG(log, - "LayoutRecordType on (ASTContext*){0} '{1}' for (RecordDecl*)" - "{2} [name = '{3}']", + "LayoutRecordType on (ASTContext*){0:x} '{1}' for (RecordDecl*)" + "{2:x} [name = '{3}']", &dest_ctx, TypeSystemClang::GetASTContext(&dest_ctx)->getDisplayName(), record, record->getName()); @@ -703,7 +703,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( if (log) { LLDB_LOG(log, "LRT returned:"); -LLDB_LOG(log, "LRT Original = (RecordDecl*){0}", +LLDB_LOG(log, "LRT Original = (RecordDecl*){0:x}", static_cast(origin_record.decl)); LLDB_LOG(log, "LRT Size = {0}", size); LLDB_LOG(log, "LRT Alignment = {0}", alignment); @@ -712,7 +712,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( fe = record->field_end(); fi != fe; ++fi) { LLDB_LOG(log, - "LRT (FieldDecl*){0}, Name = '{1}', Type = '{2}', Offset = " + "LRT (FieldDecl*){0:x}, Name = '{1}', Type = '{2}', Offset = " "{3} bits", *fi, fi->getName(), fi->getType().getAsString(), field_offsets[*fi]); @@ -734,7 +734,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( DynCast(base_record); LLDB_LOG(log, - "LRT {0}(CXXRecordDecl*){1}, Name = '{2}', Offset = " + "LRT {0}(CXXRecordDecl*){1:x}, Name = '{2}', Offset = " "{3} chars", (is_virtual ? "Virtual " : ""), base_cxx_record.decl, base_cxx_record.decl->getName(), @@ -1025,7 +1025,7 @@ void ClangASTImporter::ForgetDestination(clang::ASTContext *dst_ast) { Log *log = GetLog(LLDBLog::Expressions); LLDB_LOG(log, - "[ClangASTImporter] Forgetting destination (ASTContext*){0}", + "[ClangASTImporter] Forgetting destination (ASTContext*){0:x}", dst_ast); m_metadata_map.erase(dst_ast); @@ -1039,7 +1039,7 @@ void ClangASTImporter::ForgetSource(clang::ASTContext *dst_ast, LLDB_LOG(log, "[ClangASTImporter] Forgetting source->dest " - "(ASTContext*){0}->(ASTContext*){1}", + "(ASTContext*){0:x}->(ASTContext*){1:x}",
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Log pointers as hex (PR #91989)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff abd314938dda1b117f289be5e630e43e68533929 1fba045db76f51a2dbd9a2a3cb9879858b5b653e -- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 2889bef26c..44071d1ea7 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -711,11 +711,11 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( for (RecordDecl::field_iterator fi = record->field_begin(), fe = record->field_end(); fi != fe; ++fi) { - LLDB_LOG(log, - "LRT (FieldDecl*){0:x}, Name = '{1}', Type = '{2}', Offset = " - "{3} bits", - *fi, fi->getName(), fi->getType().getAsString(), - field_offsets[*fi]); + LLDB_LOG( + log, + "LRT (FieldDecl*){0:x}, Name = '{1}', Type = '{2}', Offset = " + "{3} bits", + *fi, fi->getName(), fi->getType().getAsString(), field_offsets[*fi]); } DeclFromParser parser_cxx_record = DynCast(parser_record); @@ -1164,9 +1164,10 @@ void ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo( from_named_decl->printName(name_stream); name_stream.flush(); } -LLDB_LOG(log_ast, " [ClangASTImporter][TUDecl: {0:x}] Imported " - "({1}Decl*){2:x}, named {3} (from " - "(Decl*){4:x})", +LLDB_LOG(log_ast, + " [ClangASTImporter][TUDecl: {0:x}] Imported " + "({1}Decl*){2:x}, named {3} (from " + "(Decl*){4:x})", static_cast(to->getTranslationUnitDecl()), from->getDeclKindName(), static_cast(to), name_string, static_cast(from)); @@ -1294,10 +1295,11 @@ void ClangASTImporter::ASTImporterDelegate::Imported(clang::Decl *from, from_named_decl->printName(name_stream); name_stream.flush(); - LLDB_LOG(log, - "[ClangASTImporter] Imported ({0}Decl*){1:x}, named {2} (from " - "(Decl*){3:x}), metadata {4}", - from->getDeclKindName(), to, name_string, from, user_id); + LLDB_LOG( + log, + "[ClangASTImporter] Imported ({0}Decl*){1:x}, named {2} (from " + "(Decl*){3:x}), metadata {4}", + from->getDeclKindName(), to, name_string, from, user_id); } else { LLDB_LOG(log, "[ClangASTImporter] Imported ({0}Decl*){1:x} (from " `` https://github.com/llvm/llvm-project/pull/91989 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem][NFCI] Log creation of new TypeSystem instances to expression log (PR #91985)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/91985 >From 1ccf935f97b21e0bd87955f3313cb47261d11379 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 13 May 2024 15:34:24 +0100 Subject: [PATCH 1/2] [lldb][TypeSystem][NFCI] Log creation of new TypeSystem instances to expression log We emit `ASTContext` and `TypeSystem` pointers into the `expr` log but there is no easy way (that I know of) to correlate the pointer value back to an easily readible form. This patch simply logs the name of the `TypeSystem` and the associated `ASTContext` into the `expr` channel whenever we create a new `TypeSystemClang`. --- .../TypeSystem/Clang/TypeSystemClang.cpp | 21 +++ .../TypeSystem/Clang/TypeSystemClang.h| 4 +++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index d0033fcd9cdfc..a7b5c55098de2 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -501,6 +501,8 @@ TypeSystemClang::TypeSystemClang(llvm::StringRef name, // The caller didn't pass an ASTContext so create a new one for this // TypeSystemClang. CreateASTContext(); + + LogCreation(); } TypeSystemClang::TypeSystemClang(llvm::StringRef name, @@ -510,6 +512,8 @@ TypeSystemClang::TypeSystemClang(llvm::StringRef name, m_ast_up.reset(&existing_ctxt); GetASTMap().Insert(&existing_ctxt, this); + + LogCreation(); } // Destructor @@ -544,13 +548,16 @@ lldb::TypeSystemSP TypeSystemClang::CreateInstance(lldb::LanguageType language, } } + lldb::TypeSystemSP instance; + if (module) { std::string ast_name = "ASTContext for '" + module->GetFileSpec().GetPath() + "'"; -return std::make_shared(ast_name, triple); +instance = std::make_shared(ast_name, triple); } else if (target && target->IsValid()) -return std::make_shared(*target, triple); - return lldb::TypeSystemSP(); +instance = std::make_shared(*target, triple); + + return instance; } LanguageSet TypeSystemClang::GetSupportedLanguagesForTypes() { @@ -630,7 +637,7 @@ void TypeSystemClang::SetExternalSource( ast.setExternalSource(ast_source_up); } -ASTContext &TypeSystemClang::getASTContext() { +ASTContext &TypeSystemClang::getASTContext() const { assert(m_ast_up); return *m_ast_up; } @@ -9750,3 +9757,9 @@ bool TypeSystemClang::SetDeclIsForcefullyCompleted(const clang::TagDecl *td) { metadata->SetIsForcefullyCompleted(); return true; } + +void TypeSystemClang::LogCreation() const { + if (auto *log = GetLog(LLDBLog::Expressions)) +LLDB_LOG(log, "Created new TypeSystem for (ASTContext*){0:x} '{1}'", + &getASTContext(), getDisplayName()); +} diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 59ca69622d9e8..6ba2c44c36584 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -162,7 +162,7 @@ class TypeSystemClang : public TypeSystem { llvm::StringRef getDisplayName() const { return m_display_name; } /// Returns the clang::ASTContext instance managed by this TypeSystemClang. - clang::ASTContext &getASTContext(); + clang::ASTContext &getASTContext() const; clang::MangleContext *getMangleContext(); @@ -1166,6 +1166,8 @@ class TypeSystemClang : public TypeSystem { bool IsTypeImpl(lldb::opaque_compiler_type_t type, llvm::function_ref predicate) const; + void LogCreation() const; + // Classes that inherit from TypeSystemClang can see and modify these std::string m_target_triple; std::unique_ptr m_ast_up; >From 8cb97736d9ba9bd2f1a0bc10f96c3ad7b9f9f59f Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 13 May 2024 17:19:21 +0100 Subject: [PATCH 2/2] fixup! revert obsolete changes --- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index a7b5c55098de2..8b9ef98071e68 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -548,16 +548,14 @@ lldb::TypeSystemSP TypeSystemClang::CreateInstance(lldb::LanguageType language, } } - lldb::TypeSystemSP instance; - if (module) { std::string ast_name = "ASTContext for '" + module->GetFileSpec().GetPath() + "'"; -instance = std::make_shared(ast_name, triple); +return std::make_shared(ast_name, triple); } else if (target && target->IsValid()) -instance = std::make_shared(*target, triple); +return std::make_shared(*target, triple); - return instance; + return
[Lldb-commits] [lldb] [lldb][TypeSystem][NFCI] Log creation of new TypeSystem instances to expression log (PR #91985)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/91985 >From 1ccf935f97b21e0bd87955f3313cb47261d11379 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 13 May 2024 15:34:24 +0100 Subject: [PATCH 1/3] [lldb][TypeSystem][NFCI] Log creation of new TypeSystem instances to expression log We emit `ASTContext` and `TypeSystem` pointers into the `expr` log but there is no easy way (that I know of) to correlate the pointer value back to an easily readible form. This patch simply logs the name of the `TypeSystem` and the associated `ASTContext` into the `expr` channel whenever we create a new `TypeSystemClang`. --- .../TypeSystem/Clang/TypeSystemClang.cpp | 21 +++ .../TypeSystem/Clang/TypeSystemClang.h| 4 +++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index d0033fcd9cdfc..a7b5c55098de2 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -501,6 +501,8 @@ TypeSystemClang::TypeSystemClang(llvm::StringRef name, // The caller didn't pass an ASTContext so create a new one for this // TypeSystemClang. CreateASTContext(); + + LogCreation(); } TypeSystemClang::TypeSystemClang(llvm::StringRef name, @@ -510,6 +512,8 @@ TypeSystemClang::TypeSystemClang(llvm::StringRef name, m_ast_up.reset(&existing_ctxt); GetASTMap().Insert(&existing_ctxt, this); + + LogCreation(); } // Destructor @@ -544,13 +548,16 @@ lldb::TypeSystemSP TypeSystemClang::CreateInstance(lldb::LanguageType language, } } + lldb::TypeSystemSP instance; + if (module) { std::string ast_name = "ASTContext for '" + module->GetFileSpec().GetPath() + "'"; -return std::make_shared(ast_name, triple); +instance = std::make_shared(ast_name, triple); } else if (target && target->IsValid()) -return std::make_shared(*target, triple); - return lldb::TypeSystemSP(); +instance = std::make_shared(*target, triple); + + return instance; } LanguageSet TypeSystemClang::GetSupportedLanguagesForTypes() { @@ -630,7 +637,7 @@ void TypeSystemClang::SetExternalSource( ast.setExternalSource(ast_source_up); } -ASTContext &TypeSystemClang::getASTContext() { +ASTContext &TypeSystemClang::getASTContext() const { assert(m_ast_up); return *m_ast_up; } @@ -9750,3 +9757,9 @@ bool TypeSystemClang::SetDeclIsForcefullyCompleted(const clang::TagDecl *td) { metadata->SetIsForcefullyCompleted(); return true; } + +void TypeSystemClang::LogCreation() const { + if (auto *log = GetLog(LLDBLog::Expressions)) +LLDB_LOG(log, "Created new TypeSystem for (ASTContext*){0:x} '{1}'", + &getASTContext(), getDisplayName()); +} diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 59ca69622d9e8..6ba2c44c36584 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -162,7 +162,7 @@ class TypeSystemClang : public TypeSystem { llvm::StringRef getDisplayName() const { return m_display_name; } /// Returns the clang::ASTContext instance managed by this TypeSystemClang. - clang::ASTContext &getASTContext(); + clang::ASTContext &getASTContext() const; clang::MangleContext *getMangleContext(); @@ -1166,6 +1166,8 @@ class TypeSystemClang : public TypeSystem { bool IsTypeImpl(lldb::opaque_compiler_type_t type, llvm::function_ref predicate) const; + void LogCreation() const; + // Classes that inherit from TypeSystemClang can see and modify these std::string m_target_triple; std::unique_ptr m_ast_up; >From 8cb97736d9ba9bd2f1a0bc10f96c3ad7b9f9f59f Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 13 May 2024 17:19:21 +0100 Subject: [PATCH 2/3] fixup! revert obsolete changes --- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index a7b5c55098de2..8b9ef98071e68 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -548,16 +548,14 @@ lldb::TypeSystemSP TypeSystemClang::CreateInstance(lldb::LanguageType language, } } - lldb::TypeSystemSP instance; - if (module) { std::string ast_name = "ASTContext for '" + module->GetFileSpec().GetPath() + "'"; -instance = std::make_shared(ast_name, triple); +return std::make_shared(ast_name, triple); } else if (target && target->IsValid()) -instance = std::make_shared(*target, triple); +return std::make_shared(*target, triple); - return instance; + return
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Log pointers as hex (PR #91989)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/91989 >From 1fba045db76f51a2dbd9a2a3cb9879858b5b653e Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 13 May 2024 14:50:07 +0100 Subject: [PATCH 1/3] [lldb][ExpressionParser][NFCI] Log pointers as hex This ensures that we log pointers as lower-case hex. E.g., instead of: ``` LayoutRecordType on (ASTContext*)0x00010E78D600 'scratch ASTContext' for (RecordDecl*)0x00010E797 ``` we now log: ``` ``` --- .../Clang/ClangASTImporter.cpp| 48 +-- .../ExpressionParser/Clang/ClangASTSource.cpp | 44 - 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 30b50df79da90..2889bef26c3bc 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -313,8 +313,8 @@ CompilerType ClangASTImporter::DeportType(TypeSystemClang &dst, return {}; LLDB_LOG(log, - "[ClangASTImporter] DeportType called on ({0}Type*){1} " - "from (ASTContext*){2} to (ASTContext*){3}", + "[ClangASTImporter] DeportType called on ({0}Type*){1:x} " + "from (ASTContext*){2:x} to (ASTContext*){3:x}", src_type.GetTypeName(), src_type.GetOpaqueQualType(), &src_ctxt->getASTContext(), &dst.getASTContext()); @@ -334,8 +334,8 @@ clang::Decl *ClangASTImporter::DeportDecl(clang::ASTContext *dst_ctx, clang::ASTContext *src_ctx = &decl->getASTContext(); LLDB_LOG(log, - "[ClangASTImporter] DeportDecl called on ({0}Decl*){1} from " - "(ASTContext*){2} to (ASTContext*){3}", + "[ClangASTImporter] DeportDecl called on ({0}Decl*){1:x} from " + "(ASTContext*){2:x} to (ASTContext*){3:x}", decl->getDeclKindName(), decl, src_ctx, dst_ctx); DeclContextOverride decl_context_override; @@ -352,8 +352,8 @@ clang::Decl *ClangASTImporter::DeportDecl(clang::ASTContext *dst_ctx, return nullptr; LLDB_LOG(log, - "[ClangASTImporter] DeportDecl deported ({0}Decl*){1} to " - "({2}Decl*){3}", + "[ClangASTImporter] DeportDecl deported ({0}Decl*){1:x} to " + "({2}Decl*){3:x}", decl->getDeclKindName(), decl, result->getDeclKindName(), result); return result; @@ -637,8 +637,8 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( clang::ASTContext &dest_ctx = record->getASTContext(); LLDB_LOG(log, - "LayoutRecordType on (ASTContext*){0} '{1}' for (RecordDecl*)" - "{2} [name = '{3}']", + "LayoutRecordType on (ASTContext*){0:x} '{1}' for (RecordDecl*)" + "{2:x} [name = '{3}']", &dest_ctx, TypeSystemClang::GetASTContext(&dest_ctx)->getDisplayName(), record, record->getName()); @@ -703,7 +703,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( if (log) { LLDB_LOG(log, "LRT returned:"); -LLDB_LOG(log, "LRT Original = (RecordDecl*){0}", +LLDB_LOG(log, "LRT Original = (RecordDecl*){0:x}", static_cast(origin_record.decl)); LLDB_LOG(log, "LRT Size = {0}", size); LLDB_LOG(log, "LRT Alignment = {0}", alignment); @@ -712,7 +712,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( fe = record->field_end(); fi != fe; ++fi) { LLDB_LOG(log, - "LRT (FieldDecl*){0}, Name = '{1}', Type = '{2}', Offset = " + "LRT (FieldDecl*){0:x}, Name = '{1}', Type = '{2}', Offset = " "{3} bits", *fi, fi->getName(), fi->getType().getAsString(), field_offsets[*fi]); @@ -734,7 +734,7 @@ bool ClangASTImporter::importRecordLayoutFromOrigin( DynCast(base_record); LLDB_LOG(log, - "LRT {0}(CXXRecordDecl*){1}, Name = '{2}', Offset = " + "LRT {0}(CXXRecordDecl*){1:x}, Name = '{2}', Offset = " "{3} chars", (is_virtual ? "Virtual " : ""), base_cxx_record.decl, base_cxx_record.decl->getName(), @@ -1025,7 +1025,7 @@ void ClangASTImporter::ForgetDestination(clang::ASTContext *dst_ast) { Log *log = GetLog(LLDBLog::Expressions); LLDB_LOG(log, - "[ClangASTImporter] Forgetting destination (ASTContext*){0}", + "[ClangASTImporter] Forgetting destination (ASTContext*){0:x}", dst_ast); m_metadata_map.erase(dst_ast); @@ -1039,7 +1039,7 @@ void ClangASTImporter::ForgetSource(clang::ASTContext *dst_ast, LLDB_LOG(log, "[ClangASTImporter] Forgetting source->dest " - "(ASTContext*){0}->(ASTContext*){1}", + "(ASTContext*){0:x}->(ASTContext*){1:x}",
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -147,96 +148,111 @@ class ProcessInstanceInfo : public ProcessInfo { ProcessInstanceInfo() = default; ProcessInstanceInfo(const char *name, const ArchSpec &arch, lldb::pid_t pid) - : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX), -m_parent_pid(LLDB_INVALID_PROCESS_ID) {} + : ProcessInfo(name, arch, pid) {} void Clear() { ProcessInfo::Clear(); -m_euid = UINT32_MAX; -m_egid = UINT32_MAX; -m_parent_pid = LLDB_INVALID_PROCESS_ID; +m_euid = std::nullopt; +m_egid = std::nullopt; +m_parent_pid = std::nullopt; } - uint32_t GetEffectiveUserID() const { return m_euid; } + uint32_t GetEffectiveUserID() const { return m_euid.value(); } feg208 wrote: I don't have particularly strong feelings here but we should keep it consistent between this class and the parent class which has GetUID and GetGID neither of which returns a std::optional. Given the risk of std::bad_optional_access getting thrown thoughout I am coming to the notion that maybe we shouldn't use optionals here? If we have optionals I guess we are mandating that the paired IsValid must exist and must always be called. That seems like it'll create challenges. Can we go back to using invalid values like UINT32_MAX and maybe alter the interface to use optionals in a subsequent pr? This way we could debate the merits of this approach independent of adding a niceness field. https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
@@ -731,8 +747,11 @@ class Debugger : public std::enable_shared_from_this, lldb::TargetSP m_dummy_target_sp; Diagnostics::CallbackID m_diagnostics_callback_id; - lldb_private::DebuggerDestroyCallback m_destroy_callback = nullptr; - void *m_destroy_callback_baton = nullptr; + std::recursive_mutex m_destroy_callback_mutex; royitaqi wrote: Recursive is necessary. This is because HandleDestroyCallbacks will invoke callbacks, which can call Add/RemoveDestoryCallback. All three functions use the same mutex, so it has to be recursive. See more: https://github.com/llvm/llvm-project/pull/89868#discussion_r1588099079 https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
https://github.com/royitaqi edited https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/91944 >From de7135a8a4a40b5aa5ac1f44e58d62874c96448b Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 14:45:33 +0400 Subject: [PATCH 1/4] [lldb] Add lldbutil.target_install() helper It can be used in tests #91918, #91931 and such. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 16 1 file changed, 16 insertions(+) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 58eb37fd742d7..e67b68b727b80 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,6 +1654,22 @@ def find_library_callable(test): ) +def target_install(test, filename): +path = test.getBuildArtifact(filename) +if lldb.remote_platform: +remote_path = lldbutil.append_to_process_working_directory(test, filename) +err = lldb.remote_platform.Install( +lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) +) +if err.Fail(): +raise Exception( +"remote_platform.Install('%s', '%s') failed: %s" +% (path, remote_path, err) +) +path = remote_path +return path + + def read_file_on_target(test, remote): if lldb.remote_platform: local = test.getBuildArtifact("file_from_target") >From b4f5798258b29996b6805037bed70a819a671618 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 15:51:36 +0400 Subject: [PATCH 2/4] Added the full local path as optional parameter. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index e67b68b727b80..4290fe94590a1 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,10 +1654,14 @@ def find_library_callable(test): ) -def target_install(test, filename): -path = test.getBuildArtifact(filename) +def target_install(test, filename=None, path=None): +test.assertTrue(filename or path, "filename or path must be specified.") +if filename is None: +filename = os.path.basename(path) +if path is None: +path = test.getBuildArtifact(filename) if lldb.remote_platform: -remote_path = lldbutil.append_to_process_working_directory(test, filename) +remote_path = append_to_process_working_directory(test, filename) err = lldb.remote_platform.Install( lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) ) >From 79831a4c0e276caf4952b4d5748d5d7ba9e59365 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 20:02:41 +0400 Subject: [PATCH 3/4] Renamed to `install_to_target`. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index 4290fe94590a1..e1aa8d63b9b72 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,7 +1654,7 @@ def find_library_callable(test): ) -def target_install(test, filename=None, path=None): +def install_to_target(test, filename=None, path=None): test.assertTrue(filename or path, "filename or path must be specified.") if filename is None: filename = os.path.basename(path) >From eb1d82b19891b0369aa80cc6202b4c7e5c2e66c3 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Mon, 13 May 2024 20:23:57 +0400 Subject: [PATCH 4/4] Simplified. --- lldb/packages/Python/lldbsuite/test/lldbutil.py | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py index e1aa8d63b9b72..1ec036f885e7e 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py @@ -1654,13 +1654,9 @@ def find_library_callable(test): ) -def install_to_target(test, filename=None, path=None): -test.assertTrue(filename or path, "filename or path must be specified.") -if filename is None: -filename = os.path.basename(path) -if path is None: -path = test.getBuildArtifact(filename) +def install_to_target(test, path): if lldb.remote_platform: +filename = os.path.basename(path) remote_path = append_to_process_working_directory(test, filename) err = lldb.remote_platform.Install( lldb.SBFileSpec(path, True), lldb.SBFileSpec(remote_path, False) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma
[Lldb-commits] [lldb] [lldb] Add lldbutil.target_install() helper (PR #91944)
@@ -1654,6 +1654,26 @@ def find_library_callable(test): ) +def target_install(test, filename=None, path=None): +test.assertTrue(filename or path, "filename or path must be specified.") +if filename is None: +filename = os.path.basename(path) +if path is None: +path = test.getBuildArtifact(filename) slydiman wrote: Ok. I kept only the full path parameter. Updated. Thanks. https://github.com/llvm/llvm-project/pull/91944 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
hawkinsw wrote: > > Sorry for the nits! I am not smart enough to contribute on the technical > > merits, so I am trying to find some way to help! > > Ehhh grammar is not where I shine. I rolled them up Not at all! I thought the comments were really excellent and helpful! I just wanted to help if I could! https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
@@ -147,96 +148,111 @@ class ProcessInstanceInfo : public ProcessInfo { ProcessInstanceInfo() = default; ProcessInstanceInfo(const char *name, const ArchSpec &arch, lldb::pid_t pid) - : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX), -m_parent_pid(LLDB_INVALID_PROCESS_ID) {} + : ProcessInfo(name, arch, pid) {} void Clear() { ProcessInfo::Clear(); -m_euid = UINT32_MAX; -m_egid = UINT32_MAX; -m_parent_pid = LLDB_INVALID_PROCESS_ID; +m_euid = std::nullopt; +m_egid = std::nullopt; +m_parent_pid = std::nullopt; } - uint32_t GetEffectiveUserID() const { return m_euid; } + uint32_t GetEffectiveUserID() const { return m_euid.value(); } feg208 wrote: Maybe a better approach is to keep the optionals in the members that are particular to ProcessInstanceInfo (i.e. the time fields, priority, and the zombie fields) and revert the pid fields and the pid and uid/gid fields to the previous and punt that to a later pr. How does that sound? https://github.com/llvm/llvm-project/pull/91544 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [openmp] [polly] fix(python): fix comparison to True/False (PR #91858)
https://github.com/bulbazord approved this pull request. LGTM The PR summary is empty, but it looks like the commit message has an explanation for this change, so that's fine. https://github.com/llvm/llvm-project/pull/91858 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Adds additional fields to ProcessInfo (PR #91544)
https://github.com/feg208 updated https://github.com/llvm/llvm-project/pull/91544 >From dbb4b4126754be15ffd9d12d9997e4969f7ba5cf Mon Sep 17 00:00:00 2001 From: Fred Grim Date: Wed, 8 May 2024 15:36:16 -0700 Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo To implement SaveCore for elf binaries we need to populate some additional fields in the prpsinfo struct. Those fields are the nice value of the process whose core is to be taken as well as a boolean flag indicating whether or not that process is a zombie. This commit adds those as well as tests to ensure that the values are consistent with expectations --- lldb/include/lldb/Utility/ProcessInfo.h | 94 +++-- lldb/source/Host/linux/Host.cpp | 32 ++--- lldb/source/Utility/ProcessInfo.cpp | 11 ++- lldb/unittests/Host/linux/HostTest.cpp | 21 ++ 4 files changed, 109 insertions(+), 49 deletions(-) diff --git a/lldb/include/lldb/Utility/ProcessInfo.h b/lldb/include/lldb/Utility/ProcessInfo.h index 54ac000dc7fc2..995873a6869e0 100644 --- a/lldb/include/lldb/Utility/ProcessInfo.h +++ b/lldb/include/lldb/Utility/ProcessInfo.h @@ -15,6 +15,7 @@ #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/NameMatches.h" #include "lldb/Utility/StructuredData.h" +#include #include namespace lldb_private { @@ -147,72 +148,71 @@ class ProcessInstanceInfo : public ProcessInfo { ProcessInstanceInfo() = default; ProcessInstanceInfo(const char *name, const ArchSpec &arch, lldb::pid_t pid) - : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX), -m_parent_pid(LLDB_INVALID_PROCESS_ID) {} + : ProcessInfo(name, arch, pid) {} void Clear() { ProcessInfo::Clear(); -m_euid = UINT32_MAX; -m_egid = UINT32_MAX; -m_parent_pid = LLDB_INVALID_PROCESS_ID; +m_euid = std::nullopt; +m_egid = std::nullopt; +m_parent_pid = std::nullopt; } - uint32_t GetEffectiveUserID() const { return m_euid; } + uint32_t GetEffectiveUserID() const { return m_euid.value(); } - uint32_t GetEffectiveGroupID() const { return m_egid; } + uint32_t GetEffectiveGroupID() const { return m_egid.value(); } - bool EffectiveUserIDIsValid() const { return m_euid != UINT32_MAX; } + bool EffectiveUserIDIsValid() const { return m_euid.has_value(); } - bool EffectiveGroupIDIsValid() const { return m_egid != UINT32_MAX; } + bool EffectiveGroupIDIsValid() const { return m_egid.has_value(); } void SetEffectiveUserID(uint32_t uid) { m_euid = uid; } void SetEffectiveGroupID(uint32_t gid) { m_egid = gid; } - lldb::pid_t GetParentProcessID() const { return m_parent_pid; } + lldb::pid_t GetParentProcessID() const { return m_parent_pid.value(); } void SetParentProcessID(lldb::pid_t pid) { m_parent_pid = pid; } - bool ParentProcessIDIsValid() const { -return m_parent_pid != LLDB_INVALID_PROCESS_ID; - } + bool ParentProcessIDIsValid() const { return m_parent_pid.has_value(); } - lldb::pid_t GetProcessGroupID() const { return m_process_group_id; } + lldb::pid_t GetProcessGroupID() const { return m_process_group_id.value(); } void SetProcessGroupID(lldb::pid_t pgrp) { m_process_group_id = pgrp; } - bool ProcessGroupIDIsValid() const { -return m_process_group_id != LLDB_INVALID_PROCESS_ID; - } + bool ProcessGroupIDIsValid() const { return m_process_group_id.has_value(); } - lldb::pid_t GetProcessSessionID() const { return m_process_session_id; } + lldb::pid_t GetProcessSessionID() const { +return m_process_session_id.value(); + } void SetProcessSessionID(lldb::pid_t session) { m_process_session_id = session; } bool ProcessSessionIDIsValid() const { -return m_process_session_id != LLDB_INVALID_PROCESS_ID; +return m_process_session_id.has_value(); } - struct timespec GetUserTime() const { return m_user_time; } + struct timespec GetUserTime() const { return m_user_time.value(); } void SetUserTime(struct timespec utime) { m_user_time = utime; } bool UserTimeIsValid() const { -return m_user_time.tv_sec > 0 || m_user_time.tv_usec > 0; +return m_user_time.has_value() && + (m_user_time->tv_sec > 0 || m_user_time->tv_usec > 0); } - struct timespec GetSystemTime() const { return m_system_time; } + struct timespec GetSystemTime() const { return m_system_time.value(); } void SetSystemTime(struct timespec stime) { m_system_time = stime; } bool SystemTimeIsValid() const { -return m_system_time.tv_sec > 0 || m_system_time.tv_usec > 0; +return m_system_time.has_value() && + (m_system_time->tv_sec > 0 || m_system_time->tv_usec > 0); } struct timespec GetCumulativeUserTime() const { -return m_cumulative_user_time; +return m_cumulative_user_time.value(); } void SetCumulativeUserTime(struct timespec cutime) { @@ -220,12 +220,13 @@ class ProcessInstanceInfo : public ProcessInfo { } bool CumulativeUserTimeIsValid() cons
[Lldb-commits] [lldb] [lldb][ExpressionParser][NFCI] Log pointers as hex (PR #91989)
https://github.com/bulbazord approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/91989 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem][NFCI] Log creation of new TypeSystem instances to expression log (PR #91985)
https://github.com/bulbazord approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/91985 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
https://github.com/Jlalond created https://github.com/llvm/llvm-project/pull/92002 Currently in Core dumps, the entire pthread is copied, including the unused space beyond the stack pointer. This causes large amounts of core dump inflation when the number of threads is high, but the stack usage is low. Such as when an application is using a thread pool. This change will optimize for these situations in addition to generally improving the core dump performance for all of lldb. >From 2d192f640b332c2f1381cf96b75be60ad18de3ac Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Fri, 10 May 2024 09:35:11 -0700 Subject: [PATCH 1/3] change core dump stacks to only include up to the stack pointer (+ red zone) Add python tests to verify we can read up to sp + redzone - 1. --- lldb/source/Target/Process.cpp | 14 +++--- .../TestProcessSaveCoreMinidump.py | 12 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 25afade9a8275..a11e45909202f 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -3857,8 +3857,8 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) { // case we should tell it to stop doing that. Normally, we don't NEED // to do that because we will next close the communication to the stub // and that will get it to shut down. But there are remote debugging -// cases where relying on that side-effect causes the shutdown to be -// flakey, so we should send a positive signal to interrupt the wait. +// cases where relying on that side-effect causes the shutdown to be +// flakey, so we should send a positive signal to interrupt the wait. Status error = HaltPrivate(); BroadcastEvent(eBroadcastBitInterrupt, nullptr); } else if (StateIsRunningState(m_last_broadcast_state)) { @@ -6410,12 +6410,20 @@ GetCoreFileSaveRangesStackOnly(Process &process, if (!reg_ctx_sp) continue; const addr_t sp = reg_ctx_sp->GetSP(); +const size_t red_zone = process.GetABI()->GetRedZoneSize(); lldb_private::MemoryRegionInfo sp_region; if (process.GetMemoryRegionInfo(sp, sp_region).Success()) { // Only add this region if not already added above. If our stack pointer // is pointing off in the weeds, we will want this range. - if (stack_bases.count(sp_region.GetRange().GetRangeBase()) == 0) + if (stack_bases.count(sp_region.GetRange().GetRangeBase()) == 0) { +// Take only the start of the stack to the stack pointer and include the redzone. +// Because stacks grow 'down' to include the red_zone we have to subtract it from the sp. +const size_t stack_head = (sp - red_zone); +const size_t stack_size = sp_region.GetRange().GetRangeEnd() - (stack_head); +sp_region.GetRange().SetRangeBase(stack_head); +sp_region.GetRange().SetByteSize(stack_size); AddRegion(sp_region, try_dirty_pages, ranges); + } } } } diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py index 9fe5e89142987..123bbd472be05 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -14,6 +14,7 @@ class ProcessSaveCoreMinidumpTestCase(TestBase): def verify_core_file( self, core_path, expected_pid, expected_modules, expected_threads ): +breakpoint() # To verify, we'll launch with the mini dump target = self.dbg.CreateTarget(None) process = target.LoadCore(core_path) @@ -36,11 +37,22 @@ def verify_core_file( self.assertEqual(module_file_name, expected_file_name) self.assertEqual(module.GetUUIDString(), expected.GetUUIDString()) +red_zone = process.GetTarget().GetStackRedZoneSize() for thread_idx in range(process.GetNumThreads()): thread = process.GetThreadAtIndex(thread_idx) self.assertTrue(thread.IsValid()) thread_id = thread.GetThreadID() self.assertIn(thread_id, expected_threads) +oldest_frame = thread.GetFrameAtIndex(thread.GetNumFrames() - 1) +stack_start = oldest_frame.GetSymbol().GetStartAddress().GetFileAddress() +frame = thread.GetFrameAtIndex(0) +sp = frame.GetSP() +stack_size = stack_start - (sp - red_zone) +byte_array = process.ReadMemory(sp - red_zone + 1, stack_size, error) +self.assertTrue(error.Success(), "Failed to read stack") +self.assertEqual(stack_size - 1, len(byte_array), "Incorrect stack size read"), + + self.db
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
Jlalond wrote: @clayborg Could you take a look at this? https://github.com/llvm/llvm-project/pull/92002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/92002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) Changes Currently in Core dumps, the entire pthread is copied, including the unused space beyond the stack pointer. This causes large amounts of core dump inflation when the number of threads is high, but the stack usage is low. Such as when an application is using a thread pool. This change will optimize for these situations in addition to generally improving the core dump performance for all of lldb. --- Full diff: https://github.com/llvm/llvm-project/pull/92002.diff 2 Files Affected: - (modified) lldb/source/Target/Process.cpp (+11-3) - (modified) lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py (+29-8) ``diff diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 25afade9a8275..a11e45909202f 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -3857,8 +3857,8 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) { // case we should tell it to stop doing that. Normally, we don't NEED // to do that because we will next close the communication to the stub // and that will get it to shut down. But there are remote debugging -// cases where relying on that side-effect causes the shutdown to be -// flakey, so we should send a positive signal to interrupt the wait. +// cases where relying on that side-effect causes the shutdown to be +// flakey, so we should send a positive signal to interrupt the wait. Status error = HaltPrivate(); BroadcastEvent(eBroadcastBitInterrupt, nullptr); } else if (StateIsRunningState(m_last_broadcast_state)) { @@ -6410,12 +6410,20 @@ GetCoreFileSaveRangesStackOnly(Process &process, if (!reg_ctx_sp) continue; const addr_t sp = reg_ctx_sp->GetSP(); +const size_t red_zone = process.GetABI()->GetRedZoneSize(); lldb_private::MemoryRegionInfo sp_region; if (process.GetMemoryRegionInfo(sp, sp_region).Success()) { // Only add this region if not already added above. If our stack pointer // is pointing off in the weeds, we will want this range. - if (stack_bases.count(sp_region.GetRange().GetRangeBase()) == 0) + if (stack_bases.count(sp_region.GetRange().GetRangeBase()) == 0) { +// Take only the start of the stack to the stack pointer and include the redzone. +// Because stacks grow 'down' to include the red_zone we have to subtract it from the sp. +const size_t stack_head = (sp - red_zone); +const size_t stack_size = sp_region.GetRange().GetRangeEnd() - (stack_head); +sp_region.GetRange().SetRangeBase(stack_head); +sp_region.GetRange().SetByteSize(stack_size); AddRegion(sp_region, try_dirty_pages, ranges); + } } } } diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py index 9fe5e89142987..56f75ec7e9708 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -9,10 +9,9 @@ from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil - class ProcessSaveCoreMinidumpTestCase(TestBase): def verify_core_file( -self, core_path, expected_pid, expected_modules, expected_threads +self, core_path, expected_pid, expected_modules, expected_threads, stacks_to_sps_map ): # To verify, we'll launch with the mini dump target = self.dbg.CreateTarget(None) @@ -36,17 +35,37 @@ def verify_core_file( self.assertEqual(module_file_name, expected_file_name) self.assertEqual(module.GetUUIDString(), expected.GetUUIDString()) +red_zone = process.GetTarget().GetStackRedZoneSize() for thread_idx in range(process.GetNumThreads()): thread = process.GetThreadAtIndex(thread_idx) self.assertTrue(thread.IsValid()) thread_id = thread.GetThreadID() self.assertIn(thread_id, expected_threads) +frame = thread.GetFrameAtIndex(0) +sp_region = lldb.SBMemoryRegionInfo() +sp = frame.GetSP() +err = process.GetMemoryRegionInfo(sp, sp_region) +self.assertTrue(err.Success(), err.GetCString()) +error = lldb.SBError() +# Try to read at the end of the stack red zone and succeed +process.ReadMemory(sp_region.GetRegionEnd() - 1, 1, error) +self.assertTrue(error.Success(), error.GetCString()) +# Try to read just past the red zone and fail +process.ReadMemory(sp_region.GetRegionEnd() + 1, 1, error) +# Try to read from the base of the s
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
@@ -6410,12 +6410,20 @@ GetCoreFileSaveRangesStackOnly(Process &process, if (!reg_ctx_sp) continue; const addr_t sp = reg_ctx_sp->GetSP(); +const size_t red_zone = process.GetABI()->GetRedZoneSize(); bulbazord wrote: Asking for my understanding: This will only really do anything if the size of the red zone is greater than 0 right? Otherwise, this change is a no-op. Is that right? https://github.com/llvm/llvm-project/pull/92002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: > It's not completely correct -- the frame for `signal_generating_add` is > missing. That's what the error message is complaining about. Ah, thanks, I missed that! Let me debug it and comment further https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
@@ -6410,12 +6410,20 @@ GetCoreFileSaveRangesStackOnly(Process &process, if (!reg_ctx_sp) continue; const addr_t sp = reg_ctx_sp->GetSP(); +const size_t red_zone = process.GetABI()->GetRedZoneSize(); Jlalond wrote: Correct, but on systems where it would be greater than 0, we ensure we capture it for the memory dump as well. https://github.com/llvm/llvm-project/pull/92002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
https://github.com/jeffreytan81 requested changes to this pull request. https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
@@ -1689,35 +1689,56 @@ void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback, void SBDebugger::SetDestroyCallback( lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) { LLDB_INSTRUMENT_VA(this, destroy_callback, baton); + if (m_opaque_sp) { -return m_opaque_sp->SetDestroyCallback( -destroy_callback, baton); +m_opaque_sp->SetDestroyCallback(destroy_callback, baton); } } +lldb::destroy_callback_token_t +SBDebugger::AddDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback, + void *baton) { + LLDB_INSTRUMENT_VA(this, destroy_callback, baton); + + if (m_opaque_sp) { +return m_opaque_sp->AddDestroyCallback(destroy_callback, baton); + } + return LLDB_INVALID_DESTROY_CALLBACK_TOKEN; +} + +bool SBDebugger::RemoveDestroyCallback(lldb::destroy_callback_token_t token) { + LLDB_INSTRUMENT_VA(this, token); + + if (m_opaque_sp) { +return m_opaque_sp->RemoveDestroyCallback(token); + } + return false; +} + SBTrace SBDebugger::LoadTraceFromFile(SBError &error, const SBFileSpec &trace_description_file) { LLDB_INSTRUMENT_VA(this, error, trace_description_file); + return SBTrace::LoadTraceFromFile(error, *this, trace_description_file); } void SBDebugger::RequestInterrupt() { LLDB_INSTRUMENT_VA(this); - + if (m_opaque_sp) -m_opaque_sp->RequestInterrupt(); +m_opaque_sp->RequestInterrupt(); } void SBDebugger::CancelInterruptRequest() { LLDB_INSTRUMENT_VA(this); - + if (m_opaque_sp) -m_opaque_sp->CancelInterruptRequest(); +m_opaque_sp->CancelInterruptRequest(); } bool SBDebugger::InterruptRequested() { LLDB_INSTRUMENT_VA(this); - + if (m_opaque_sp) jeffreytan81 wrote: Undo all these unnecessary changes. https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] SBDebugger: Add new APIs `AddDestroyCallback` and `RemoveDestroyCallback` (PR #89868)
@@ -743,9 +743,28 @@ DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback, } void Debugger::HandleDestroyCallback() { - if (m_destroy_callback) { -m_destroy_callback(GetID(), m_destroy_callback_baton); -m_destroy_callback = nullptr; + std::lock_guard guard(m_destroy_callback_mutex); + const lldb::user_id_t user_id = GetID(); + // This loop handles the case where callbacks are added/removed by existing + // callbacks during the loop, as the following: + // - Added callbacks will always be invoked. + // - Removed callbacks will never be invoked. That is *unless* the loop + // happens to invoke the said callbacks first, before they get removed. + // In this case, the callbacks gets invoked, and the removal return false. + // + // In the removal case, because the order of the container is random, it's + // wise to not depend on the order and instead implement logic inside the + // callbacks to decide if their work should be skipped. + while (m_destroy_callback_and_baton.size()) { +auto iter = m_destroy_callback_and_baton.begin(); +const lldb::destroy_callback_token_t &token = iter->first; +const lldb_private::DebuggerDestroyCallback &callback = iter->second.first; +void *const &baton = iter->second.second; jeffreytan81 wrote: I do not think you need to take reference to them. Copy by value is good to avoid indirection. https://github.com/llvm/llvm-project/pull/89868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits