date:20230111

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Enrico Loparco via Phabricator via lldb-commits

eloparco updated this revision to Diff 488117.
eloparco added a comment.

Add binding and test for SBTarget::GetMaximumOpcodeByteSize()


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

Files:
  lldb/bindings/interface/SBTarget.i
  lldb/include/lldb/API/SBTarget.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/source/API/SBTarget.cpp
  lldb/test/API/python_api/target/TestTargetAPI.py
  lldb/test/API/tools/lldb-vscode/disassemble/Makefile
  lldb/test/API/tools/lldb-vscode/disassemble/TestVSCode_disassemble.py
  lldb/test/API/tools/lldb-vscode/disassemble/main.cpp
  lldb/tools/lldb-vscode/CMakeLists.txt
  lldb/tools/lldb-vscode/DisassembledInstruction.cpp
  lldb/tools/lldb-vscode/DisassembledInstruction.h
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/LLDBUtils.cpp
  lldb/tools/lldb-vscode/LLDBUtils.h
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/VSCodeForward.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1451,8 +1451,7 @@
   // which may affect the outcome of tests.
   bool source_init_file = GetBoolean(arguments, "sourceInitFile", true);
 
-  g_vsc.debugger =
-  lldb::SBDebugger::Create(source_init_file, log_cb, nullptr);
+  g_vsc.debugger = lldb::SBDebugger::Create(source_init_file, log_cb, nullptr);
   g_vsc.progress_event_thread = std::thread(ProgressEventThreadFunction);
 
   // Start our event thread so we can receive events from the debugger, target,
@@ -1542,6 +1541,8 @@
   // The debug adapter supports 'logMessage' in breakpoint.
   body.try_emplace("supportsLogPoints", true);
 
+  body.try_emplace("supportsDisassembleRequest", true);
+
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
@@ -2117,6 +2118,178 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+std::vector
+_get_instructions_from_memory(lldb::addr_t start, uint64_t count) {
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+
+  lldb::SBError error;
+  std::vector buffer(count, 0);
+  const size_t bytes_read __attribute__((unused)) = process.ReadMemory(
+  start, static_cast(buffer.data()), count, error);
+  assert(bytes_read == count && error.Success() &&
+ "unable to read byte range from memory");
+
+  // If base_addr starts in the middle of an instruction,
+  // that first instruction will not be parsed correctly (negligible)
+  std::vector sb_instructions;
+  const auto base_addr = lldb::SBAddress(start, g_vsc.target);
+  lldb::SBInstructionList instructions =
+  g_vsc.target.GetInstructions(base_addr, buffer.data(), count);
+
+  for (size_t i = 0; i < instructions.GetSize(); i++) {
+auto instr = instructions.GetInstructionAtIndex(i);
+sb_instructions.emplace_back(instr);
+  }
+  return sb_instructions;
+}
+
+auto _handle_disassemble_positive_offset(lldb::addr_t base_addr,
+ int64_t instruction_offset,
+ uint64_t instruction_count) {
+  llvm::json::Array response_instructions;
+
+  /**
+   * For positive offsets, we use the `ReadInstructions()` API to get
+   * `instruction_offset + instruction_count` instructions after the
+   * `base_addr`.
+   */
+  auto start_addr = lldb::SBAddress(base_addr, g_vsc.target);
+  lldb::SBInstructionList instructions = g_vsc.target.ReadInstructions(
+  start_addr, instruction_offset + instruction_count);
+
+  const auto num_instrs_to_skip = static_cast(instruction_offset);
+  for (size_t i = num_instrs_to_skip; i < instructions.GetSize(); ++i) {
+lldb::SBInstruction instr = instructions.GetInstructionAtIndex(i);
+
+auto disass_instr =
+CreateDisassembledInstruction(DisassembledInstruction(instr));
+response_instructions.emplace_back(std::move(disass_instr));
+  }
+
+  return response_instructions;
+}
+
+auto _handle_disassemble_negative_offset(
+lldb::addr_t base_addr, int64_t instruction_offset,
+uint64_t instruction_count,
+std::optional memory_reference) {
+  llvm::json::Array response_instructions;
+
+  const auto max_instruction_size = g_vsc.target.GetMaximumOpcodeByteSize();
+  const auto bytes_offset = -instruction_offset * max_instruction_size;
+  auto start_addr = base_addr - bytes_offset;
+  const auto disassemble_bytes = instruction_count * max_instruction_size;
+
+  /**
+   * For negative offsets, we do not know what `start_addr` corresponds to the
+   * instruction located `instruction_offset` instructions before `base_addr`
+   * since on some architectures opcodes have variable length.
+   *
+   * To address that, we need to read at least starting from `start_addr =
+   * base_addr

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Enrico Loparco via Phabricator via lldb-commits

eloparco added a comment.

@jingham I added the test that was missing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

2023-01-11 Thread Pavel Kosov via Phabricator via lldb-commits

kpdev42 updated this revision to Diff 488154.
kpdev42 added a comment.

Renaming and cleanup according to review


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140368/new/

https://reviews.llvm.org/D140368

Files:
  lldb/source/Target/StopInfo.cpp
  lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/Makefile
  
lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
  lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp

Index: lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
@@ -0,0 +1,11 @@
+int func_1() { return 1; }
+
+int func_2() {
+  func_1(); // breakpoint_0
+  return 1 + func_1();  // breakpoint_1
+}
+
+int main(int argc, char const *argv[]) {
+  func_2();  // Start from here
+  return 0;
+}
Index: lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
@@ -0,0 +1,121 @@
+"""
+Test that breakpoints (reason = breakpoint) have more priority than
+plan completion (reason = step in/out/over) when reporting stop reason after step,
+in particular 'step out' and 'step over', and in addition 'step in'.
+Check for correct StopReason when stepping to the line with breakpoint,
+which should be eStopReasonBreakpoint in general,
+and eStopReasonPlanComplete when breakpoint's condition fails or it is disabled.
+"""
+
+
+import unittest2
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ThreadPlanUserBreakpointsTestCase(TestBase):
+
+def setUp(self):
+TestBase.setUp(self)
+
+# Build and run to starting breakpoint
+self.build()
+src = lldb.SBFileSpec('main.cpp')
+(self.target, self.process, self.thread, _) = \
+lldbutil.run_to_source_breakpoint(self, '// Start from here', src)
+
+# Setup two more breakpoints
+self.breakpoints = [self.target.BreakpointCreateBySourceRegex('breakpoint_%i' % i, src)
+for i in range(2)]
+self.assertTrue(
+all(bp and bp.GetNumLocations() == 1 for bp in self.breakpoints),
+VALID_BREAKPOINT)
+
+def check_correct_stop_reason(self, breakpoint_idx, condition):
+self.assertState(self.process.GetState(), lldb.eStateStopped)
+if condition:
+# All breakpoints active, stop reason is breakpoint
+thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[breakpoint_idx])
+self.assertEquals(self.thread, thread1, "Didn't stop at breakpoint %i." % breakpoint_idx)
+else:
+# Breakpoints are inactive, stop reason is plan complete
+self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete,
+'Expected stop reason to be step into/over/out for inactive breakpoint %i line.' % breakpoint_idx)
+
+def change_breakpoints(self, action):
+for bp in self.breakpoints:
+action(bp)
+
+def check_thread_plan_user_breakpoint(self, condition, set_up_breakpoint_func):
+# Make breakpoints active/inactive in different ways
+self.change_breakpoints(lambda bp: set_up_breakpoint_func(condition, bp))
+
+self.thread.StepInto()
+# We should be stopped at the breakpoint_0 line with the correct stop reason
+self.check_correct_stop_reason(0, condition)
+
+# This step-over creates a step-out from `func_1` plan
+self.thread.StepOver()
+# We should be stopped at the breakpoint_1 line with the correct stop reason
+self.check_correct_stop_reason(1, condition)
+
+# Check explicit step-out
+# Make sure we install the breakpoint at the right address:
+# step-out might stop on different lines, if the compiler
+# did or did not emit more instructions after the return
+return_addr = self.thread.GetFrameAtIndex(1).GetPC()
+step_out_breakpoint = self.target.BreakpointCreateByAddress(return_addr)
+self.assertTrue(step_out_breakpoint, VALID_BREAKPOINT)
+set_up_breakpoint_func(condition, step_out_breakpoint)
+self.breakpoints.append(step_out_breakpoint)
+self.thread.StepOut()
+# We should be stopped somewhere in the main frame with the correct stop reason
+self.check_correct_stop_reason(2, condition)
+
+# Run the process until termination
+self.process.Continue()
+self.assertState(self.process.GetState(), lldb.eStateExited)

[Lldb-commits] [PATCH] D141330: [lldb] Limit 8b259fe573e1 to dSYMs

2023-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D141330#4037932 , @JDevlieghere 
wrote:

> In D141330#4037925 , @aprantl wrote:
>
>> Should this be true for a fully linked ELF executable, too?
>
> Sounds reasonable, but adding @labath and @DavidSpickett to confirm. This is 
> trivial to extend later.

It sounds like this is depends on whoever produced the file, not on the file 
format.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141330/new/

https://reviews.llvm.org/D141330

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141021: [lldb] Remove tools copied into LLDB.framework before install

2023-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Does this mean that the in-tree lldb will cease to be functional once someone 
"installs" it? (at least until the next rebuild)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141021/new/

https://reviews.llvm.org/D141021

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

2023-01-11 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/examples/python/scripted_process/scripted_platform.py:64
+def launch_process(self, launch_info):
+""" Launch a scripted process.
+

Does this really have to be a scripted process? Ideally, one could also launch 
a "normal" ProcessGdbRemote this way. This is pretty much what happens in 
PlatformQemuUser, and if that worked, then we could theoretically replace that 
built-in platform with a simple python script.



Comment at: lldb/examples/python/scripted_process/scripted_platform.py:76
+def kill_process(self, pid):
+""" Kill a scripted process.
+

ditto



Comment at: lldb/examples/python/scripted_process/scripted_platform.py:31
+def list_processes(self):
+""" Get a list of processes that can be ran on the platform.
+

mib wrote:
> labath wrote:
> > mib wrote:
> > > labath wrote:
> > > > mib wrote:
> > > > > labath wrote:
> > > > > > mib wrote:
> > > > > > > mib wrote:
> > > > > > > > mib wrote:
> > > > > > > > > labath wrote:
> > > > > > > > > > I am surprised that you want to go down the "run" path for 
> > > > > > > > > > this functionality. I think most of the launch 
> > > > > > > > > > functionality does not make sense for this use case (e.g., 
> > > > > > > > > > you can't provide arguments to these processes, when you 
> > > > > > > > > > "run" them, can you?), and it is not consistent with what 
> > > > > > > > > > the "process listing" functionality does for regular 
> > > > > > > > > > platforms.
> > > > > > > > > > 
> > > > > > > > > > OTOH, the "attach" flow makes perfect sense here -- you 
> > > > > > > > > > take the pid of an existing process, attach to it, and stop 
> > > > > > > > > > it at a random point in its execution. You can't customize 
> > > > > > > > > > anything about how that process is run (because it's 
> > > > > > > > > > already running) -- all you can do is choose how you want 
> > > > > > > > > > to select the target process.
> > > > > > > > > For now, there is no support for attaching to a scripted 
> > > > > > > > > process, because we didn't have any use for it quite yet: 
> > > > > > > > > cripted processes were mostly used for doing post-mortem 
> > > > > > > > > debugging, so we "ran" them artificially in lldb by providing 
> > > > > > > > > some launch options (the name of the class managing the 
> > > > > > > > > process and an optional user-provided dictionary) through the 
> > > > > > > > > command line or using an `SBLaunchInfo` object.
> > > > > > > > > 
> > > > > > > > > I guess I'll need to extend the `platform process 
> > > > > > > > > launch/attach` commands and `SBAttachInfo` object to also 
> > > > > > > > > support these options since they're required for the scripted 
> > > > > > > > > process instantiation.
> > > > > > > > > 
> > > > > > > > > Note that we aren't really attaching to the real running 
> > > > > > > > > process, we're creating a scripted process that knows how to 
> > > > > > > > > read memory to mock the real process.
> > > > > > > > @labath, I'll do that work on a follow-up patch
> > > > > > > @labath here D139945 :) 
> > > > > > Thanks. However, are you still planning to use the launch path for 
> > > > > > your feature? Because if you're not, then I think this comment 
> > > > > > should say "Get a list of processes that **are running**" (or that 
> > > > > > **can be attached to**).
> > > > > > 
> > > > > > And if you are, then I'd like to hear your thoughts on the 
> > > > > > discrepancy between what "launching" means for scripted and 
> > > > > > non-scripted platforms.
> > > > > > 
> > > > > The way I see it is that the scripted platform will create a process 
> > > > > with the right process plugin. In the case of scripted processes, the 
> > > > > `ProcessLaunchInfo` argument should have the script class name set 
> > > > > (which automatically sets the process plugin name to 
> > > > > "ScriptedProcess" in the launch info). Once the process is 
> > > > > instantiated (before the launch), the scripted platform will need to 
> > > > > redirect to process stop events through its event multiplexer. So the 
> > > > > way I see it essentially, running a regular process with the scripted 
> > > > > platform should be totally transparent.
> > > > > 
> > > > > Something that is also worth discussing IMO, is the discrepancy 
> > > > > between launching and attaching from the scripted platform:
> > > > > 
> > > > > One possibility could be that `platform process launch` would launch 
> > > > > all the scripted processes listed by the scripted platform and set 
> > > > > them up with the multiplexer, whereas `platform process attach` would 
> > > > > just create a scripted process individually. I know this doesn't 
> > > > > match the current behavior of the platform commands so if you guys 
> > > > > think we should preserve the expected behavior, I guess.

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

2023-01-11 Thread Simon Pilgrim via Phabricator via lldb-commits

RKSimon added a comment.

@ayermolo This has broken some buildbots - please can you take a look? 
https://lab.llvm.org/buildbot/#/builders/245/builds/3279


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139379/new/

https://reviews.llvm.org/D139379

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

2023-01-11 Thread Alexander Yermolovich via Phabricator via lldb-commits

ayermolo added a comment.

In D139379#4043492 , @RKSimon wrote:

> @ayermolo This has broken some buildbots - please can you take a look? 
> https://lab.llvm.org/buildbot/#/builders/245/builds/3279

ok


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139379/new/

https://reviews.llvm.org/D139379

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

2023-01-11 Thread Nikita Popov via Phabricator via lldb-commits

nikic added a comment.

Just to check, this isn't going to cause some warning spew about all those 
OptTable implementations being non-final?




Comment at: llvm/include/llvm/Option/OptTable.h:256
 
+/// Specialization of
+class GenericOptTable : public OptTable {

Comment looks a bit unfinished :)



Comment at: llvm/unittests/Option/OptionParsingTest.cpp:87
+
+template  class DISABLED_OptTableTest : public ::testing::Test {};
+

What's this about?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140800/new/

https://reviews.llvm.org/D140800

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

2023-01-11 Thread Alexander Yermolovich via Phabricator via lldb-commits

ayermolo added a comment.

https://reviews.llvm.org/D141504
TBH not 100% sure on the fix, but looking where it's failing, maybe it's some 
kind of interaction with APIs returning 64bit number, and 32bit format macros 
on ARM? 
I don't have access to ARM machine.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139379/new/

https://reviews.llvm.org/D139379

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141330: [lldb] Limit 8b259fe573e1 to dSYMs

2023-01-11 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

In D141330#4043397 , @labath wrote:

> In D141330#4037932 , @JDevlieghere 
> wrote:
>
>> In D141330#4037925 , @aprantl 
>> wrote:
>>
>>> Should this be true for a fully linked ELF executable, too?
>>
>> Sounds reasonable, but adding @labath and @DavidSpickett to confirm. This is 
>> trivial to extend later.
>
> It sounds like this is depends on whoever produced the file, not on the file 
> format.

It's not even the producer (compiler/assembler) - it's the linker (or whatever 
links the dwarf) that needs to generate the table, and we can't detect that 
from the debug info.

We need (1) a guarantee that every compile unit which generated code in the 
debug_info has an entry in debug_aranges, (2) a guarantee that all of the 
address ranges that come from the CU are described, and (3) a guarantee that 
the ranges in debug_aranges are unique, that no address will map to multiple 
compile units.  The description in the dwarf v5 doc includes,

  By scanning the table, a debugger can quickly decide which compilation unit 
to look in to find the debugging information for an object that has a given 
address.

  If the range of addresses covered by the text and/or data of a compilation 
unit is not contiguous, then there may be multiple address range descriptors 
for that compilation unit.

(I dislike it when the standard says "there MAY be multiple address range 
descriptors" -- does this mean if I have a noncontiguous CUs A and B 
interleaved in the final binary, the debug_aranges can overlap?)

There's no guarantee for (1) if a debug_aranges table is present, but maybe we 
can simply assume that any producer producing debug_aranges has done so 
comprehensively.  I couldn't imagine why it wouldn't be that way.

The point of this patch, of course, is to skip the verification of (1) - that 
every CU that generated functions in the final binary has a debug_aranges 
entry.  It turns out we have compile units in debug_info that don't emit any 
functions, and lldb would see those as missing from debug_aranges and iterate 
over the DIEs looking for subprograms.

Maybe lldb should simply trust that if debug_aranges exists, all of 1-3 are 
true, and revisit it when/if we get bug reports of some random toolchain in the 
world violating that assumption.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141330/new/

https://reviews.llvm.org/D141330

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141425: [lldb] Add --gdb-format flag to dwim-print

2023-01-11 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added a comment.

@jingham The brief answer is that decisions have been based on compatibility 
with the behavior of `p`.

1. `expression` (also `p`) and `frame variable`, only support the the format 
part of gdb options. Neither support `--count` or `--size` and so don't support 
the equivalent gdb options.
2. `p` takes no flags (other than the gdb-format), and `dwim-print` matches 
that behavior.

Taking the example of `dwim-print -fA variable`, if the user were to do the 
same but with an expression, they'd have to write `dwim-print -fA -- 
expression`. Will there be users who know and want the printing options you 
mentioned (synthetic/raw, depth, etc) but who try to use those with 
`dwim-print` instead of directly using `v` or `e`?

My expectation has been that `dwim-print` would (1) generally not be used 
directly, but be via an alias (either `p` or another choice) and that (2) for 
compatibility, the alias would be `dwim-print --`.

Thoughts?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141425/new/

https://reviews.llvm.org/D141425

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141021: [lldb] Remove tools copied into LLDB.framework before install

2023-01-11 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

In D141021#4043411 , @labath wrote:

> Does this mean that the in-tree lldb will cease to be functional once someone 
> "installs" it? (at least until the next rebuild)

I believe so, but only in the case where you are building the LLDB framework on 
a macOS machine. I don't think this is ideal behavior but I personally think 
the tradeoff is worth it since you have deterministic installations behavior 
w.r.t. RPATHs.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141021/new/

https://reviews.llvm.org/D141021

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

2023-01-11 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda accepted this revision.
jasonmolenda added a comment.

LGTM.  Jim?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140249/new/

https://reviews.llvm.org/D140249

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140293: [lldb] Update custom commands to always be overrriden

2023-01-11 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM. The pattern of `debugger.HandleCommand("command script add -o -c [...]")` 
seems rather tedious. Would it make sense to add a wrapper to the SBAPI that 
takes care of that?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140293/new/

https://reviews.llvm.org/D140293

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

2023-01-11 Thread Jim Ingham via Phabricator via lldb-commits

jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for figuring this out!




Comment at: lldb/source/Target/StopInfo.cpp:540
 
+if (m_should_stop && !bp_loc_sp->GetBreakpoint().IsInternal())
+  internal_breakpoint = false;

DavidSpickett wrote:
> I think the key here is the `m_should_stop` check (the rest looks equivalent 
> to what is there already). What exactly does that achieve?
The point is that once we've looked at all the locations that were in the site 
we stopped at, if all the breakpoints that said we should stop here were 
internal, then we must be implementing a thread plan and we should give the 
thread plan a chance to set the stop info.  But if there were any non-internal 
breakpoints that say we should stop, then we should report the breakpoint stop 
reason instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140368/new/

https://reviews.llvm.org/D140368

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Jim Ingham via Phabricator via lldb-commits

jingham accepted this revision.
jingham added a comment.

That looks fine.  I'm removing my objection, but that's just to the SB API 
parts, I'm not commenting on the vscode part.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

2023-01-11 Thread Jim Ingham via Phabricator via lldb-commits

jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140249/new/

https://reviews.llvm.org/D140249

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140293: [lldb] Update custom commands to always be overrriden

2023-01-11 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

In D140293#4044524 , @JDevlieghere 
wrote:

> LGTM. The pattern of `debugger.HandleCommand("command script add -o -c 
> [...]")` seems rather tedious. Would it make sense to add a wrapper to the 
> SBAPI that takes care of that?

I think that would make sense. I'll do it in a follow-up!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140293/new/

https://reviews.llvm.org/D140293

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141539: [lldb][test] Replace use of p with expression (NFC)

2023-01-11 Thread Dave Lee via Phabricator via lldb-commits

kastiglione created this revision.
kastiglione added a reviewer: aprantl.
Herald added subscribers: kbarton, nemanjai.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

In API tests, replace use of the `p` alias with the `expression` command.

To avoid conflating tests of the alias with tests of the expression command,
this patch canonicalizes to the use `expression`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141539

Files:
  
lldb/test/API/commands/expression/codegen-crash-typedefdecl-not-in_declcontext/main.cpp
  
lldb/test/API/commands/expression/persist_objc_pointeetype/TestPersistObjCPointeeType.py
  lldb/test/API/commands/expression/rdar44436068/main.c
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  lldb/test/API/commands/target/dump-pcm-info/TestDumpPCMInfo.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-ptr-to-array/TestPtrToArrayFormatting.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-smart-array/TestDataFormatterSmartArray.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/list/TestDataFormatterGenericList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/multiset/TestDataFormatterGenericMultiSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/vector/TestDataFormatterLibcxxVector.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/map/TestDataFormatterStdMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py
  lldb/test/API/functionalities/inferior-assert/TestInferiorAssert.py
  lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashing.py
  lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
  
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferior.py
  
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
  lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py
  lldb/test/API/functionalities/set-data/TestSetData.py
  lldb/test/API/functionalities/ubsan/user-expression/TestUbsanUserExpression.py
  lldb/test/API/lang/c/enum_types/TestEnumTypes.py
  lldb/test/API/lang/c/strings/TestCStrings.py
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py
  lldb/test/API/lang/objc/modules-auto-import/TestModulesAutoImport.py
  lldb/test/API/lang/objc/modules/TestObjCModules.py
  lldb/test/API/lang/objc/objc-struct-argument/TestObjCStructArgument.py
  lldb/test/API/lang/objc/radar-9691614/TestObjCMethodReturningBOOL.py
  
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
  lldb/test/API/linux/builtin_trap/TestBuiltinTrap.py
  lldb/test/API/lua_api/TestFileHandle.lua
  lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
  lldb/test/API/macosx/macCatalyst/TestMacCatalyst.py
  
lldb/test/API/macosx/macCatalystAppMacOSFramework/TestMacCatalystAppWithMacOSFramework.py
  lldb/test/API/python_api/file_handle/TestFileHandle.py

Index: lldb/test/API/python_api/file_handle/TestFileHandle.py
===
--- lldb/test/API/python_api/file_handle/TestFileHandle.py
+++ lldb/test/API/python_api/file_handle/TestFileHandle.py
@@ -135,7 +135,7 @@
 def test_legacy_file_out(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetOutputFileHandle(f, False)
-self.handleCmd('p/x 3735928559', collect_result=False, check=False)
+self.handleCmd('expression/x 3735928559', collect_result=False, check=False)
 with open(self.out_filename, 'r') as f:
 self.assertIn('deadbeef', f.read())
 
@@ -359,7 +359,7 @@
 
 
 def test_string_inout(self):
-inf = io.StringIO("help help\np/x ~0\n")
+inf = io.StringIO("help help\nexpression/x ~0\n")
 outf = io.StringIO()
 status = self.dbg.SetOutputFile(lldb.SBFile(outf))
 self.assertSuccess(status)
Index: lldb/test/API/macosx/macCatalystAppMacOSFramework/TestMacCatalystAppWithMacOSFramework.py
===
--- lldb/test/API/macosx/macCatalystAppMacOSFramework/TestMacCatalystAppWithMacOSFramework.py
+++ lldb/test/API

[Lldb-commits] [lldb] f2f3b1a - [lldb] Do not deallocate memory after exec

2023-01-11 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2023-01-11T12:33:41-08:00
New Revision: f2f3b1a87ad294433986683864f75575c3a81779

URL: 
https://github.com/llvm/llvm-project/commit/f2f3b1a87ad294433986683864f75575c3a81779
DIFF: 
https://github.com/llvm/llvm-project/commit/f2f3b1a87ad294433986683864f75575c3a81779.diff

LOG: [lldb] Do not deallocate memory after exec

After an exec has occured, resources used to manage the state of a
Process are cleaned up. One such resource is the AllocatedMemoryCache
which keeps track of memory allocations made in the process for things
like expression evaluation. After an exec is performed, the allocated
memory regions in the process are gone, so it does not make sense to try
to deallocate those regions.

rdar://103188106

Differential Revision: https://reviews.llvm.org/D140249

Added: 


Modified: 
lldb/include/lldb/Target/Memory.h
lldb/source/Target/Memory.cpp
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Memory.h 
b/lldb/include/lldb/Target/Memory.h
index b3ad22aff16bd..8c564e8f5a8c5 100644
--- a/lldb/include/lldb/Target/Memory.h
+++ b/lldb/include/lldb/Target/Memory.h
@@ -116,7 +116,7 @@ class AllocatedMemoryCache {
 
   ~AllocatedMemoryCache();
 
-  void Clear();
+  void Clear(bool deallocate_memory);
 
   lldb::addr_t AllocateMemory(size_t byte_size, uint32_t permissions,
   Status &error);

diff  --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp
index a07a72f31922e..d4dedeea7c2c5 100644
--- a/lldb/source/Target/Memory.cpp
+++ b/lldb/source/Target/Memory.cpp
@@ -332,9 +332,9 @@ AllocatedMemoryCache::AllocatedMemoryCache(Process &process)
 
 AllocatedMemoryCache::~AllocatedMemoryCache() = default;
 
-void AllocatedMemoryCache::Clear() {
+void AllocatedMemoryCache::Clear(bool deallocate_memory) {
   std::lock_guard guard(m_mutex);
-  if (m_process.IsAlive()) {
+  if (m_process.IsAlive() && deallocate_memory) {
 PermissionsToBlockMap::iterator pos, end = m_memory_map.end();
 for (pos = m_memory_map.begin(); pos != end; ++pos)
   m_process.DoDeallocateMemory(pos->second->GetBaseAddress());

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 1abe12f2a5920..e0cca0595ee30 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -543,7 +543,7 @@ void Process::Finalize() {
   m_notifications.swap(empty_notifications);
   m_image_tokens.clear();
   m_memory_cache.Clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*deallocate_memory=*/true);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
@@ -5657,7 +5657,9 @@ void Process::DidExec() {
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_image_tokens.clear();
-  m_allocated_memory_cache.Clear();
+  // After an exec, the inferior is a new process and these memory regions are
+  // no longer allocated.
+  m_allocated_memory_cache.Clear(/*deallocte_memory=*/false);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140249: [lldb] Do not deallocate memory after exec

2023-01-11 Thread Alex Langford via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf2f3b1a87ad2: [lldb] Do not deallocate memory after exec 
(authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140249/new/

https://reviews.llvm.org/D140249

Files:
  lldb/include/lldb/Target/Memory.h
  lldb/source/Target/Memory.cpp
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -543,7 +543,7 @@
   m_notifications.swap(empty_notifications);
   m_image_tokens.clear();
   m_memory_cache.Clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*deallocate_memory=*/true);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
@@ -5657,7 +5657,9 @@
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_image_tokens.clear();
-  m_allocated_memory_cache.Clear();
+  // After an exec, the inferior is a new process and these memory regions are
+  // no longer allocated.
+  m_allocated_memory_cache.Clear(/*deallocte_memory=*/false);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
Index: lldb/source/Target/Memory.cpp
===
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -332,9 +332,9 @@
 
 AllocatedMemoryCache::~AllocatedMemoryCache() = default;
 
-void AllocatedMemoryCache::Clear() {
+void AllocatedMemoryCache::Clear(bool deallocate_memory) {
   std::lock_guard guard(m_mutex);
-  if (m_process.IsAlive()) {
+  if (m_process.IsAlive() && deallocate_memory) {
 PermissionsToBlockMap::iterator pos, end = m_memory_map.end();
 for (pos = m_memory_map.begin(); pos != end; ++pos)
   m_process.DoDeallocateMemory(pos->second->GetBaseAddress());
Index: lldb/include/lldb/Target/Memory.h
===
--- lldb/include/lldb/Target/Memory.h
+++ lldb/include/lldb/Target/Memory.h
@@ -116,7 +116,7 @@
 
   ~AllocatedMemoryCache();
 
-  void Clear();
+  void Clear(bool deallocate_memory);
 
   lldb::addr_t AllocateMemory(size_t byte_size, uint32_t permissions,
   Status &error);


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -543,7 +543,7 @@
   m_notifications.swap(empty_notifications);
   m_image_tokens.clear();
   m_memory_cache.Clear();
-  m_allocated_memory_cache.Clear();
+  m_allocated_memory_cache.Clear(/*deallocate_memory=*/true);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
@@ -5657,7 +5657,9 @@
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_image_tokens.clear();
-  m_allocated_memory_cache.Clear();
+  // After an exec, the inferior is a new process and these memory regions are
+  // no longer allocated.
+  m_allocated_memory_cache.Clear(/*deallocte_memory=*/false);
   {
 std::lock_guard guard(m_language_runtimes_mutex);
 m_language_runtimes.clear();
Index: lldb/source/Target/Memory.cpp
===
--- lldb/source/Target/Memory.cpp
+++ lldb/source/Target/Memory.cpp
@@ -332,9 +332,9 @@
 
 AllocatedMemoryCache::~AllocatedMemoryCache() = default;
 
-void AllocatedMemoryCache::Clear() {
+void AllocatedMemoryCache::Clear(bool deallocate_memory) {
   std::lock_guard guard(m_mutex);
-  if (m_process.IsAlive()) {
+  if (m_process.IsAlive() && deallocate_memory) {
 PermissionsToBlockMap::iterator pos, end = m_memory_map.end();
 for (pos = m_memory_map.begin(); pos != end; ++pos)
   m_process.DoDeallocateMemory(pos->second->GetBaseAddress());
Index: lldb/include/lldb/Target/Memory.h
===
--- lldb/include/lldb/Target/Memory.h
+++ lldb/include/lldb/Target/Memory.h
@@ -116,7 +116,7 @@
 
   ~AllocatedMemoryCache();
 
-  void Clear();
+  void Clear(bool deallocate_memory);
 
   lldb::addr_t AllocateMemory(size_t byte_size, uint32_t permissions,
   Status &error);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

2023-01-11 Thread serge via Phabricator via lldb-commits

serge-sans-paille updated this revision to Diff 488344.
serge-sans-paille added a comment.

Nits + rebase on main branch


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140800/new/

https://reviews.llvm.org/D140800

Files:
  clang/lib/Driver/DriverOptions.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/MachO/Driver.h
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
  llvm/lib/Option/OptTable.cpp
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
  llvm/tools/llvm-ifs/llvm-ifs.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-nm/llvm-nm.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-readobj/llvm-readobj.cpp
  llvm/tools/llvm-size/llvm-size.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -237,8 +237,14 @@
   CurPrefix = NewPrefix;
   }
 
-  // Dump prefixes.
+  DenseSet PrefixesUnionSet;
+  for (const auto &Prefix : Prefixes)
+PrefixesUnionSet.insert(Prefix.first.begin(), Prefix.first.end());
+  SmallVector PrefixesUnion(PrefixesUnionSet.begin(),
+   PrefixesUnionSet.end());
+  array_pod_sort(PrefixesUnion.begin(), PrefixesUnion.end());
 
+  // Dump prefixes.
   OS << "/\n";
   OS << "// Prefixes\n\n";
   OS << "#ifdef PREFIX\n";
@@ -259,6 +265,20 @@
   OS << "#undef COMMA\n";
   OS << "#endif // PREFIX\n\n";
 
+  // Dump prefix unions.
+  OS << "/\n";
+  OS << "// Prefix Union\n\n";
+  OS << "#ifdef PREFIX_UNION\n";
+  OS << "#define COMMA ,\n";
+  OS << "PREFIX_UNION({\n";
+  for (const auto &Prefix : PrefixesUnion) {
+OS << "llvm::StringLiteral(\"" << Prefix << "\") COMMA ";
+  }
+  OS << "llvm::StringLiteral(\"\")})\n";
+  OS << "#undef COMMA\n";
+  OS << "#endif // PREFIX_UNION\n\n";
+
+  // Dump groups.
   OS << "/\n";
   OS << "// ValuesCode\n\n";
   OS << "#ifdef OPTTABLE_VALUES_CODE\n";
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -32,6 +32,14 @@
 #include "Opts.inc"
 #undef PREFIX
 
+static constexpr const StringLiteral PrefixTable_init[] =
+#define PREFIX_UNION(VALUES) VALUES
+#include "Opts.inc"
+#undef PREFIX_UNION
+;
+static constexpr const ArrayRef
+PrefixTable(PrefixTable_init, std::size(PrefixTable_init) - 1);
+
 enum OptionFlags {
   OptFlag1 = (1 << 4),
   OptFlag2 = (1 << 5),
@@ -48,10 +56,16 @@
 };
 
 namespace {
-class TestOptTable : public OptTable {
+class TestOptTable : public GenericOptTable {
 public:
   TestOptTable(bool IgnoreCase = false)
-: OptTable(InfoTable, IgnoreCase) {}
+  : GenericOptTable(InfoTable, IgnoreCase) {}
+};
+
+class TestPrecomputedOptTable : public PrecomputedOptTable {
+public:
+  TestPrecomputedOptTable(bool IgnoreCase = false)
+  : PrecomputedOptTable(InfoTable, PrefixTable, IgnoreCase) {}
 };
 }
 
@@ -67,8 +81,20 @@
   "-Gchuu", "2"
   };
 
-TEST(Option, OptionParsing) {
-  TestOptTable T;
+// Test fixture
+template  class OptTableTest : public ::testing::Test {};
+
+template  class DISABLED_OptTableTest : public ::testing::Test {};
+
+// Test both precomputed and computed OptTables with the same suite of tests.
+using OptTableTestTypes =
+::testing::Types;
+
+TYPED_TEST_SUITE(OptTableTest, OptTableTestTypes, );
+TYPED_TEST_SUITE(DISABLED_OptTableTest, OptTableTestTypes, );
+
+TYPED_TEST(OptTableTest, OptionParsing) {
+  TypeParam T;
   unsigned MAI, MAC;
   InputArgList AL = T.ParseArgs(Args, MAI, MAC);
 
@@ -114,8 +140,8 @@
   EXPECT_EQ("desu", StringRef(ASL[1]));
 }
 
-TEST(Option, ParseWithFlagExclusions) {
-  TestOptTable T;
+TYPED_TEST(OptTableTest, ParseWithFlagExclusions) {
+  TypeParam T;
   unsigned MAI, MAC;
 
   // Exclude flag3 to avoid parsing as OPT_SLASH_C.
@@ -142,8 +168,8 @@
   EXPECT_EQ("bar", AL.getLastArgValue(OPT_C

[Lldb-commits] [PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

2023-01-11 Thread serge via Phabricator via lldb-commits

serge-sans-paille marked 2 inline comments as done.
serge-sans-paille added a comment.

In D140800#4043723 , @nikic wrote:

> Just to check, this isn't going to cause some warning spew about all those 
> OptTable implementations being non-final?

nope. Why would there be?




Comment at: llvm/unittests/Option/OptionParsingTest.cpp:327
 
-TEST(DISABLED_Option, FindNearestFIXME) {
-  TestOptTable T;

@nikic: the `DISABLED_` prefix seems to be a gtest convention, see 
https://github.com/google/googletest/blob/main/docs/advanced.md#temporarily-disabling-tests


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140800/new/

https://reviews.llvm.org/D140800

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140253: [debugserver] Clear memory allocations after exec

2023-01-11 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140253/new/

https://reviews.llvm.org/D140253

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 58def62 - [debugserver] Clear memory allocations after exec

2023-01-11 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2023-01-11T13:37:16-08:00
New Revision: 58def623ca46f107d185d7f02b267ffa751aff97

URL: 
https://github.com/llvm/llvm-project/commit/58def623ca46f107d185d7f02b267ffa751aff97
DIFF: 
https://github.com/llvm/llvm-project/commit/58def623ca46f107d185d7f02b267ffa751aff97.diff

LOG: [debugserver] Clear memory allocations after exec

After an exec, the inferior is a new process and none of these memory
regions are still allocated. Clear them out.

Differential Revision: https://reviews.llvm.org/D140253

Added: 


Modified: 
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/MacOSX/MachTask.h
lldb/tools/debugserver/source/MacOSX/MachTask.mm

Removed: 




diff  --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm 
b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 3aa2a84bc359f..d524a49defb90 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -2285,6 +2285,7 @@ static bool mach_header_validity_test(uint32_t magic, 
uint32_t cputype) {
 m_thread_list.Clear();
 m_activities.Clear();
 m_breakpoints.DisableAll();
+m_task.ClearAllocations();
   }
 
   if (m_sent_interrupt_signo != 0) {

diff  --git a/lldb/tools/debugserver/source/MacOSX/MachTask.h 
b/lldb/tools/debugserver/source/MacOSX/MachTask.h
index a58d5bed3b068..286a57b9e0dd1 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachTask.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachTask.h
@@ -61,6 +61,7 @@ class MachTask {
 
   nub_addr_t AllocateMemory(nub_size_t size, uint32_t permissions);
   nub_bool_t DeallocateMemory(nub_addr_t addr);
+  void ClearAllocations();
 
   mach_port_t ExceptionPort() const;
   bool ExceptionPortIsValid() const;

diff  --git a/lldb/tools/debugserver/source/MacOSX/MachTask.mm 
b/lldb/tools/debugserver/source/MacOSX/MachTask.mm
index 1e03faf3f2e23..feec85df9ef18 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachTask.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachTask.mm
@@ -999,6 +999,13 @@ static void 
get_threads_profile_data(DNBProfileDataScanType scanType,
   return false;
 }
 
+//--
+// MachTask::ClearAllocations
+//--
+void MachTask::ClearAllocations() {
+  m_allocations.clear();
+}
+
 void MachTask::TaskPortChanged(task_t task)
 {
   m_task = task;



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140253: [debugserver] Clear memory allocations after exec

2023-01-11 Thread Alex Langford via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG58def623ca46: [debugserver] Clear memory allocations after 
exec (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140253/new/

https://reviews.llvm.org/D140253

Files:
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/MacOSX/MachTask.h
  lldb/tools/debugserver/source/MacOSX/MachTask.mm


Index: lldb/tools/debugserver/source/MacOSX/MachTask.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachTask.mm
+++ lldb/tools/debugserver/source/MacOSX/MachTask.mm
@@ -999,6 +999,13 @@
   return false;
 }
 
+//--
+// MachTask::ClearAllocations
+//--
+void MachTask::ClearAllocations() {
+  m_allocations.clear();
+}
+
 void MachTask::TaskPortChanged(task_t task)
 {
   m_task = task;
Index: lldb/tools/debugserver/source/MacOSX/MachTask.h
===
--- lldb/tools/debugserver/source/MacOSX/MachTask.h
+++ lldb/tools/debugserver/source/MacOSX/MachTask.h
@@ -61,6 +61,7 @@
 
   nub_addr_t AllocateMemory(nub_size_t size, uint32_t permissions);
   nub_bool_t DeallocateMemory(nub_addr_t addr);
+  void ClearAllocations();
 
   mach_port_t ExceptionPort() const;
   bool ExceptionPortIsValid() const;
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -2285,6 +2285,7 @@
 m_thread_list.Clear();
 m_activities.Clear();
 m_breakpoints.DisableAll();
+m_task.ClearAllocations();
   }
 
   if (m_sent_interrupt_signo != 0) {


Index: lldb/tools/debugserver/source/MacOSX/MachTask.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachTask.mm
+++ lldb/tools/debugserver/source/MacOSX/MachTask.mm
@@ -999,6 +999,13 @@
   return false;
 }
 
+//--
+// MachTask::ClearAllocations
+//--
+void MachTask::ClearAllocations() {
+  m_allocations.clear();
+}
+
 void MachTask::TaskPortChanged(task_t task)
 {
   m_task = task;
Index: lldb/tools/debugserver/source/MacOSX/MachTask.h
===
--- lldb/tools/debugserver/source/MacOSX/MachTask.h
+++ lldb/tools/debugserver/source/MacOSX/MachTask.h
@@ -61,6 +61,7 @@
 
   nub_addr_t AllocateMemory(nub_size_t size, uint32_t permissions);
   nub_bool_t DeallocateMemory(nub_addr_t addr);
+  void ClearAllocations();
 
   mach_port_t ExceptionPort() const;
   bool ExceptionPortIsValid() const;
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -2285,6 +2285,7 @@
 m_thread_list.Clear();
 m_activities.Clear();
 m_breakpoints.DisableAll();
+m_task.ClearAllocations();
   }
 
   if (m_sent_interrupt_signo != 0) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

Are you planning on updating the reverse disassembly code still on this patch?




Comment at: lldb/include/lldb/API/SBTarget.h:844
 
+  uint32_t GetMaximumOpcodeByteSize() const;
+

Do we still need this API change with the new approach? We can't really use 
this unless we include GetMinimumOpcodeByteSize() and only use this value to 
backup if the min and max are the same.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2183-2194
+  /**
+   * For negative offsets, we do not know what `start_addr` corresponds to the
+   * instruction located `instruction_offset` instructions before `base_addr`
+   * since on some architectures opcodes have variable length.
+   *
+   * To address that, we need to read at least starting from `start_addr =
+   * base_addr + instruction_offset * max_instruction_size` (pruning is done if

Use C++ comments instead of C style comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141539: [lldb][test] Replace use of p with expression (NFC)

2023-01-11 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

I think it makes sense to have one test that tests the `p` command alias and 
its parameter handling and have all other tests be explicit about whether they 
test the expression evaluator or the command alias. As such, this is a good 
canonicalization.




Comment at: lldb/test/API/python_api/file_handle/TestFileHandle.py:138
 self.dbg.SetOutputFileHandle(f, False)
-self.handleCmd('p/x 3735928559', collect_result=False, check=False)
+self.handleCmd('expression/x 3735928559', collect_result=False, 
check=False)
 with open(self.out_filename, 'r') as f:

not your code, but does the `/x` serve any purpose here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141539/new/

https://reviews.llvm.org/D141539

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

2023-01-11 Thread Alexander Yermolovich via Phabricator via lldb-commits

ayermolo updated this revision to Diff 488393.
ayermolo added a comment.

Trying a fix for arm.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139379/new/

https://reviews.llvm.org/D139379

Files:
  bolt/lib/Core/DebugData.cpp
  bolt/lib/Rewrite/DWARFRewriter.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
  llvm/lib/DWP/DWP.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/test/DebugInfo/X86/debug-cu-index-unknown-section.s
  llvm/test/DebugInfo/X86/dwp-v2-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v2-tu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
  llvm/test/DebugInfo/dwarfdump-dwp.test
  llvm/test/tools/llvm-dwp/X86/debug_macro_v5.s
  llvm/test/tools/llvm-dwp/X86/info-v5.s
  llvm/test/tools/llvm-dwp/X86/loclists.s
  llvm/test/tools/llvm-dwp/X86/merge.test
  llvm/test/tools/llvm-dwp/X86/rnglists.s
  llvm/test/tools/llvm-dwp/X86/simple.test
  llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
  llvm/test/tools/llvm-dwp/X86/unknown-section-id.s

Index: llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
===
--- llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
+++ llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
@@ -17,7 +17,7 @@
 # CHECK:  Index Signature INFO ABBREV
 # CHECK-NOT:  Unknown
 # CHECK:  -
-# CHECK-NEXT: 1 0x1122 [0x, 0x0014) [0x, 0x0009)
+# CHECK-NEXT: 1 0x1122 [0x, 0x0014) [0x, 0x0009)
 # CHECK-NOT:  [
 
 # CHECK:  .debug_tu_index contents:
@@ -25,7 +25,7 @@
 # CHECK:  Index Signature TYPES ABBREV
 # CHECK-NOT:  Unknown
 # CHECK:  -
-# CHECK-NEXT: 2 0x1133 [0x, 0x0019) [0x0009, 0x0014)
+# CHECK-NEXT: 2 0x1133 [0x, 0x0019) [0x0009, 0x0014)
 # CHECK-NOT:  [
 
 .section .debug_abbrev.dwo, "e", @progbits
Index: llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
===
--- llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
+++ llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
@@ -13,9 +13,9 @@
 # CHECK: 0x001b: Type Unit: length = 0x0017, format = DWARF32, version = 0x0005, unit_type = DW_UT_split_type, abbr_offset = 0x, addr_size = 0x08, name = '', type_signature = [[TUID2:.*]], type_offset = 0x0019 (next unit at 0x0036)
 # CHECK-DAG: .debug_tu_index contents:
 # CHECK: version = 5, units = 2, slots = 4
-# CHECK: Index Signature  INFO ABBREV
-# CHECK: 1 [[TUID1]]  [0x, 0x001b) [0x, 0x0010)
-# CHECK: 4 [[TUID2]]  [0x001b, 0x0036) [0x, 0x0010)
+# CHECK: Index Signature  INFO ABBREV
+# CHECK: 1 [[TUID1]]  [0x, 0x001b) [0x, 0x0010)
+# CHECK: 4 [[TUID2]]  [0x001b, 0x0036) [0x, 0x0010)
 
 .section	.debug_info.dwo,"e",@progbits
 .long	.Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
Index: llvm/test/tools/llvm-dwp/X86/simple.test
===
--- llvm/test/tools/llvm-dwp/X86/simple.test
+++ llvm/test/tools/llvm-dwp/X86/simple.test
@@ -28,9 +28,9 @@
 CHECK: DW_TAG_formal_parameter
 
 CHECK: .debug_info.dwo contents:
-CHECK: [[AOFF:0x[0-9a-f]*]]:
+CHECK: 0x[[#%.8x,AOFF:]]:
 CHECK-LABEL: Compile Unit: length = {{.*}}, version = 0x0004, abbr_offset =
-CHECK: 0x[[AAOFF]], addr_size = 0x08 (next unit at [[BOFF:.*]])
+CHECK: 0x[[AAOFF]], addr_size = 0x08 (next unit at 0x[[#%.8x,BOFF:]])
 CHECK: DW_TAG_compile_unit
 CHECK:   DW_AT_name {{.*}} "a.cpp"
 CHECK:   DW_AT_GNU_dwo_id {{.*}} ([[DWOA:.*]])
@@ -40,9 +40,9 @@
 NOTYP: DW_AT_name {{.*}} "foo"
 TYPES: DW_AT_signature {{.*}} ([[FOOSIG:.*]])
 
-CHECK: [[BOFF]]:
+CHECK: 0x[[#BOFF]]:
 CHECK-LABEL: Compile Unit: length = {{.*}}, version = 0x0004, abbr_offset =
-CHECK: 0x[[BAOFF]], addr_size = 0x08 (next unit at [[XOFF:.*]])
+CHECK: 0x[[BAOFF]], addr_size = 0x08 (next unit at 0x[[#%.8x,XOFF:]])
 CHECK:   DW_AT_name {{.*}} "b.cpp"
 CHECK:   DW_AT_GNU_dwo_id {{.*}} ([[DWOB:.*]])
 CHECK:   DW_TAG_structure_type
@@ -54,32 +54,32 @@
 
 NOTYP-NOT: .debug_types.dwo contents:
 TYPES-LABEL: .debug_types.dwo contents:
-TYPES: [[FOOUOFF:0x[0-9a-f]*]]:
+TYPES: 0x[[#%.8x,FOOUOFF:]]:
 TYPES-LABEL: Type Unit: length = 0x0020, format = DWARF32, version = 0x0004, abbr_offset =
-TYPES: 0x[[AAOFF]], addr_size = 0x08, name = 'foo', type_signature = [[FOOSIG]], type

[Lldb-commits] [PATCH] D141539: [lldb][test] Replace use of p with expression (NFC)

2023-01-11 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added a comment.

I will add explicit tests for `p`.




Comment at: lldb/test/API/python_api/file_handle/TestFileHandle.py:138
 self.dbg.SetOutputFileHandle(f, False)
-self.handleCmd('p/x 3735928559', collect_result=False, check=False)
+self.handleCmd('expression/x 3735928559', collect_result=False, 
check=False)
 with open(self.out_filename, 'r') as f:

aprantl wrote:
> not your code, but does the `/x` serve any purpose here?
a couple lines below they read in the output from this command, and compare hex 
3735928559 to "deadbeef".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141539/new/

https://reviews.llvm.org/D141539

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141539: [lldb][test] Replace use of p with expression (NFC)

2023-01-11 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added inline comments.



Comment at: 
lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py:74
+self.expect("expression/d ((int*)&first)[0]", substrs=['= 5'])
+self.expect("expression/d ((int*)&second)[0]", substrs=['= 6'])
 self.assertEqual(first_sym.GetStartAddress().GetLoadAddress(target), 

@aprantl I don't see why these `/d` are not needed though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141539/new/

https://reviews.llvm.org/D141539

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: 
lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py:74-81
+num_a = array_find(locals, lambda x: x['name'] == 'num_a')
+self.assertIsNotNone(num_a)
+
+# Get and verify the data breakpoint info
+data_breakpoint_info = self.vscode.request_dataBreakpointInfo(
+num_a['name'], 1
+)

This might fail on some systems if they put the value for the local variables 
"num_a" or "num_b" in a register. So you might need to modify this test to make 
sure it will run on any systems. The other thing you can do is to take the 
address of any local variable to ensure it is kept in memory, so adding "int 
*num_a_ptr = &num_a;" and "int *num_b_ptr = &num_b;" might help ensure this 
test always works.



Comment at: 
lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py:95
+expected_line = line_number('main.cpp', '// num_a second')
+self.verify_watchpoint_hit(expected_line)

I would suggest adding a test for a constant global value that appears in a 
section (like .rodata) that has no write permissions and verify that we 
correctly figure out that we can only do "read" access on such a variable (see 
code changes I suggest where we check the memory region).

We also need to test error conditions out when a variable is not in memory. If 
you compile some C code with optimizations, you can try to create a variable 
that is in a register with "int num_a = 1;" and in the test grab the SBValue 
for this local variable and test if the value has an invalid load address



Comment at: lldb/tools/lldb-vscode/Watchpoint.cpp:34-36
+  if (!this->enabled) {
+return;
+  }

no braces on single line if statements per llvm coding guidelines.



Comment at: lldb/tools/lldb-vscode/Watchpoint.cpp:38-44
+  this->watchpoint =
+  g_vsc.target.WatchAddress(this->addr, this->size,
+this->type == WatchpointType::Read ||
+this->type == WatchpointType::ReadWrite,
+this->type == WatchpointType::Write ||
+this->type == WatchpointType::ReadWrite,
+this->error);

We should be using the "SBValue::Watch(...)" instead of 
SBTarget.WatchAddress(...). If you create a watchpoint for a local variable 
that is on the stack, this will cause all sorts of problems if the variable 
goes out of scope. SBValue::Watch(...) has the smarts to be able to figure out 
if a variable is local, and I seem to remember that it will disable this 
watchpoint as soon as a local variable goes out of scope, though I might not be 
remember things correctly. It can also figure out if a variable is currently in 
memory or not and it won't set a watchpoint if possible. 

I might recommend that this Watchpoint class gets created with a SBValue, and 
then we use that for setting the watchpoint, the we set the watchpoint using:
```
lldb::SBWatchpoint lldb::SBValue::Watch(bool resolve_location, bool read, bool 
write, SBError &error);
```




Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2429
+
+  auto v_address = variable.GetLoadAddress();
+  auto v_size = variable.GetByteSize();

Some variables don't have load addresses. This can happen when a variable is in 
a register. If a variable is in a register, then this function will return 
LLDB_INVALID_ADDRESS, and if that happens you will need to return something 
appropriate, like maybe returning "null" for the "dataId" in the response and 
then filling in an appropriate message in the "description" as it is documented 
above as "UI string that describes on what data the breakpoint is set on or why 
a data breakpoint is not available". 



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2439-2442
+  llvm::json::Array access_types;
+  access_types.emplace_back("read");
+  access_types.emplace_back("write");
+  access_types.emplace_back("readWrite");

if "v_address != LLDB_INVALID_ADDRESS", then you can ask for the memory region 
for this address using:
```
lldb::SBError lldb::SBProcess::GetMemoryRegionInfo(lldb::addr_t load_addr, 
lldb::SBMemoryRegionInfo ®ion_info);
```
If you get an error back, then this memory address is unreadable and you should 
return an error. If no error is returned, then you should check if the memory 
region has read or write permissions with:
```
llvm::json::Array access_types;
lldb::SBMemoryRegionInfo region_info;
lldb::SBError error = g_vsc.target.GetProcess().GetMemoryRegionInfo(v_address, 
region_info);
if (error.Success()) {
  if (region_info.IsReadable()) {
access_types.emplace_back("r

[Lldb-commits] [lldb] 7fc7934 - [llvm][dwwarf] Change CU/TU index to 64-bit

2023-01-11 Thread Alexander Yermolovich via lldb-commits


Author: Alexander Yermolovich
Date: 2023-01-11T15:07:11-08:00
New Revision: 7fc79340234f3acd354c52bd8bf2cf44f38fc4be

URL: 
https://github.com/llvm/llvm-project/commit/7fc79340234f3acd354c52bd8bf2cf44f38fc4be
DIFF: 
https://github.com/llvm/llvm-project/commit/7fc79340234f3acd354c52bd8bf2cf44f38fc4be.diff

LOG: [llvm][dwwarf] Change CU/TU index to 64-bit

Changed contribution data structure to 64 bit. I added the 32bit and 64bit
accessors to make it explicit where we use 32bit and where we use 64bit. Also to
make sure sure we catch all the cases where this data structure is used.

Reviewed By: dblaikie

Differential Revision: https://reviews.llvm.org/D139379

Added: 


Modified: 
bolt/lib/Core/DebugData.cpp
bolt/lib/Rewrite/DWARFRewriter.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
llvm/lib/DWP/DWP.cpp
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
llvm/test/DebugInfo/X86/debug-cu-index-unknown-section.s
llvm/test/DebugInfo/X86/dwp-v2-cu-index.s
llvm/test/DebugInfo/X86/dwp-v2-tu-index.s
llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
llvm/test/DebugInfo/dwarfdump-dwp.test
llvm/test/tools/llvm-dwp/X86/debug_macro_v5.s
llvm/test/tools/llvm-dwp/X86/info-v5.s
llvm/test/tools/llvm-dwp/X86/loclists.s
llvm/test/tools/llvm-dwp/X86/merge.test
llvm/test/tools/llvm-dwp/X86/rnglists.s
llvm/test/tools/llvm-dwp/X86/simple.test
llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
llvm/test/tools/llvm-dwp/X86/unknown-section-id.s

Removed: 




diff  --git a/bolt/lib/Core/DebugData.cpp b/bolt/lib/Core/DebugData.cpp
index fa5505ad78aa6..2cbd119e69fe2 100644
--- a/bolt/lib/Core/DebugData.cpp
+++ b/bolt/lib/Core/DebugData.cpp
@@ -1241,8 +1241,8 @@ void DebugAbbrevWriter::addUnitAbbreviations(DWARFUnit 
&Unit) {
 
   const DWARFUnitIndex::Entry::SectionContribution *DWOContrubution =
   DWOEntry->getContribution(DWARFSectionKind::DW_SECT_ABBREV);
-  AbbrevContents = AbbrevSectionContents.substr(DWOContrubution->Offset,
-DWOContrubution->Length);
+  AbbrevContents = AbbrevSectionContents.substr(
+  DWOContrubution->getOffset(), DWOContrubution->getLength());
 } else if (!Unit.isDWOUnit()) {
   const uint64_t StartOffset = Unit.getAbbreviationsOffset();
 

diff  --git a/bolt/lib/Rewrite/DWARFRewriter.cpp 
b/bolt/lib/Rewrite/DWARFRewriter.cpp
index a76d128ea4e60..5b8a28465fd7c 100644
--- a/bolt/lib/Rewrite/DWARFRewriter.cpp
+++ b/bolt/lib/Rewrite/DWARFRewriter.cpp
@@ -1212,11 +1212,11 @@ updateDebugData(DWARFContext &DWCtx, std::string 
&Storage,
   const DWARFUnitIndex::Entry::SectionContribution;
   auto getSliceData = [&](const DWARFUnitIndex::Entry *DWOEntry,
   StringRef OutData, DWARFSectionKind Sec,
-  uint32_t &DWPOffset) -> StringRef {
+  uint64_t &DWPOffset) -> StringRef {
 if (DWOEntry) {
   DWOSectionContribution *DWOContrubution = DWOEntry->getContribution(Sec);
-  DWPOffset = DWOContrubution->Offset;
-  OutData = OutData.substr(DWPOffset, DWOContrubution->Length);
+  DWPOffset = DWOContrubution->getOffset();
+  OutData = OutData.substr(DWPOffset, DWOContrubution->getLength());
 }
 return OutData;
   };
@@ -1227,7 +1227,7 @@ updateDebugData(DWARFContext &DWCtx, std::string &Storage,
 
   Streamer.switchSection(SectionIter->second.first);
   StringRef OutData = SectionContents;
-  uint32_t DWPOffset = 0;
+  uint64_t DWPOffset = 0;
 
   switch (SectionIter->second.second) {
   default: {
@@ -1310,14 +1310,15 @@ static std::string extractDWOTUFromDWP(
   // Sorting so it's easy to compare output.
   // They should be sharing the same Abbrev.
   llvm::sort(TUContributions, [](const TUEntry &V1, const TUEntry &V2) -> bool 
{
-return V1.second->Offset < V2.second->Offset;
+return V1.second->getOffset() < V2.second->getOffset();
   });
 
   for (auto &PairEntry : TUContributions) {
 const DWARFUnitIndex::Entry::SectionContribution *C = PairEntry.second;
 const uint64_t TUSignature = PairEntry.first;
-DWOTUSection.append(Contents.slice(C->Offset, C->Offset + 
C->Length).str());
-TUContributionsToCU.push_back({TUSignature, C->Length});
+DWOTUSection.append(
+Contents.slice(C->getOffset(), C->getOffset() + C->getLength()).str());
+TUContributionsToCU.push_back({TUSignature, C->getLength32()});
   }
   return DWOTUSection;
 }
@@ -1357,11 +1358,12 @@ static void extractTypesFromDWPDWARF5(
   llvm::sort(TUContributions,
  [](const DWARFUn

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

2023-01-11 Thread Alexander Yermolovich via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7fc79340234f: [llvm][dwwarf] Change CU/TU index to 64-bit 
(authored by ayermolo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139379/new/

https://reviews.llvm.org/D139379

Files:
  bolt/lib/Core/DebugData.cpp
  bolt/lib/Rewrite/DWARFRewriter.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
  llvm/lib/DWP/DWP.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/test/DebugInfo/X86/debug-cu-index-unknown-section.s
  llvm/test/DebugInfo/X86/dwp-v2-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v2-tu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
  llvm/test/DebugInfo/dwarfdump-dwp.test
  llvm/test/tools/llvm-dwp/X86/debug_macro_v5.s
  llvm/test/tools/llvm-dwp/X86/info-v5.s
  llvm/test/tools/llvm-dwp/X86/loclists.s
  llvm/test/tools/llvm-dwp/X86/merge.test
  llvm/test/tools/llvm-dwp/X86/rnglists.s
  llvm/test/tools/llvm-dwp/X86/simple.test
  llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
  llvm/test/tools/llvm-dwp/X86/unknown-section-id.s

Index: llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
===
--- llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
+++ llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
@@ -17,7 +17,7 @@
 # CHECK:  Index Signature INFO ABBREV
 # CHECK-NOT:  Unknown
 # CHECK:  -
-# CHECK-NEXT: 1 0x1122 [0x, 0x0014) [0x, 0x0009)
+# CHECK-NEXT: 1 0x1122 [0x, 0x0014) [0x, 0x0009)
 # CHECK-NOT:  [
 
 # CHECK:  .debug_tu_index contents:
@@ -25,7 +25,7 @@
 # CHECK:  Index Signature TYPES ABBREV
 # CHECK-NOT:  Unknown
 # CHECK:  -
-# CHECK-NEXT: 2 0x1133 [0x, 0x0019) [0x0009, 0x0014)
+# CHECK-NEXT: 2 0x1133 [0x, 0x0019) [0x0009, 0x0014)
 # CHECK-NOT:  [
 
 .section .debug_abbrev.dwo, "e", @progbits
Index: llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
===
--- llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
+++ llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
@@ -13,9 +13,9 @@
 # CHECK: 0x001b: Type Unit: length = 0x0017, format = DWARF32, version = 0x0005, unit_type = DW_UT_split_type, abbr_offset = 0x, addr_size = 0x08, name = '', type_signature = [[TUID2:.*]], type_offset = 0x0019 (next unit at 0x0036)
 # CHECK-DAG: .debug_tu_index contents:
 # CHECK: version = 5, units = 2, slots = 4
-# CHECK: Index Signature  INFO ABBREV
-# CHECK: 1 [[TUID1]]  [0x, 0x001b) [0x, 0x0010)
-# CHECK: 4 [[TUID2]]  [0x001b, 0x0036) [0x, 0x0010)
+# CHECK: Index Signature  INFO ABBREV
+# CHECK: 1 [[TUID1]]  [0x, 0x001b) [0x, 0x0010)
+# CHECK: 4 [[TUID2]]  [0x001b, 0x0036) [0x, 0x0010)
 
 .section	.debug_info.dwo,"e",@progbits
 .long	.Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
Index: llvm/test/tools/llvm-dwp/X86/simple.test
===
--- llvm/test/tools/llvm-dwp/X86/simple.test
+++ llvm/test/tools/llvm-dwp/X86/simple.test
@@ -28,9 +28,9 @@
 CHECK: DW_TAG_formal_parameter
 
 CHECK: .debug_info.dwo contents:
-CHECK: [[AOFF:0x[0-9a-f]*]]:
+CHECK: 0x[[#%.8x,AOFF:]]:
 CHECK-LABEL: Compile Unit: length = {{.*}}, version = 0x0004, abbr_offset =
-CHECK: 0x[[AAOFF]], addr_size = 0x08 (next unit at [[BOFF:.*]])
+CHECK: 0x[[AAOFF]], addr_size = 0x08 (next unit at 0x[[#%.8x,BOFF:]])
 CHECK: DW_TAG_compile_unit
 CHECK:   DW_AT_name {{.*}} "a.cpp"
 CHECK:   DW_AT_GNU_dwo_id {{.*}} ([[DWOA:.*]])
@@ -40,9 +40,9 @@
 NOTYP: DW_AT_name {{.*}} "foo"
 TYPES: DW_AT_signature {{.*}} ([[FOOSIG:.*]])
 
-CHECK: [[BOFF]]:
+CHECK: 0x[[#BOFF]]:
 CHECK-LABEL: Compile Unit: length = {{.*}}, version = 0x0004, abbr_offset =
-CHECK: 0x[[BAOFF]], addr_size = 0x08 (next unit at [[XOFF:.*]])
+CHECK: 0x[[BAOFF]], addr_size = 0x08 (next unit at 0x[[#%.8x,XOFF:]])
 CHECK:   DW_AT_name {{.*}} "b.cpp"
 CHECK:   DW_AT_GNU_dwo_id {{.*}} ([[DWOB:.*]])
 CHECK:   DW_TAG_structure_type
@@ -54,32 +54,32 @@
 
 NOTYP-NOT: .debug_types.dwo contents:
 TYPES-LABEL: .debug_types.dwo contents:
-TYPES: [[FOOUOFF:0x[0-9a-f]*]]:
+TYPES: 0x[[#%.8x,FOOUOFF:]]:
 TYPES-LABEL: Type Unit: length = 0x0020, format =

[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Each SymbolFile has its own type list that keeps the shared pointers to all 
types. So there should be no need for any of these changes here unless someone 
isn't correctly storing a newly created type in the SymbolFile's type list. You 
can see the types being stored in code like:

  TypeSP type_sp(... /* create type here*/ ...);
  dwarf->GetTypeList().Insert(type_sp); // Store the share reference in the 
symbol file's type list 




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:208-209
 
   dwarf->GetTypeList().Insert(type_sp);
-  dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+  dwarf->GetDIEToType()[die.GetDIE()] = type_sp;
   clang::TagDecl *tag_decl = TypeSystemClang::GetAsTagDecl(type);

Above the "type_sp" is immediately stored in the SymbolFile's type list. So we 
don't need to do anything here and it _is_ safe to just store a "Type *".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141318/new/

https://reviews.llvm.org/D141318

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

> Storing raw pointers in DieToTypePtr may cause use-after-frees to occur, 
> since there are no guarantees that the shared pointers that owns the 
> underlying pointer to the type are kept around as long as the map. Change the 
> map to store a shared pointer instead.

This isn't true. SymbolFile has a m_type_list that is used to store all shared 
pointers to any types that are created. Since SymbolFileDWARF is an instance of 
SymbolFile, it is safe to store "Type *" anywhere inside this class because as 
along as the SymbolFileDWARF is around, so will the SymbolFile that owns all of 
the types. If there are any places where someone is creating a type and _not_ 
storing it in the SymbolFile::m_type_list, then this is the bug and that should 
be fixed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141318/new/

https://reviews.llvm.org/D141318

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

SymbolVendorELF.cpp is using this as well:

  dsym_objfile_sp->SetType(ObjectFile::eTypeDebugInfo);

They must have copied and pasted some code. So not sure this is safe enough as 
is to implement. There are also a few places that try to check for a dSYM file 
using this kind of method, might be nice to add a method, that is done right, 
that detects if we really have a dSYM or not in ObjectFile.h/.cpp. Wasm also 
sets the file type to debug info and so does breakpad.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68655/new/

https://reviews.llvm.org/D68655

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

There have also been some bugs in .debug_aranges and some folks want to get rid 
of .debug_aranges all together and rely only upon the DW_TAG_compile_unit's 
DW_AT_ranges. Tons of details in this patch:

https://reviews.llvm.org/D136395

So long story short, some people believe we should not produce or use 
.debug_aranges anymore, and want LLDB changed to ignore it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68655/new/

https://reviews.llvm.org/D68655

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Enrico Loparco via Phabricator via lldb-commits

eloparco added a comment.

In D140358#4045210 , @clayborg wrote:

> Are you planning on updating the reverse disassembly code still on this patch?

What do you mean by "reverse disassembly"?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D68655#4045420 , @clayborg wrote:

> SymbolVendorELF.cpp is using this as well:
>
>   dsym_objfile_sp->SetType(ObjectFile::eTypeDebugInfo);
>
> They must have copied and pasted some code. So not sure this is safe enough 
> as is to implement. There are also a few places that try to check for a dSYM 
> file using this kind of method, might be nice to add a method, that is done 
> right, that detects if we really have a dSYM or not in ObjectFile.h/.cpp. 
> Wasm also sets the file type to debug info and so does breakpad.

The best way to check for dSYM would be to read the bytes from the mach-o 
header that specify MH_DSYM from the data for the object file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68655/new/

https://reviews.llvm.org/D68655

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D140358#4045442 , @eloparco wrote:

> In D140358#4045210 , @clayborg 
> wrote:
>
>> Are you planning on updating the reverse disassembly code still on this 
>> patch?
>
> What do you mean by "reverse disassembly"?

I mean when we try and back up by some offset.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

2023-01-11 Thread Augusto Noronha via Phabricator via lldb-commits

augusto2112 added a comment.

In D141318#4045372 , @clayborg wrote:

> Each SymbolFile has its own type list that keeps the shared pointers to all 
> types. So there should be no need for any of these changes here unless 
> someone isn't correctly storing a newly created type in the SymbolFile's type 
> list. You can see the types being stored in code like:
>
>   TypeSP type_sp(... /* create type here*/ ...);
>   dwarf->GetTypeList().Insert(type_sp); // Store the share reference in the 
> symbol file's type list 

What triggered the use after free for me was this snippet in 
`DWARFASTParserClang::ParseTypeModifier`:

...
TypeSP lldb_function_type_sp = ParseTypeFromDWARF(
sc, function_type, &function_type_is_new_pointer);

if (lldb_function_type_sp) {
  clang_type = m_ast.CreateBlockPointerType(
  lldb_function_type_sp->GetForwardCompilerType());
  encoding_data_type = Type::eEncodingIsUID;
  attrs.type.Clear();
  resolve_state = Type::ResolveState::Full;
}
  }

`lldb_function_type_sp` is created, the underlying pointer is placed in the 
map, the SP is destroyed at the end of the scope, and any lookups for that type 
will get back the dangling pointer.

I could update this particular instance, but this just seems like a really easy 
mistake to make... There's nothing to suggest to callers that they must update 
the `m_type_list` when they ask for a new type. Wouldn't it be better we 
prevent this somehow, by updating the raw pointers of the map by being either 
unique, shared or weak pointers?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141318/new/

https://reviews.llvm.org/D141318

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

2023-01-11 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

In D68655#4045423 , @clayborg wrote:

> There have also been some bugs in .debug_aranges and some folks want to get 
> rid of .debug_aranges all together and rely only upon the 
> DW_TAG_compile_unit's DW_AT_ranges. Tons of details in this patch:
>
> https://reviews.llvm.org/D136395
>
> So long story short, some people believe we should not produce or use 
> .debug_aranges anymore, and want LLDB changed to ignore it.

I'm honestly curious how the DW_TAG_compile_unit's DW_AT_ranges (or 
DW_AT_high/low_pc) would be authoritative, but the ranges for the CU in 
debug_aranges would be incorrect.  If the linker is allowed to reorder 
functions (does this not happen on linux?), then the compiler can't know the 
address ranges of the CU, only the linker (or post-linker) can know what 
address ranges are owned by this compile unit.  So the linker had to create the 
DW_TAG_compile_unit's DW_AT_ranges list after linking is complete, and the 
linker is also the part of the toolchain that creates debug_aranges.  If it is 
creating an incorrect debug_aranges entry, how can it possibly do the correct 
thing when rewriting the DW_TAG_compile_unit's DW_AT_ranges?

On Darwin, we usually get a DW_AT_high/low_pc on the compile_unit in the .o 
files, where all of the functions are contiguous.  When dsymutil links the 
debug information into the dSYM, it includes the address ranges of every 
DW_TAG_subprogram that was included in the final executable in the 
debug_aranges.  It doesn't modify DW_AT_high/low_pc in the compile_unit, it 
merely updates the addresses to their linked addresses.  But lldb doesn't use 
those for anything - either it trusts the debug_aranges entries, or lldb scans 
all of the subprograms itself to construct the aranges (same thing dsymutil 
did).

tl;dr I don't see what using DW_AT_ranges in the compile_unit accomplishes 
versus debug_aranges.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68655/new/

https://reviews.llvm.org/D68655

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Enrico Loparco via Phabricator via lldb-commits

eloparco added inline comments.



Comment at: lldb/include/lldb/API/SBTarget.h:844
 
+  uint32_t GetMaximumOpcodeByteSize() const;
+

clayborg wrote:
> Do we still need this API change with the new approach? We can't really use 
> this unless we include GetMinimumOpcodeByteSize() and only use this value to 
> backup if the min and max are the same.
In principle we could assume a maximum opcode of size 16 in the calculation, 
but knowing the maximum actually supported by the architecture can save us from 
reading unnecessary chunks of memory. In this way, we avoid reading 
instructions that will then be discarded because resulting in more than 
`instructionCount` total instructions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Enrico Loparco via Phabricator via lldb-commits

eloparco added a comment.

In D140358#4045450 , @clayborg wrote:

> In D140358#4045442 , @eloparco 
> wrote:
>
>> In D140358#4045210 , @clayborg 
>> wrote:
>>
>>> Are you planning on updating the reverse disassembly code still on this 
>>> patch?
>>
>> What do you mean by "reverse disassembly"?
>
> I mean when we try and back up by some offset.

Sorry, still not clear. So probably not intended for this patch :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Enrico Loparco via Phabricator via lldb-commits

eloparco updated this revision to Diff 488415.
eloparco added a comment.

Use single-line comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

Files:
  lldb/bindings/interface/SBTarget.i
  lldb/include/lldb/API/SBTarget.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/source/API/SBTarget.cpp
  lldb/test/API/python_api/target/TestTargetAPI.py
  lldb/test/API/tools/lldb-vscode/disassemble/Makefile
  lldb/test/API/tools/lldb-vscode/disassemble/TestVSCode_disassemble.py
  lldb/test/API/tools/lldb-vscode/disassemble/main.cpp
  lldb/tools/lldb-vscode/CMakeLists.txt
  lldb/tools/lldb-vscode/DisassembledInstruction.cpp
  lldb/tools/lldb-vscode/DisassembledInstruction.h
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/LLDBUtils.cpp
  lldb/tools/lldb-vscode/LLDBUtils.h
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/VSCodeForward.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1451,8 +1451,7 @@
   // which may affect the outcome of tests.
   bool source_init_file = GetBoolean(arguments, "sourceInitFile", true);
 
-  g_vsc.debugger =
-  lldb::SBDebugger::Create(source_init_file, log_cb, nullptr);
+  g_vsc.debugger = lldb::SBDebugger::Create(source_init_file, log_cb, nullptr);
   g_vsc.progress_event_thread = std::thread(ProgressEventThreadFunction);
 
   // Start our event thread so we can receive events from the debugger, target,
@@ -1542,6 +1541,8 @@
   // The debug adapter supports 'logMessage' in breakpoint.
   body.try_emplace("supportsLogPoints", true);
 
+  body.try_emplace("supportsDisassembleRequest", true);
+
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
@@ -2117,6 +2118,173 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+std::vector
+_get_instructions_from_memory(lldb::addr_t start, uint64_t count) {
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+
+  lldb::SBError error;
+  std::vector buffer(count, 0);
+  const size_t bytes_read __attribute__((unused)) = process.ReadMemory(
+  start, static_cast(buffer.data()), count, error);
+  assert(bytes_read == count && error.Success() &&
+ "unable to read byte range from memory");
+
+  // If base_addr starts in the middle of an instruction,
+  // that first instruction will not be parsed correctly (negligible)
+  std::vector sb_instructions;
+  const auto base_addr = lldb::SBAddress(start, g_vsc.target);
+  lldb::SBInstructionList instructions =
+  g_vsc.target.GetInstructions(base_addr, buffer.data(), count);
+
+  for (size_t i = 0; i < instructions.GetSize(); i++) {
+auto instr = instructions.GetInstructionAtIndex(i);
+sb_instructions.emplace_back(instr);
+  }
+  return sb_instructions;
+}
+
+auto _handle_disassemble_positive_offset(lldb::addr_t base_addr,
+ int64_t instruction_offset,
+ uint64_t instruction_count) {
+  llvm::json::Array response_instructions;
+
+  // For positive offsets, we use the `ReadInstructions()` API to get
+  // `instruction_offset + instruction_count` instructions after the
+  // `base_addr`.
+  auto start_addr = lldb::SBAddress(base_addr, g_vsc.target);
+  lldb::SBInstructionList instructions = g_vsc.target.ReadInstructions(
+  start_addr, instruction_offset + instruction_count);
+
+  const auto num_instrs_to_skip = static_cast(instruction_offset);
+  for (size_t i = num_instrs_to_skip; i < instructions.GetSize(); ++i) {
+lldb::SBInstruction instr = instructions.GetInstructionAtIndex(i);
+
+auto disass_instr =
+CreateDisassembledInstruction(DisassembledInstruction(instr));
+response_instructions.emplace_back(std::move(disass_instr));
+  }
+
+  return response_instructions;
+}
+
+auto _handle_disassemble_negative_offset(
+lldb::addr_t base_addr, int64_t instruction_offset,
+uint64_t instruction_count,
+std::optional memory_reference) {
+  llvm::json::Array response_instructions;
+
+  const auto max_instruction_size = g_vsc.target.GetMaximumOpcodeByteSize();
+  const auto bytes_offset = -instruction_offset * max_instruction_size;
+  auto start_addr = base_addr - bytes_offset;
+  const auto disassemble_bytes = instruction_count * max_instruction_size;
+
+  // For negative offsets, we do not know what `start_addr` corresponds to the
+  // instruction located `instruction_offset` instructions before `base_addr`
+  // since on some architectures opcodes have variable length.
+  // To address that, we need to read at least starting from `start_addr =
+  // base_addr + instruction_offset * max_instruction_size` (pruning is done i

[Lldb-commits] [PATCH] D141318: [lldb] Store shared pointers in DieToTypePtr map instead of raw pointers

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D141318#4045459 , @augusto2112 
wrote:

> In D141318#4045372 , @clayborg 
> wrote:
>
>> Each SymbolFile has its own type list that keeps the shared pointers to all 
>> types. So there should be no need for any of these changes here unless 
>> someone isn't correctly storing a newly created type in the SymbolFile's 
>> type list. You can see the types being stored in code like:
>>
>>   TypeSP type_sp(... /* create type here*/ ...);
>>   dwarf->GetTypeList().Insert(type_sp); // Store the share reference in the 
>> symbol file's type list 
>
> What triggered the use after free for me was this snippet in 
> `DWARFASTParserClang::ParseTypeModifier`:
>
> ...
> TypeSP lldb_function_type_sp = ParseTypeFromDWARF(
> sc, function_type, &function_type_is_new_pointer);
>   
> if (lldb_function_type_sp) {
>   clang_type = m_ast.CreateBlockPointerType(
>   lldb_function_type_sp->GetForwardCompilerType());
>   encoding_data_type = Type::eEncodingIsUID;
>   attrs.type.Clear();
>   resolve_state = Type::ResolveState::Full;
> }
>   }
>
> `lldb_function_type_sp` is created, the underlying pointer is placed in the 
> map, the SP is destroyed at the end of the scope, and any lookups for that 
> type will get back the dangling pointer.
>
> I could update this particular instance, but this just seems like a really 
> easy mistake to make... There's nothing to suggest to callers that they must 
> update the `m_type_list` when they ask for a new type. Wouldn't it be better 
> we prevent this somehow, by updating the raw pointers of the map by being 
> either unique, shared or weak pointers?

We really need the SymbolFile to own (have a strong reference to) all types 
that each SymbolFile subclass creates. One idea would be to make the 
lldb_private::Type's constructor protected and then friend class the SymbolFile 
class only, and then add an accessor where all types must be created via a 
SymbolFile API. Right now anyone can create a type with:

  TypeSP type_sp(new Type(...));

This could turn into:

  TypeSP type_sp(symbol_file->CreateType(...));

The main issue is shared pointer and weak pointer objects are more expensive 
objects as they are two pointers each and we need to keep these extra maps, 
like the the dwarf->GetDIEToType() as small as they can be. We also need to the 
type to not disappear as it needs to be owned by the SymbolFile classes. Not 
sure if anyone is creating types outside of the SymbolFile or subclasses.

So my requirement is that all types created by SymbolFile objects must have a 
single strong reference to the type in SymbolFile::m_type_list. Right now this 
isn't enforced, but it would be great if it was.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141318/new/

https://reviews.llvm.org/D141318

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Enrico Loparco via Phabricator via lldb-commits

eloparco added a comment.

In D140358#4045469 , @eloparco wrote:

> In D140358#4045450 , @clayborg 
> wrote:
>
>> In D140358#4045442 , @eloparco 
>> wrote:
>>
>>> In D140358#4045210 , @clayborg 
>>> wrote:
>>>
 Are you planning on updating the reverse disassembly code still on this 
 patch?
>>>
>>> What do you mean by "reverse disassembly"?
>>
>> I mean when we try and back up by some offset.
>
> Sorry, still not clear. So probably not intended for this patch :)

If you mean setting breakpoints and stepping through disassembled instructions, 
that's outside the purpose of this initial patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D68655#4045461 , @jasonmolenda 
wrote:

> In D68655#4045423 , @clayborg wrote:
>
>> There have also been some bugs in .debug_aranges and some folks want to get 
>> rid of .debug_aranges all together and rely only upon the 
>> DW_TAG_compile_unit's DW_AT_ranges. Tons of details in this patch:
>>
>> https://reviews.llvm.org/D136395
>>
>> So long story short, some people believe we should not produce or use 
>> .debug_aranges anymore, and want LLDB changed to ignore it.
>
> I'm honestly curious how the DW_TAG_compile_unit's DW_AT_ranges (or 
> DW_AT_high/low_pc) would be authoritative, but the ranges for the CU in 
> debug_aranges would be incorrect.  If the linker is allowed to reorder 
> functions (does this not happen on linux?), then the compiler can't know the 
> address ranges of the CU, only the linker (or post-linker) can know what 
> address ranges are owned by this compile unit.  So the linker had to create 
> the DW_TAG_compile_unit's DW_AT_ranges list after linking is complete, and 
> the linker is also the part of the toolchain that creates debug_aranges.  If 
> it is creating an incorrect debug_aranges entry, how can it possibly do the 
> correct thing when rewriting the DW_TAG_compile_unit's DW_AT_ranges?

They are all the same from the linker perspective: both .debug_aranges (for 
accelerator table) and .debug_ranges (for DW_AT_ranges) have relocations on 
them. As linkers move stuff around they apply the relocations and they all stay 
up to date. Anything without a relocation has its start address zeroed out.

> On Darwin, we usually get a DW_AT_high/low_pc on the compile_unit in the .o 
> files, where all of the functions are contiguous.  When dsymutil links the 
> debug information into the dSYM, it includes the address ranges of every 
> DW_TAG_subprogram that was included in the final executable in the 
> debug_aranges.  It doesn't modify DW_AT_high/low_pc in the compile_unit, it 
> merely updates the addresses to their linked addresses.  But lldb doesn't use 
> those for anything - either it trusts the debug_aranges entries, or lldb 
> scans all of the subprograms itself to construct the aranges (same thing 
> dsymutil did).

dsymutil generates correct .debug_aranges by looking at the final DWARF. Note 
that there are no .debug_aranges sections in .o files since we don't use them, 
they only get produced if we have a dSYM for darwin.

> tl;dr I don't see what using DW_AT_ranges in the compile_unit accomplishes 
> versus debug_aranges.

Different things are included in DW_AT_ranges, like address ranges for global 
and static variables. .debug_aranges only has functions, no globals or statics, 
so if you are trying to find a global variable by address, you can't rely on 
.debug_aranges. Nothing in the DWARF spec states things clearly enough for 
different compilers to know what to include in .debug_aranges  and the compiler 
uint DW_AT_ranges. Each compiler (GCC and clang) do their own thing and often 
do things differently. So the other way to add functionality like this would be 
to check the target triple and check for Apple as the vendor maybe. But we 
still need know if we have a dSYM file or not, because if not, we can't use 
.debug_aranges as .o files don't have them, but they also don't claim to be of 
type ObjectFile::eTypeDebugInfo.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68655/new/

https://reviews.llvm.org/D68655

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

2023-01-11 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D68655#4045561 , @clayborg wrote:

> But we still need know if we have a dSYM file or not, because if not, we 
> can't use .debug_aranges as .o files don't have them, but they also don't 
> claim to be of type ObjectFile::eTypeDebugInfo.

I came to the same realization and fixed that in 
https://reviews.llvm.org/rG9b737f148d88501a0a778e1adacf342108286bb0


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68655/new/

https://reviews.llvm.org/D68655

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

I mean we can not just subtract something, any number, from any address unless 
we have fixed size opcodes. If we do this for x86, you can get complete garbage 
with no hope of ever getting back on track and this disassembly just won't make 
sense at all and will be useless. I thought my x86 example spelled out why it 
is bad to backup. If we start disassembling in the middle of an opcode, we can 
attempt to disassemble immediate values that are encoded into the middle of an 
opcode. Since x86 instructions can be 1 byte to 15 bytes, we might disassembly 
garbage and never align up to real opcodes.

So I see a few solutions:

- use functions and symbols to get actual boundaries, and use sections to 
detect when there are instruction (code) and not
- check if min and max opcode sizes are the same and only try to backup if they 
are the same, but still use sections to disassembly data as .byte or .long 
directives


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D68655#4045604 , @JDevlieghere 
wrote:

> In D68655#4045561 , @clayborg wrote:
>
>> But we still need know if we have a dSYM file or not, because if not, we 
>> can't use .debug_aranges as .o files don't have them, but they also don't 
>> claim to be of type ObjectFile::eTypeDebugInfo.
>
> I came to the same realization and fixed that in 
> https://reviews.llvm.org/rG9b737f148d88501a0a778e1adacf342108286bb0

nice! approved that one as the detection is solid.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68655/new/

https://reviews.llvm.org/D68655

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Enrico Loparco via Phabricator via lldb-commits

eloparco added inline comments.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2177
+  const auto bytes_offset = -instruction_offset * bytes_per_instruction;
+  auto start_addr = base_addr - bytes_offset;
+  const auto disassemble_bytes = instruction_count * bytes_per_instruction;

eloparco wrote:
> clayborg wrote:
> > eloparco wrote:
> > > clayborg wrote:
> > > > This will work if the min and max opcode byte size are the same, like 
> > > > for arm64, the min and max are 4. This won't work for x86 or arm32 in 
> > > > thumb mode. So when backing up, we need to do an address lookup on the 
> > > > address we think we want to go to, and then adjust the starting address 
> > > > accordingly. Something like:
> > > > 
> > > > ```
> > > > SBAddress start_sbaddr = (base_addr - bytes_offset, g_vsc.target);
> > > > ```
> > > > now we have a section offset address that can tell us more about what 
> > > > it is. We can find the SBFunction or SBSymbol for this address and use 
> > > > those to find the right instructions. This will allow us to correctly 
> > > > disassemble code bytes. 
> > > > 
> > > > We can also look at the section that the memory comes from and see what 
> > > > the section contains. If the section is data, then emit something like:
> > > > ```
> > > > 0x1000 .byte 0x23
> > > > 0x1001 .byte 0x34
> > > > ...
> > > > ```
> > > > To find the section type we can do:
> > > > ```
> > > > SBSection section = start_sbaddr.GetSection();
> > > > if (section.IsValid() && section.GetSectionType() == 
> > > > lldb::eSectionTypeCode) {
> > > >  // Disassemble from a valid boundary
> > > > } else {
> > > >   // Emit a byte or long at a time with ".byte 0xXX" or other ASM 
> > > > directive for binary data
> > > > }
> > > > ```
> > > > We need to ensure we start disassembling on the correct instruction 
> > > > boundary as well as our math for "start_addr" might be in between 
> > > > actual opcode boundaries. If we are in a lldb::eSectionTypeCode, then 
> > > > we know we have instructions, and if we are not, then we can emit 
> > > > ".byte" or other binary data directives. So if we do have 
> > > > lldb::eSectionTypeCode as our section type, then we should have a 
> > > > function or symbol, and we can get instructions from those objects 
> > > > easily:
> > > > 
> > > > ```
> > > > if (section.IsValid() && section.GetSectionType() == 
> > > > lldb::eSectionTypeCode) {
> > > >  lldb::SBInstructionList instructions;
> > > >  lldb::SBFunction function = start_sbaddr.GetFunction();
> > > >  if (function.IsValid()) {
> > > > instructions = function.GetInstructions(g_vsc.target);
> > > >  } else {
> > > > symbol = start_sbaddr.GetSymbol();
> > > > if (symbol.IsValid())
> > > >   instructions = symbol.GetInstructions(g_vsc.target);
> > > > }
> > > > const size_t num_instrs = instructions.GetSize();
> > > > if (num_instrs > 0) {
> > > >   // we found instructions from a function or symbol and we need to 
> > > >   // find the matching instruction that we want to start from by 
> > > > iterating
> > > >   // over the instructions and finding the right address
> > > >   size_t matching_idx = num_instrs; // Invalid index
> > > >   for (size_t i=0; i > > > lldb::SBInstruction inst = instructions.GetInstructionAtIndex(i);
> > > > if (inst.GetAddress().GetLoadAddress(g_vsc.target) >= start_addr) {
> > > >   matching_idx = i;
> > > >   break;
> > > > }
> > > >   }
> > > >   if (matching_idx < num_instrs) {
> > > > // Now we can print the instructions from [matching_idx, num_instrs)
> > > > // then we need to repeat the search for the next function or 
> > > > symbol. 
> > > > // note there may be bytes between functions or symbols which we 
> > > > can disassemble
> > > > // by calling _get_instructions_from_memory(...) but we must find 
> > > > the next
> > > > // symbol or function boundary and get back on track
> > > >   }
> > > >   
> > > > ```
> > > Sorry, I should have provided a proper explanation.
> > > 
> > > I use the maximum instruction size as the "worst case". Basically, I need 
> > > to read a portion of memory but I do not know the start address and the 
> > > size. For the start address, if I want to read N instructions before 
> > > `base_addr` I need to read at least starting from `base_addr - N * 
> > > max_instruction_size`: if all instructions are of size 
> > > `max_instruction_size` I will read exactly N instructions; otherwise I 
> > > will read more than N instructions and prune the additional ones 
> > > afterwards. Same for applies for the size.
> > > 
> > > Since `start_addr` is based on a "worst case", it may be an address in 
> > > the middle of an instruction. In that case, that first instruction will 
> > > be misinterpreted, but I think that is negligible.
> > > 
> > > The logic is similar to what is implemented in other VS Code extensions, 
> > > like 
> > > https://github.com/vadimcn/vs

[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

2023-01-11 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2177
+  const auto max_instruction_size = g_vsc.target.GetMaximumOpcodeByteSize();
+  const auto bytes_offset = -instruction_offset * max_instruction_size;
+  auto start_addr = base_addr - bytes_offset;

Just checked out your changes, and you are still just subtracting a value from 
the start address and attempting to disassemble from memory which is the 
problem. We need to take that subtracted address, and look it up as suggested 
in previous code examples I posted. If you find a function to symbol, ask those 
objects for their instructions. and then try to use those. 

But basically for _any_ disassembly this is what I would recommend doing:
- first resolve the "start_address" (no matter how you come up the address) 
that want to disassemble into a SBAddress
- check its section. If the section is valid and contains instructions, call a 
function that will disassemble the address range for the section that starts at 
"start_address" and ends at the end of the section. We can call this 
"disassemble_code" as a function. More details on this below
- If the section does not contain instructions, just read the bytes and emit a 
lines like:
```
0x1000 .byte 0x12
0x1000 .byte 0x34
...
```

Now for the disassemble_code function. We know the address range for this is in 
code. We then need to resolve the address passed to "disassemble_code" into a 
SBAddress and ask that address for a SBFunction or SBSymbol as I mentioned. 
Then we ask the SBFunction or SBSymbol for all instructions that they contain, 
and then use any instructions that fall into the range we have. If there is no 
SBFunction or SBSymbol, then disassemble an instruction at a time and then see 
if the new address will resolve to a function or symbol.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

2023-01-11 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

I know this is all moot because the dSYM-specific patch landed, but I am 
curious about this part,

In D68655#4045561 , @clayborg wrote:

> 

> Different things are included in DW_AT_ranges, like address ranges for global 
> and static variables. .debug_aranges only has functions, no globals or 
> statics, so if you are trying to find a global variable by address, you can't 
> rely on .debug_aranges. Nothing in the DWARF spec states things clearly 
> enough for different compilers to know what to include in .debug_aranges  and 
> the compiler uint DW_AT_ranges.

The standard says,

"Each descriptor is a triple consisting of a segment selector, the beginning 
address within that segment of a range of text or data covered by some entry 
owned by the corresponding compilation unit, followed by the non-zero length of 
that range"

It is pretty clear on the point that any part of the address space that can be 
attributed to a compile_unit can be included in the debug_aranges range list - 
if only code is included, that's a choice of the aranges writer.  lldb itself, 
if debug_aranges is missing or doesn't include a CU, steps through the line 
table concatenating addresses for all the line entries in the CU (v. 
DWARFCompileUnit::BuildAddressRangeTable ) - it doesn't include data.  (it will 
also use DW_AT_ranges from the compile_unit but I worry more about this being 
copied verbatim from the .o file into the linked DWARF than I worry about 
debug_aranges, personally)

In a DW_TAG_compile_unit, the DW_AT_ranges that the compiler puts in the .o 
file isn't relevant to the final linked debug information, unless it included 
the discrete range of every item which might be linked into the final binary, 
and the DWARF linker only copies the range entries that were in the final 
binary in the linked dwarf DW_AT_ranges range list.   Or if the dwarf linker 
knows how to scan the DWARF compile_unit like lldb does, concatenating line 
table entries or DW_TAG_subprogram's.

If any dwarf linker is doing all of this work to construct an accurate & 
comprehensive compile_unit DW_AT_ranges, why couldn't it be trusted to also 
have an identical/accurate dwarf_aranges for this cu?  I don't see the 
difference.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68655/new/

https://reviews.llvm.org/D68655

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

2023-01-11 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

I guess to say it shorter.  If I have a dwarf_aranges, that means the dwarf 
linker created it.  And if it created it, surely its at least based off of the 
subprogram address ranges or the line table -- that is, the text address 
ranges.  If I have a DW_TAG_compile_unit DW_AT_ranges, either the compiler (to 
the .o file) created it, in which case I really am suspicious of those ranges 
because the compiler can't know which symbols will end up in the final 
executable, and the addresses in the ranges were simply translated to the final 
executable address equivalents.  Or it was rewritten by a dwarf linker that 
parsed the DWARF and knew how to correctly calculate the addresses that 
correspond to that compile unit.

if anything, I would trust a dwarf_aranges entry for a CU before I would trust 
the CU's DW_AT_ranges list.  Both have to be written by the dwarf linker to be 
correct, but only the former is written ONLY by the dwarf linker.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68655/new/

https://reviews.llvm.org/D68655

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

2023-01-11 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib marked 5 inline comments as done.
mib added inline comments.



Comment at: lldb/examples/python/scripted_process/scripted_platform.py:31
+def list_processes(self):
+""" Get a list of processes that can be ran on the platform.
+

labath wrote:
> mib wrote:
> > labath wrote:
> > > mib wrote:
> > > > labath wrote:
> > > > > mib wrote:
> > > > > > labath wrote:
> > > > > > > mib wrote:
> > > > > > > > mib wrote:
> > > > > > > > > mib wrote:
> > > > > > > > > > labath wrote:
> > > > > > > > > > > I am surprised that you want to go down the "run" path 
> > > > > > > > > > > for this functionality. I think most of the launch 
> > > > > > > > > > > functionality does not make sense for this use case 
> > > > > > > > > > > (e.g., you can't provide arguments to these processes, 
> > > > > > > > > > > when you "run" them, can you?), and it is not consistent 
> > > > > > > > > > > with what the "process listing" functionality does for 
> > > > > > > > > > > regular platforms.
> > > > > > > > > > > 
> > > > > > > > > > > OTOH, the "attach" flow makes perfect sense here -- you 
> > > > > > > > > > > take the pid of an existing process, attach to it, and 
> > > > > > > > > > > stop it at a random point in its execution. You can't 
> > > > > > > > > > > customize anything about how that process is run (because 
> > > > > > > > > > > it's already running) -- all you can do is choose how you 
> > > > > > > > > > > want to select the target process.
> > > > > > > > > > For now, there is no support for attaching to a scripted 
> > > > > > > > > > process, because we didn't have any use for it quite yet: 
> > > > > > > > > > cripted processes were mostly used for doing post-mortem 
> > > > > > > > > > debugging, so we "ran" them artificially in lldb by 
> > > > > > > > > > providing some launch options (the name of the class 
> > > > > > > > > > managing the process and an optional user-provided 
> > > > > > > > > > dictionary) through the command line or using an 
> > > > > > > > > > `SBLaunchInfo` object.
> > > > > > > > > > 
> > > > > > > > > > I guess I'll need to extend the `platform process 
> > > > > > > > > > launch/attach` commands and `SBAttachInfo` object to also 
> > > > > > > > > > support these options since they're required for the 
> > > > > > > > > > scripted process instantiation.
> > > > > > > > > > 
> > > > > > > > > > Note that we aren't really attaching to the real running 
> > > > > > > > > > process, we're creating a scripted process that knows how 
> > > > > > > > > > to read memory to mock the real process.
> > > > > > > > > @labath, I'll do that work on a follow-up patch
> > > > > > > > @labath here D139945 :) 
> > > > > > > Thanks. However, are you still planning to use the launch path 
> > > > > > > for your feature? Because if you're not, then I think this 
> > > > > > > comment should say "Get a list of processes that **are running**" 
> > > > > > > (or that **can be attached to**).
> > > > > > > 
> > > > > > > And if you are, then I'd like to hear your thoughts on the 
> > > > > > > discrepancy between what "launching" means for scripted and 
> > > > > > > non-scripted platforms.
> > > > > > > 
> > > > > > The way I see it is that the scripted platform will create a 
> > > > > > process with the right process plugin. In the case of scripted 
> > > > > > processes, the `ProcessLaunchInfo` argument should have the script 
> > > > > > class name set (which automatically sets the process plugin name to 
> > > > > > "ScriptedProcess" in the launch info). Once the process is 
> > > > > > instantiated (before the launch), the scripted platform will need 
> > > > > > to redirect to process stop events through its event multiplexer. 
> > > > > > So the way I see it essentially, running a regular process with the 
> > > > > > scripted platform should be totally transparent.
> > > > > > 
> > > > > > Something that is also worth discussing IMO, is the discrepancy 
> > > > > > between launching and attaching from the scripted platform:
> > > > > > 
> > > > > > One possibility could be that `platform process launch` would 
> > > > > > launch all the scripted processes listed by the scripted platform 
> > > > > > and set them up with the multiplexer, whereas `platform process 
> > > > > > attach` would just create a scripted process individually. I know 
> > > > > > this doesn't match the current behavior of the platform commands so 
> > > > > > if you guys think we should preserve the expected behavior, I guess.
> > > > > > 
> > > > > > May be @jingham has some opinion about this ?
> > > > > Before we do that, maybe we could take a step back. Could you explain 
> > > > > why you chose to use the "launch" flow for this use case?
> > > > > 
> > > > > To me, it just seems confusing to be using "launching" for any of 
> > > > > this, particularly given that "attaching" looks like a much better 
> > > > > match for what is happening here:
> > > > > - launch allows y

[Lldb-commits] [PATCH] D139250: [lldb] Add ScriptedPlatform python implementation

2023-01-11 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 488485.
mib marked an inline comment as done.
mib added a comment.

Address @labath comments:

- Rephrase documentation to remove any `Scripted Process` occurrence
- Add `attach_to_process` affordance to python `Scripted Platform` class


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139250/new/

https://reviews.llvm.org/D139250

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/scripted_platform.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
  lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
  lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -200,16 +200,9 @@
   return python::PythonObject();
 }
 
-python::PythonObject lldb_private::LLDBSwigPythonCreateScriptedProcess(
+python::PythonObject lldb_private::LLDBSwigPythonCreateScriptedObject(
 const char *python_class_name, const char *session_dictionary_name,
-const lldb::TargetSP &target_sp, const StructuredDataImpl &args_impl,
-std::string &error_string) {
-  return python::PythonObject();
-}
-
-python::PythonObject lldb_private::LLDBSwigPythonCreateScriptedThread(
-const char *python_class_name, const char *session_dictionary_name,
-const lldb::ProcessSP &process_sp, const StructuredDataImpl &args_impl,
+lldb::ExecutionContextRefSP exe_ctx_sp, const StructuredDataImpl &args_impl,
 std::string &error_string) {
   return python::PythonObject();
 }
Index: lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
@@ -13,8 +13,8 @@
 return module
 return None
 
-def __init__(self, target: lldb.SBTarget, args : lldb.SBStructuredData):
-super().__init__(target, args)
+def __init__(self, exe_ctx: lldb.SBExecutionContext, args : lldb.SBStructuredData):
+super().__init__(exe_ctx, args)
 
 self.corefile_target = None
 self.corefile_process = None
@@ -25,7 +25,7 @@
 idx = self.backing_target_idx.GetIntegerValue(42)
 if self.backing_target_idx.GetType() == lldb.eStructuredDataTypeString:
 idx = int(self.backing_target_idx.GetStringValue(100))
-self.corefile_target = target.GetDebugger().GetTargetAtIndex(idx)
+self.corefile_target = self.target.GetDebugger().GetTargetAtIndex(idx)
 self.corefile_process = self.corefile_target.GetProcess()
 for corefile_thread in self.corefile_process:
 structured_data = lldb.SBStructuredData()
Index: lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
@@ -7,8 +7,8 @@
 from lldb.plugins.scripted_process import ScriptedThread
 
 class InvalidScriptedProcess(ScriptedProcess):
-def __init__(self, target: lldb.SBTarget, args : lldb.SBStructuredData):
-super().__init__(target, args)
+def __init__(self, exe_ctx: lldb.SBExecutionContext, args : lldb.SBStructuredData):
+super().__init__(exe_ctx, args)
 self.threads[0] = InvalidScriptedThread(self, None)
 
 def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
Index: lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -7,8 +7,8 @@
 from lldb.plugins.scripted_process import ScriptedThread
 
 class DummyScriptedProcess(ScriptedProcess):
-def __init__(self, target: lldb.SBTarget, args : lldb.SBStructuredData):
-super().__init__(target, args)

[Lldb-commits] [PATCH] D139251: [lldb/Interpreter] Introduce ScriptedPlatform{, Python}Interface

2023-01-11 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 488488.
mib added a comment.

Add `AttachToProcess` method to interface


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139251/new/

https://reviews.llvm.org/D139251

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Interpreter/ScriptedPlatformInterface.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.h
@@ -0,0 +1,44 @@
+//===-- ScriptedPlatformPythonInterface.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_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPLATFORMPYTHONINTERFACE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPLATFORMPYTHONINTERFACE_H
+
+#include "lldb/Host/Config.h"
+
+#if LLDB_ENABLE_PYTHON
+
+#include "ScriptedPythonInterface.h"
+#include "lldb/Interpreter/ScriptedPlatformInterface.h"
+
+namespace lldb_private {
+class ScriptedPlatformPythonInterface : public ScriptedPlatformInterface,
+public ScriptedPythonInterface {
+public:
+  ScriptedPlatformPythonInterface(ScriptInterpreterPythonImpl &interpreter);
+
+  StructuredData::GenericSP
+  CreatePluginObject(const llvm::StringRef class_name,
+ ExecutionContext &exe_ctx,
+ StructuredData::DictionarySP args_sp,
+ StructuredData::Generic *script_obj = nullptr) override;
+
+  StructuredData::DictionarySP ListProcesses() override;
+
+  StructuredData::DictionarySP GetProcessInfo(lldb::pid_t) override;
+
+  Status AttachToProcess(lldb::ProcessAttachInfoSP attach_info) override;
+
+  Status LaunchProcess(lldb::ProcessLaunchInfoSP launch_info) override;
+
+  Status KillProcess(lldb::pid_t pid) override;
+};
+} // namespace lldb_private
+
+#endif // LLDB_ENABLE_PYTHON
+#endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDPLATFORMPYTHONINTERFACE_H
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.cpp
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPlatformPythonInterface.cpp
@@ -0,0 +1,108 @@
+//===-- ScriptedPlatformPythonInterface.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/Config.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-enumerations.h"
+
+#if LLDB_ENABLE_PYTHON
+
+// LLDB Python header must be included first
+#include "lldb-python.h"
+
+#include "SWIGPythonBridge.h"
+#include "ScriptInterpreterPythonImpl.h"
+#include "ScriptedPlatformPythonInterface.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::python;
+using Locker = ScriptInterpreterPythonImpl::Locker;
+
+ScriptedPlatformPythonInterface::ScriptedPlatformPythonInterface(
+ScriptInterpreterPythonImpl &interpreter)
+: ScriptedPlatformInterface(), ScriptedPythonInterface(interpreter) {}
+
+StructuredData::GenericSP ScriptedPlatformPythonInterface::CreatePluginObject(
+llvm::StringRef class_name, ExecutionContext &exe_ctx,
+StructuredData::DictionarySP args_sp, StructuredData::Generic *script_obj) {
+  if (class_name.empty())
+return {};
+
+  StructuredDataImpl args_impl(args_sp);
+  std::string error_string;
+
+  Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
+ Locker::FreeLock);
+
+  lldb::ExecutionContextRefSP exe_ctx_ref_sp =
+  std::make_shared(exe_ctx);
+
+  PythonObject ret_val = LLDBSwigPythonCreateScriptedObject(
+  class_name.str().c_str(), m_interpreter.GetDictionaryName(),
+  exe_ctx_ref_sp, args_impl, error_string);
+
+  m_object_instance_sp =
+  StructuredData::GenericSP(new StructuredPythonObject(std::move(ret_val)));
+
+  return m_object_instance_sp;
+}
+
+StructuredData::DictionarySP ScriptedPlatformPythonInterface::ListProcesses() {
+  Stat

[Lldb-commits] [PATCH] D139945: [lldb] Add scripted process launch/attach option to platform commands

2023-01-11 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

In D139945#4011466 , @labath wrote:

> In D139945#4009427 , @JDevlieghere 
> wrote:
>
>> In D139945#3999351 , @labath wrote:
>>
>>> For a "plugin", the scripted process is sure getting a lot of special 
>>> handling in generic code. (I know this isn't being introduced here, but I 
>>> wasn't involved in the previous review -- and I'm not actually sure I want 
>>> to be involved here). I don't think that's necessarily a bad thing in this 
>>> case, but maybe we should not be calling it a plugin in that case? We 
>>> already have a couple of precedents for putting implementations of 
>>> "pluggable" classes into generic code -- ProcessTrace for instance. And 
>>> just like in the case of ProcessTrace (where the real plugin is the thing 
>>> which handles the trace file format), here the real plugin would the the 
>>> scripting language backing the scripted process?
>>>
>>> (Apart from that, this patch seems fine.)
>>
>> Maybe one way around this is to have some generic metadata that gets passed 
>> to the plugin, that can be different per plugin?
>
> That might work, but I don't think it's really necessary. My only issue is 
> one of nomenclature. I don't see a problem with having a concrete Process 
> subclass in the lldb core. My problem is with calling that class a "plugin", 
> and pretending it's pluggable, but then introducing backdoor dependencies to 
> it (usually by referring to with via its (string) plugin name). You could try 
> to solve that by making it genuinely pluggable, but that could be overkill, 
> given that it already contains *is* pluggable, only in a different way -- one 
> can plugin a different scripting language to implement the bindings (and then 
> obviously the user can plug in to implement those bindings).

@labath I'm taking note of your comment but as you said it's the pluggable 
design of scripted processes is not introduced in this patch. I also have some 
plans for Scripted Processes that may involve rethinking its pluggable design, 
so I'll keep your feedback in mind for when I implement those changes :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139945/new/

https://reviews.llvm.org/D139945

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

61 matches

Mail list logo