[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/96260 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH 1/2] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- lldb/include/lldb/Target/Thread.h | 29 ++ .../Process/Utility/StopInfoMachException.cpp | 296 +++--- .../Process/Windows/Common/ProcessWindows.cpp | 16 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 118 +++ .../Process/scripted/ScriptedThread.cpp | 9 + lldb/source/Target/Thread.cpp | 17 +- .../TestConsecutiveBreakpoints.py | 26 +- .../TestStepOverBreakpoint.py | 6 +- 8 files changed, 235 insertions(+), 282 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index c17bddf4d98b8..1e1aead896018 100644 --- a/lldb/include/lldb/Target/Thread.h +++
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Unify Platform::ResolveExecutable (PR #96256)
https://github.com/jasonmolenda approved this pull request. LGTM, nice cleanup. https://github.com/llvm/llvm-project/pull/96256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Revert "[lldb/crashlog] Make interactive mode the new default" (PR #96263)
https://github.com/medismailben closed https://github.com/llvm/llvm-project/pull/96263 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1373f7c - Revert "[lldb/crashlog] Make interactive mode the new default" (#96263)
Author: Med Ismail Bennani
Date: 2024-06-20T18:23:50-07:00
New Revision: 1373f7c714824f5957aa5fabf8370286f86e6b14
URL:
https://github.com/llvm/llvm-project/commit/1373f7c714824f5957aa5fabf8370286f86e6b14
DIFF:
https://github.com/llvm/llvm-project/commit/1373f7c714824f5957aa5fabf8370286f86e6b14.diff
LOG: Revert "[lldb/crashlog] Make interactive mode the new default" (#96263)
Reverts llvm/llvm-project#94575 since introduces test failure:
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/6166/
Added:
Modified:
lldb/examples/python/crashlog.py
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
Removed:
diff --git a/lldb/examples/python/crashlog.py
b/lldb/examples/python/crashlog.py
index d3952e377c657..1c0d717ce455c 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -31,7 +31,6 @@
import concurrent.futures
import contextlib
import datetime
-import enum
import json
import os
import platform
@@ -46,6 +45,7 @@
import time
import uuid
+
print_lock = threading.RLock()
try:
@@ -1582,12 +1582,9 @@ def synchronous(debugger):
debugger.RunCommandInterpreter(True, False, run_options, 0,
False, True)
-class CrashLogLoadingMode(str, enum.Enum):
-batch = "batch"
-interactive = "interactive"
-
-
-def CreateSymbolicateCrashLogOptions(command_name, description):
+def CreateSymbolicateCrashLogOptions(
+command_name, description, add_interactive_options
+):
usage = "crashlog [options] [FILE ...]"
arg_parser = argparse.ArgumentParser(
description=description,
@@ -1603,12 +1600,6 @@ def CreateSymbolicateCrashLogOptions(command_name,
description):
help="crash report(s) to symbolicate",
)
-arg_parser.add_argument(
-"-m",
-"--mode",
-choices=[mode.value for mode in CrashLogLoadingMode],
-help="change how the symbolicated process and threads are displayed to
the user (default: interactive)",
-)
arg_parser.add_argument(
"--version",
"-V",
@@ -1745,35 +1736,36 @@ def CreateSymbolicateCrashLogOptions(command_name,
description):
help=argparse.SUPPRESS,
default=False,
)
-arg_parser.add_argument(
-"--target",
-"-t",
-dest="target_path",
-help="the target binary path that should be used for interactive
crashlog (optional)",
-default=None,
-)
-arg_parser.add_argument(
-"--skip-status",
-"-s",
-dest="skip_status",
-action="store_true",
-help="prevent the interactive crashlog to dump the process status and
thread backtrace at launch",
-default=False,
-)
-legacy_group = arg_parser.add_mutually_exclusive_group()
-legacy_group.add_argument(
-"-i",
-"--interactive",
-action="store_true",
-help=argparse.SUPPRESS,
-)
-legacy_group.add_argument(
-"-b",
-"--batch",
-action="store_true",
-help=argparse.SUPPRESS,
-)
-
+if add_interactive_options:
+arg_parser.add_argument(
+"-i",
+"--interactive",
+action="store_true",
+help="parse a crash log and load it in a ScriptedProcess",
+default=False,
+)
+arg_parser.add_argument(
+"-b",
+"--batch",
+action="store_true",
+help="dump symbolicated stackframes without creating a debug
session",
+default=True,
+)
+arg_parser.add_argument(
+"--target",
+"-t",
+dest="target_path",
+help="the target binary path that should be used for interactive
crashlog (optional)",
+default=None,
+)
+arg_parser.add_argument(
+"--skip-status",
+"-s",
+dest="skip_status",
+action="store_true",
+help="prevent the interactive crashlog to dump the process status
and thread backtrace at launch",
+default=False,
+)
return arg_parser
@@ -1786,7 +1778,7 @@ def CrashLogOptionParser():
created that has all of the shared libraries loaded at the load addresses
found in the crash log file. This allows
you to explore the program as if it were stopped at the locations described in
the crash log and functions can
be disassembled and lookups can be performed using the addresses found in the
crash log."""
-return CreateSymbolicateCrashLogOptions("crashlog", description)
+
[Lldb-commits] [lldb] Revert "[lldb/crashlog] Make interactive mode the new default" (PR #96263)
https://github.com/medismailben updated
https://github.com/llvm/llvm-project/pull/96263
>From b9af881866b3702be5d5bf55f694d4eb051e2872 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Thu, 20 Jun 2024 18:19:26 -0700
Subject: [PATCH] Revert "[lldb/crashlog] Make interactive mode the new default
(#94575)"
This reverts commit aafa0ef900791857f55629bcf61c37f53cc0d2af.
---
lldb/examples/python/crashlog.py | 124 +++---
.../Python/Crashlog/altered_threadState.test | 2 +-
.../Python/Crashlog/json.test | 6 +-
.../Python/Crashlog/no_threadState.test | 2 +-
.../skipped_status_interactive_crashlog.test | 2 +-
.../Python/Crashlog/text.test | 2 +-
6 files changed, 55 insertions(+), 83 deletions(-)
diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py
index d3952e377c657..1c0d717ce455c 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -31,7 +31,6 @@
import concurrent.futures
import contextlib
import datetime
-import enum
import json
import os
import platform
@@ -46,6 +45,7 @@
import time
import uuid
+
print_lock = threading.RLock()
try:
@@ -1582,12 +1582,9 @@ def synchronous(debugger):
debugger.RunCommandInterpreter(True, False, run_options, 0,
False, True)
-class CrashLogLoadingMode(str, enum.Enum):
-batch = "batch"
-interactive = "interactive"
-
-
-def CreateSymbolicateCrashLogOptions(command_name, description):
+def CreateSymbolicateCrashLogOptions(
+command_name, description, add_interactive_options
+):
usage = "crashlog [options] [FILE ...]"
arg_parser = argparse.ArgumentParser(
description=description,
@@ -1603,12 +1600,6 @@ def CreateSymbolicateCrashLogOptions(command_name,
description):
help="crash report(s) to symbolicate",
)
-arg_parser.add_argument(
-"-m",
-"--mode",
-choices=[mode.value for mode in CrashLogLoadingMode],
-help="change how the symbolicated process and threads are displayed to
the user (default: interactive)",
-)
arg_parser.add_argument(
"--version",
"-V",
@@ -1745,35 +1736,36 @@ def CreateSymbolicateCrashLogOptions(command_name,
description):
help=argparse.SUPPRESS,
default=False,
)
-arg_parser.add_argument(
-"--target",
-"-t",
-dest="target_path",
-help="the target binary path that should be used for interactive
crashlog (optional)",
-default=None,
-)
-arg_parser.add_argument(
-"--skip-status",
-"-s",
-dest="skip_status",
-action="store_true",
-help="prevent the interactive crashlog to dump the process status and
thread backtrace at launch",
-default=False,
-)
-legacy_group = arg_parser.add_mutually_exclusive_group()
-legacy_group.add_argument(
-"-i",
-"--interactive",
-action="store_true",
-help=argparse.SUPPRESS,
-)
-legacy_group.add_argument(
-"-b",
-"--batch",
-action="store_true",
-help=argparse.SUPPRESS,
-)
-
+if add_interactive_options:
+arg_parser.add_argument(
+"-i",
+"--interactive",
+action="store_true",
+help="parse a crash log and load it in a ScriptedProcess",
+default=False,
+)
+arg_parser.add_argument(
+"-b",
+"--batch",
+action="store_true",
+help="dump symbolicated stackframes without creating a debug
session",
+default=True,
+)
+arg_parser.add_argument(
+"--target",
+"-t",
+dest="target_path",
+help="the target binary path that should be used for interactive
crashlog (optional)",
+default=None,
+)
+arg_parser.add_argument(
+"--skip-status",
+"-s",
+dest="skip_status",
+action="store_true",
+help="prevent the interactive crashlog to dump the process status
and thread backtrace at launch",
+default=False,
+)
return arg_parser
@@ -1786,7 +1778,7 @@ def CrashLogOptionParser():
created that has all of the shared libraries loaded at the load addresses
found in the crash log file. This allows
you to explore the program as if it were stopped at the locations described in
the crash log and functions can
be disassembled and lookups can be performed using the addresses found in the
crash log."""
-return CreateSymbolicateCrashLogOptions("crashlog", description)
+return CreateSymbolicateCrashLogOptions("crashlog", description, True)
def SymbolicateCrashLogs(debugger, command_args, result, is_command):
@@ -1802,35 +1794,8 @@ def SymbolicateCrashLogs(debugger, command_args, result,
is_command):
result.SetError(str(e))
[Lldb-commits] [lldb] Revert "[lldb/crashlog] Make interactive mode the new default" (PR #96263)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Med Ismail Bennani (medismailben)
Changes
Reverts llvm/llvm-project#94575 since introduces test failure:
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/6166/
---
Full diff: https://github.com/llvm/llvm-project/pull/96263.diff
6 Files Affected:
- (modified) lldb/examples/python/crashlog.py (+48-76)
- (modified)
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test
(+1-1)
- (modified) lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test (+3-3)
- (modified)
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test (+1-1)
- (modified)
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
(+1-1)
- (modified) lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test (+1-1)
``diff
diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py
index d3952e377c657..1c0d717ce455c 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -31,7 +31,6 @@
import concurrent.futures
import contextlib
import datetime
-import enum
import json
import os
import platform
@@ -46,6 +45,7 @@
import time
import uuid
+
print_lock = threading.RLock()
try:
@@ -1582,12 +1582,9 @@ def synchronous(debugger):
debugger.RunCommandInterpreter(True, False, run_options, 0,
False, True)
-class CrashLogLoadingMode(str, enum.Enum):
-batch = "batch"
-interactive = "interactive"
-
-
-def CreateSymbolicateCrashLogOptions(command_name, description):
+def CreateSymbolicateCrashLogOptions(
+command_name, description, add_interactive_options
+):
usage = "crashlog [options] [FILE ...]"
arg_parser = argparse.ArgumentParser(
description=description,
@@ -1603,12 +1600,6 @@ def CreateSymbolicateCrashLogOptions(command_name,
description):
help="crash report(s) to symbolicate",
)
-arg_parser.add_argument(
-"-m",
-"--mode",
-choices=[mode.value for mode in CrashLogLoadingMode],
-help="change how the symbolicated process and threads are displayed to
the user (default: interactive)",
-)
arg_parser.add_argument(
"--version",
"-V",
@@ -1745,35 +1736,36 @@ def CreateSymbolicateCrashLogOptions(command_name,
description):
help=argparse.SUPPRESS,
default=False,
)
-arg_parser.add_argument(
-"--target",
-"-t",
-dest="target_path",
-help="the target binary path that should be used for interactive
crashlog (optional)",
-default=None,
-)
-arg_parser.add_argument(
-"--skip-status",
-"-s",
-dest="skip_status",
-action="store_true",
-help="prevent the interactive crashlog to dump the process status and
thread backtrace at launch",
-default=False,
-)
-legacy_group = arg_parser.add_mutually_exclusive_group()
-legacy_group.add_argument(
-"-i",
-"--interactive",
-action="store_true",
-help=argparse.SUPPRESS,
-)
-legacy_group.add_argument(
-"-b",
-"--batch",
-action="store_true",
-help=argparse.SUPPRESS,
-)
-
+if add_interactive_options:
+arg_parser.add_argument(
+"-i",
+"--interactive",
+action="store_true",
+help="parse a crash log and load it in a ScriptedProcess",
+default=False,
+)
+arg_parser.add_argument(
+"-b",
+"--batch",
+action="store_true",
+help="dump symbolicated stackframes without creating a debug
session",
+default=True,
+)
+arg_parser.add_argument(
+"--target",
+"-t",
+dest="target_path",
+help="the target binary path that should be used for interactive
crashlog (optional)",
+default=None,
+)
+arg_parser.add_argument(
+"--skip-status",
+"-s",
+dest="skip_status",
+action="store_true",
+help="prevent the interactive crashlog to dump the process status
and thread backtrace at launch",
+default=False,
+)
return arg_parser
@@ -1786,7 +1778,7 @@ def CrashLogOptionParser():
created that has all of the shared libraries loaded at the load addresses
found in the crash log file. This allows
you to explore the program as if it were stopped at the locations described in
the crash log and functions can
be disassembled and lookups can be performed using the addresses found in the
crash log."""
-return CreateSymbolicateCrashLogOptions("crashlog", description)
+return CreateSymbolicateCrashLogOptions("crashlog", description, True)
def SymbolicateCrashLogs(debugger, command_args, result, is_command):
@@ -1802,35 +1794,8 @@ def
[Lldb-commits] [lldb] Revert "[lldb/crashlog] Make interactive mode the new default" (PR #96263)
https://github.com/medismailben created
https://github.com/llvm/llvm-project/pull/96263
Reverts llvm/llvm-project#94575 since introduces test failure:
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/6166/
>From b9af881866b3702be5d5bf55f694d4eb051e2872 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Thu, 20 Jun 2024 18:19:26 -0700
Subject: [PATCH] Revert "[lldb/crashlog] Make interactive mode the new default
(#94575)"
This reverts commit aafa0ef900791857f55629bcf61c37f53cc0d2af.
---
lldb/examples/python/crashlog.py | 124 +++---
.../Python/Crashlog/altered_threadState.test | 2 +-
.../Python/Crashlog/json.test | 6 +-
.../Python/Crashlog/no_threadState.test | 2 +-
.../skipped_status_interactive_crashlog.test | 2 +-
.../Python/Crashlog/text.test | 2 +-
6 files changed, 55 insertions(+), 83 deletions(-)
diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py
index d3952e377c657..1c0d717ce455c 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -31,7 +31,6 @@
import concurrent.futures
import contextlib
import datetime
-import enum
import json
import os
import platform
@@ -46,6 +45,7 @@
import time
import uuid
+
print_lock = threading.RLock()
try:
@@ -1582,12 +1582,9 @@ def synchronous(debugger):
debugger.RunCommandInterpreter(True, False, run_options, 0,
False, True)
-class CrashLogLoadingMode(str, enum.Enum):
-batch = "batch"
-interactive = "interactive"
-
-
-def CreateSymbolicateCrashLogOptions(command_name, description):
+def CreateSymbolicateCrashLogOptions(
+command_name, description, add_interactive_options
+):
usage = "crashlog [options] [FILE ...]"
arg_parser = argparse.ArgumentParser(
description=description,
@@ -1603,12 +1600,6 @@ def CreateSymbolicateCrashLogOptions(command_name,
description):
help="crash report(s) to symbolicate",
)
-arg_parser.add_argument(
-"-m",
-"--mode",
-choices=[mode.value for mode in CrashLogLoadingMode],
-help="change how the symbolicated process and threads are displayed to
the user (default: interactive)",
-)
arg_parser.add_argument(
"--version",
"-V",
@@ -1745,35 +1736,36 @@ def CreateSymbolicateCrashLogOptions(command_name,
description):
help=argparse.SUPPRESS,
default=False,
)
-arg_parser.add_argument(
-"--target",
-"-t",
-dest="target_path",
-help="the target binary path that should be used for interactive
crashlog (optional)",
-default=None,
-)
-arg_parser.add_argument(
-"--skip-status",
-"-s",
-dest="skip_status",
-action="store_true",
-help="prevent the interactive crashlog to dump the process status and
thread backtrace at launch",
-default=False,
-)
-legacy_group = arg_parser.add_mutually_exclusive_group()
-legacy_group.add_argument(
-"-i",
-"--interactive",
-action="store_true",
-help=argparse.SUPPRESS,
-)
-legacy_group.add_argument(
-"-b",
-"--batch",
-action="store_true",
-help=argparse.SUPPRESS,
-)
-
+if add_interactive_options:
+arg_parser.add_argument(
+"-i",
+"--interactive",
+action="store_true",
+help="parse a crash log and load it in a ScriptedProcess",
+default=False,
+)
+arg_parser.add_argument(
+"-b",
+"--batch",
+action="store_true",
+help="dump symbolicated stackframes without creating a debug
session",
+default=True,
+)
+arg_parser.add_argument(
+"--target",
+"-t",
+dest="target_path",
+help="the target binary path that should be used for interactive
crashlog (optional)",
+default=None,
+)
+arg_parser.add_argument(
+"--skip-status",
+"-s",
+dest="skip_status",
+action="store_true",
+help="prevent the interactive crashlog to dump the process status
and thread backtrace at launch",
+default=False,
+)
return arg_parser
@@ -1786,7 +1778,7 @@ def CrashLogOptionParser():
created that has all of the shared libraries loaded at the load addresses
found in the crash log file. This allows
you to explore the program as if it were stopped at the locations described in
the crash log and functions can
be disassembled and lookups can be performed using the addresses found in the
crash log."""
-return CreateSymbolicateCrashLogOptions("crashlog", description)
+return CreateSymbolicateCrashLogOptions("crashlog", description, True)
def SymbolicateCrashLogs(debugger, command_args, result,
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
jasonmolenda wrote: This patch definitely needs review from Jim Ingham, who is out this week. @ZequanWu and @cmtice FYI. I tested this on arm64 macOS and on aarch64 Ubuntu - the important Process changes are were in ProcessGDBRemote, StopInfoMachException, and in ProcessWindows, I believe my testing will have exercised the first two set of changes (the largest and most complex). I updated ProcessWindows as I believe it needs to be updated, but don't have a way of testing it short of merging the PR and seeing what happens to the windows CI bot. @AlexK0 if you have a setup to build and run the testsuite on windows, could you try it with this patch some time when you're able? I can't see this being merged for at least another 5-6 days, there's no rush. You can download the unidiff style diff of the patch from https://patch-diff.githubusercontent.com/raw/llvm/llvm-project/pull/96260.diff https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jason Molenda (jasonmolenda) Changes lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- Patch is 33.48 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/96260.diff 8 Files Affected: - (modified) lldb/include/lldb/Target/Thread.h (+29) - (modified) lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp (+118-178) - (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp (+3-13) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+36-82) - (modified) lldb/source/Plugins/Process/scripted/ScriptedThread.cpp (+9) - (modified) lldb/source/Target/Thread.cpp (+16-1) - (modified) lldb/test/API/functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py (+19-7) - (modified) lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py (+5-1) ``diff diff --git a/lldb/include/lldb/Target/Thread.h
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda created https://github.com/llvm/llvm-project/pull/96260 lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite
[Lldb-commits] [lldb] [API] add GetSyntheticValue (PR #95959)
https://github.com/v-bulle updated
https://github.com/llvm/llvm-project/pull/95959
>From 27a00b54bc991dfb4747e0d37b15878beebaabba Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Wed, 12 Jun 2024 14:23:15 -0700
Subject: [PATCH 1/3] [API] add GetSyntheticValue
---
lldb/include/lldb/API/SBValue.h | 2 ++
lldb/source/API/SBValue.cpp | 12
2 files changed, 14 insertions(+)
diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h
index 65920c76df7a8..bec816fb45184 100644
--- a/lldb/include/lldb/API/SBValue.h
+++ b/lldb/include/lldb/API/SBValue.h
@@ -89,6 +89,8 @@ class LLDB_API SBValue {
lldb::SBValue GetNonSyntheticValue();
+ lldb::SBValue GetSyntheticValue();
+
lldb::DynamicValueType GetPreferDynamicValue();
void SetPreferDynamicValue(lldb::DynamicValueType use_dynamic);
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 9d7efba024d11..6b77c0e95cedd 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -761,6 +761,18 @@ lldb::SBValue SBValue::GetNonSyntheticValue() {
return value_sb;
}
+lldb::SBValue SBValue::GetSyntheticValue() {
+ LLDB_INSTRUMENT_VA(this);
+
+ SBValue value_sb;
+ if (IsValid()) {
+ValueImplSP proxy_sp(new ValueImpl(m_opaque_sp->GetRootSP(),
+ m_opaque_sp->GetUseDynamic(), true));
+value_sb.SetSP(proxy_sp);
+ }
+ return value_sb;
+}
+
lldb::DynamicValueType SBValue::GetPreferDynamicValue() {
LLDB_INSTRUMENT_VA(this);
>From e0ed752849486f67d9fddbef3767a1756afd1ab2 Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Thu, 20 Jun 2024 17:04:15 -0700
Subject: [PATCH 2/3] add test
---
.../formatters/TestFormattersSBAPI.py | 12
lldb/test/API/python_api/formatters/synth.py | 28 +--
2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/lldb/test/API/python_api/formatters/TestFormattersSBAPI.py
b/lldb/test/API/python_api/formatters/TestFormattersSBAPI.py
index 7e802f92da352..460afbc1202cf 100644
--- a/lldb/test/API/python_api/formatters/TestFormattersSBAPI.py
+++ b/lldb/test/API/python_api/formatters/TestFormattersSBAPI.py
@@ -155,6 +155,18 @@ def cleanup():
],
)
+self.dbg.GetCategory("CCCSynth2").SetEnabled(True)
+self.expect(
+"frame variable ccc",
+matching=True,
+substrs=[
+"CCC object with leading synthetic value (int) b = 222",
+"a = 111",
+"b = 222",
+"c = 333",
+],
+)
+
foo_var = (
self.dbg.GetSelectedTarget()
.GetProcess()
diff --git a/lldb/test/API/python_api/formatters/synth.py
b/lldb/test/API/python_api/formatters/synth.py
index 474c18bc62ebd..2b8f7c86ac6f1 100644
--- a/lldb/test/API/python_api/formatters/synth.py
+++ b/lldb/test/API/python_api/formatters/synth.py
@@ -29,11 +29,19 @@ def ccc_summary(sbvalue, internal_dict):
# This tests that the SBValue.GetNonSyntheticValue() actually returns a
# non-synthetic value. If it does not, then
sbvalue.GetChildMemberWithName("a")
# in the following statement will call the 'get_child_index' method of the
-# synthetic child provider CCCSynthProvider below (which raises an
-# exception).
+# synthetic child provider CCCSynthProvider below (which return the "b"
field").
return "CCC object with leading value " +
str(sbvalue.GetChildMemberWithName("a"))
+def ccc_synthetic(sbvalue, internal_dict):
+sbvalue = sbvalue.GetSyntheticValue()
+# This tests that the SBValue.GetNonSyntheticValue() actually returns a
+# synthetic value. If it does, then sbvalue.GetChildMemberWithName("a")
+# in the following statement will call the 'get_child_index' method of the
+# synthetic child provider CCCSynthProvider below (which return the "b"
field").
+return "CCC object with leading synthetic value " +
str(sbvalue.GetChildMemberWithName("a"))
+
+
class CCCSynthProvider(object):
def __init__(self, sbvalue, internal_dict):
self._sbvalue = sbvalue
@@ -42,6 +50,9 @@ def num_children(self):
return 3
def get_child_index(self, name):
+if name == "a":
+# Return b for test.
+return 1
raise RuntimeError("I don't want to be called!")
def get_child_at_index(self, index):
@@ -119,3 +130,16 @@ def __lldb_init_module(debugger, dict):
"synth.empty2_summary", lldb.eTypeOptionHideEmptyAggregates
),
)
+cat2 = debugger.CreateCategory("CCCSynth2")
+cat2.AddTypeSynthetic(
+lldb.SBTypeNameSpecifier("CCC"),
+lldb.SBTypeSynthetic.CreateWithClassName(
+"synth.CCCSynthProvider", lldb.eTypeOptionCascade
+),
+)
+cat2.AddTypeSummary(
+lldb.SBTypeNameSpecifier("CCC"),
+lldb.SBTypeSummary.CreateWithFunctionName(
+
[Lldb-commits] [lldb] [API] add GetSyntheticValue (PR #95959)
github-actions[bot] wrote:
:warning: Python code formatter, darker found issues in your code. :warning:
You can test this locally with the following command:
``bash
darker --check --diff -r
d8091522664248a4ba73d8d1e7fa6ac57bfcf67c...e0ed752849486f67d9fddbef3767a1756afd1ab2
lldb/test/API/python_api/formatters/TestFormattersSBAPI.py
lldb/test/API/python_api/formatters/synth.py
``
View the diff from darker here.
``diff
--- synth.py2024-06-21 00:23:31.00 +
+++ synth.py2024-06-21 00:26:49.436805 +
@@ -37,11 +37,13 @@
sbvalue = sbvalue.GetSyntheticValue()
# This tests that the SBValue.GetNonSyntheticValue() actually returns a
# synthetic value. If it does, then sbvalue.GetChildMemberWithName("a")
# in the following statement will call the 'get_child_index' method of the
# synthetic child provider CCCSynthProvider below (which return the "b"
field").
-return "CCC object with leading synthetic value " +
str(sbvalue.GetChildMemberWithName("a"))
+return "CCC object with leading synthetic value " + str(
+sbvalue.GetChildMemberWithName("a")
+)
class CCCSynthProvider(object):
def __init__(self, sbvalue, internal_dict):
self._sbvalue = sbvalue
``
https://github.com/llvm/llvm-project/pull/95959
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Unify Platform::ResolveExecutable (PR #96256)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/96256
>From 41f7b780548ccfc15ad22da51742f1809c34c103 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Thu, 20 Jun 2024 13:51:53 -0700
Subject: [PATCH] [lldb] Unify Platform::ResolveExecutable duplication
The Platform class currently has two functions to resolve an executable:
`ResolveExecutable` and `ResolveRemoteExecutable`. The former strictly
deals with local files while the latter can handle potentially remote
files. I couldn't figure out why the distinction matters, at the latter
is a super-set of the former.
To make things even more confusion, we had a similar but not identical
implementation in RemoteAwarePlatform where its implementation of
`ResolveExecutable` could handle remote files. To top it all off, we had
copy-pasted implementation, dead code included in
`PlatformAppleSimulator` and `PlatformRemoteDarwinDevice`.
I went ahead and unified all the different implementation on the
original `ResolveRemoteExecutable` implementation. As far as I can tell,
it should work for every other platform, and the test suite (on macOS)
seems to agree with me, except for a small wording change.
---
lldb/include/lldb/Target/Platform.h | 7 +-
.../include/lldb/Target/RemoteAwarePlatform.h | 4 -
.../MacOSX/PlatformAppleSimulator.cpp | 75 --
.../Platform/MacOSX/PlatformAppleSimulator.h | 4 -
.../MacOSX/PlatformRemoteDarwinDevice.cpp | 65
.../MacOSX/PlatformRemoteDarwinDevice.h | 4 -
lldb/source/Target/Platform.cpp | 48 +-
lldb/source/Target/RemoteAwarePlatform.cpp| 139 --
.../TestAssertMessages.py | 2 +-
.../tools/lldb-dap/launch/TestDAP_launch.py | 2 +-
.../Target/RemoteAwarePlatformTest.cpp| 13 +-
11 files changed, 16 insertions(+), 347 deletions(-)
diff --git a/lldb/include/lldb/Target/Platform.h
b/lldb/include/lldb/Target/Platform.h
index e05c79cb501bf..e54e1460acee7 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -139,7 +139,7 @@ class Platform : public PluginInterface {
/// Returns \b true if this Platform plug-in was able to find
/// a suitable executable, \b false otherwise.
virtual Status ResolveExecutable(const ModuleSpec _spec,
- lldb::ModuleSP _sp,
+ lldb::ModuleSP _module_sp,
const FileSpecList
*module_search_paths_ptr);
/// Find a symbol file given a symbol file module specification.
@@ -1004,10 +1004,7 @@ class Platform : public PluginInterface {
virtual const char *GetCacheHostname();
- virtual Status
- ResolveRemoteExecutable(const ModuleSpec _spec,
- lldb::ModuleSP _module_sp,
- const FileSpecList *module_search_paths_ptr);
+ /// The method is virtual for mocking in the unit tests.
private:
typedef std::function ModuleResolver;
diff --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h
b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index 0b9d79f9ff038..6fbeec7888a98 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -23,10 +23,6 @@ class RemoteAwarePlatform : public Platform {
bool GetModuleSpec(const FileSpec _file_spec, const ArchSpec ,
ModuleSpec _spec) override;
- Status
- ResolveExecutable(const ModuleSpec _spec, lldb::ModuleSP _sp,
-const FileSpecList *module_search_paths_ptr) override;
-
lldb::user_id_t OpenFile(const FileSpec _spec, File::OpenOptions flags,
uint32_t mode, Status ) override;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
index 0c4b566b7d535..c27a34b7201a2 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -383,81 +383,6 @@ PlatformSP PlatformAppleSimulator::CreateInstance(
return PlatformSP();
}
-Status PlatformAppleSimulator::ResolveExecutable(
-const ModuleSpec _spec, lldb::ModuleSP _module_sp,
-const FileSpecList *module_search_paths_ptr) {
- Status error;
- // Nothing special to do here, just use the actual file and architecture
-
- ModuleSpec resolved_module_spec(module_spec);
-
- // If we have "ls" as the exe_file, resolve the executable loation based on
- // the current path variables
- // TODO: resolve bare executables in the Platform SDK
- //if (!resolved_exe_file.Exists())
- //resolved_exe_file.ResolveExecutableLocation ();
-
- // Resolve any executable within a bundle on MacOSX
- // TODO: verify that this handles shallow bundles, if not then implement one
- // ourselves
-
[Lldb-commits] [lldb] [lldb] Unify Platform::ResolveExecutable (PR #96256)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
The Platform class currently has two functions to resolve an executable:
`ResolveExecutable` and `ResolveRemoteExecutable`. The former strictly deals
with local files while the latter can handle potentially remote files. I
couldn't figure out why the distinction matters, at the latter is a super-set
of the former.
To make things even more confusion, we had a similar but not identical
implementation in RemoteAwarePlatform where its implementation of
`ResolveExecutable` could handle remote files. To top it all off, we had
copy-pasted implementation, dead code included in `PlatformAppleSimulator` and
`PlatformRemoteDarwinDevice`.
I went ahead and unified all the different implementation on the original
`ResolveRemoteExecutable` implementation. As far as I can tell, it should work
for every other platform, and the test suite (on macOS) seems to agree with me,
except for a small wording change.
---
Patch is 23.46 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/96256.diff
11 Files Affected:
- (modified) lldb/include/lldb/Target/Platform.h (+4-5)
- (modified) lldb/include/lldb/Target/RemoteAwarePlatform.h (-4)
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
(-75)
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h (-4)
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
(-65)
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
(-4)
- (modified) lldb/source/Target/Platform.cpp (+5-43)
- (modified) lldb/source/Target/RemoteAwarePlatform.cpp (-139)
- (modified) lldb/test/API/assert_messages_test/TestAssertMessages.py (+1-1)
- (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+1-1)
- (modified) lldb/unittests/Target/RemoteAwarePlatformTest.cpp (+7-6)
``diff
diff --git a/lldb/include/lldb/Target/Platform.h
b/lldb/include/lldb/Target/Platform.h
index e05c79cb501bf..75c37be6c2af2 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -138,9 +138,11 @@ class Platform : public PluginInterface {
/// \return
/// Returns \b true if this Platform plug-in was able to find
/// a suitable executable, \b false otherwise.
+ /// @{
virtual Status ResolveExecutable(const ModuleSpec _spec,
- lldb::ModuleSP _sp,
+ lldb::ModuleSP _module_sp,
const FileSpecList
*module_search_paths_ptr);
+ /// @}
/// Find a symbol file given a symbol file module specification.
///
@@ -1004,10 +1006,7 @@ class Platform : public PluginInterface {
virtual const char *GetCacheHostname();
- virtual Status
- ResolveRemoteExecutable(const ModuleSpec _spec,
- lldb::ModuleSP _module_sp,
- const FileSpecList *module_search_paths_ptr);
+ /// The method is virtual for mocking in the unit tests.
private:
typedef std::function ModuleResolver;
diff --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h
b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index 0b9d79f9ff038..6fbeec7888a98 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -23,10 +23,6 @@ class RemoteAwarePlatform : public Platform {
bool GetModuleSpec(const FileSpec _file_spec, const ArchSpec ,
ModuleSpec _spec) override;
- Status
- ResolveExecutable(const ModuleSpec _spec, lldb::ModuleSP _sp,
-const FileSpecList *module_search_paths_ptr) override;
-
lldb::user_id_t OpenFile(const FileSpec _spec, File::OpenOptions flags,
uint32_t mode, Status ) override;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
index 0c4b566b7d535..c27a34b7201a2 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -383,81 +383,6 @@ PlatformSP PlatformAppleSimulator::CreateInstance(
return PlatformSP();
}
-Status PlatformAppleSimulator::ResolveExecutable(
-const ModuleSpec _spec, lldb::ModuleSP _module_sp,
-const FileSpecList *module_search_paths_ptr) {
- Status error;
- // Nothing special to do here, just use the actual file and architecture
-
- ModuleSpec resolved_module_spec(module_spec);
-
- // If we have "ls" as the exe_file, resolve the executable loation based on
- // the current path variables
- // TODO: resolve bare executables in the Platform SDK
- //if (!resolved_exe_file.Exists())
- //resolved_exe_file.ResolveExecutableLocation ();
-
- // Resolve any executable within a bundle on MacOSX
- // TODO: verify that this
[Lldb-commits] [lldb] [API] add GetSyntheticValue (PR #95959)
https://github.com/v-bulle updated
https://github.com/llvm/llvm-project/pull/95959
>From 27a00b54bc991dfb4747e0d37b15878beebaabba Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Wed, 12 Jun 2024 14:23:15 -0700
Subject: [PATCH 1/2] [API] add GetSyntheticValue
---
lldb/include/lldb/API/SBValue.h | 2 ++
lldb/source/API/SBValue.cpp | 12
2 files changed, 14 insertions(+)
diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h
index 65920c76df7a8..bec816fb45184 100644
--- a/lldb/include/lldb/API/SBValue.h
+++ b/lldb/include/lldb/API/SBValue.h
@@ -89,6 +89,8 @@ class LLDB_API SBValue {
lldb::SBValue GetNonSyntheticValue();
+ lldb::SBValue GetSyntheticValue();
+
lldb::DynamicValueType GetPreferDynamicValue();
void SetPreferDynamicValue(lldb::DynamicValueType use_dynamic);
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 9d7efba024d11..6b77c0e95cedd 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -761,6 +761,18 @@ lldb::SBValue SBValue::GetNonSyntheticValue() {
return value_sb;
}
+lldb::SBValue SBValue::GetSyntheticValue() {
+ LLDB_INSTRUMENT_VA(this);
+
+ SBValue value_sb;
+ if (IsValid()) {
+ValueImplSP proxy_sp(new ValueImpl(m_opaque_sp->GetRootSP(),
+ m_opaque_sp->GetUseDynamic(), true));
+value_sb.SetSP(proxy_sp);
+ }
+ return value_sb;
+}
+
lldb::DynamicValueType SBValue::GetPreferDynamicValue() {
LLDB_INSTRUMENT_VA(this);
>From e0ed752849486f67d9fddbef3767a1756afd1ab2 Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Thu, 20 Jun 2024 17:04:15 -0700
Subject: [PATCH 2/2] add test
---
.../formatters/TestFormattersSBAPI.py | 12
lldb/test/API/python_api/formatters/synth.py | 28 +--
2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/lldb/test/API/python_api/formatters/TestFormattersSBAPI.py
b/lldb/test/API/python_api/formatters/TestFormattersSBAPI.py
index 7e802f92da352..460afbc1202cf 100644
--- a/lldb/test/API/python_api/formatters/TestFormattersSBAPI.py
+++ b/lldb/test/API/python_api/formatters/TestFormattersSBAPI.py
@@ -155,6 +155,18 @@ def cleanup():
],
)
+self.dbg.GetCategory("CCCSynth2").SetEnabled(True)
+self.expect(
+"frame variable ccc",
+matching=True,
+substrs=[
+"CCC object with leading synthetic value (int) b = 222",
+"a = 111",
+"b = 222",
+"c = 333",
+],
+)
+
foo_var = (
self.dbg.GetSelectedTarget()
.GetProcess()
diff --git a/lldb/test/API/python_api/formatters/synth.py
b/lldb/test/API/python_api/formatters/synth.py
index 474c18bc62ebd..2b8f7c86ac6f1 100644
--- a/lldb/test/API/python_api/formatters/synth.py
+++ b/lldb/test/API/python_api/formatters/synth.py
@@ -29,11 +29,19 @@ def ccc_summary(sbvalue, internal_dict):
# This tests that the SBValue.GetNonSyntheticValue() actually returns a
# non-synthetic value. If it does not, then
sbvalue.GetChildMemberWithName("a")
# in the following statement will call the 'get_child_index' method of the
-# synthetic child provider CCCSynthProvider below (which raises an
-# exception).
+# synthetic child provider CCCSynthProvider below (which return the "b"
field").
return "CCC object with leading value " +
str(sbvalue.GetChildMemberWithName("a"))
+def ccc_synthetic(sbvalue, internal_dict):
+sbvalue = sbvalue.GetSyntheticValue()
+# This tests that the SBValue.GetNonSyntheticValue() actually returns a
+# synthetic value. If it does, then sbvalue.GetChildMemberWithName("a")
+# in the following statement will call the 'get_child_index' method of the
+# synthetic child provider CCCSynthProvider below (which return the "b"
field").
+return "CCC object with leading synthetic value " +
str(sbvalue.GetChildMemberWithName("a"))
+
+
class CCCSynthProvider(object):
def __init__(self, sbvalue, internal_dict):
self._sbvalue = sbvalue
@@ -42,6 +50,9 @@ def num_children(self):
return 3
def get_child_index(self, name):
+if name == "a":
+# Return b for test.
+return 1
raise RuntimeError("I don't want to be called!")
def get_child_at_index(self, index):
@@ -119,3 +130,16 @@ def __lldb_init_module(debugger, dict):
"synth.empty2_summary", lldb.eTypeOptionHideEmptyAggregates
),
)
+cat2 = debugger.CreateCategory("CCCSynth2")
+cat2.AddTypeSynthetic(
+lldb.SBTypeNameSpecifier("CCC"),
+lldb.SBTypeSynthetic.CreateWithClassName(
+"synth.CCCSynthProvider", lldb.eTypeOptionCascade
+),
+)
+cat2.AddTypeSummary(
+lldb.SBTypeNameSpecifier("CCC"),
+lldb.SBTypeSummary.CreateWithFunctionName(
+
[Lldb-commits] [lldb] [lldb] Unify Platform::ResolveExecutable (PR #96256)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/96256
The Platform class currently has two functions to resolve an executable:
`ResolveExecutable` and `ResolveRemoteExecutable`. The former strictly deals
with local files while the latter can handle potentially remote files. I
couldn't figure out why the distinction matters, at the latter is a super-set
of the former.
To make things even more confusion, we had a similar but not identical
implementation in RemoteAwarePlatform where its implementation of
`ResolveExecutable` could handle remote files. To top it all off, we had
copy-pasted implementation, dead code included in `PlatformAppleSimulator` and
`PlatformRemoteDarwinDevice`.
I went ahead and unified all the different implementation on the original
`ResolveRemoteExecutable` implementation. As far as I can tell, it should work
for every other platform, and the test suite (on macOS) seems to agree with me,
except for a small wording change.
>From b94b23f65f0c5ceb9bdfd21d4355959da1e9f2c9 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Thu, 20 Jun 2024 13:51:53 -0700
Subject: [PATCH] [lldb] Unify Platform::ResolveExecutable duplication
The Platform class currently has two functions to resolve an executable:
`ResolveExecutable` and `ResolveRemoteExecutable`. The former strictly
deals with local files while the latter can handle potentially remote
files. I couldn't figure out why the distinction matters, at the latter
is a super-set of the former.
To make things even more confusion, we had a similar but not identical
implementation in RemoteAwarePlatform where its implementation of
`ResolveExecutable` could handle remote files. To top it all off, we had
copy-pasted implementation, dead code included in
`PlatformAppleSimulator` and `PlatformRemoteDarwinDevice`.
I went ahead and unified all the different implementation on the
original `ResolveRemoteExecutable` implementation. As far as I can tell,
it should work for every other platform, and the test suite (on macOS)
seems to agree with me, except for a small wording change.
---
lldb/include/lldb/Target/Platform.h | 9 +-
.../include/lldb/Target/RemoteAwarePlatform.h | 4 -
.../MacOSX/PlatformAppleSimulator.cpp | 75 --
.../Platform/MacOSX/PlatformAppleSimulator.h | 4 -
.../MacOSX/PlatformRemoteDarwinDevice.cpp | 65
.../MacOSX/PlatformRemoteDarwinDevice.h | 4 -
lldb/source/Target/Platform.cpp | 48 +-
lldb/source/Target/RemoteAwarePlatform.cpp| 139 --
.../TestAssertMessages.py | 2 +-
.../tools/lldb-dap/launch/TestDAP_launch.py | 2 +-
.../Target/RemoteAwarePlatformTest.cpp| 13 +-
11 files changed, 18 insertions(+), 347 deletions(-)
diff --git a/lldb/include/lldb/Target/Platform.h
b/lldb/include/lldb/Target/Platform.h
index e05c79cb501bf..75c37be6c2af2 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -138,9 +138,11 @@ class Platform : public PluginInterface {
/// \return
/// Returns \b true if this Platform plug-in was able to find
/// a suitable executable, \b false otherwise.
+ /// @{
virtual Status ResolveExecutable(const ModuleSpec _spec,
- lldb::ModuleSP _sp,
+ lldb::ModuleSP _module_sp,
const FileSpecList
*module_search_paths_ptr);
+ /// @}
/// Find a symbol file given a symbol file module specification.
///
@@ -1004,10 +1006,7 @@ class Platform : public PluginInterface {
virtual const char *GetCacheHostname();
- virtual Status
- ResolveRemoteExecutable(const ModuleSpec _spec,
- lldb::ModuleSP _module_sp,
- const FileSpecList *module_search_paths_ptr);
+ /// The method is virtual for mocking in the unit tests.
private:
typedef std::function ModuleResolver;
diff --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h
b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index 0b9d79f9ff038..6fbeec7888a98 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -23,10 +23,6 @@ class RemoteAwarePlatform : public Platform {
bool GetModuleSpec(const FileSpec _file_spec, const ArchSpec ,
ModuleSpec _spec) override;
- Status
- ResolveExecutable(const ModuleSpec _spec, lldb::ModuleSP _sp,
-const FileSpecList *module_search_paths_ptr) override;
-
lldb::user_id_t OpenFile(const FileSpec _spec, File::OpenOptions flags,
uint32_t mode, Status ) override;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
index 0c4b566b7d535..c27a34b7201a2 100644
---
[Lldb-commits] [lldb] aafa0ef - [lldb/crashlog] Make interactive mode the new default (#94575)
Author: Med Ismail Bennani Date: 2024-06-20T17:23:18-07:00 New Revision: aafa0ef900791857f55629bcf61c37f53cc0d2af URL: https://github.com/llvm/llvm-project/commit/aafa0ef900791857f55629bcf61c37f53cc0d2af DIFF: https://github.com/llvm/llvm-project/commit/aafa0ef900791857f55629bcf61c37f53cc0d2af.diff LOG: [lldb/crashlog] Make interactive mode the new default (#94575) This patch makes interactive mode as the default when using the crashlog command. It replaces the existing `-i|--interactive` flag with a new `-m|--mode` option, that can either be `interactive` or `batch`. By default, when the option is not explicitely set by the user, the interactive mode is selected, however, lldb will fallback to batch mode if the command interpreter is not interactive or if stdout is not a tty. This also adds some railguards to prevent users from using interactive only options with the batch mode and updates the tests accordingly. rdar://97801509 Differential Revision: https://reviews.llvm.org/D141658 Signed-off-by: Med Ismail Bennani Added: Modified: lldb/examples/python/crashlog.py lldb/test/Shell/ScriptInterpreter/Python/Crashlog/altered_threadState.test lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no_threadState.test lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test Removed: diff --git a/lldb/examples/python/crashlog.py b/lldb/examples/python/crashlog.py index 1c0d717ce455c..d3952e377c657 100755 --- a/lldb/examples/python/crashlog.py +++ b/lldb/examples/python/crashlog.py @@ -31,6 +31,7 @@ import concurrent.futures import contextlib import datetime +import enum import json import os import platform @@ -45,7 +46,6 @@ import time import uuid - print_lock = threading.RLock() try: @@ -1582,9 +1582,12 @@ def synchronous(debugger): debugger.RunCommandInterpreter(True, False, run_options, 0, False, True) -def CreateSymbolicateCrashLogOptions( -command_name, description, add_interactive_options -): +class CrashLogLoadingMode(str, enum.Enum): +batch = "batch" +interactive = "interactive" + + +def CreateSymbolicateCrashLogOptions(command_name, description): usage = "crashlog [options] [FILE ...]" arg_parser = argparse.ArgumentParser( description=description, @@ -1600,6 +1603,12 @@ def CreateSymbolicateCrashLogOptions( help="crash report(s) to symbolicate", ) +arg_parser.add_argument( +"-m", +"--mode", +choices=[mode.value for mode in CrashLogLoadingMode], +help="change how the symbolicated process and threads are displayed to the user (default: interactive)", +) arg_parser.add_argument( "--version", "-V", @@ -1736,36 +1745,35 @@ def CreateSymbolicateCrashLogOptions( help=argparse.SUPPRESS, default=False, ) -if add_interactive_options: -arg_parser.add_argument( -"-i", -"--interactive", -action="store_true", -help="parse a crash log and load it in a ScriptedProcess", -default=False, -) -arg_parser.add_argument( -"-b", -"--batch", -action="store_true", -help="dump symbolicated stackframes without creating a debug session", -default=True, -) -arg_parser.add_argument( -"--target", -"-t", -dest="target_path", -help="the target binary path that should be used for interactive crashlog (optional)", -default=None, -) -arg_parser.add_argument( -"--skip-status", -"-s", -dest="skip_status", -action="store_true", -help="prevent the interactive crashlog to dump the process status and thread backtrace at launch", -default=False, -) +arg_parser.add_argument( +"--target", +"-t", +dest="target_path", +help="the target binary path that should be used for interactive crashlog (optional)", +default=None, +) +arg_parser.add_argument( +"--skip-status", +"-s", +dest="skip_status", +action="store_true", +help="prevent the interactive crashlog to dump the process status and thread backtrace at launch", +default=False, +) +legacy_group = arg_parser.add_mutually_exclusive_group() +legacy_group.add_argument( +"-i", +"--interactive", +action="store_true", +help=argparse.SUPPRESS, +) +legacy_group.add_argument( +"-b", +"--batch", +action="store_true", +help=argparse.SUPPRESS, +) +
[Lldb-commits] [lldb] [lldb/crashlog] Make interactive mode the new default (PR #94575)
https://github.com/medismailben closed https://github.com/llvm/llvm-project/pull/94575 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a unit test for SBBreakpoint::SetCallback (PR #96001)
@@ -0,0 +1,93 @@
+//===-- TestBreakpointSetCallback.cpp
+//===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Progress.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/lldb-private-enumerations.h"
+#include "lldb/lldb-types.h"
+#include "gtest/gtest.h"
+#include
+#include
+#include
+
+using namespace lldb_private;
+using namespace lldb;
+
+#define EXPECTED_BREAKPOINT_ID 1
+#define EXPECTED_BREAKPOINT_LOCATION_ID 0
+
+class BreakpointSetCallbackTest : public ::testing::Test {
+public:
+ static void CheckCallbackArgs(void *baton, StoppointCallbackContext *context,
+lldb::user_id_t break_id,
+lldb::user_id_t break_loc_id,
+lldb::user_id_t expected_breakpoint_id,
+lldb::user_id_t expected_breakpoint_loc_id,
+TargetSP expected_target_sp) {
+EXPECT_EQ(context->exe_ctx_ref.GetTargetSP(), expected_target_sp);
+EXPECT_EQ(baton, "hello");
+EXPECT_EQ(break_id, expected_breakpoint_id);
+EXPECT_EQ(break_loc_id, expected_breakpoint_loc_id);
+ }
+
+protected:
+ void SetUp() override {
+std::call_once(TestUtilities::g_debugger_initialize_flag,
+ []() { Debugger::Initialize(nullptr); });
+
+// Set up the debugger, make sure that was done properly.
+ArchSpec arch("x86_64-apple-macosx-");
+Platform::SetHostPlatform(
+PlatformRemoteMacOSX::CreateInstance(true, ));
+
+m_debugger_sp = Debugger::CreateInstance();
+
+// Create target
+m_debugger_sp->GetTargetList().CreateTarget(*m_debugger_sp, "", arch,
+
lldb_private::eLoadDependentsNo,
+m_platform_sp, m_target_sp);
+// Create breakpoint
+m_breakpoint_sp = m_target_sp->CreateBreakpoint(0xDEADBEEF, false, false);
+ };
+
+ DebuggerSP m_debugger_sp;
+ PlatformSP m_platform_sp;
+ TargetSP m_target_sp;
+ BreakpointSP m_breakpoint_sp;
+ lldb::user_id_t expected_breakpoint_id;
+ lldb::user_id_t expected_breakpoint_loc_id;
+ SubsystemRAII
+ subsystems;
+};
+
+TEST_F(BreakpointSetCallbackTest, TestBreakpointSetCallback) {
+ void *baton = (void *)"hello";
+ TargetSP current_target = m_target_sp;
chelcassanova wrote:
That only works if I capture `this`, trying to capture just `m_target_sp` won't
work (unless we move the target setup into the test body)
https://github.com/llvm/llvm-project/pull/96001
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [API] add GetSyntheticValue (PR #95959)
https://github.com/v-bulle updated
https://github.com/llvm/llvm-project/pull/95959
>From 27a00b54bc991dfb4747e0d37b15878beebaabba Mon Sep 17 00:00:00 2001
From: Vincent Belliard
Date: Wed, 12 Jun 2024 14:23:15 -0700
Subject: [PATCH] [API] add GetSyntheticValue
---
lldb/include/lldb/API/SBValue.h | 2 ++
lldb/source/API/SBValue.cpp | 12
2 files changed, 14 insertions(+)
diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h
index 65920c76df7a8..bec816fb45184 100644
--- a/lldb/include/lldb/API/SBValue.h
+++ b/lldb/include/lldb/API/SBValue.h
@@ -89,6 +89,8 @@ class LLDB_API SBValue {
lldb::SBValue GetNonSyntheticValue();
+ lldb::SBValue GetSyntheticValue();
+
lldb::DynamicValueType GetPreferDynamicValue();
void SetPreferDynamicValue(lldb::DynamicValueType use_dynamic);
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 9d7efba024d11..6b77c0e95cedd 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -761,6 +761,18 @@ lldb::SBValue SBValue::GetNonSyntheticValue() {
return value_sb;
}
+lldb::SBValue SBValue::GetSyntheticValue() {
+ LLDB_INSTRUMENT_VA(this);
+
+ SBValue value_sb;
+ if (IsValid()) {
+ValueImplSP proxy_sp(new ValueImpl(m_opaque_sp->GetRootSP(),
+ m_opaque_sp->GetUseDynamic(), true));
+value_sb.SetSP(proxy_sp);
+ }
+ return value_sb;
+}
+
lldb::DynamicValueType SBValue::GetPreferDynamicValue() {
LLDB_INSTRUMENT_VA(this);
___
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 semantics of SupportFile equivalence (PR #95606)
JDevlieghere wrote: `IsProbablySame` works for me https://github.com/llvm/llvm-project/pull/95606 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #96243)
https://github.com/clayborg closed https://github.com/llvm/llvm-project/pull/96243 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #96243)
clayborg wrote: I got the original PR back on track... https://github.com/llvm/llvm-project/pull/96243 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
clayborg wrote: @ayermolo: you suggestion worked. thanks. This PR is back on track... @labath: I implemented your suggestions, let me know if this look ok now https://github.com/llvm/llvm-project/pull/87740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)
https://github.com/clayborg updated
https://github.com/llvm/llvm-project/pull/87740
>From badd915257bb192add91696e0b8530c057bd385f Mon Sep 17 00:00:00 2001
From: Greg Clayton
Date: Sat, 30 Mar 2024 10:50:34 -0700
Subject: [PATCH 01/11] Add support for using foreign type units in
.debug_names.
This patch adds support for the new foreign type unit support in .debug_names.
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp
file
- fix crashers that happen due to PeekDIEName() using wrong offsets
---
.../SymbolFile/DWARF/DWARFDebugInfo.cpp | 18
.../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h | 2 +
.../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 65 -
.../SymbolFile/DWARF/DebugNamesDWARFIndex.h | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.cpp | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.h | 7 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 66 --
.../SymbolFile/DWARF/SymbolFileDWARF.h| 9 ++
.../DWARF/x86/dwp-foreign-type-units.cpp | 91 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.h | 11 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 13 +++
11 files changed, 257 insertions(+), 37 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index c37cc91e08ed1..056c6d4b0605f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -222,6 +222,20 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section
section,
return result;
}
+DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef _ref) {
+ // Make sure we get the correct SymbolFileDWARF from the DIERef before
+ // asking for information from a debug info object. We might start with the
+ // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
+ // might be pointing to a specific .dwo file or to the .dwp file. So this
+ // makes sure we get the right SymbolFileDWARF instance before finding the
+ // DWARFUnit that contains the offset. If we just use this object to do the
+ // search, we might be using the wrong .debug_info section from the wrong
+ // file with an offset meant for a different section.
+ SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
+ return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
+ die_ref.die_offset());
+}
+
DWARFUnit *
DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
dw_offset_t die_offset) {
@@ -232,6 +246,10 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section
section,
return result;
}
+const std::shared_ptr DWARFDebugInfo::GetDwpSymbolFile() {
+ return m_dwarf.GetDwpSymbolFile();
+}
+
DWARFTypeUnit *DWARFDebugInfo::GetTypeUnitForHash(uint64_t hash) {
auto pos = llvm::lower_bound(m_type_hash_to_unit_index,
std::make_pair(hash, 0u), llvm::less_first());
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index 4706b55d38ea9..4d4555a338252 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -52,6 +52,8 @@ class DWARFDebugInfo {
const DWARFDebugAranges ();
+ const std::shared_ptr GetDwpSymbolFile();
+
protected:
typedef std::vector UnitColl;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 1d17f20670eed..d815d345b08ee 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -34,6 +34,18 @@ DebugNamesDWARFIndex::Create(Module ,
DWARFDataExtractor debug_names,
module, std::move(index_up), debug_names, debug_str, dwarf));
}
+
+llvm::DenseSet
+DebugNamesDWARFIndex::GetTypeUnitSigs(const DebugNames _names) {
+ llvm::DenseSet result;
+ for (const DebugNames::NameIndex : debug_names) {
+const uint32_t num_tus = ni.getForeignTUCount();
+for (uint32_t tu = 0; tu < num_tus; ++tu)
+ result.insert(ni.getForeignTUSignature(tu));
+ }
+ return result;
+}
+
llvm::DenseSet
DebugNamesDWARFIndex::GetUnits(const DebugNames _names) {
llvm::DenseSet result;
@@ -48,17 +60,22 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames
_names) {
return result;
}
+DWARFTypeUnit *
+DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry ) const
{
+ std::optional type_sig = entry.getForeignTUTypeSignature();
+ if (type_sig)
+if (auto dwp_sp = m_debug_info.GetDwpSymbolFile())
+ return
[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #96243)
https://github.com/clayborg created
https://github.com/llvm/llvm-project/pull/96243
This patch adds support for the new foreign type unit support in .debug_names.
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp
file
- fix crashers that happen due to PeekDIEName() using wrong offsets
- support finding type unit entries in .dwp and .dwo files
This PR is a redo of https://github.com/llvm/llvm-project/pull/87740 after that
branch had merging issues
>From c800f88b19d6e470e73c350ea5bcc88ce299f0eb Mon Sep 17 00:00:00 2001
From: Greg Clayton
Date: Sat, 30 Mar 2024 10:50:34 -0700
Subject: [PATCH 01/10] Add support for using foreign type units in
.debug_names.
This patch adds support for the new foreign type unit support in .debug_names.
Features include:
- don't manually index foreign TUs if we have info for them
- only use the type unit entries that match the .dwo files when we have a .dwp
file
- fix crashers that happen due to PeekDIEName() using wrong offsets
---
.../SymbolFile/DWARF/DWARFDebugInfo.cpp | 18
.../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h | 2 +
.../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 65 -
.../SymbolFile/DWARF/DebugNamesDWARFIndex.h | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.cpp | 6 +-
.../SymbolFile/DWARF/ManualDWARFIndex.h | 7 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 66 --
.../SymbolFile/DWARF/SymbolFileDWARF.h| 9 ++
.../DWARF/x86/dwp-foreign-type-units.cpp | 91 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.h | 11 +++
.../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 13 +++
11 files changed, 257 insertions(+), 37 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index c37cc91e08ed1..056c6d4b0605f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -222,6 +222,20 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section
section,
return result;
}
+DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef _ref) {
+ // Make sure we get the correct SymbolFileDWARF from the DIERef before
+ // asking for information from a debug info object. We might start with the
+ // DWARFDebugInfo for the main executable in a split DWARF and the DIERef
+ // might be pointing to a specific .dwo file or to the .dwp file. So this
+ // makes sure we get the right SymbolFileDWARF instance before finding the
+ // DWARFUnit that contains the offset. If we just use this object to do the
+ // search, we might be using the wrong .debug_info section from the wrong
+ // file with an offset meant for a different section.
+ SymbolFileDWARF *dwarf = m_dwarf.GetDIERefSymbolFile(die_ref);
+ return dwarf->DebugInfo().GetUnitContainingDIEOffset(die_ref.section(),
+ die_ref.die_offset());
+}
+
DWARFUnit *
DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
dw_offset_t die_offset) {
@@ -232,6 +246,10 @@ DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section
section,
return result;
}
+const std::shared_ptr DWARFDebugInfo::GetDwpSymbolFile() {
+ return m_dwarf.GetDwpSymbolFile();
+}
+
DWARFTypeUnit *DWARFDebugInfo::GetTypeUnitForHash(uint64_t hash) {
auto pos = llvm::lower_bound(m_type_hash_to_unit_index,
std::make_pair(hash, 0u), llvm::less_first());
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index 4706b55d38ea9..4d4555a338252 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -52,6 +52,8 @@ class DWARFDebugInfo {
const DWARFDebugAranges ();
+ const std::shared_ptr GetDwpSymbolFile();
+
protected:
typedef std::vector UnitColl;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 1d17f20670eed..d815d345b08ee 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -34,6 +34,18 @@ DebugNamesDWARFIndex::Create(Module ,
DWARFDataExtractor debug_names,
module, std::move(index_up), debug_names, debug_str, dwarf));
}
+
+llvm::DenseSet
+DebugNamesDWARFIndex::GetTypeUnitSigs(const DebugNames _names) {
+ llvm::DenseSet result;
+ for (const DebugNames::NameIndex : debug_names) {
+const uint32_t num_tus = ni.getForeignTUCount();
+for (uint32_t tu = 0; tu < num_tus; ++tu)
+ result.insert(ni.getForeignTUSignature(tu));
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
@@ -43,8 +43,7 @@ CompilerType RegisterTypeBuilderClang::GetRegisterType( ScratchTypeSystemClang::GetForTarget(m_target); assert(type_system); - std::string register_type_name = "__lldb_register_fields_"; - register_type_name += name; + std::string register_type_name = "__lldb_register_fields_" + name; medismailben wrote: Yeah, I was thinking making a `Twine` then building a `std::string` out of it. Nvm https://github.com/llvm/llvm-project/pull/95768 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a unit test for SBBreakpoint::SetCallback (PR #96001)
https://github.com/medismailben approved this pull request. LGTM, with comments addressed. https://github.com/llvm/llvm-project/pull/96001 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a unit test for SBBreakpoint::SetCallback (PR #96001)
@@ -0,0 +1,93 @@
+//===-- TestBreakpointSetCallback.cpp
+//===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
+#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Progress.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/lldb-private-enumerations.h"
+#include "lldb/lldb-types.h"
+#include "gtest/gtest.h"
+#include
+#include
+#include
+
+using namespace lldb_private;
+using namespace lldb;
+
+#define EXPECTED_BREAKPOINT_ID 1
+#define EXPECTED_BREAKPOINT_LOCATION_ID 0
+
+class BreakpointSetCallbackTest : public ::testing::Test {
+public:
+ static void CheckCallbackArgs(void *baton, StoppointCallbackContext *context,
+lldb::user_id_t break_id,
+lldb::user_id_t break_loc_id,
+lldb::user_id_t expected_breakpoint_id,
+lldb::user_id_t expected_breakpoint_loc_id,
+TargetSP expected_target_sp) {
+EXPECT_EQ(context->exe_ctx_ref.GetTargetSP(), expected_target_sp);
+EXPECT_EQ(baton, "hello");
+EXPECT_EQ(break_id, expected_breakpoint_id);
+EXPECT_EQ(break_loc_id, expected_breakpoint_loc_id);
+ }
+
+protected:
+ void SetUp() override {
+std::call_once(TestUtilities::g_debugger_initialize_flag,
+ []() { Debugger::Initialize(nullptr); });
+
+// Set up the debugger, make sure that was done properly.
+ArchSpec arch("x86_64-apple-macosx-");
+Platform::SetHostPlatform(
+PlatformRemoteMacOSX::CreateInstance(true, ));
+
+m_debugger_sp = Debugger::CreateInstance();
+
+// Create target
+m_debugger_sp->GetTargetList().CreateTarget(*m_debugger_sp, "", arch,
+
lldb_private::eLoadDependentsNo,
+m_platform_sp, m_target_sp);
+// Create breakpoint
+m_breakpoint_sp = m_target_sp->CreateBreakpoint(0xDEADBEEF, false, false);
+ };
+
+ DebuggerSP m_debugger_sp;
+ PlatformSP m_platform_sp;
+ TargetSP m_target_sp;
+ BreakpointSP m_breakpoint_sp;
+ lldb::user_id_t expected_breakpoint_id;
+ lldb::user_id_t expected_breakpoint_loc_id;
+ SubsystemRAII
+ subsystems;
+};
+
+TEST_F(BreakpointSetCallbackTest, TestBreakpointSetCallback) {
+ void *baton = (void *)"hello";
+ TargetSP current_target = m_target_sp;
medismailben wrote:
nit: you don't need to make a new various from `m_target_sp`, just capture it
directly in the lambda.
https://github.com/llvm/llvm-project/pull/96001
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a unit test for SBBreakpoint::SetCallback (PR #96001)
@@ -99,10 +99,15 @@ typedef std::optional (*SymbolLocatorLocateExecutableSymbolFile)( typedef bool (*SymbolLocatorDownloadObjectAndSymbolFile)( ModuleSpec _spec, Status , bool force_lookup, bool copy_executable); -typedef bool (*BreakpointHitCallback)(void *baton, - StoppointCallbackContext *context, - lldb::user_id_t break_id, - lldb::user_id_t break_loc_id); +// typedef bool (*BreakpointHitCallback)(void *baton, medismailben wrote: deadcode ? https://github.com/llvm/llvm-project/pull/96001 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a unit test for SBBreakpoint::SetCallback (PR #96001)
@@ -99,10 +99,15 @@ typedef std::optional (*SymbolLocatorLocateExecutableSymbolFile)( typedef bool (*SymbolLocatorDownloadObjectAndSymbolFile)( ModuleSpec _spec, Status , bool force_lookup, bool copy_executable); -typedef bool (*BreakpointHitCallback)(void *baton, - StoppointCallbackContext *context, - lldb::user_id_t break_id, - lldb::user_id_t break_loc_id); +// typedef bool (*BreakpointHitCallback)(void *baton, +// StoppointCallbackContext *context, +// lldb::user_id_t break_id, +// lldb::user_id_t break_loc_id); +typedef std::functionhttps://github.com/llvm/llvm-project/pull/96001 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a unit test for SBBreakpoint::SetCallback (PR #96001)
https://github.com/medismailben edited https://github.com/llvm/llvm-project/pull/96001 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
@@ -4254,14 +4356,55 @@ static std::vector
ParseFlagsFields(XMLNode flags_node,
});
if (name && start && end) {
- if (*start > *end) {
+ if (*start > *end)
LLDB_LOG(
log,
"ProcessGDBRemote::ParseFlagsFields Start {0} > end {1} in field "
"\"{2}\", ignoring",
*start, *end, name->data());
- } else {
-fields.push_back(RegisterFlags::Field(name->str(), *start, *end));
+ else {
+if (RegisterFlags::Field::GetSizeInBits(*start, *end) > 64)
+ LLDB_LOG(log,
+ "ProcessGDBRemote::ParseFlagsFields Ignoring field \"{2}\" "
+ "that has "
+ " size > 64 bits, this is not supported",
jasonmolenda wrote:
there's a whitespace at the end of `"that has "`, we don't need one at the
start.
https://github.com/llvm/llvm-project/pull/95768
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
https://github.com/jasonmolenda approved this pull request. This looks good to me. The only thing I could suggest is that we check the specified fields/size are less than 64 bits when reading the enum field definitions, but that means I can specify fields above 32 bits and then associate that with a 32-bit cpsr register and it won't be detected. I feel like it's critical we be that careful with our input handling, but I thought I'd mention it to see what you think. https://github.com/llvm/llvm-project/pull/95768 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
@@ -370,6 +370,16 @@ void FieldEnum::Enumerator::ToXML(Stream ) const {
escaped_name.c_str(), m_value);
}
+void FieldEnum::Enumerator::log(Log *log) const {
jasonmolenda wrote:
These seem closer to Dump methods that output to a Log instead of a
StringStream. Maybe DumpToLog()? I wasn't sure what the method would do based
on the name.
https://github.com/llvm/llvm-project/pull/95768
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/95768 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a unit test for SBBreakpoint::SetCallback (PR #96001)
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/96001
>From 2cfcef620125ed7d0ba84acfa20de3064437282c Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Thu, 13 Jun 2024 16:02:07 -0700
Subject: [PATCH] add unit test for breakpoint::setcallback
---
lldb/include/lldb/lldb-private-interfaces.h | 13 ++-
lldb/source/Breakpoint/BreakpointOptions.cpp | 18 +---
lldb/unittests/CMakeLists.txt | 1 +
lldb/unittests/Callback/CMakeLists.txt| 12 +++
.../Callback/TestBreakpointSetCallback.cpp| 93 +++
5 files changed, 120 insertions(+), 17 deletions(-)
create mode 100644 lldb/unittests/Callback/CMakeLists.txt
create mode 100644 lldb/unittests/Callback/TestBreakpointSetCallback.cpp
diff --git a/lldb/include/lldb/lldb-private-interfaces.h
b/lldb/include/lldb/lldb-private-interfaces.h
index 53d5fbb84cc92..136b644133d32 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -99,10 +99,15 @@ typedef std::optional
(*SymbolLocatorLocateExecutableSymbolFile)(
typedef bool (*SymbolLocatorDownloadObjectAndSymbolFile)(
ModuleSpec _spec, Status , bool force_lookup,
bool copy_executable);
-typedef bool (*BreakpointHitCallback)(void *baton,
- StoppointCallbackContext *context,
- lldb::user_id_t break_id,
- lldb::user_id_t break_loc_id);
+// typedef bool (*BreakpointHitCallback)(void *baton,
+// StoppointCallbackContext *context,
+// lldb::user_id_t break_id,
+// lldb::user_id_t break_loc_id);
+typedef std::function
+BreakpointHitCallback;
+
typedef bool (*WatchpointHitCallback)(void *baton,
StoppointCallbackContext *context,
lldb::user_id_t watch_id);
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp
b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 6c6037dd9edd3..1db8401698114 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -102,19 +102,11 @@ const char *BreakpointOptions::g_option_names[(
"ConditionText", "IgnoreCount",
"EnabledState", "OneShotState", "AutoContinue"};
-bool BreakpointOptions::NullCallback(void *baton,
- StoppointCallbackContext *context,
- lldb::user_id_t break_id,
- lldb::user_id_t break_loc_id) {
- return true;
-}
-
// BreakpointOptions constructor
BreakpointOptions::BreakpointOptions(bool all_flags_set)
-: m_callback(BreakpointOptions::NullCallback),
- m_baton_is_command_baton(false), m_callback_is_synchronous(false),
- m_enabled(true), m_one_shot(false), m_ignore_count(0),
- m_condition_text_hash(0), m_inject_condition(false),
+: m_callback(nullptr), m_baton_is_command_baton(false),
+ m_callback_is_synchronous(false), m_enabled(true), m_one_shot(false),
+ m_ignore_count(0), m_condition_text_hash(0), m_inject_condition(false),
m_auto_continue(false), m_set_flags(0) {
if (all_flags_set)
m_set_flags.Set(~((Flags::ValueType)0));
@@ -420,7 +412,7 @@ void BreakpointOptions::SetCallback(
}
void BreakpointOptions::ClearCallback() {
- m_callback = BreakpointOptions::NullCallback;
+ m_callback = nullptr;
m_callback_is_synchronous = false;
m_callback_baton_sp.reset();
m_baton_is_command_baton = false;
@@ -449,7 +441,7 @@ bool
BreakpointOptions::InvokeCallback(StoppointCallbackContext *context,
}
bool BreakpointOptions::HasCallback() const {
- return m_callback != BreakpointOptions::NullCallback;
+ return static_cast(m_callback);
}
bool BreakpointOptions::GetCommandLineCallbacks(StringList _list) {
diff --git a/lldb/unittests/CMakeLists.txt b/lldb/unittests/CMakeLists.txt
index a2585a94b6155..cc9d45ebf981d 100644
--- a/lldb/unittests/CMakeLists.txt
+++ b/lldb/unittests/CMakeLists.txt
@@ -52,6 +52,7 @@ if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
add_subdirectory(API)
endif()
add_subdirectory(Breakpoint)
+add_subdirectory(Callback)
add_subdirectory(Core)
add_subdirectory(DataFormatter)
add_subdirectory(Disassembler)
diff --git a/lldb/unittests/Callback/CMakeLists.txt
b/lldb/unittests/Callback/CMakeLists.txt
new file mode 100644
index 0..b9e0ef5a396e3
--- /dev/null
+++ b/lldb/unittests/Callback/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_lldb_unittest(LLDBCallbackTests
+ TestBreakpointSetCallback.cpp
+
+ LINK_LIBS
+lldbBreakpoint
+lldbCore
+LLVMTestingSupport
+lldbUtilityHelpers
+lldbPluginPlatformMacOSX
+ LINK_COMPONENTS
+Support
+ )
diff --git a/lldb/unittests/Callback/TestBreakpointSetCallback.cpp
[Lldb-commits] [lldb] Add a unit test for SBBreakpoint::SetCallback (PR #96001)
https://github.com/chelcassanova updated
https://github.com/llvm/llvm-project/pull/96001
>From aa575bbc2b91837851631cf7a14e03398e2efa48 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova
Date: Thu, 13 Jun 2024 16:02:07 -0700
Subject: [PATCH] add unit test for breakpoint::setcallback
---
lldb/include/lldb/lldb-private-interfaces.h | 9 +-
lldb/source/Breakpoint/BreakpointOptions.cpp | 13 +--
lldb/unittests/CMakeLists.txt | 1 +
lldb/unittests/Callback/CMakeLists.txt| 12 +++
.../Callback/TestBreakpointSetCallback.cpp| 88 +++
5 files changed, 111 insertions(+), 12 deletions(-)
create mode 100644 lldb/unittests/Callback/CMakeLists.txt
create mode 100644 lldb/unittests/Callback/TestBreakpointSetCallback.cpp
diff --git a/lldb/include/lldb/lldb-private-interfaces.h
b/lldb/include/lldb/lldb-private-interfaces.h
index 53d5fbb84cc92..39ecf5f05fb85 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -99,10 +99,15 @@ typedef std::optional
(*SymbolLocatorLocateExecutableSymbolFile)(
typedef bool (*SymbolLocatorDownloadObjectAndSymbolFile)(
ModuleSpec _spec, Status , bool force_lookup,
bool copy_executable);
-typedef bool (*BreakpointHitCallback)(void *baton,
+// typedef bool (*BreakpointHitCallback)(void *baton,
+// StoppointCallbackContext *context,
+// lldb::user_id_t break_id,
+// lldb::user_id_t break_loc_id);
+typedef std::function
BreakpointHitCallback;
+
typedef bool (*WatchpointHitCallback)(void *baton,
StoppointCallbackContext *context,
lldb::user_id_t watch_id);
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp
b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 6c6037dd9edd3..f2ce40d02ffb4 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -102,16 +102,9 @@ const char *BreakpointOptions::g_option_names[(
"ConditionText", "IgnoreCount",
"EnabledState", "OneShotState", "AutoContinue"};
-bool BreakpointOptions::NullCallback(void *baton,
- StoppointCallbackContext *context,
- lldb::user_id_t break_id,
- lldb::user_id_t break_loc_id) {
- return true;
-}
-
// BreakpointOptions constructor
BreakpointOptions::BreakpointOptions(bool all_flags_set)
-: m_callback(BreakpointOptions::NullCallback),
+: m_callback(nullptr),
m_baton_is_command_baton(false), m_callback_is_synchronous(false),
m_enabled(true), m_one_shot(false), m_ignore_count(0),
m_condition_text_hash(0), m_inject_condition(false),
@@ -420,7 +413,7 @@ void BreakpointOptions::SetCallback(
}
void BreakpointOptions::ClearCallback() {
- m_callback = BreakpointOptions::NullCallback;
+ m_callback = nullptr;
m_callback_is_synchronous = false;
m_callback_baton_sp.reset();
m_baton_is_command_baton = false;
@@ -449,7 +442,7 @@ bool
BreakpointOptions::InvokeCallback(StoppointCallbackContext *context,
}
bool BreakpointOptions::HasCallback() const {
- return m_callback != BreakpointOptions::NullCallback;
+ return static_cast(m_callback);
}
bool BreakpointOptions::GetCommandLineCallbacks(StringList _list) {
diff --git a/lldb/unittests/CMakeLists.txt b/lldb/unittests/CMakeLists.txt
index a2585a94b6155..cc9d45ebf981d 100644
--- a/lldb/unittests/CMakeLists.txt
+++ b/lldb/unittests/CMakeLists.txt
@@ -52,6 +52,7 @@ if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
add_subdirectory(API)
endif()
add_subdirectory(Breakpoint)
+add_subdirectory(Callback)
add_subdirectory(Core)
add_subdirectory(DataFormatter)
add_subdirectory(Disassembler)
diff --git a/lldb/unittests/Callback/CMakeLists.txt
b/lldb/unittests/Callback/CMakeLists.txt
new file mode 100644
index 0..b9e0ef5a396e3
--- /dev/null
+++ b/lldb/unittests/Callback/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_lldb_unittest(LLDBCallbackTests
+ TestBreakpointSetCallback.cpp
+
+ LINK_LIBS
+lldbBreakpoint
+lldbCore
+LLVMTestingSupport
+lldbUtilityHelpers
+lldbPluginPlatformMacOSX
+ LINK_COMPONENTS
+Support
+ )
diff --git a/lldb/unittests/Callback/TestBreakpointSetCallback.cpp
b/lldb/unittests/Callback/TestBreakpointSetCallback.cpp
new file mode 100644
index 0..7318e524025e4
--- /dev/null
+++ b/lldb/unittests/Callback/TestBreakpointSetCallback.cpp
@@ -0,0 +1,88 @@
+//===-- TestBreakpointSetCallback.cpp
+//===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
[Lldb-commits] [lldb] [lldb] Fix the semantics of SupportFile equivalence (PR #95606)
jasonmolenda wrote: > Unfortunately, I don't know of a concise way to express this difference, > maybe because the concept is not concise to begin with. `IsProbablySameFile`, > `MayDescribeSameFile`, `PotentiallySameFile` ? I was thinking about this too, and the best idea I had was `IsProbablySame`. https://github.com/llvm/llvm-project/pull/95606 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
https://github.com/mbucko updated
https://github.com/llvm/llvm-project/pull/95997
>From 3ba6a550cb4731b9754646665e18fcdc6e255bd4 Mon Sep 17 00:00:00 2001
From: Miro Bucko
Date: Tue, 18 Jun 2024 14:35:55 -0700
Subject: [PATCH] [lldb] Fix SBAddressRange validation checks.
---
lldb/include/lldb/API/SBAddressRange.h| 6
lldb/include/lldb/API/SBAddressRangeList.h| 2 ++
lldb/source/API/SBAddressRange.cpp| 29 +++
lldb/source/API/SBAddressRangeList.cpp| 17 +++
.../address_range/TestAddressRange.py | 2 +-
5 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/lldb/include/lldb/API/SBAddressRange.h
b/lldb/include/lldb/API/SBAddressRange.h
index 152bd82426af1..5c4d6b8c5683d 100644
--- a/lldb/include/lldb/API/SBAddressRange.h
+++ b/lldb/include/lldb/API/SBAddressRange.h
@@ -11,6 +11,10 @@
#include "lldb/API/SBDefines.h"
+namespace lldb_private {
+class AddressRange;
+}
+
namespace lldb {
class LLDB_API SBAddressRange {
@@ -58,6 +62,8 @@ class LLDB_API SBAddressRange {
friend class SBFunction;
friend class SBProcess;
+ lldb_private::AddressRange () const;
+
AddressRangeUP m_opaque_up;
};
diff --git a/lldb/include/lldb/API/SBAddressRangeList.h
b/lldb/include/lldb/API/SBAddressRangeList.h
index a123287ef1b4f..5a4eeecf37dc9 100644
--- a/lldb/include/lldb/API/SBAddressRangeList.h
+++ b/lldb/include/lldb/API/SBAddressRangeList.h
@@ -46,6 +46,8 @@ class LLDB_API SBAddressRangeList {
friend class SBBlock;
friend class SBProcess;
+ lldb_private::AddressRangeListImpl () const;
+
std::unique_ptr m_opaque_up;
};
diff --git a/lldb/source/API/SBAddressRange.cpp
b/lldb/source/API/SBAddressRange.cpp
index 9b1affdade439..5834ebe5e63c0 100644
--- a/lldb/source/API/SBAddressRange.cpp
+++ b/lldb/source/API/SBAddressRange.cpp
@@ -50,9 +50,7 @@ const SBAddressRange ::operator=(const
SBAddressRange ) {
bool SBAddressRange::operator==(const SBAddressRange ) {
LLDB_INSTRUMENT_VA(this, rhs);
- if (!IsValid() || !rhs.IsValid())
-return false;
- return m_opaque_up->operator==(*(rhs.m_opaque_up));
+ return ref().operator==(rhs.ref());
}
bool SBAddressRange::operator!=(const SBAddressRange ) {
@@ -64,40 +62,35 @@ bool SBAddressRange::operator!=(const SBAddressRange ) {
void SBAddressRange::Clear() {
LLDB_INSTRUMENT_VA(this);
- m_opaque_up.reset();
+ ref().Clear();
}
bool SBAddressRange::IsValid() const {
LLDB_INSTRUMENT_VA(this);
- return m_opaque_up && m_opaque_up->IsValid();
+ return ref().IsValid();
}
lldb::SBAddress SBAddressRange::GetBaseAddress() const {
LLDB_INSTRUMENT_VA(this);
- if (!IsValid())
-return lldb::SBAddress();
- return lldb::SBAddress(m_opaque_up->GetBaseAddress());
+ return lldb::SBAddress(ref().GetBaseAddress());
}
lldb::addr_t SBAddressRange::GetByteSize() const {
LLDB_INSTRUMENT_VA(this);
- if (!IsValid())
-return 0;
- return m_opaque_up->GetByteSize();
+ return ref().GetByteSize();
}
bool SBAddressRange::GetDescription(SBStream ,
const SBTarget target) {
LLDB_INSTRUMENT_VA(this, description, target);
- Stream = description.ref();
- if (!IsValid()) {
-stream << "";
-return true;
- }
- m_opaque_up->GetDescription(, target.GetSP().get());
- return true;
+ return ref().GetDescription((), target.GetSP().get());
+}
+
+lldb_private::AddressRange ::ref() const {
+ assert(m_opaque_up && "opaque pointer must always be valid");
+ return *m_opaque_up;
}
diff --git a/lldb/source/API/SBAddressRangeList.cpp
b/lldb/source/API/SBAddressRangeList.cpp
index 20660b3ff2088..957155d5125ef 100644
--- a/lldb/source/API/SBAddressRangeList.cpp
+++ b/lldb/source/API/SBAddressRangeList.cpp
@@ -37,40 +37,40 @@ SBAddressRangeList::operator=(const SBAddressRangeList
) {
LLDB_INSTRUMENT_VA(this, rhs);
if (this != )
-*m_opaque_up = *rhs.m_opaque_up;
+ref() = rhs.ref();
return *this;
}
uint32_t SBAddressRangeList::GetSize() const {
LLDB_INSTRUMENT_VA(this);
- return m_opaque_up->GetSize();
+ return ref().GetSize();
}
SBAddressRange SBAddressRangeList::GetAddressRangeAtIndex(uint64_t idx) {
LLDB_INSTRUMENT_VA(this, idx);
SBAddressRange sb_addr_range;
- (*sb_addr_range.m_opaque_up) = m_opaque_up->GetAddressRangeAtIndex(idx);
+ (*sb_addr_range.m_opaque_up) = ref().GetAddressRangeAtIndex(idx);
return sb_addr_range;
}
void SBAddressRangeList::Clear() {
LLDB_INSTRUMENT_VA(this);
- m_opaque_up->Clear();
+ ref().Clear();
}
void SBAddressRangeList::Append(const SBAddressRange _addr_range) {
LLDB_INSTRUMENT_VA(this, sb_addr_range);
- m_opaque_up->Append(*sb_addr_range.m_opaque_up);
+ ref().Append(*sb_addr_range.m_opaque_up);
}
void SBAddressRangeList::Append(const SBAddressRangeList _addr_range_list) {
LLDB_INSTRUMENT_VA(this, sb_addr_range_list);
- m_opaque_up->Append(*sb_addr_range_list.m_opaque_up);
+
[Lldb-commits] [lldb] [LLDB] Add an assert to verify sign_bit_pos is within the valid range (NFC) (PR #95678)
https://github.com/fmayer approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/95678 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -64,7 +64,7 @@ bool SBAddressRange::operator!=(const SBAddressRange ) {
void SBAddressRange::Clear() {
LLDB_INSTRUMENT_VA(this);
- m_opaque_up.reset();
+ m_opaque_up->Clear();
mbucko wrote:
correct
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [DWIMPrint] Move the setting of the result status into dump_val_object (PR #96232)
https://github.com/adrian-prantl closed https://github.com/llvm/llvm-project/pull/96232 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 9473e16 - [DWIMPrint] Move the setting of the result status into dump_val_object (#96232)
Author: Adrian Prantl
Date: 2024-06-20T12:50:26-07:00
New Revision: 9473e162b92a7e0bb1471eaaa6cbd6b5fc104fed
URL:
https://github.com/llvm/llvm-project/commit/9473e162b92a7e0bb1471eaaa6cbd6b5fc104fed
DIFF:
https://github.com/llvm/llvm-project/commit/9473e162b92a7e0bb1471eaaa6cbd6b5fc104fed.diff
LOG: [DWIMPrint] Move the setting of the result status into dump_val_object
(#96232)
Previously the result would get overwritten by a success on all code
paths.
This is another NFC change for TypeSystemClang, because an object
description cannot actually fail there. It will have different behavior
in the Swift plugin.
Added:
Modified:
lldb/source/Commands/CommandObjectDWIMPrint.cpp
Removed:
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index c1549ca6933fc..b7cd955e00203 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -141,10 +141,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
maybe_add_hint(output);
result.GetOutputStream() << output;
} else {
- if (llvm::Error error =
- valobj.Dump(result.GetOutputStream(), dump_options))
+ llvm::Error error =
+valobj.Dump(result.GetOutputStream(), dump_options);
+ if (error) {
result.AppendError(toString(std::move(error)));
+return;
+ }
}
+result.SetStatus(eReturnStatusSuccessFinishResult);
};
// First, try `expr` as the name of a frame variable.
@@ -165,7 +169,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
}
dump_val_object(*valobj_sp);
- result.SetStatus(eReturnStatusSuccessFinishResult);
return;
}
}
@@ -176,7 +179,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
if (auto var_sp = state->GetVariable(expr))
if (auto valobj_sp = var_sp->GetValueObject()) {
dump_val_object(*valobj_sp);
- result.SetStatus(eReturnStatusSuccessFinishResult);
return;
}
@@ -197,34 +199,36 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
error_stream << "" << fixed_expression << "\n";
}
-if (expr_result == eExpressionCompleted) {
- if (verbosity != eDWIMPrintVerbosityNone) {
-StringRef flags;
-if (args.HasArgs())
- flags = args.GetArgStringWithDelimiter();
-result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
-expr);
- }
-
- if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
-dump_val_object(*valobj_sp);
-
- if (suppress_result)
-if (auto result_var_sp =
-target.GetPersistentVariable(valobj_sp->GetName())) {
- auto language = valobj_sp->GetPreferredDisplayLanguage();
- if (auto *persistent_state =
- target.GetPersistentExpressionStateForLanguage(language))
-persistent_state->RemovePersistentVariable(result_var_sp);
-}
-
- result.SetStatus(eReturnStatusSuccessFinishResult);
-} else {
+// If the expression failed, return an error.
+if (expr_result != eExpressionCompleted) {
if (valobj_sp)
result.SetError(valobj_sp->GetError());
else
result.AppendErrorWithFormatv(
"unknown error evaluating expression `{0}`", expr);
+ return;
+}
+
+if (verbosity != eDWIMPrintVerbosityNone) {
+ StringRef flags;
+ if (args.HasArgs())
+flags = args.GetArgStringWithDelimiter();
+ result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
+ expr);
}
+
+if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+ dump_val_object(*valobj_sp);
+else
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+
+if (suppress_result)
+ if (auto result_var_sp =
+ target.GetPersistentVariable(valobj_sp->GetName())) {
+auto language = valobj_sp->GetPreferredDisplayLanguage();
+if (auto *persistent_state =
+target.GetPersistentExpressionStateForLanguage(language))
+ persistent_state->RemovePersistentVariable(result_var_sp);
+ }
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [DWIMPrint] Move the setting of the result status into dump_val_object (PR #96232)
https://github.com/kastiglione approved this pull request. thanks https://github.com/llvm/llvm-project/pull/96232 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [DWIMPrint] Move the setting of the result status into dump_val_object (PR #96232)
https://github.com/adrian-prantl edited https://github.com/llvm/llvm-project/pull/96232 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [DWIMPrint] Move the setting of the result status into dump_val_object (PR #96232)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/96232
>From 73309c4f67ea550b9af336cecc30dd242d89c08b Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Thu, 20 Jun 2024 12:38:58 -0700
Subject: [PATCH] [DWIMPrint] Move the setting of the result status into
dump_val_object
Previously the result would get overwritten by a succ on all code
paths.
This is another NFC change for TypeSystemClang, because an object
description cannot actually fail there. It will have different
behavior in the Swift plugin.
---
.../Commands/CommandObjectDWIMPrint.cpp | 58 ++-
1 file changed, 31 insertions(+), 27 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index c1549ca6933fc..b7cd955e00203 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -141,10 +141,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
maybe_add_hint(output);
result.GetOutputStream() << output;
} else {
- if (llvm::Error error =
- valobj.Dump(result.GetOutputStream(), dump_options))
+ llvm::Error error =
+valobj.Dump(result.GetOutputStream(), dump_options);
+ if (error) {
result.AppendError(toString(std::move(error)));
+return;
+ }
}
+result.SetStatus(eReturnStatusSuccessFinishResult);
};
// First, try `expr` as the name of a frame variable.
@@ -165,7 +169,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
}
dump_val_object(*valobj_sp);
- result.SetStatus(eReturnStatusSuccessFinishResult);
return;
}
}
@@ -176,7 +179,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
if (auto var_sp = state->GetVariable(expr))
if (auto valobj_sp = var_sp->GetValueObject()) {
dump_val_object(*valobj_sp);
- result.SetStatus(eReturnStatusSuccessFinishResult);
return;
}
@@ -197,34 +199,36 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
error_stream << "" << fixed_expression << "\n";
}
-if (expr_result == eExpressionCompleted) {
- if (verbosity != eDWIMPrintVerbosityNone) {
-StringRef flags;
-if (args.HasArgs())
- flags = args.GetArgStringWithDelimiter();
-result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
-expr);
- }
-
- if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
-dump_val_object(*valobj_sp);
-
- if (suppress_result)
-if (auto result_var_sp =
-target.GetPersistentVariable(valobj_sp->GetName())) {
- auto language = valobj_sp->GetPreferredDisplayLanguage();
- if (auto *persistent_state =
- target.GetPersistentExpressionStateForLanguage(language))
-persistent_state->RemovePersistentVariable(result_var_sp);
-}
-
- result.SetStatus(eReturnStatusSuccessFinishResult);
-} else {
+// If the expression failed, return an error.
+if (expr_result != eExpressionCompleted) {
if (valobj_sp)
result.SetError(valobj_sp->GetError());
else
result.AppendErrorWithFormatv(
"unknown error evaluating expression `{0}`", expr);
+ return;
+}
+
+if (verbosity != eDWIMPrintVerbosityNone) {
+ StringRef flags;
+ if (args.HasArgs())
+flags = args.GetArgStringWithDelimiter();
+ result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
+ expr);
}
+
+if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+ dump_val_object(*valobj_sp);
+else
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+
+if (suppress_result)
+ if (auto result_var_sp =
+ target.GetPersistentVariable(valobj_sp->GetName())) {
+auto language = valobj_sp->GetPreferredDisplayLanguage();
+if (auto *persistent_state =
+target.GetPersistentExpressionStateForLanguage(language))
+ persistent_state->RemovePersistentVariable(result_var_sp);
+ }
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [DWIMPrint] Move the setting of the result status into dump_val_object (PR #96232)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Adrian Prantl (adrian-prantl)
Changes
This is another NFC change for TypeSystemClang, because an object description
cannot actually fail there. It will have different behavior in the Swift plugin.
---
Full diff: https://github.com/llvm/llvm-project/pull/96232.diff
1 Files Affected:
- (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+31-27)
``diff
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index c1549ca6933fc..b7cd955e00203 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -141,10 +141,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
maybe_add_hint(output);
result.GetOutputStream() << output;
} else {
- if (llvm::Error error =
- valobj.Dump(result.GetOutputStream(), dump_options))
+ llvm::Error error =
+valobj.Dump(result.GetOutputStream(), dump_options);
+ if (error) {
result.AppendError(toString(std::move(error)));
+return;
+ }
}
+result.SetStatus(eReturnStatusSuccessFinishResult);
};
// First, try `expr` as the name of a frame variable.
@@ -165,7 +169,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
}
dump_val_object(*valobj_sp);
- result.SetStatus(eReturnStatusSuccessFinishResult);
return;
}
}
@@ -176,7 +179,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
if (auto var_sp = state->GetVariable(expr))
if (auto valobj_sp = var_sp->GetValueObject()) {
dump_val_object(*valobj_sp);
- result.SetStatus(eReturnStatusSuccessFinishResult);
return;
}
@@ -197,34 +199,36 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
error_stream << "" << fixed_expression << "\n";
}
-if (expr_result == eExpressionCompleted) {
- if (verbosity != eDWIMPrintVerbosityNone) {
-StringRef flags;
-if (args.HasArgs())
- flags = args.GetArgStringWithDelimiter();
-result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
-expr);
- }
-
- if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
-dump_val_object(*valobj_sp);
-
- if (suppress_result)
-if (auto result_var_sp =
-target.GetPersistentVariable(valobj_sp->GetName())) {
- auto language = valobj_sp->GetPreferredDisplayLanguage();
- if (auto *persistent_state =
- target.GetPersistentExpressionStateForLanguage(language))
-persistent_state->RemovePersistentVariable(result_var_sp);
-}
-
- result.SetStatus(eReturnStatusSuccessFinishResult);
-} else {
+// If the expression failed, return an error.
+if (expr_result != eExpressionCompleted) {
if (valobj_sp)
result.SetError(valobj_sp->GetError());
else
result.AppendErrorWithFormatv(
"unknown error evaluating expression `{0}`", expr);
+ return;
+}
+
+if (verbosity != eDWIMPrintVerbosityNone) {
+ StringRef flags;
+ if (args.HasArgs())
+flags = args.GetArgStringWithDelimiter();
+ result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
+ expr);
}
+
+if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+ dump_val_object(*valobj_sp);
+else
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+
+if (suppress_result)
+ if (auto result_var_sp =
+ target.GetPersistentVariable(valobj_sp->GetName())) {
+auto language = valobj_sp->GetPreferredDisplayLanguage();
+if (auto *persistent_state =
+target.GetPersistentExpressionStateForLanguage(language))
+ persistent_state->RemovePersistentVariable(result_var_sp);
+ }
}
}
``
https://github.com/llvm/llvm-project/pull/96232
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [DWIMPrint] Move the setting of the result status into dump_val_object (PR #96232)
https://github.com/adrian-prantl created
https://github.com/llvm/llvm-project/pull/96232
This is another NFC change for TypeSystemClang, because an object description
cannot actually fail there. It will have different behavior in the Swift plugin.
>From 942199ba4e70c4dc3bfbf7182e34863daf352f73 Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Thu, 20 Jun 2024 12:38:58 -0700
Subject: [PATCH] [DWIMPrint] Move the setting of the result status into
dump_val_object
This is another NFC change for TypeSystemClang, because an object
description cannot actually fail there. It will have different
behavior in the Swift plugin.
---
.../Commands/CommandObjectDWIMPrint.cpp | 58 ++-
1 file changed, 31 insertions(+), 27 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index c1549ca6933fc..b7cd955e00203 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -141,10 +141,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
maybe_add_hint(output);
result.GetOutputStream() << output;
} else {
- if (llvm::Error error =
- valobj.Dump(result.GetOutputStream(), dump_options))
+ llvm::Error error =
+valobj.Dump(result.GetOutputStream(), dump_options);
+ if (error) {
result.AppendError(toString(std::move(error)));
+return;
+ }
}
+result.SetStatus(eReturnStatusSuccessFinishResult);
};
// First, try `expr` as the name of a frame variable.
@@ -165,7 +169,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
}
dump_val_object(*valobj_sp);
- result.SetStatus(eReturnStatusSuccessFinishResult);
return;
}
}
@@ -176,7 +179,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
if (auto var_sp = state->GetVariable(expr))
if (auto valobj_sp = var_sp->GetValueObject()) {
dump_val_object(*valobj_sp);
- result.SetStatus(eReturnStatusSuccessFinishResult);
return;
}
@@ -197,34 +199,36 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
error_stream << "" << fixed_expression << "\n";
}
-if (expr_result == eExpressionCompleted) {
- if (verbosity != eDWIMPrintVerbosityNone) {
-StringRef flags;
-if (args.HasArgs())
- flags = args.GetArgStringWithDelimiter();
-result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
-expr);
- }
-
- if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
-dump_val_object(*valobj_sp);
-
- if (suppress_result)
-if (auto result_var_sp =
-target.GetPersistentVariable(valobj_sp->GetName())) {
- auto language = valobj_sp->GetPreferredDisplayLanguage();
- if (auto *persistent_state =
- target.GetPersistentExpressionStateForLanguage(language))
-persistent_state->RemovePersistentVariable(result_var_sp);
-}
-
- result.SetStatus(eReturnStatusSuccessFinishResult);
-} else {
+// If the expression failed, return an error.
+if (expr_result != eExpressionCompleted) {
if (valobj_sp)
result.SetError(valobj_sp->GetError());
else
result.AppendErrorWithFormatv(
"unknown error evaluating expression `{0}`", expr);
+ return;
+}
+
+if (verbosity != eDWIMPrintVerbosityNone) {
+ StringRef flags;
+ if (args.HasArgs())
+flags = args.GetArgStringWithDelimiter();
+ result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
+ expr);
}
+
+if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+ dump_val_object(*valobj_sp);
+else
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+
+if (suppress_result)
+ if (auto result_var_sp =
+ target.GetPersistentVariable(valobj_sp->GetName())) {
+auto language = valobj_sp->GetPreferredDisplayLanguage();
+if (auto *persistent_state =
+target.GetPersistentExpressionStateForLanguage(language))
+ persistent_state->RemovePersistentVariable(result_var_sp);
+ }
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][AST] fix ast-print of extern with >=2 declarators, fixed (PR #93913)
temyurchenko wrote: > Do you need someone to land these changes on your behalf? Yeah, I don't have the rights :( https://github.com/llvm/llvm-project/pull/93913 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2800,6 +2809,10 @@ void PruneThreadPlans(); virtual size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, Status ) = 0; + void DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr, + const uint8_t *buf, size_t size, AddressRanges , + size_t alignment, size_t max_matches); + mbucko wrote: Yeah possibly the FindRangesInMemory too. I was going to add that later once I know exactly how I'm going to override these. https://github.com/llvm/llvm-project/pull/95007 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -101,3 +101,5 @@ bool SBAddressRange::GetDescription(SBStream ,
m_opaque_up->GetDescription(, target.GetSP().get());
return true;
}
+
+lldb_private::AddressRange ::ref() const { return *m_opaque_up;
}
clayborg wrote:
This is no longer needed right? This PR contains this fix right?
https://github.com/llvm/llvm-project/pull/95997
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2800,6 +2809,10 @@ void PruneThreadPlans(); virtual size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, Status ) = 0; + void DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr, + const uint8_t *buf, size_t size, AddressRanges , + size_t alignment, size_t max_matches); + clayborg wrote: We might want this to be virtual to allow core files to do this quicker than having to read all memory into a buffer. https://github.com/llvm/llvm-project/pull/95007 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -92,3 +92,7 @@ bool SBAddressRangeList::GetDescription(SBStream ,
stream << "]";
return true;
}
+
+lldb_private::AddressRanges ::ref() const {
+ return m_opaque_up->ref();
+}
clayborg wrote:
we might want to add an `assert()` call here like you did for
`SBAddressRange::ref()`?
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -64,7 +64,7 @@ bool SBAddressRange::operator!=(const SBAddressRange ) {
void SBAddressRange::Clear() {
LLDB_INSTRUMENT_VA(this);
- m_opaque_up.reset();
+ m_opaque_up->Clear();
clayborg wrote:
This is no longer needed right? This PR contains this fix right?
https://github.com/llvm/llvm-project/pull/95997
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -2007,6 +2007,135 @@ size_t Process::ReadMemory(addr_t addr, void *buf,
size_t size, Status ) {
}
}
+void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr,
+ const uint8_t *buf, size_t size,
+ AddressRanges , size_t alignment,
+ size_t max_matches) {
+ // Inputs are already validated in FindInMemory() functions.
+ assert(buf != nullptr);
+ assert(size > 0);
+ assert(alignment > 0);
+ assert(max_matches > 0);
+ assert(start_addr != LLDB_INVALID_ADDRESS);
+ assert(end_addr != LLDB_INVALID_ADDRESS);
+ assert(start_addr < end_addr);
+
+ lldb::addr_t start = start_addr;
+ if (alignment > 1) {
+// Align to an address alignment boundary
+const uint64_t align_offset = start % alignment;
+if (align_offset > 0)
+ start += alignment - align_offset;
+ }
+ while (matches.size() < max_matches && (start + size) < end_addr) {
+const lldb::addr_t found_addr = FindInMemory(start, end_addr, buf, size);
+if (found_addr == LLDB_INVALID_ADDRESS)
+ break;
+
+if (found_addr % alignment) {
+ ++start;
+ continue;
+}
+
+matches.emplace_back(found_addr, size);
+start = found_addr + alignment;
+ }
+}
+
+AddressRanges Process::FindRangesInMemory(const uint8_t *buf, uint64_t size,
+ const AddressRanges ,
+ size_t alignment, size_t max_matches,
+ Status ) {
+ AddressRanges matches;
+ if (buf == nullptr) {
+error.SetErrorString("buffer is null");
+return matches;
+ }
+ if (size == 0) {
+error.SetErrorString("buffer size is zero");
+return matches;
+ }
+ if (ranges.empty()) {
+error.SetErrorString("empty ranges");
+return matches;
+ }
+ if (alignment == 0) {
+error.SetErrorString("alignment must be greater than zero");
+return matches;
+ }
+ if (max_matches == 0) {
+error.SetErrorStringWithFormat("max_matches must be greater than zero");
clayborg wrote:
use `error.SetErrorString()` if there are no formats being applied
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
@@ -58,6 +58,8 @@ class LLDB_API SBAddressRange {
friend class SBFunction;
friend class SBProcess;
+ lldb_private::AddressRange () const;
+
clayborg wrote:
This is no longer needed right? This PR contains this fix right?
https://github.com/llvm/llvm-project/pull/95997
https://github.com/llvm/llvm-project/pull/95007
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/95997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Fix type definition search with simple template names (PR #95905)
@@ -3073,14 +3073,43 @@
SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE ) {
// See comments below about -gsimple-template-names for why we attempt to
// compute missing template parameter names.
-ConstString template_params;
-if (type_system) {
- DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
- if (dwarf_ast)
-template_params = dwarf_ast->GetDIEClassTemplateParams(die);
+std::vector template_params;
+DWARFDeclContext die_dwarf_decl_ctx;
+DWARFASTParser *dwarf_ast = type_system ? type_system->GetDWARFParser() :
nullptr;
+for (DWARFDIE ctx_die = die; ctx_die && !isUnitType(ctx_die.Tag());
+ ctx_die = ctx_die.GetParentDeclContextDIE()) {
+ die_dwarf_decl_ctx.AppendDeclContext(ctx_die.Tag(), ctx_die.GetName());
+ template_params.push_back(
+ (ctx_die.IsStructUnionOrClass() && dwarf_ast)
+ ? dwarf_ast->GetDIEClassTemplateParams(ctx_die)
+ : "");
}
+const bool any_template_params = llvm::any_of(
+template_params, [](llvm::StringRef p) { return !p.empty(); });
-const DWARFDeclContext die_dwarf_decl_ctx = die.GetDWARFDeclContext();
+auto die_matches = [&](DWARFDIE type_die) {
+ // Resolve the type if both have the same tag or {class, struct} tags.
+ const bool tag_matches =
+ type_die.Tag() == tag ||
+ (IsStructOrClassTag(type_die.Tag()) && IsStructOrClassTag(tag));
dwblaikie wrote:
FWIW, I'm pretty sure that the only difference between class and struct is the
access control.
Though C++ technically rejects things like forward declaring a thing as a
struct, then actually defining it as a class - not sure if that'd ever come up
for lldb (for instance when interacting with clang header modules? If the DWARF
contained a declaration of a thing and always created it as a struct, but then
tried to load a module with that name as a class definition) I guess it'd also
maybe break technically valid user code in expression evaluation - if the name
was used for a varibale and a type, the user should be able to disambiguate in
favor of the type by saying, eg: "sizeof(class MyType)" but if lldb made
everything a struct ... hmm, nope, that'd be fine:
```
$ cat test.cpp
struct MyType { };
int MyType = 3;
static_assert(sizeof(class MyType) == 1);
$ clang++-tot test.cpp -fsyntax-only
```
I guess printing types in lldb would be technically wrong, since it'd print as
"struct MyType {" instead of "class MyType {"?
(but it might mean not having to have the access modifier hack? If everything
was made as a struct, and none of the members were given other access modifiers
- everything would be accessible by default anyway)
```
https://github.com/llvm/llvm-project/pull/95905
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make LanguageRuntime::GetTypeBitSize return an optional (NFC) (PR #96013)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/96013 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 5e9f247 - [lldb] Make LanguageRuntime::GetTypeBitSize return an optional (NFC) (#96013)
Author: Jonas Devlieghere
Date: 2024-06-20T10:46:26-07:00
New Revision: 5e9f247c064cb2361cd641f62eb4b7049d21641a
URL:
https://github.com/llvm/llvm-project/commit/5e9f247c064cb2361cd641f62eb4b7049d21641a
DIFF:
https://github.com/llvm/llvm-project/commit/5e9f247c064cb2361cd641f62eb4b7049d21641a.diff
LOG: [lldb] Make LanguageRuntime::GetTypeBitSize return an optional (NFC)
(#96013)
Make LanguageRuntime::GetTypeBitSize return an optional. This should be
NFC, though the ObjCLanguageRuntime implementation is (possibly) more
defensive against returning 0.
I'm not sure if it's possible for both `m_ivar.size` and `m_ivar.offset`
to be zero. Previously, we'd return 0 and cache it, only to discard it
the next time when finding it in the cache, and recomputing it again.
The new code will avoid putting it in the cache in the first place.
Added:
Modified:
lldb/include/lldb/Target/LanguageRuntime.h
lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
Removed:
diff --git a/lldb/include/lldb/Target/LanguageRuntime.h
b/lldb/include/lldb/Target/LanguageRuntime.h
index d093dccfeae85..954d454c78589 100644
--- a/lldb/include/lldb/Target/LanguageRuntime.h
+++ b/lldb/include/lldb/Target/LanguageRuntime.h
@@ -170,9 +170,9 @@ class LanguageRuntime : public Runtime, public
PluginInterface {
return m_process->GetTarget().GetSearchFilterForModule(nullptr);
}
- virtual bool GetTypeBitSize(const CompilerType _type,
- uint64_t ) {
-return false;
+ virtual std::optional
+ GetTypeBitSize(const CompilerType _type) {
+return {};
}
virtual void SymbolsDidLoad(const ModuleList _list) {}
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
index ba52444f0c2fc..a812ffba7a648 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
@@ -350,18 +350,17 @@ ObjCLanguageRuntime::EncodingToTypeSP
ObjCLanguageRuntime::GetEncodingToType() {
return nullptr;
}
-bool ObjCLanguageRuntime::GetTypeBitSize(const CompilerType _type,
- uint64_t ) {
+std::optional
+ObjCLanguageRuntime::GetTypeBitSize(const CompilerType _type) {
void *opaque_ptr = compiler_type.GetOpaqueQualType();
- size = m_type_size_cache.Lookup(opaque_ptr);
- // an ObjC object will at least have an ISA, so 0 is definitely not OK
- if (size > 0)
-return true;
+ uint64_t cached_size = m_type_size_cache.Lookup(opaque_ptr);
+ if (cached_size > 0)
+return cached_size;
ClassDescriptorSP class_descriptor_sp =
GetClassDescriptorFromClassName(compiler_type.GetTypeName());
if (!class_descriptor_sp)
-return false;
+return {};
int32_t max_offset = INT32_MIN;
uint64_t sizeof_max = 0;
@@ -377,11 +376,13 @@ bool ObjCLanguageRuntime::GetTypeBitSize(const
CompilerType _type,
}
}
- size = 8 * (max_offset + sizeof_max);
- if (found)
+ uint64_t size = 8 * (max_offset + sizeof_max);
+ if (found && size > 0) {
m_type_size_cache.Insert(opaque_ptr, size);
+return size;
+ }
- return found;
+ return {};
}
lldb::BreakpointPreconditionSP
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
index 0a8b6e8d56ed0..ffe9725fa6826 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -107,7 +107,7 @@ class ObjCLanguageRuntime : public LanguageRuntime {
int64_t *value_bits = nullptr,
uint64_t *payload = nullptr) = 0;
/// @}
-
+
virtual uint64_t GetInstanceSize() = 0;
// use to implement version-specific additional constraints on pointers
@@ -321,8 +321,8 @@ class ObjCLanguageRuntime : public LanguageRuntime {
m_negative_complete_class_cache.clear();
}
- bool GetTypeBitSize(const CompilerType _type,
- uint64_t ) override;
+ std::optional
+ GetTypeBitSize(const CompilerType _type) override;
/// Check whether the name is "self" or "_cmd" and should show up in
/// "frame variable".
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 0270c67366d1d..eb7f6232ac981 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4744,11 +4744,11 @@
TypeSystemClang::GetBitSize(lldb::opaque_compiler_type_t type,
ExecutionContext
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/95997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
https://github.com/mbucko updated
https://github.com/llvm/llvm-project/pull/95997
>From ca4dcf373b0617ab588ee804e3d28f8d311da06a Mon Sep 17 00:00:00 2001
From: Miro Bucko
Date: Tue, 18 Jun 2024 14:35:55 -0700
Subject: [PATCH] [lldb] Fix SBAddressRange validation checks.
---
lldb/include/lldb/API/SBAddressRange.h| 6
lldb/include/lldb/API/SBAddressRangeList.h| 2 ++
lldb/source/API/SBAddressRange.cpp| 29 +++
lldb/source/API/SBAddressRangeList.cpp| 17 +++
.../address_range/TestAddressRange.py | 2 +-
5 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/lldb/include/lldb/API/SBAddressRange.h
b/lldb/include/lldb/API/SBAddressRange.h
index 152bd82426af1..5c4d6b8c5683d 100644
--- a/lldb/include/lldb/API/SBAddressRange.h
+++ b/lldb/include/lldb/API/SBAddressRange.h
@@ -11,6 +11,10 @@
#include "lldb/API/SBDefines.h"
+namespace lldb_private {
+class AddressRange;
+}
+
namespace lldb {
class LLDB_API SBAddressRange {
@@ -58,6 +62,8 @@ class LLDB_API SBAddressRange {
friend class SBFunction;
friend class SBProcess;
+ lldb_private::AddressRange () const;
+
AddressRangeUP m_opaque_up;
};
diff --git a/lldb/include/lldb/API/SBAddressRangeList.h
b/lldb/include/lldb/API/SBAddressRangeList.h
index a123287ef1b4f..5a4eeecf37dc9 100644
--- a/lldb/include/lldb/API/SBAddressRangeList.h
+++ b/lldb/include/lldb/API/SBAddressRangeList.h
@@ -46,6 +46,8 @@ class LLDB_API SBAddressRangeList {
friend class SBBlock;
friend class SBProcess;
+ lldb_private::AddressRangeListImpl () const;
+
std::unique_ptr m_opaque_up;
};
diff --git a/lldb/source/API/SBAddressRange.cpp
b/lldb/source/API/SBAddressRange.cpp
index 9b1affdade439..5834ebe5e63c0 100644
--- a/lldb/source/API/SBAddressRange.cpp
+++ b/lldb/source/API/SBAddressRange.cpp
@@ -50,9 +50,7 @@ const SBAddressRange ::operator=(const
SBAddressRange ) {
bool SBAddressRange::operator==(const SBAddressRange ) {
LLDB_INSTRUMENT_VA(this, rhs);
- if (!IsValid() || !rhs.IsValid())
-return false;
- return m_opaque_up->operator==(*(rhs.m_opaque_up));
+ return ref().operator==(rhs.ref());
}
bool SBAddressRange::operator!=(const SBAddressRange ) {
@@ -64,40 +62,35 @@ bool SBAddressRange::operator!=(const SBAddressRange ) {
void SBAddressRange::Clear() {
LLDB_INSTRUMENT_VA(this);
- m_opaque_up.reset();
+ ref().Clear();
}
bool SBAddressRange::IsValid() const {
LLDB_INSTRUMENT_VA(this);
- return m_opaque_up && m_opaque_up->IsValid();
+ return ref().IsValid();
}
lldb::SBAddress SBAddressRange::GetBaseAddress() const {
LLDB_INSTRUMENT_VA(this);
- if (!IsValid())
-return lldb::SBAddress();
- return lldb::SBAddress(m_opaque_up->GetBaseAddress());
+ return lldb::SBAddress(ref().GetBaseAddress());
}
lldb::addr_t SBAddressRange::GetByteSize() const {
LLDB_INSTRUMENT_VA(this);
- if (!IsValid())
-return 0;
- return m_opaque_up->GetByteSize();
+ return ref().GetByteSize();
}
bool SBAddressRange::GetDescription(SBStream ,
const SBTarget target) {
LLDB_INSTRUMENT_VA(this, description, target);
- Stream = description.ref();
- if (!IsValid()) {
-stream << "";
-return true;
- }
- m_opaque_up->GetDescription(, target.GetSP().get());
- return true;
+ return ref().GetDescription((), target.GetSP().get());
+}
+
+lldb_private::AddressRange ::ref() const {
+ assert(m_opaque_up && "opaque pointer must always be valid");
+ return *m_opaque_up;
}
diff --git a/lldb/source/API/SBAddressRangeList.cpp
b/lldb/source/API/SBAddressRangeList.cpp
index 20660b3ff2088..957155d5125ef 100644
--- a/lldb/source/API/SBAddressRangeList.cpp
+++ b/lldb/source/API/SBAddressRangeList.cpp
@@ -37,40 +37,40 @@ SBAddressRangeList::operator=(const SBAddressRangeList
) {
LLDB_INSTRUMENT_VA(this, rhs);
if (this != )
-*m_opaque_up = *rhs.m_opaque_up;
+ref() = rhs.ref();
return *this;
}
uint32_t SBAddressRangeList::GetSize() const {
LLDB_INSTRUMENT_VA(this);
- return m_opaque_up->GetSize();
+ return ref().GetSize();
}
SBAddressRange SBAddressRangeList::GetAddressRangeAtIndex(uint64_t idx) {
LLDB_INSTRUMENT_VA(this, idx);
SBAddressRange sb_addr_range;
- (*sb_addr_range.m_opaque_up) = m_opaque_up->GetAddressRangeAtIndex(idx);
+ (*sb_addr_range.m_opaque_up) = ref().GetAddressRangeAtIndex(idx);
return sb_addr_range;
}
void SBAddressRangeList::Clear() {
LLDB_INSTRUMENT_VA(this);
- m_opaque_up->Clear();
+ ref().Clear();
}
void SBAddressRangeList::Append(const SBAddressRange _addr_range) {
LLDB_INSTRUMENT_VA(this, sb_addr_range);
- m_opaque_up->Append(*sb_addr_range.m_opaque_up);
+ ref().Append(*sb_addr_range.m_opaque_up);
}
void SBAddressRangeList::Append(const SBAddressRangeList _addr_range_list) {
LLDB_INSTRUMENT_VA(this, sb_addr_range_list);
- m_opaque_up->Append(*sb_addr_range_list.m_opaque_up);
+
[Lldb-commits] [lldb] Convert ValueObject::Dump() to return llvm::Error() (NFCish) (PR #95857)
https://github.com/adrian-prantl closed https://github.com/llvm/llvm-project/pull/95857 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Convert ValueObject::Dump() to return llvm::Error() (NFCish) (PR #95857)
adrian-prantl wrote: Landed in f1edc0459a4cc8cd4d00331c2a5cb318955318b2 https://github.com/llvm/llvm-project/pull/95857 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f1edc04 - Reformat test (NFC)
Author: Adrian Prantl
Date: 2024-06-20T10:32:07-07:00
New Revision: f1edc0459a4cc8cd4d00331c2a5cb318955318b2
URL:
https://github.com/llvm/llvm-project/commit/f1edc0459a4cc8cd4d00331c2a5cb318955318b2
DIFF:
https://github.com/llvm/llvm-project/commit/f1edc0459a4cc8cd4d00331c2a5cb318955318b2.diff
LOG: Reformat test (NFC)
Added:
Modified:
lldb/test/API/commands/dwim-print/TestDWIMPrint.py
Removed:
diff --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index 785a9621c99dc..b40924a182e37 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -121,9 +121,7 @@ def test_empty_expression(self):
def test_nested_values(self):
"""Test dwim-print with nested values (structs, etc)."""
self.build()
-lldbutil.run_to_source_breakpoint(
-self, "break here", lldb.SBFileSpec("main.c")
-)
+lldbutil.run_to_source_breakpoint(self, "break here",
lldb.SBFileSpec("main.c"))
self.runCmd("settings set auto-one-line-summaries false")
self._expect_cmd(f"dwim-print s", "frame variable")
self._expect_cmd(f"dwim-print (struct Structure)s", "expression")
@@ -131,9 +129,7 @@ def test_nested_values(self):
def test_summary_strings(self):
"""Test dwim-print with nested values (structs, etc)."""
self.build()
-lldbutil.run_to_source_breakpoint(
-self, "break here", lldb.SBFileSpec("main.c")
-)
+lldbutil.run_to_source_breakpoint(self, "break here",
lldb.SBFileSpec("main.c"))
self.runCmd("settings set auto-one-line-summaries false")
self.runCmd("type summary add -e -s 'stub summary' Structure")
self._expect_cmd(f"dwim-print s", "frame variable")
@@ -142,17 +138,13 @@ def test_summary_strings(self):
def test_void_result(self):
"""Test dwim-print does not surface an error message for void
expressions."""
self.build()
-lldbutil.run_to_source_breakpoint(
-self, "break here", lldb.SBFileSpec("main.c")
-)
+lldbutil.run_to_source_breakpoint(self, "break here",
lldb.SBFileSpec("main.c"))
self.expect("dwim-print (void)15", matching=False,
patterns=["(?i)error"])
def test_preserves_persistent_variables(self):
"""Test dwim-print does not delete persistent variables."""
self.build()
-lldbutil.run_to_source_breakpoint(
-self, "break here", lldb.SBFileSpec("main.c")
-)
+lldbutil.run_to_source_breakpoint(self, "break here",
lldb.SBFileSpec("main.c"))
self.expect("dwim-print int $i = 15")
# Run the same expression twice and verify success. This ensures the
# first command does not delete the persistent variable.
@@ -162,7 +154,5 @@ def test_preserves_persistent_variables(self):
def test_missing_type(self):
"""The expected output of po opaque is its address (no error)"""
self.build()
-lldbutil.run_to_source_breakpoint(
-self, "break here", lldb.SBFileSpec("main.c")
-)
-self.expect("dwim-print -O -- opaque", substrs=['0x'])
+lldbutil.run_to_source_breakpoint(self, "break here",
lldb.SBFileSpec("main.c"))
+self.expect("dwim-print -O -- opaque", substrs=["0x"])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] b8f0ca0 - Factor out expression result error strings.
Author: Adrian Prantl
Date: 2024-06-20T10:32:06-07:00
New Revision: b8f0ca09b66716fc337448a2e43146038bf115ab
URL:
https://github.com/llvm/llvm-project/commit/b8f0ca09b66716fc337448a2e43146038bf115ab
DIFF:
https://github.com/llvm/llvm-project/commit/b8f0ca09b66716fc337448a2e43146038bf115ab.diff
LOG: Factor out expression result error strings.
Added:
lldb/include/lldb/Utility/ErrorMessages.h
lldb/source/Utility/ErrorMessages.cpp
Modified:
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/source/Utility/CMakeLists.txt
Removed:
diff --git a/lldb/include/lldb/Utility/ErrorMessages.h
b/lldb/include/lldb/Utility/ErrorMessages.h
new file mode 100644
index 0..116e29e0e6187
--- /dev/null
+++ b/lldb/include/lldb/Utility/ErrorMessages.h
@@ -0,0 +1,22 @@
+//===-- ErrorMessages.h -*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_UTILITY_ERROR_MESSAGES_H
+#define LLDB_UTILITY_ERROR_MESSAGES_H
+
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include
+
+namespace lldb_private {
+
+/// Produce a human-readable rendition of an ExpressionResults value.
+std::string toString(lldb::ExpressionResults e);
+
+} // namespace lldb_private
+#endif
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index da995de1407c4..40c915b2c94fc 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -48,6 +48,7 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Host/StreamFile.h"
+#include "lldb/Utility/ErrorMessages.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/State.h"
@@ -1817,51 +1818,8 @@ CommandInterpreter::PreprocessToken(std::string
_str) {
error = expr_result_valobj_sp->GetError();
if (error.Success()) {
-switch (expr_result) {
-case eExpressionSetupError:
- error.SetErrorStringWithFormat(
- "expression setup error for the expression '%s'", expr_str.c_str());
- break;
-case eExpressionParseError:
- error.SetErrorStringWithFormat(
- "expression parse error for the expression '%s'", expr_str.c_str());
- break;
-case eExpressionResultUnavailable:
- error.SetErrorStringWithFormat(
- "expression error fetching result for the expression '%s'",
- expr_str.c_str());
- break;
-case eExpressionCompleted:
- break;
-case eExpressionDiscarded:
- error.SetErrorStringWithFormat(
- "expression discarded for the expression '%s'", expr_str.c_str());
- break;
-case eExpressionInterrupted:
- error.SetErrorStringWithFormat(
- "expression interrupted for the expression '%s'", expr_str.c_str());
- break;
-case eExpressionHitBreakpoint:
- error.SetErrorStringWithFormat(
- "expression hit breakpoint for the expression '%s'",
- expr_str.c_str());
- break;
-case eExpressionTimedOut:
- error.SetErrorStringWithFormat(
- "expression timed out for the expression '%s'", expr_str.c_str());
- break;
-case eExpressionStoppedForDebug:
- error.SetErrorStringWithFormat("expression stop at entry point "
- "for debugging for the "
- "expression '%s'",
- expr_str.c_str());
- break;
-case eExpressionThreadVanished:
- error.SetErrorStringWithFormat(
- "expression thread vanished for the expression '%s'",
- expr_str.c_str());
- break;
-}
+std::string result = lldb_private::toString(expr_result);
+error.SetErrorString(result + "for the expression '" + expr_str + "'");
}
return error;
}
diff --git
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
index 800eda925591e..5ff267720629e 100644
---
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -31,6 +31,7 @@
#include "lldb/Target/Target.h"
#include "lldb/Target/Thread.h"
#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/ErrorMessages.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Scalar.h"
@@ -201,7 +202,7 @@
[Lldb-commits] [lldb] f900644 - Refactor GetObjectDescription() to return llvm::Expected (NFC)
Author: Adrian Prantl
Date: 2024-06-20T10:32:06-07:00
New Revision: f900644ae2b6c7a485673974688a62c3f3301dcc
URL:
https://github.com/llvm/llvm-project/commit/f900644ae2b6c7a485673974688a62c3f3301dcc
DIFF:
https://github.com/llvm/llvm-project/commit/f900644ae2b6c7a485673974688a62c3f3301dcc.diff
LOG: Refactor GetObjectDescription() to return llvm::Expected (NFC)
This is de facto an NFC change for Objective-C but will benefit the
Swift language plugin.
Added:
Modified:
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
lldb/include/lldb/Target/LanguageRuntime.h
lldb/source/API/SBValue.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/DataFormatters/ValueObjectPrinter.cpp
lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.h
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.h
Removed:
diff --git a/lldb/include/lldb/Core/ValueObject.h
b/lldb/include/lldb/Core/ValueObject.h
index 205d9ad9b1953..93eb3e8f590f4 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -537,7 +537,7 @@ class ValueObject {
std::string ,
const TypeSummaryOptions );
- const char *GetObjectDescription();
+ llvm::Expected GetObjectDescription();
bool HasSpecialPrintableRepresentation(
ValueObjectRepresentationStyle val_obj_display,
diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index 7460370c77e80..f9deb6ebf8d6d 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -75,7 +75,7 @@ class ValueObjectPrinter {
void SetupMostSpecializedValue();
- const char *GetDescriptionForDisplay();
+ llvm::Expected GetDescriptionForDisplay();
const char *GetRootNameForDisplay();
@@ -108,7 +108,8 @@ class ValueObjectPrinter {
bool PrintValueAndSummaryIfNeeded(bool _printed, bool
_printed);
- bool PrintObjectDescriptionIfNeeded(bool value_printed, bool
summary_printed);
+ llvm::Error PrintObjectDescriptionIfNeeded(bool value_printed,
+ bool summary_printed);
bool
ShouldPrintChildren(DumpValueObjectOptions::PointerDepth _ptr_depth);
@@ -132,7 +133,7 @@ class ValueObjectPrinter {
PrintChildren(bool value_printed, bool summary_printed,
const DumpValueObjectOptions::PointerDepth _ptr_depth);
- void PrintChildrenIfNeeded(bool value_printed, bool summary_printed);
+ llvm::Error PrintChildrenIfNeeded(bool value_printed, bool summary_printed);
bool PrintChildrenOneLiner(bool hide_names);
diff --git a/lldb/include/lldb/Target/LanguageRuntime.h
b/lldb/include/lldb/Target/LanguageRuntime.h
index a2a9c0163f082..d093dccfeae85 100644
--- a/lldb/include/lldb/Target/LanguageRuntime.h
+++ b/lldb/include/lldb/Target/LanguageRuntime.h
@@ -73,11 +73,12 @@ class LanguageRuntime : public Runtime, public
PluginInterface {
return nullptr;
}
- virtual bool GetObjectDescription(Stream , ValueObject ) = 0;
-
- virtual bool GetObjectDescription(Stream , Value ,
-ExecutionContextScope *exe_scope) = 0;
+ virtual llvm::Error GetObjectDescription(Stream ,
+ ValueObject ) = 0;
+ virtual llvm::Error
+ GetObjectDescription(Stream , Value ,
+ ExecutionContextScope *exe_scope) = 0;
struct VTableInfo {
Address addr; /// Address of the vtable's virtual function table
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 1b9cae6e350aa..10a691c403419 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -379,7 +379,10 @@ const char *SBValue::GetObjectDescription() {
if (!value_sp)
return nullptr;
- return ConstString(value_sp->GetObjectDescription()).GetCString();
+ llvm::Expected str = value_sp->GetObjectDescription();
+ if (!str)
+return ConstString("error: " + toString(str.takeError())).AsCString();
+ return ConstString(*str).AsCString();
}
SBType SBValue::GetType() {
diff --git a/lldb/source/Core/ValueObject.cpp
b/lldb/source/Core/ValueObject.cpp
index cf0f557fd9129..8f72efc2299b4 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -989,41 +989,46 @@ ValueObject::ReadPointedString(lldb::WritableDataBufferSP
_sp,
return {total_bytes_read,
[Lldb-commits] [lldb] d1bc75c - Convert ValueObject::Dump() to return llvm::Error() (NFCish)
Author: Adrian Prantl
Date: 2024-06-20T10:32:06-07:00
New Revision: d1bc75c0bce141b94f9afadfde4e784760735ec0
URL:
https://github.com/llvm/llvm-project/commit/d1bc75c0bce141b94f9afadfde4e784760735ec0
DIFF:
https://github.com/llvm/llvm-project/commit/d1bc75c0bce141b94f9afadfde4e784760735ec0.diff
LOG: Convert ValueObject::Dump() to return llvm::Error() (NFCish)
This change by itself has no measurable effect on the LLDB
testsuite. I'm making it in preparation for threading through more
errors in the Swift language plugin.
Added:
Modified:
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
lldb/source/API/SBValue.cpp
lldb/source/Breakpoint/Watchpoint.cpp
lldb/source/Commands/CommandObjectDWIMPrint.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Core/DumpRegisterValue.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/DataFormatters/ValueObjectPrinter.cpp
lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
lldb/test/API/commands/dwim-print/TestDWIMPrint.py
lldb/test/API/commands/dwim-print/main.c
lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s
lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp
Removed:
diff --git a/lldb/include/lldb/Core/ValueObject.h
b/lldb/include/lldb/Core/ValueObject.h
index db1fdae170ed0..205d9ad9b1953 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -694,9 +694,9 @@ class ValueObject {
virtual SymbolContextScope *GetSymbolContextScope();
- void Dump(Stream );
+ llvm::Error Dump(Stream );
- void Dump(Stream , const DumpValueObjectOptions );
+ llvm::Error Dump(Stream , const DumpValueObjectOptions );
static lldb::ValueObjectSP
CreateValueObjectFromExpression(llvm::StringRef name,
diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index fb5d60ba30d77..7460370c77e80 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -33,7 +33,7 @@ class ValueObjectPrinter {
~ValueObjectPrinter() = default;
- bool PrintValueObject();
+ llvm::Error PrintValueObject();
protected:
typedef std::set InstancePointersSet;
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 9d7efba024d11..1b9cae6e350aa 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -1233,7 +1233,10 @@ bool SBValue::GetDescription(SBStream ) {
DumpValueObjectOptions options;
options.SetUseDynamicType(m_opaque_sp->GetUseDynamic());
options.SetUseSyntheticValue(m_opaque_sp->GetUseSynthetic());
-value_sp->Dump(strm, options);
+if (llvm::Error error = value_sp->Dump(strm, options)) {
+ strm << "error: " << toString(std::move(error));
+ return false;
+}
} else {
strm.PutCString("No value");
}
diff --git a/lldb/source/Breakpoint/Watchpoint.cpp
b/lldb/source/Breakpoint/Watchpoint.cpp
index edb1a0e93460c..715e83c76697b 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -299,7 +299,9 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char
*prefix) const {
.SetHideRootType(true)
.SetHideRootName(true)
.SetHideName(true);
-m_old_value_sp->Dump(strm, options);
+if (llvm::Error error = m_old_value_sp->Dump(strm, options))
+ strm << "error: " << toString(std::move(error));
+
if (strm.GetData())
values_ss.Printf("old value: %s", strm.GetData());
}
@@ -322,7 +324,9 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char
*prefix) const {
.SetHideRootType(true)
.SetHideRootName(true)
.SetHideName(true);
-m_new_value_sp->Dump(strm, options);
+if (llvm::Error error = m_new_value_sp->Dump(strm, options))
+ strm << "error: " << toString(std::move(error));
+
if (strm.GetData())
values_ss.Printf("new value: %s", strm.GetData());
}
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 57a372a762e15..c1549ca6933fc 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -133,12 +133,17 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
auto dump_val_object = [&](ValueObject ) {
if (is_po) {
StreamString temp_result_stream;
-
[Lldb-commits] [lldb] [lldb] Make LanguageRuntime::GetTypeBitSize return an optional (NFC) (PR #96013)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/96013
>From 663bee087c345ed0d5759906d780cb4a75facac8 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Tue, 18 Jun 2024 19:32:44 -0700
Subject: [PATCH 1/2] [lldb] Make LanguageRuntime::GetTypeBitSize return an
optional (NFC)
Make LanguageRuntime::GetTypeBitSize return an optional. This should be
NFC, though the ObjCLanguageRuntime implementation is (possibly) more
defensive against returning 0.
I'm not sure if it's possible for both `m_ivar.size` and `m_ivar.offset`
to be zero. Previously, we'd return 0 and cache it, only to discard it
the next time when finding it in the cache, and recomputing it again.
The new code will avoid putting it in the cache in the first place.
---
lldb/include/lldb/Target/LanguageRuntime.h| 6 +++---
.../ObjC/ObjCLanguageRuntime.cpp | 21 ++-
.../ObjC/ObjCLanguageRuntime.h| 6 +++---
.../TypeSystem/Clang/TypeSystemClang.cpp | 11 +-
4 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/lldb/include/lldb/Target/LanguageRuntime.h
b/lldb/include/lldb/Target/LanguageRuntime.h
index a2a9c0163f082..58874c4188034 100644
--- a/lldb/include/lldb/Target/LanguageRuntime.h
+++ b/lldb/include/lldb/Target/LanguageRuntime.h
@@ -169,9 +169,9 @@ class LanguageRuntime : public Runtime, public
PluginInterface {
return m_process->GetTarget().GetSearchFilterForModule(nullptr);
}
- virtual bool GetTypeBitSize(const CompilerType _type,
- uint64_t ) {
-return false;
+ virtual std::optional
+ GetTypeBitSize(const CompilerType _type) {
+return {};
}
virtual void SymbolsDidLoad(const ModuleList _list) {}
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
index ba52444f0c2fc..a812ffba7a648 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
@@ -350,18 +350,17 @@ ObjCLanguageRuntime::EncodingToTypeSP
ObjCLanguageRuntime::GetEncodingToType() {
return nullptr;
}
-bool ObjCLanguageRuntime::GetTypeBitSize(const CompilerType _type,
- uint64_t ) {
+std::optional
+ObjCLanguageRuntime::GetTypeBitSize(const CompilerType _type) {
void *opaque_ptr = compiler_type.GetOpaqueQualType();
- size = m_type_size_cache.Lookup(opaque_ptr);
- // an ObjC object will at least have an ISA, so 0 is definitely not OK
- if (size > 0)
-return true;
+ uint64_t cached_size = m_type_size_cache.Lookup(opaque_ptr);
+ if (cached_size > 0)
+return cached_size;
ClassDescriptorSP class_descriptor_sp =
GetClassDescriptorFromClassName(compiler_type.GetTypeName());
if (!class_descriptor_sp)
-return false;
+return {};
int32_t max_offset = INT32_MIN;
uint64_t sizeof_max = 0;
@@ -377,11 +376,13 @@ bool ObjCLanguageRuntime::GetTypeBitSize(const
CompilerType _type,
}
}
- size = 8 * (max_offset + sizeof_max);
- if (found)
+ uint64_t size = 8 * (max_offset + sizeof_max);
+ if (found && size > 0) {
m_type_size_cache.Insert(opaque_ptr, size);
+return size;
+ }
- return found;
+ return {};
}
lldb::BreakpointPreconditionSP
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
index 0a8b6e8d56ed0..ffe9725fa6826 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -107,7 +107,7 @@ class ObjCLanguageRuntime : public LanguageRuntime {
int64_t *value_bits = nullptr,
uint64_t *payload = nullptr) = 0;
/// @}
-
+
virtual uint64_t GetInstanceSize() = 0;
// use to implement version-specific additional constraints on pointers
@@ -321,8 +321,8 @@ class ObjCLanguageRuntime : public LanguageRuntime {
m_negative_complete_class_cache.clear();
}
- bool GetTypeBitSize(const CompilerType _type,
- uint64_t ) override;
+ std::optional
+ GetTypeBitSize(const CompilerType _type) override;
/// Check whether the name is "self" or "_cmd" and should show up in
/// "frame variable".
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index dbe6238d4fe5a..f9cdf357acdde 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4744,11 +4744,12 @@
TypeSystemClang::GetBitSize(lldb::opaque_compiler_type_t type,
ExecutionContext exe_ctx(exe_scope);
Process *process = exe_ctx.GetProcessPtr();
if (process) {
-ObjCLanguageRuntime *objc_runtime =
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
clayborg wrote:
> This class behaves quite differently from other SB API classes. Normally, the
> opaque pointer can be cleared to release the potentially more resource heavy
> private counterpart. `AddressRange` is a pretty simple class, so I understand
> that it makes things easier if we guarantee the pointer is always valid, but
> it is somewhat of a surprise.
>
> Personally, I think consistency beats the small advantage of not having to
> check the pointer. If we want to stick to this approach, I'd like to see an
> assert that makes it clear that in this class, we have a precondition that
> the pointer is always valid:
>
> ```
> assert(m_opaque_up && "opaque pointer must always be valid");
> ```
For simple classes, there is no need to clear the unique pointer, so I like
this approach for small classes.
We can use the assert in a new protected `ref()` method:
```
lldb_private::AddressRange & SBAddressRange::ref() {
assert(m_opaque_up && "opaque pointer must always be valid");
return *m_opaque_up;
}
```
And then have everything that accesses `m_opaque_up` use the `ref()` function.
It is similar to other classes and makes the code nicer when we don't see
direct uses of `m_opaque_up`
https://github.com/llvm/llvm-project/pull/95997
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Convert ValueObject::Dump() to return llvm::Error() (NFCish) (PR #95857)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/95857
>From 7e3d1420a941431de223098ee153d2d4c63cfbfc Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Mon, 17 Jun 2024 14:29:01 -0700
Subject: [PATCH 1/4] Convert ValueObject::Dump() to return llvm::Error()
(NFCish)
This change by itself has no measurable effect on the LLDB
testsuite. I'm making it in preparation for threading through more
errors in the Swift language plugin.
---
lldb/include/lldb/Core/ValueObject.h | 4 ++--
.../lldb/DataFormatters/ValueObjectPrinter.h | 2 +-
lldb/source/API/SBValue.cpp| 5 -
lldb/source/Breakpoint/Watchpoint.cpp | 8 ++--
.../source/Commands/CommandObjectDWIMPrint.cpp | 9 +++--
.../Commands/CommandObjectExpression.cpp | 6 +-
lldb/source/Commands/CommandObjectFrame.cpp| 18 +-
lldb/source/Commands/CommandObjectMemory.cpp | 5 -
lldb/source/Commands/CommandObjectTarget.cpp | 3 ++-
lldb/source/Commands/CommandObjectThread.cpp | 14 ++
lldb/source/Core/DumpRegisterValue.cpp | 3 ++-
lldb/source/Core/FormatEntity.cpp | 11 +--
lldb/source/Core/ValueObject.cpp | 9 ++---
.../DataFormatters/ValueObjectPrinter.cpp | 18 +++---
lldb/source/Plugins/REPL/Clang/ClangREPL.cpp | 4 +++-
.../API/commands/dwim-print/TestDWIMPrint.py | 16
lldb/test/API/commands/dwim-print/main.c | 6 +-
.../DWARF/x86/class-type-nullptr-deref.s | 2 +-
.../DWARF/x86/debug-types-signature-loop.s | 2 +-
.../DumpValueObjectOptionsTests.cpp| 7 ---
20 files changed, 108 insertions(+), 44 deletions(-)
diff --git a/lldb/include/lldb/Core/ValueObject.h
b/lldb/include/lldb/Core/ValueObject.h
index db1fdae170ed0..205d9ad9b1953 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -694,9 +694,9 @@ class ValueObject {
virtual SymbolContextScope *GetSymbolContextScope();
- void Dump(Stream );
+ llvm::Error Dump(Stream );
- void Dump(Stream , const DumpValueObjectOptions );
+ llvm::Error Dump(Stream , const DumpValueObjectOptions );
static lldb::ValueObjectSP
CreateValueObjectFromExpression(llvm::StringRef name,
diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index fb5d60ba30d77..7460370c77e80 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -33,7 +33,7 @@ class ValueObjectPrinter {
~ValueObjectPrinter() = default;
- bool PrintValueObject();
+ llvm::Error PrintValueObject();
protected:
typedef std::set InstancePointersSet;
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 9d7efba024d11..1b9cae6e350aa 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -1233,7 +1233,10 @@ bool SBValue::GetDescription(SBStream ) {
DumpValueObjectOptions options;
options.SetUseDynamicType(m_opaque_sp->GetUseDynamic());
options.SetUseSyntheticValue(m_opaque_sp->GetUseSynthetic());
-value_sp->Dump(strm, options);
+if (llvm::Error error = value_sp->Dump(strm, options)) {
+ strm << "error: " << toString(std::move(error));
+ return false;
+}
} else {
strm.PutCString("No value");
}
diff --git a/lldb/source/Breakpoint/Watchpoint.cpp
b/lldb/source/Breakpoint/Watchpoint.cpp
index edb1a0e93460c..715e83c76697b 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -299,7 +299,9 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char
*prefix) const {
.SetHideRootType(true)
.SetHideRootName(true)
.SetHideName(true);
-m_old_value_sp->Dump(strm, options);
+if (llvm::Error error = m_old_value_sp->Dump(strm, options))
+ strm << "error: " << toString(std::move(error));
+
if (strm.GetData())
values_ss.Printf("old value: %s", strm.GetData());
}
@@ -322,7 +324,9 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char
*prefix) const {
.SetHideRootType(true)
.SetHideRootName(true)
.SetHideName(true);
-m_new_value_sp->Dump(strm, options);
+if (llvm::Error error = m_new_value_sp->Dump(strm, options))
+ strm << "error: " << toString(std::move(error));
+
if (strm.GetData())
values_ss.Printf("new value: %s", strm.GetData());
}
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 57a372a762e15..c1549ca6933fc 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -133,12 +133,17 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
auto
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
https://github.com/mbucko updated
https://github.com/llvm/llvm-project/pull/95997
>From 59c382f0b06d632c05baeb357c0390a2423932fc Mon Sep 17 00:00:00 2001
From: Miro Bucko
Date: Tue, 18 Jun 2024 14:35:55 -0700
Subject: [PATCH] [lldb] Fix SBAddressRange validation checks.
---
lldb/source/API/SBAddressRange.cpp| 25 ---
lldb/source/API/SBAddressRangeList.cpp| 8 ++
.../address_range/TestAddressRange.py | 2 +-
3 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/lldb/source/API/SBAddressRange.cpp
b/lldb/source/API/SBAddressRange.cpp
index 9b1affdade439..44f2c15635d13 100644
--- a/lldb/source/API/SBAddressRange.cpp
+++ b/lldb/source/API/SBAddressRange.cpp
@@ -50,8 +50,8 @@ const SBAddressRange ::operator=(const
SBAddressRange ) {
bool SBAddressRange::operator==(const SBAddressRange ) {
LLDB_INSTRUMENT_VA(this, rhs);
- if (!IsValid() || !rhs.IsValid())
-return false;
+ assert(m_opaque_up && "opaque pointer must always be valid");
+ assert(rhs.m_opaque_up && "opaque pointer must always be valid");
return m_opaque_up->operator==(*(rhs.m_opaque_up));
}
@@ -64,28 +64,28 @@ bool SBAddressRange::operator!=(const SBAddressRange ) {
void SBAddressRange::Clear() {
LLDB_INSTRUMENT_VA(this);
- m_opaque_up.reset();
+ assert(m_opaque_up && "opaque pointer must always be valid");
+ m_opaque_up->Clear();
}
bool SBAddressRange::IsValid() const {
LLDB_INSTRUMENT_VA(this);
- return m_opaque_up && m_opaque_up->IsValid();
+ assert(m_opaque_up && "opaque pointer must always be valid");
+ return m_opaque_up->IsValid();
}
lldb::SBAddress SBAddressRange::GetBaseAddress() const {
LLDB_INSTRUMENT_VA(this);
- if (!IsValid())
-return lldb::SBAddress();
+ assert(m_opaque_up && "opaque pointer must always be valid");
return lldb::SBAddress(m_opaque_up->GetBaseAddress());
}
lldb::addr_t SBAddressRange::GetByteSize() const {
LLDB_INSTRUMENT_VA(this);
- if (!IsValid())
-return 0;
+ assert(m_opaque_up && "opaque pointer must always be valid");
return m_opaque_up->GetByteSize();
}
@@ -93,11 +93,6 @@ bool SBAddressRange::GetDescription(SBStream ,
const SBTarget target) {
LLDB_INSTRUMENT_VA(this, description, target);
- Stream = description.ref();
- if (!IsValid()) {
-stream << "";
-return true;
- }
- m_opaque_up->GetDescription(, target.GetSP().get());
- return true;
+ assert(m_opaque_up && "opaque pointer must always be valid");
+ return m_opaque_up->GetDescription((), target.GetSP().get());
}
diff --git a/lldb/source/API/SBAddressRangeList.cpp
b/lldb/source/API/SBAddressRangeList.cpp
index 20660b3ff2088..ddb5a0a8a3d0b 100644
--- a/lldb/source/API/SBAddressRangeList.cpp
+++ b/lldb/source/API/SBAddressRangeList.cpp
@@ -36,6 +36,8 @@ const SBAddressRangeList &
SBAddressRangeList::operator=(const SBAddressRangeList ) {
LLDB_INSTRUMENT_VA(this, rhs);
+ assert(m_opaque_up && "opaque pointer must always be valid");
+ assert(rhs.m_opaque_up && "opaque pointer must always be valid");
if (this != )
*m_opaque_up = *rhs.m_opaque_up;
return *this;
@@ -44,12 +46,14 @@ SBAddressRangeList::operator=(const SBAddressRangeList
) {
uint32_t SBAddressRangeList::GetSize() const {
LLDB_INSTRUMENT_VA(this);
+ assert(m_opaque_up && "opaque pointer must always be valid");
return m_opaque_up->GetSize();
}
SBAddressRange SBAddressRangeList::GetAddressRangeAtIndex(uint64_t idx) {
LLDB_INSTRUMENT_VA(this, idx);
+ assert(m_opaque_up && "opaque pointer must always be valid");
SBAddressRange sb_addr_range;
(*sb_addr_range.m_opaque_up) = m_opaque_up->GetAddressRangeAtIndex(idx);
return sb_addr_range;
@@ -58,18 +62,21 @@ SBAddressRange
SBAddressRangeList::GetAddressRangeAtIndex(uint64_t idx) {
void SBAddressRangeList::Clear() {
LLDB_INSTRUMENT_VA(this);
+ assert(m_opaque_up && "opaque pointer must always be valid");
m_opaque_up->Clear();
}
void SBAddressRangeList::Append(const SBAddressRange _addr_range) {
LLDB_INSTRUMENT_VA(this, sb_addr_range);
+ assert(m_opaque_up && "opaque pointer must always be valid");
m_opaque_up->Append(*sb_addr_range.m_opaque_up);
}
void SBAddressRangeList::Append(const SBAddressRangeList _addr_range_list) {
LLDB_INSTRUMENT_VA(this, sb_addr_range_list);
+ assert(m_opaque_up && "opaque pointer must always be valid");
m_opaque_up->Append(*sb_addr_range_list.m_opaque_up);
}
@@ -77,6 +84,7 @@ bool SBAddressRangeList::GetDescription(SBStream ,
const SBTarget ) {
LLDB_INSTRUMENT_VA(this, description, target);
+ assert(m_opaque_up && "opaque pointer must always be valid");
const uint32_t num_ranges = GetSize();
bool is_first = true;
Stream = description.ref();
diff --git a/lldb/test/API/python_api/address_range/TestAddressRange.py
b/lldb/test/API/python_api/address_range/TestAddressRange.py
index
[Lldb-commits] [lldb] [lldb] Make LanguageRuntime::GetTypeBitSize return an optional (NFC) (PR #96013)
@@ -4744,11 +4744,12 @@
TypeSystemClang::GetBitSize(lldb::opaque_compiler_type_t type,
ExecutionContext exe_ctx(exe_scope);
Process *process = exe_ctx.GetProcessPtr();
if (process) {
-ObjCLanguageRuntime *objc_runtime = ObjCLanguageRuntime::Get(*process);
-if (objc_runtime) {
- uint64_t bit_size = 0;
- if (objc_runtime->GetTypeBitSize(GetType(qual_type), bit_size))
-return bit_size;
+if (ObjCLanguageRuntime *objc_runtime =
+ObjCLanguageRuntime::Get(*process)) {
+ std::optional bit_size =
+ objc_runtime->GetTypeBitSize(GetType(qual_type));
+ if (bit_size)
+return *bit_size;
Michael137 wrote:
Aah you're right, missed the fallthrough here. Could fold the `GetTypeBitSize`
call into the if-statement, like you did for `ObjCLanguageRuntime::Get`. But
either way, LGTM
https://github.com/llvm/llvm-project/pull/96013
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 0ec567c - Revert "[lldb][ObjC] Don't query objective-c runtime for decls in C++ contexts (#95963)"
Author: Michael Buch
Date: 2024-06-20T17:53:37+01:00
New Revision: 0ec567c370df86893a22bf59d2716f6e553ca63b
URL:
https://github.com/llvm/llvm-project/commit/0ec567c370df86893a22bf59d2716f6e553ca63b
DIFF:
https://github.com/llvm/llvm-project/commit/0ec567c370df86893a22bf59d2716f6e553ca63b.diff
LOG: Revert "[lldb][ObjC] Don't query objective-c runtime for decls in C++
contexts (#95963)"
This reverts commit dadf960607bb429baebd3f523ce5b93260a154d2.
The commit caused `TestEarlyProcessLaunch.py` to fail on the
macOS bots.
Added:
Modified:
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/test/API/lang/objcxx/objc-from-cpp-frames-without-debuginfo/TestObjCFromCppFramesWithoutDebugInfo.py
Removed:
lldb/test/Shell/Expr/TestObjCInCXXContext.test
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index 1fdd272dcbece..82a7a2cc3f1ef 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -637,7 +637,7 @@ void ClangASTSource::FindExternalVisibleDecls(
FindDeclInModules(context, name);
}
- if (!context.m_found_type && m_ast_context->getLangOpts().ObjC) {
+ if (!context.m_found_type) {
FindDeclInObjCRuntime(context, name);
}
}
diff --git
a/lldb/test/API/lang/objcxx/objc-from-cpp-frames-without-debuginfo/TestObjCFromCppFramesWithoutDebugInfo.py
b/lldb/test/API/lang/objcxx/objc-from-cpp-frames-without-debuginfo/TestObjCFromCppFramesWithoutDebugInfo.py
index 497c0dd128f48..ef8d5540fa4ef 100644
---
a/lldb/test/API/lang/objcxx/objc-from-cpp-frames-without-debuginfo/TestObjCFromCppFramesWithoutDebugInfo.py
+++
b/lldb/test/API/lang/objcxx/objc-from-cpp-frames-without-debuginfo/TestObjCFromCppFramesWithoutDebugInfo.py
@@ -15,11 +15,4 @@ def test(self):
(_, process, _, _) = lldbutil.run_to_name_breakpoint(self, "main")
self.assertState(process.GetState(), lldb.eStateStopped)
-
-# Tests that we can use builtin Objective-C identifiers.
self.expect("expr id", error=False)
-
-# Tests that we can lookup Objective-C decls in the ObjC runtime
plugin.
-self.expect_expr(
-"NSString *c; c == nullptr", result_value="true",
result_type="bool"
-)
diff --git a/lldb/test/Shell/Expr/TestObjCInCXXContext.test
b/lldb/test/Shell/Expr/TestObjCInCXXContext.test
deleted file mode 100644
index 8537799bdeb67..0
--- a/lldb/test/Shell/Expr/TestObjCInCXXContext.test
+++ /dev/null
@@ -1,21 +0,0 @@
-// UNSUPPORTED: system-linux, system-windows
-
-// Tests that we don't consult the the Objective-C runtime
-// plugin when in a purely C++ context.
-//
-// RUN: %clangxx_host %p/Inputs/objc-cast.cpp -g -o %t
-// RUN: %lldb %t \
-// RUN: -o "b main" -o run \
-// RUN: -o "expression --language objective-c -- NSString * a; a" \
-// RUN: -o "expression --language objective-c++ -- NSString * b; b" \
-// RUN: -o "expression NSString" \
-// RUN: 2>&1 | FileCheck %s
-
-// CHECK: (lldb) expression --language objective-c -- NSString * a; a
-// CHECK-NEXT: (NSString *){{.*}}= nil
-
-// CHECK: (lldb) expression --language objective-c++ -- NSString * b; b
-// CHECK-NEXT: (NSString *){{.*}}= nil
-
-// CHECK: (lldb) expression NSString
-// CHECK-NEXT: error:{{.*}} use of undeclared identifier 'NSString'
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 869f551 - [lldb] Give more time to test/API/multiple-debuggers
Author: Adrian Prantl
Date: 2024-06-20T09:45:58-07:00
New Revision: 869f5517605224944d6037716e234d9f1f0e7067
URL:
https://github.com/llvm/llvm-project/commit/869f5517605224944d6037716e234d9f1f0e7067
DIFF:
https://github.com/llvm/llvm-project/commit/869f5517605224944d6037716e234d9f1f0e7067.diff
LOG: [lldb] Give more time to test/API/multiple-debuggers
This test occasionally fails on two of the busiest CI bots (asan and
matrix), and we can't reproduce it locally. This leads to the
hypothesis that the test is timing out (in the sense of the number of
"join attempts" performed by this test's driver).
This commit doubles the number of iterations performed and also does
an NFC refactor of the main test loop so that it can be more easily
understood.
Added:
Modified:
lldb/test/API/api/multiple-debuggers/multi-process-driver.cpp
Removed:
diff --git a/lldb/test/API/api/multiple-debuggers/multi-process-driver.cpp
b/lldb/test/API/api/multiple-debuggers/multi-process-driver.cpp
index 5cf5ff3562030..c9c0bcf641e08 100644
--- a/lldb/test/API/api/multiple-debuggers/multi-process-driver.cpp
+++ b/lldb/test/API/api/multiple-debuggers/multi-process-driver.cpp
@@ -216,6 +216,22 @@ void *do_one_debugger (void *in)
return (void*) 1;
}
+int count_completed_threads(int num_threads) {
+ int num_completed_threads = 0;
+ for (int i = 0; i < num_threads; i++)
+if (completed_threads_array[i])
+ num_completed_threads++;
+ return num_completed_threads;
+}
+
+int count_successful_threads(int num_threads) {
+ int num_successful_threads = 0;
+ for (int i = 0; i < num_threads; i++)
+if (successful_threads_array[i])
+ num_successful_threads++;
+ return num_successful_threads;
+}
+
int main (int argc, char **argv)
{
#if !defined(_MSC_VER)
@@ -241,26 +257,15 @@ int main (int argc, char **argv)
}
-int max_time_to_wait = 20; // 20 iterations, or 60 seconds
-int iter = 0;
-while (1)
-{
+int max_time_to_wait = 40; // 40 iterations, or 120 seconds
+if (getenv("ASAN_OPTIONS"))
+ max_time_to_wait *= 4;
+for (int iter = 0; iter < max_time_to_wait; iter++) {
std::this_thread::sleep_for(std::chrono::seconds(3));
-bool all_done = true;
-int successful_threads = 0;
-int total_completed_threads = 0;
-for (uint64_t i = 0; i < NUMBER_OF_SIMULTANEOUS_DEBUG_SESSIONS; i++)
-{
-if (successful_threads_array[i] == true)
-successful_threads++;
-if (completed_threads_array[i] == true)
-total_completed_threads++;
-if (completed_threads_array[i] == false)
-{
-all_done = false;
-}
-}
-if (all_done)
+int successful_threads =
count_successful_threads(NUMBER_OF_SIMULTANEOUS_DEBUG_SESSIONS);
+int total_completed_threads =
count_completed_threads(NUMBER_OF_SIMULTANEOUS_DEBUG_SESSIONS);
+
+if (total_completed_threads == NUMBER_OF_SIMULTANEOUS_DEBUG_SESSIONS)
{
#if DEBUG == 1
printf ("All threads completed.\n");
@@ -275,14 +280,14 @@ int main (int argc, char **argv)
printf ("%d threads completed so far (%d successfully), out of
%d\n", total_completed_threads, successful_threads,
NUMBER_OF_SIMULTANEOUS_DEBUG_SESSIONS);
#endif
}
-if (iter++ == max_time_to_wait)
-{
-printf ("reached maximum timeout but only %d threads have
completed so far (%d successfully), out of %d. Exiting.\n",
total_completed_threads, successful_threads,
NUMBER_OF_SIMULTANEOUS_DEBUG_SESSIONS);
-break;
-}
+if (iter == max_time_to_wait)
+ printf("reached maximum timeout but only %d threads have completed "
+ "so far "
+ "(%d successfully), out of %d. Exiting.\n",
+ total_completed_threads, successful_threads,
+ NUMBER_OF_SIMULTANEOUS_DEBUG_SESSIONS);
}
-
SBDebugger::Terminate();
exit (1);
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [clang][AST] fix ast-print of extern with >=2 declarators, fixed (PR #93913)
AaronBallman wrote: Do you need someone to land these changes on your behalf? https://github.com/llvm/llvm-project/pull/93913 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Merge CompilerContextKind::{Class, Struct} (PR #96145)
https://github.com/Michael137 approved this pull request. https://github.com/llvm/llvm-project/pull/96145 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
mbucko wrote: > This class behaves quite differently from other SB API classes. Normally, the > opaque pointer can be cleared to release the potentially more resource heavy > private counterpart. `AddressRange` is a pretty simple class, so I understand > that it makes things easier if we guarantee the pointer is always valid, but > it is somewhat of a surprise. > > Personally, I think consistency beats the small advantage of not having to > check the pointer. If we want to stick to this approach, I'd like to see an > assert that makes it clear that in this class, we have a precondition that > the pointer is always valid: > > ``` > assert(m_opaque_up && "opaque pointer must always be valid"); > ``` So you're ok with keeping the code as is plus adding the asserts? https://github.com/llvm/llvm-project/pull/95997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make LanguageRuntime::GetTypeBitSize return an optional (NFC) (PR #96013)
@@ -4744,11 +4744,12 @@
TypeSystemClang::GetBitSize(lldb::opaque_compiler_type_t type,
ExecutionContext exe_ctx(exe_scope);
Process *process = exe_ctx.GetProcessPtr();
if (process) {
-ObjCLanguageRuntime *objc_runtime = ObjCLanguageRuntime::Get(*process);
-if (objc_runtime) {
- uint64_t bit_size = 0;
- if (objc_runtime->GetTypeBitSize(GetType(qual_type), bit_size))
-return bit_size;
+if (ObjCLanguageRuntime *objc_runtime =
+ObjCLanguageRuntime::Get(*process)) {
+ std::optional bit_size =
+ objc_runtime->GetTypeBitSize(GetType(qual_type));
+ if (bit_size)
+return *bit_size;
JDevlieghere wrote:
That might be the right thing to do, but would be a change in behavior, as now,
if it fails, we'll fall through to the `default` case first.
https://github.com/llvm/llvm-project/pull/96013
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][API] Add Find(Ranges)InMemory() to Process SB API (PR #95007)
https://github.com/mbucko updated
https://github.com/llvm/llvm-project/pull/95007
>From bae5474315a8a9053033c90237e1eacd9e56e39c Mon Sep 17 00:00:00 2001
From: Miro Bucko
Date: Tue, 4 Jun 2024 12:01:48 -0700
Subject: [PATCH] [lldb][API] Add Find(Ranges)InMemory() to Process SB API
Test Plan:
llvm-lit
llvm-project/lldb/test/API/python_api/find_in_memory/TestFindInMemory.py
llvm-project/lldb/test/API/python_api/find_in_memory/TestFindRangesInMemory.py
Reviewers: clayborg
Tasks: lldb
---
lldb/bindings/python/python-typemaps.swig | 3 +-
lldb/include/lldb/API/SBAddressRange.h| 2 +
lldb/include/lldb/API/SBAddressRangeList.h| 2 +
lldb/include/lldb/API/SBProcess.h | 10 +
lldb/include/lldb/Core/AddressRangeListImpl.h | 4 +
lldb/include/lldb/Target/Process.h| 13 ++
lldb/source/API/SBAddressRange.cpp| 4 +-
lldb/source/API/SBAddressRangeList.cpp| 4 +
lldb/source/API/SBProcess.cpp | 58 -
lldb/source/Target/Process.cpp| 129 ++
.../API/python_api/find_in_memory/Makefile| 3 +
.../find_in_memory/TestFindInMemory.py| 131 +++
.../find_in_memory/TestFindRangesInMemory.py | 221 ++
.../find_in_memory/address_ranges_helper.py | 73 ++
.../API/python_api/find_in_memory/main.cpp| 27 +++
15 files changed, 677 insertions(+), 7 deletions(-)
create mode 100644 lldb/test/API/python_api/find_in_memory/Makefile
create mode 100644 lldb/test/API/python_api/find_in_memory/TestFindInMemory.py
create mode 100644
lldb/test/API/python_api/find_in_memory/TestFindRangesInMemory.py
create mode 100644
lldb/test/API/python_api/find_in_memory/address_ranges_helper.py
create mode 100644 lldb/test/API/python_api/find_in_memory/main.cpp
diff --git a/lldb/bindings/python/python-typemaps.swig
b/lldb/bindings/python/python-typemaps.swig
index c39594c7df041..f8c33e15c03e6 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -257,7 +257,8 @@ AND call SWIG_fail at the same time, because it will result
in a double free.
}
// For SBProcess::WriteMemory, SBTarget::GetInstructions and
SBDebugger::DispatchInput.
%typemap(in) (const void *buf, size_t size),
- (const void *data, size_t data_len) {
+ (const void *data, size_t data_len),
+ (const void *buf, uint64_t size) {
if (PythonString::Check($input)) {
PythonString str(PyRefType::Borrowed, $input);
$1 = (void *)str.GetString().data();
diff --git a/lldb/include/lldb/API/SBAddressRange.h
b/lldb/include/lldb/API/SBAddressRange.h
index 152bd82426af1..ef8ce9ba9977d 100644
--- a/lldb/include/lldb/API/SBAddressRange.h
+++ b/lldb/include/lldb/API/SBAddressRange.h
@@ -58,6 +58,8 @@ class LLDB_API SBAddressRange {
friend class SBFunction;
friend class SBProcess;
+ lldb_private::AddressRange () const;
+
AddressRangeUP m_opaque_up;
};
diff --git a/lldb/include/lldb/API/SBAddressRangeList.h
b/lldb/include/lldb/API/SBAddressRangeList.h
index a123287ef1b4f..9e4d747685e63 100644
--- a/lldb/include/lldb/API/SBAddressRangeList.h
+++ b/lldb/include/lldb/API/SBAddressRangeList.h
@@ -46,6 +46,8 @@ class LLDB_API SBAddressRangeList {
friend class SBBlock;
friend class SBProcess;
+ lldb_private::AddressRanges () const;
+
std::unique_ptr m_opaque_up;
};
diff --git a/lldb/include/lldb/API/SBProcess.h
b/lldb/include/lldb/API/SBProcess.h
index f1b5d1fb92ce2..a6ab7ae759918 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -209,6 +209,16 @@ class LLDB_API SBProcess {
lldb::addr_t ReadPointerFromMemory(addr_t addr, lldb::SBError );
+ lldb::SBAddressRangeList FindRangesInMemory(const void *buf, uint64_t size,
+ const SBAddressRangeList ,
+ uint32_t alignment,
+ uint32_t max_matches,
+ SBError );
+
+ lldb::addr_t FindInMemory(const void *buf, uint64_t size,
+const SBAddressRange , uint32_t alignment,
+SBError );
+
// Events
static lldb::StateType GetStateFromEvent(const lldb::SBEvent );
diff --git a/lldb/include/lldb/Core/AddressRangeListImpl.h
b/lldb/include/lldb/Core/AddressRangeListImpl.h
index 46ebfe73d4d92..6742e6ead87de 100644
--- a/lldb/include/lldb/Core/AddressRangeListImpl.h
+++ b/lldb/include/lldb/Core/AddressRangeListImpl.h
@@ -13,7 +13,9 @@
#include
namespace lldb {
+class SBAddressRangeList;
class SBBlock;
+class SBProcess;
}
namespace lldb_private {
@@ -39,7 +41,9 @@ class AddressRangeListImpl {
lldb_private::AddressRange GetAddressRangeAtIndex(size_t index);
private:
+ friend class lldb::SBAddressRangeList;
friend class lldb::SBBlock;
+ friend class lldb::SBProcess;
[Lldb-commits] [lldb] [lldb] Fix SBAddressRange validation checks. (PR #95997)
https://github.com/JDevlieghere requested changes to this pull request. This class behaves quite differently from other SB API classes. Normally, the opaque pointer can be cleared to release the potentially more resource heavy private counterpart. `AddressRange` is a pretty simple class, so I understand that it makes things easier if we guarantee the pointer is always valid, but it is somewhat of a surprise. Personally, I think consistency beats the small advantage of not having to check the pointer. If we want to stick to this approach, I'd like to see an assert that makes it clear that in this class, we have a precondition that the pointer is always valid: ``` assert(m_opaque_up && "opaque pointer must always be valid"); ``` https://github.com/llvm/llvm-project/pull/95997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix printing of unsigned enum bitfields when they contain the max value (PR #96202)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/96202 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix printing of unsigned enum bitfields when they contain the max value (PR #96202)
@@ -0,0 +1,31 @@
+"""
+Test that the expression parser accounts for the underlying type of bitfield
+enums when looking for matching values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestBitfieldEnum(TestBase):
+def test_bitfield_enums(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.cpp", False)
+)
+
+self.expect_expr(
+"bfs",
+result_type="BitfieldStruct",
+result_children=[
+ValueCheck(name="signed_min", value="min"),
+ValueCheck(name="signed_other", value="-1"),
+ValueCheck(name="signed_max", value="max"),
+ValueCheck(name="unsigned_min", value="min"),
+ValueCheck(name="unsigned_other", value="1"),
+ValueCheck(name="unsigned_max", value="max"),
DavidSpickett wrote:
Prior to this change, this printed `3`.
https://github.com/llvm/llvm-project/pull/96202
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix printing of unsigned enum bitfields when they contain the max value (PR #96202)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: David Spickett (DavidSpickett)
Changes
While testing register fields I found that if you put the max value into a
bitfield with an underlying type that is an unsigned enum, lldb would not print
the enum name.
This is because the code to match values to names wasn't checking whether the
enum's type was signed, it just assumed it was.
So for example a 2 bit field with value 3 got signed extended to -1, which
didn't match the enumerator value of 3. So lldb just printed the number instead
of the name.
For a value of 1, the top bit was 0 so the sign extend became a zero extend,
and lldb did print the name of the enumerator.
I added a new test because I needed to use C++ to get typed enums. It checks
min, max and an in between value for signed and unsigned enums applied to a
bitfield.
---
Full diff: https://github.com/llvm/llvm-project/pull/96202.diff
4 Files Affected:
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+13-5)
- (added) lldb/test/API/commands/expression/bitfield_enums/Makefile (+3)
- (added) lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py
(+31)
- (added) lldb/test/API/commands/expression/bitfield_enums/main.cpp (+24)
``diff
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 0270c67366d1d..0fdc0ff150d30 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -8639,8 +8639,13 @@ static bool DumpEnumValue(const clang::QualType
_type, Stream ,
const clang::EnumDecl *enum_decl = enutype->getDecl();
assert(enum_decl);
lldb::offset_t offset = byte_offset;
- const uint64_t enum_svalue = data.GetMaxS64Bitfield(
- , byte_size, bitfield_bit_size, bitfield_bit_offset);
+ bool qual_type_is_signed = qual_type->isSignedIntegerOrEnumerationType();
+ const uint64_t enum_svalue =
+ qual_type_is_signed
+ ? data.GetMaxS64Bitfield(, byte_size, bitfield_bit_size,
+ bitfield_bit_offset)
+ : data.GetMaxU64Bitfield(, byte_size, bitfield_bit_size,
+ bitfield_bit_offset);
bool can_be_bitfield = true;
uint64_t covered_bits = 0;
int num_enumerators = 0;
@@ -8652,8 +8657,11 @@ static bool DumpEnumValue(const clang::QualType
_type, Stream ,
// enumerators. Also 0 doesn't make sense when the enumerators are used as
// flags.
for (auto *enumerator : enum_decl->enumerators()) {
-uint64_t val = enumerator->getInitVal().getSExtValue();
-val = llvm::SignExtend64(val, 8*byte_size);
+llvm::APSInt init_val = enumerator->getInitVal();
+uint64_t val =
+qual_type_is_signed ? init_val.getSExtValue() :
init_val.getZExtValue();
+if (qual_type_is_signed)
+ val = llvm::SignExtend64(val, 8 * byte_size);
if (llvm::popcount(val) != 1 && (val & ~covered_bits) != 0)
can_be_bitfield = false;
covered_bits |= val;
@@ -8673,7 +8681,7 @@ static bool DumpEnumValue(const clang::QualType
_type, Stream ,
// No exact match, but we don't think this is a bitfield. Print the value as
// decimal.
if (!can_be_bitfield) {
-if (qual_type->isSignedIntegerOrEnumerationType())
+if (qual_type_is_signed)
s.Printf("%" PRIi64, enum_svalue);
else
s.Printf("%" PRIu64, enum_uvalue);
diff --git a/lldb/test/API/commands/expression/bitfield_enums/Makefile
b/lldb/test/API/commands/expression/bitfield_enums/Makefile
new file mode 100644
index 0..8b20bcb05
--- /dev/null
+++ b/lldb/test/API/commands/expression/bitfield_enums/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git
a/lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py
b/lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py
new file mode 100644
index 0..a484b69300e7b
--- /dev/null
+++ b/lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py
@@ -0,0 +1,31 @@
+"""
+Test that the expression parser accounts for the underlying type of bitfield
+enums when looking for matching values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestBitfieldEnum(TestBase):
+def test_bitfield_enums(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.cpp", False)
+)
+
+self.expect_expr(
+"bfs",
+result_type="BitfieldStruct",
+result_children=[
+ValueCheck(name="signed_min", value="min"),
+ValueCheck(name="signed_other", value="-1"),
+ValueCheck(name="signed_max", value="max"),
+ValueCheck(name="unsigned_min", value="min"),
+
[Lldb-commits] [lldb] [lldb] Fix printing of unsigned enum bitfields when they contain the max value (PR #96202)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/96202 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix printing of unsigned enums bitfields when they contain the max value (PR #96202)
https://github.com/DavidSpickett created
https://github.com/llvm/llvm-project/pull/96202
While testing register fields I found that if you put the max value into a
bitfield with an underlying type that is an unsigned enum, lldb would not print
the enum name.
This is because the code to match values to names wasn't checking whether the
enum's type was signed, it just assumed it was.
So for example a 2 bit field with value 3 got signed extended to -1, which
didn't match the enumerator value of 3. So lldb just printed the number instead
of the name.
For a value of 1, the top bit was 0 so the sign extend became a zero extend,
and lldb did print the name of the enumerator.
I added a new test because I needed to use C++ to get typed enums. It checks
min, max and an in between value for signed and unsigned enums applied to a
bitfield.
>From 49113d1f126d4bf048fba6ece0e28463b9ee3884 Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Thu, 20 Jun 2024 14:37:24 +
Subject: [PATCH] [lldb] Fix printing of unsigned enums used as bitfield types
While testing register fields I found that if you put the max
value into a bitfield with an underlying type that is an unsigned
enum, lldb would not print the enum name.
This is because the code to match values to names wasn't checking
whether the enum's type was signed, it just assumed it was.
So for example a 2 bit field with value 3 got signed extended
to -1, which didn't match the enumerator value of 3. So lldb
just printed the number instead of the name.
For a value of 1, the top bit was 0 so the sign extend became
a zero extend, and lldb did print the name of the enumerator.
I added a new test because I needed to use C++ to get typed
enums. It checks min, max and an in between value for signed
and unsigned enums applied to a bitfield.
---
.../TypeSystem/Clang/TypeSystemClang.cpp | 18 ---
.../expression/bitfield_enums/Makefile| 3 ++
.../bitfield_enums/TestBitfieldEnums.py | 31 +++
.../expression/bitfield_enums/main.cpp| 24 ++
4 files changed, 71 insertions(+), 5 deletions(-)
create mode 100644 lldb/test/API/commands/expression/bitfield_enums/Makefile
create mode 100644
lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py
create mode 100644 lldb/test/API/commands/expression/bitfield_enums/main.cpp
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 0270c67366d1d..0fdc0ff150d30 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -8639,8 +8639,13 @@ static bool DumpEnumValue(const clang::QualType
_type, Stream ,
const clang::EnumDecl *enum_decl = enutype->getDecl();
assert(enum_decl);
lldb::offset_t offset = byte_offset;
- const uint64_t enum_svalue = data.GetMaxS64Bitfield(
- , byte_size, bitfield_bit_size, bitfield_bit_offset);
+ bool qual_type_is_signed = qual_type->isSignedIntegerOrEnumerationType();
+ const uint64_t enum_svalue =
+ qual_type_is_signed
+ ? data.GetMaxS64Bitfield(, byte_size, bitfield_bit_size,
+ bitfield_bit_offset)
+ : data.GetMaxU64Bitfield(, byte_size, bitfield_bit_size,
+ bitfield_bit_offset);
bool can_be_bitfield = true;
uint64_t covered_bits = 0;
int num_enumerators = 0;
@@ -8652,8 +8657,11 @@ static bool DumpEnumValue(const clang::QualType
_type, Stream ,
// enumerators. Also 0 doesn't make sense when the enumerators are used as
// flags.
for (auto *enumerator : enum_decl->enumerators()) {
-uint64_t val = enumerator->getInitVal().getSExtValue();
-val = llvm::SignExtend64(val, 8*byte_size);
+llvm::APSInt init_val = enumerator->getInitVal();
+uint64_t val =
+qual_type_is_signed ? init_val.getSExtValue() :
init_val.getZExtValue();
+if (qual_type_is_signed)
+ val = llvm::SignExtend64(val, 8 * byte_size);
if (llvm::popcount(val) != 1 && (val & ~covered_bits) != 0)
can_be_bitfield = false;
covered_bits |= val;
@@ -8673,7 +8681,7 @@ static bool DumpEnumValue(const clang::QualType
_type, Stream ,
// No exact match, but we don't think this is a bitfield. Print the value as
// decimal.
if (!can_be_bitfield) {
-if (qual_type->isSignedIntegerOrEnumerationType())
+if (qual_type_is_signed)
s.Printf("%" PRIi64, enum_svalue);
else
s.Printf("%" PRIu64, enum_uvalue);
diff --git a/lldb/test/API/commands/expression/bitfield_enums/Makefile
b/lldb/test/API/commands/expression/bitfield_enums/Makefile
new file mode 100644
index 0..8b20bcb05
--- /dev/null
+++ b/lldb/test/API/commands/expression/bitfield_enums/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git
a/lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py
[Lldb-commits] [lldb] [lldb] Merge CompilerContextKind::{Class, Struct} (PR #96145)
@@ -75,20 +75,18 @@ bool
lldb_private::contextMatches(llvm::ArrayRef context_chain,
static CompilerContextKind ConvertTypeClass(lldb::TypeClass type_class) {
if (type_class == eTypeClassAny)
return CompilerContextKind::AnyType;
- uint16_t result = 0;
- if (type_class & lldb::eTypeClassClass)
-result |= (uint16_t)CompilerContextKind::Class;
- if (type_class & lldb::eTypeClassStruct)
-result |= (uint16_t)CompilerContextKind::Struct;
labath wrote:
> Hmm it might actually be useful to see both in the output I would've thought?
Maybe, I don't really have an intuitive expectation here, but I like how this
forces a consistent behavior. (There are other ways to ensure a consistent
struct/class handling, but they would require more code.)
https://github.com/llvm/llvm-project/pull/96145
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
dzhidzhoev wrote: > Can't say I'm excited to see this feature (although I admit it is looking > less daunting than I originally feared). Can you explain why you need this > feature? Is there some particular aspect of lldb's remote operation that you > think isn't well covered by the existing tests? Is there a particular > category of tests that you'd like to have more than others? How many of the > existing tests would exercise the remote platform capability in a meaningful > way (a lot of these tests don't even build runnable binaries)? I'm sorry, it took a bit longer time to answer than I expected, my apologies. In general, our goal is to maximize the functionality of the testing toolkit in case of cross-compilation+remote launch setup. I agree that not every Shell test launches binaries. According to my grep-based statistics, it's roughly one-fifth of the tests currently (117/533 test files that contain "r", "run", or "process launch" commands). Though it's still a number. Also, launching the "platform select" and "platform connect ..." commands before executing the rest of a Shell test script may be useful. Here is an example case of a potentially unexpected behavior https://github.com/llvm/llvm-project/pull/94165. So it's not always necessary to have a Shell test build and run a binary to reveal some nuances. And it seems to me, that it won't cause excessive maintenance overhead since we essentially add just two extra commands to %lldb invocation. I haven't opened an RFC since I thought it would be clearer to discuss that idea with the showcase of implementation. I'm open to moving a discussion there if it's considered as a better approach in this case (please let me know). https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
https://github.com/DavidSpickett updated
https://github.com/llvm/llvm-project/pull/95768
>From a9b542a1686be0afd73ad89a504d2c66ba6c4a4f Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Mon, 11 Mar 2024 10:56:29 +
Subject: [PATCH 1/2] [lldb] Parse and display register field enums
This teaches lldb to parse the enum XML elements sent by
lldb-server, and make use of the information in `register read`
and `register info`.
The format is described in
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Enum-Target-Types.html.
The target XML parser will drop any invalid enum or evalue. If
we find multiple evalue for the same value, we will use the last
one we find.
The order of evalues from the XML is preserved as there may be
good reason they are not in numerical order.
---
lldb/include/lldb/Target/RegisterFlags.h | 7 +
lldb/source/Core/DumpRegisterInfo.cpp | 7 +-
.../Process/gdb-remote/ProcessGDBRemote.cpp | 188 +-
.../Process/gdb-remote/ProcessGDBRemote.h | 5 +
.../RegisterTypeBuilderClang.cpp | 30 +-
lldb/source/Target/RegisterFlags.cpp | 10 +
.../gdb_remote_client/TestXMLRegisterFlags.py | 322 ++
lldb/unittests/Core/DumpRegisterInfoTest.cpp | 26 ++
8 files changed, 573 insertions(+), 22 deletions(-)
diff --git a/lldb/include/lldb/Target/RegisterFlags.h
b/lldb/include/lldb/Target/RegisterFlags.h
index 1c6bf5dcf4a7f..628c841c10d95 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -32,10 +32,15 @@ class FieldEnum {
: m_value(value), m_name(std::move(name)) {}
void ToXML(Stream ) const;
+
+void log(Log *log) const;
};
typedef std::vector Enumerators;
+ // GDB also includes a "size" that is the size of the underlying register.
+ // We will not store that here but instead use the size of the register
+ // this gets attached to when emitting XML.
FieldEnum(std::string id, const Enumerators );
const Enumerators () const { return m_enumerators; }
@@ -44,6 +49,8 @@ class FieldEnum {
void ToXML(Stream , unsigned size) const;
+ void log(Log *log) const;
+
private:
std::string m_id;
Enumerators m_enumerators;
diff --git a/lldb/source/Core/DumpRegisterInfo.cpp
b/lldb/source/Core/DumpRegisterInfo.cpp
index 8334795416902..eccc6784cd497 100644
--- a/lldb/source/Core/DumpRegisterInfo.cpp
+++ b/lldb/source/Core/DumpRegisterInfo.cpp
@@ -111,6 +111,11 @@ void lldb_private::DoDumpRegisterInfo(
};
DumpList(strm, "In sets: ", in_sets, emit_set);
- if (flags_type)
+ if (flags_type) {
strm.Printf("\n\n%s", flags_type->AsTable(terminal_width).c_str());
+
+std::string enumerators = flags_type->DumpEnums(terminal_width);
+if (enumerators.size())
+ strm << "\n\n" << enumerators;
+ }
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index a5a731981299f..4754698e6e88f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4179,21 +4179,124 @@ struct GdbServerTargetInfo {
RegisterSetMap reg_set_map;
};
-static std::vector ParseFlagsFields(XMLNode flags_node,
- unsigned size) {
+static FieldEnum::Enumerators ParseEnumEvalues(const XMLNode _node) {
+ Log *log(GetLog(GDBRLog::Process));
+ // We will use the last instance of each value. Also we preserve the order
+ // of declaration in the XML, as it may not be numerical.
+ std::map enumerators;
+
+ enum_node.ForEachChildElementWithName(
+ "evalue", [, ](const XMLNode _node) {
+std::optional name;
+std::optional value;
+
+enumerator_node.ForEachAttribute(
+[, , ](const llvm::StringRef _name,
+ const llvm::StringRef _value) {
+ if (attr_name == "name") {
+if (attr_value.size())
+ name = attr_value;
+else
+ LLDB_LOG(log, "ProcessGDBRemote::ParseEnumEvalues "
+"Ignoring empty name in evalue");
+ } else if (attr_name == "value") {
+uint64_t parsed_value = 0;
+if (llvm::to_integer(attr_value, parsed_value))
+ value = parsed_value;
+else
+ LLDB_LOG(log,
+ "ProcessGDBRemote::ParseEnumEvalues "
+ "Invalid value \"{0}\" in "
+ "evalue",
+ attr_value.data());
+ } else
+LLDB_LOG(log,
+ "ProcessGDBRemote::ParseEnumEvalues Ignoring "
+ "unknown attribute "
+ "\"{0}\" in evalue",
+ attr_name.data());
+
+ // Keep
[Lldb-commits] [lldb] [lldb] Merge CompilerContextKind::{Class, Struct} (PR #96145)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/96145 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Merge CompilerContextKind::{Class, Struct} (PR #96145)
@@ -75,20 +75,18 @@ bool
lldb_private::contextMatches(llvm::ArrayRef context_chain,
static CompilerContextKind ConvertTypeClass(lldb::TypeClass type_class) {
if (type_class == eTypeClassAny)
return CompilerContextKind::AnyType;
- uint16_t result = 0;
- if (type_class & lldb::eTypeClassClass)
-result |= (uint16_t)CompilerContextKind::Class;
- if (type_class & lldb::eTypeClassStruct)
-result |= (uint16_t)CompilerContextKind::Struct;
Michael137 wrote:
Hmm it might actually be useful to see both in the output?
https://github.com/llvm/llvm-project/pull/96145
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Fix type definition search with simple template names (PR #95905)
@@ -3073,14 +3073,43 @@
SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE ) {
// See comments below about -gsimple-template-names for why we attempt to
// compute missing template parameter names.
-ConstString template_params;
-if (type_system) {
- DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
- if (dwarf_ast)
-template_params = dwarf_ast->GetDIEClassTemplateParams(die);
+std::vector template_params;
+DWARFDeclContext die_dwarf_decl_ctx;
+DWARFASTParser *dwarf_ast = type_system ? type_system->GetDWARFParser() :
nullptr;
+for (DWARFDIE ctx_die = die; ctx_die && !isUnitType(ctx_die.Tag());
+ ctx_die = ctx_die.GetParentDeclContextDIE()) {
+ die_dwarf_decl_ctx.AppendDeclContext(ctx_die.Tag(), ctx_die.GetName());
+ template_params.push_back(
+ (ctx_die.IsStructUnionOrClass() && dwarf_ast)
+ ? dwarf_ast->GetDIEClassTemplateParams(ctx_die)
+ : "");
}
+const bool any_template_params = llvm::any_of(
+template_params, [](llvm::StringRef p) { return !p.empty(); });
-const DWARFDeclContext die_dwarf_decl_ctx = die.GetDWARFDeclContext();
+auto die_matches = [&](DWARFDIE type_die) {
+ // Resolve the type if both have the same tag or {class, struct} tags.
+ const bool tag_matches =
+ type_die.Tag() == tag ||
+ (IsStructOrClassTag(type_die.Tag()) && IsStructOrClassTag(tag));
Michael137 wrote:
> they matter for creation of clang asts
FWIW, we do disable the main distinction (access control) between the two in
the AST. But fair point, there might be other places that care, not sure off
the top of my head.
> would be to have a sort of a getCanonicalTag() which maps structure_type to
> class_type (or the other way around), and then do these comparisons on the
> "canonical" tags.
Good idea!
https://github.com/llvm/llvm-project/pull/95905
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Parse and display register field enums (PR #95768)
@@ -43,8 +43,7 @@ CompilerType RegisterTypeBuilderClang::GetRegisterType( ScratchTypeSystemClang::GetForTarget(m_target); assert(type_system); - std::string register_type_name = "__lldb_register_fields_"; - register_type_name += name; + std::string register_type_name = "__lldb_register_fields_" + name; DavidSpickett wrote: I'll always need a StringRef for GetTypeForIdentifier, and I don't think the Twine can be represented as a single StringRef in this case. So I'll have to call .str() on the Twine regardless. Unless I am missing another advantage to Twine here? https://github.com/llvm/llvm-project/pull/95768 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Merge CompilerContextKind::{Class, Struct} (PR #96145)
@@ -75,20 +75,18 @@ bool
lldb_private::contextMatches(llvm::ArrayRef context_chain,
static CompilerContextKind ConvertTypeClass(lldb::TypeClass type_class) {
if (type_class == eTypeClassAny)
return CompilerContextKind::AnyType;
- uint16_t result = 0;
- if (type_class & lldb::eTypeClassClass)
-result |= (uint16_t)CompilerContextKind::Class;
- if (type_class & lldb::eTypeClassStruct)
-result |= (uint16_t)CompilerContextKind::Struct;
labath wrote:
This is a potentially controversial change. It means that if someone does a
`image lookup -t 'struct Foo'`, we will also return a `class Foo`. This could
be viewed as confusing, or as consistent with how we handle structures/classes
elsewhere...
https://github.com/llvm/llvm-project/pull/96145
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Merge CompilerContextKind::{Class, Struct} (PR #96145)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
Our dwarf parsing code treats structures and classes as interchangable.
CompilerContextKind is used when looking DIEs for types. This makes sure we
always they're treated the same way.
See also #95905#discussion_r1645686628.
---
Full diff: https://github.com/llvm/llvm-project/pull/96145.diff
11 Files Affected:
- (modified) lldb/include/lldb/lldb-private-enumerations.h (+6-4)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp (+4-8)
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+2-4)
- (modified) lldb/source/Symbol/Type.cpp (+10-15)
- (modified) lldb/test/Shell/SymbolFile/DWARF/clang-gmodules-type-lookup.c
(+1-1)
- (modified) lldb/test/Shell/SymbolFile/DWARF/x86/compilercontext.ll (+5-5)
- (modified) lldb/test/Shell/SymbolFile/DWARF/x86/find-basic-function.cpp
(+1-1)
- (modified) lldb/test/Shell/SymbolFile/DWARF/x86/module-ownership.mm (+2-2)
- (modified) lldb/tools/lldb-test/lldb-test.cpp (+1-2)
- (modified) lldb/unittests/Symbol/TestType.cpp (+12-18)
- (modified) lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp (+4-2)
``diff
diff --git a/lldb/include/lldb/lldb-private-enumerations.h
b/lldb/include/lldb/lldb-private-enumerations.h
index 68e060f2393f7..9d18316dcea25 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -10,6 +10,7 @@
#define LLDB_LLDB_PRIVATE_ENUMERATIONS_H
#include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/BitmaskEnum.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/FormatProviders.h"
#include "llvm/Support/raw_ostream.h"
@@ -197,8 +198,7 @@ enum class CompilerContextKind : uint16_t {
TranslationUnit = 1,
Module = 1 << 1,
Namespace = 1 << 2,
- Class = 1 << 3,
- Struct = 1 << 4,
+ ClassOrStruct = 1 << 3,
Union = 1 << 5,
Function = 1 << 6,
Variable = 1 << 7,
@@ -210,10 +210,12 @@ enum class CompilerContextKind : uint16_t {
/// Match 0..n nested modules.
AnyModule = Any | Module,
/// Match any type.
- AnyType = Any | Class | Struct | Union | Enum | Typedef | Builtin,
+ AnyType = Any | ClassOrStruct | Union | Enum | Typedef | Builtin,
/// Math any declaration context.
- AnyDeclContext = Any | Namespace | Class | Struct | Union | Enum | Function
+ AnyDeclContext = Any | Namespace | ClassOrStruct | Union | Enum | Function,
+ LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/AnyDeclContext),
};
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
// Enumerations that can be used to specify the kind of metric we're looking at
// when collecting stats.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index ada3da85112fe..fb32e2adeb3fe 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -394,15 +394,13 @@ static void GetDeclContextImpl(DWARFDIE die,
case DW_TAG_namespace:
push_ctx(CompilerContextKind::Namespace, die.GetName());
break;
+case DW_TAG_class_type:
case DW_TAG_structure_type:
- push_ctx(CompilerContextKind::Struct, die.GetName());
+ push_ctx(CompilerContextKind::ClassOrStruct, die.GetName());
break;
case DW_TAG_union_type:
push_ctx(CompilerContextKind::Union, die.GetName());
break;
-case DW_TAG_class_type:
- push_ctx(CompilerContextKind::Class, die.GetName());
- break;
case DW_TAG_enumeration_type:
push_ctx(CompilerContextKind::Enum, die.GetName());
break;
@@ -456,15 +454,13 @@ static void GetTypeLookupContextImpl(DWARFDIE die,
case DW_TAG_namespace:
push_ctx(CompilerContextKind::Namespace, die.GetName());
break;
+case DW_TAG_class_type:
case DW_TAG_structure_type:
- push_ctx(CompilerContextKind::Struct, die.GetName());
+ push_ctx(CompilerContextKind::ClassOrStruct, die.GetName());
break;
case DW_TAG_union_type:
push_ctx(CompilerContextKind::Union, die.GetName());
break;
-case DW_TAG_class_type:
- push_ctx(CompilerContextKind::Class, die.GetName());
- break;
case DW_TAG_enumeration_type:
push_ctx(CompilerContextKind::Enum, die.GetName());
break;
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 0270c67366d1d..ffd9ff2021fbd 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9173,10 +9173,8 @@ static CompilerContextKind
GetCompilerKind(clang::Decl::Kind clang_kind,
if (decl_ctx) {
if (decl_ctx->isFunctionOrMethod())
return CompilerContextKind::Function;
- else if (decl_ctx->isRecord())
-return (CompilerContextKind)((uint16_t)CompilerContextKind::Class |
-
[Lldb-commits] [lldb] [lldb] Merge CompilerContextKind::{Class, Struct} (PR #96145)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/96145 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Merge CompilerContextKind::{Class, Struct} (PR #96145)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/96145
Our dwarf parsing code treats structures and classes as interchangable.
CompilerContextKind is used when looking DIEs for types. This makes sure we
always they're treated the same way.
See also #95905#discussion_r1645686628.
>From 87d0cd2391a3378adaf17c366705e3625aa6436d Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Thu, 20 Jun 2024 09:55:15 +0200
Subject: [PATCH] [lldb] Merge CompilerContextKind::{Class,Struct}
Our dwarf parsing code treats structures and classes as interchangable.
CompilerContextKind is used when looking DIEs for types. This makes sure
we always they're treated the same way.
See also #95905#discussion_r1645686628.
---
lldb/include/lldb/lldb-private-enumerations.h | 10 ---
.../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 12 +++-
.../TypeSystem/Clang/TypeSystemClang.cpp | 6 ++--
lldb/source/Symbol/Type.cpp | 25 +++-
.../DWARF/clang-gmodules-type-lookup.c| 2 +-
.../SymbolFile/DWARF/x86/compilercontext.ll | 10 +++
.../DWARF/x86/find-basic-function.cpp | 2 +-
.../SymbolFile/DWARF/x86/module-ownership.mm | 4 +--
lldb/tools/lldb-test/lldb-test.cpp| 3 +-
lldb/unittests/Symbol/TestType.cpp| 30 ---
.../SymbolFile/DWARF/DWARFDIETest.cpp | 6 ++--
11 files changed, 48 insertions(+), 62 deletions(-)
diff --git a/lldb/include/lldb/lldb-private-enumerations.h
b/lldb/include/lldb/lldb-private-enumerations.h
index 68e060f2393f7..9d18316dcea25 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -10,6 +10,7 @@
#define LLDB_LLDB_PRIVATE_ENUMERATIONS_H
#include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/BitmaskEnum.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/FormatProviders.h"
#include "llvm/Support/raw_ostream.h"
@@ -197,8 +198,7 @@ enum class CompilerContextKind : uint16_t {
TranslationUnit = 1,
Module = 1 << 1,
Namespace = 1 << 2,
- Class = 1 << 3,
- Struct = 1 << 4,
+ ClassOrStruct = 1 << 3,
Union = 1 << 5,
Function = 1 << 6,
Variable = 1 << 7,
@@ -210,10 +210,12 @@ enum class CompilerContextKind : uint16_t {
/// Match 0..n nested modules.
AnyModule = Any | Module,
/// Match any type.
- AnyType = Any | Class | Struct | Union | Enum | Typedef | Builtin,
+ AnyType = Any | ClassOrStruct | Union | Enum | Typedef | Builtin,
/// Math any declaration context.
- AnyDeclContext = Any | Namespace | Class | Struct | Union | Enum | Function
+ AnyDeclContext = Any | Namespace | ClassOrStruct | Union | Enum | Function,
+ LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/AnyDeclContext),
};
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
// Enumerations that can be used to specify the kind of metric we're looking at
// when collecting stats.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index ada3da85112fe..fb32e2adeb3fe 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -394,15 +394,13 @@ static void GetDeclContextImpl(DWARFDIE die,
case DW_TAG_namespace:
push_ctx(CompilerContextKind::Namespace, die.GetName());
break;
+case DW_TAG_class_type:
case DW_TAG_structure_type:
- push_ctx(CompilerContextKind::Struct, die.GetName());
+ push_ctx(CompilerContextKind::ClassOrStruct, die.GetName());
break;
case DW_TAG_union_type:
push_ctx(CompilerContextKind::Union, die.GetName());
break;
-case DW_TAG_class_type:
- push_ctx(CompilerContextKind::Class, die.GetName());
- break;
case DW_TAG_enumeration_type:
push_ctx(CompilerContextKind::Enum, die.GetName());
break;
@@ -456,15 +454,13 @@ static void GetTypeLookupContextImpl(DWARFDIE die,
case DW_TAG_namespace:
push_ctx(CompilerContextKind::Namespace, die.GetName());
break;
+case DW_TAG_class_type:
case DW_TAG_structure_type:
- push_ctx(CompilerContextKind::Struct, die.GetName());
+ push_ctx(CompilerContextKind::ClassOrStruct, die.GetName());
break;
case DW_TAG_union_type:
push_ctx(CompilerContextKind::Union, die.GetName());
break;
-case DW_TAG_class_type:
- push_ctx(CompilerContextKind::Class, die.GetName());
- break;
case DW_TAG_enumeration_type:
push_ctx(CompilerContextKind::Enum, die.GetName());
break;
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 0270c67366d1d..ffd9ff2021fbd 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9173,10 +9173,8 @@ static CompilerContextKind
GetCompilerKind(clang::Decl::Kind clang_kind,
if (decl_ctx) {
[Lldb-commits] [lldb] [lldb/DWARF] Fix type definition search with simple template names (PR #95905)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/95905 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
