date:20220601

[Lldb-commits] [PATCH] D126770: [lldb] [Process/FreeBSD] Do not send SIGSTOP to stopped process

2022-06-01 Thread Michał Górny via Phabricator via lldb-commits

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Do not send SIGSTOP when requested to halt a process that's already
stopped.  This results in the signal being queued for delivery once
the process is resumed, and unexpectedly stopping it again.

This fixes TestBreakpointSetRestart failure right now, and is necessary
for non-stop protocol patches to land.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D126770

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp


Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -497,6 +497,10 @@
 Status NativeProcessFreeBSD::Halt() {
   Status error;
 
+  // Do not try to stop a process that's already stopped, this may cause
+  // the SIGSTOP to get queued and stop the process again once resumed.
+  if (StateIsStoppedState(m_state, false))
+return error;
   if (kill(GetID(), SIGSTOP) != 0)
 error.SetErrorToErrno();
   return error;


Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -497,6 +497,10 @@
 Status NativeProcessFreeBSD::Halt() {
   Status error;
 
+  // Do not try to stop a process that's already stopped, this may cause
+  // the SIGSTOP to get queued and stop the process again once resumed.
+  if (StateIsStoppedState(m_state, false))
+return error;
   if (kill(GetID(), SIGSTOP) != 0)
 error.SetErrorToErrno();
   return error;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

2022-06-01 Thread Jim Ingham via Phabricator via lldb-commits

jingham created this revision.
jingham added reviewers: kastiglione, clayborg.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

At present, if you run a regex command (e.g. the `b` command) you will get two 
history entries, one for the actual command the user typed, and one for the 
resolved command:

  (lldb) b foo.c:12
  Breakpoint 1: no locations (pending).
  Breakpoint set in dummy target, will get copied into future targets.
  (lldb) history
 0: b foo.c:12
 1: breakpoint set --file 'foo.c' --line 12
 2: history

That seems wrong in principle, there shouldn't be double entry like this.  But 
it also has some subtle side effects that are even less desirable.  For 
instance, if you write a Python based command that calls "HandleCommand" of a 
regex command, then the resolved command gets inserted into the history list 
AFTER the python command the user actually ran.  That in turn means that 
up-arrow doesn't actually rerun the command the user typed, but instead runs 
the resolved regex command.

Since regex command resolution isn't stateful - the same input string will 
always resolve to the same command - there's no actual reason to do this double 
entry, just storing the command the user typed will suffice.  And there's 
already a way to see resolved commands, so you don't need it for this purpose 
either.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126789

Files:
  lldb/source/Commands/CommandObjectRegexCommand.cpp
  lldb/test/API/functionalities/history/TestHistoryRecall.py


Index: lldb/test/API/functionalities/history/TestHistoryRecall.py
===
--- lldb/test/API/functionalities/history/TestHistoryRecall.py
+++ lldb/test/API/functionalities/history/TestHistoryRecall.py
@@ -1,5 +1,5 @@
 """
-Make sure the !N and !-N commands work properly.
+Test some features of "session history" and history recall.
 """
 
 
@@ -20,9 +20,25 @@
 
 def test_history_recall(self):
 """Test the !N and !-N functionality of the command interpreter."""
-self.sample_test()
+self.do_bang_N_test()
 
-def sample_test(self):
+def test_regex_history(self):
+"""Test the regex commands don't add two elements to the history"""
+self.do_regex_history_test()
+
+def do_regex_history_test(self):
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+command = "_regexp-break foo.c:12"
+self.runCmd(command, msg="Run the regex break command", inHistory = 
True)
+interp.HandleCommand("session history", result, True)
+self.assertTrue(result.Succeeded(), "session history ran successfully")
+results = result.GetOutput()
+self.assertIn(command, results, "Recorded the actual command")
+self.assertNotIn("breakpoint set --file 'foo.c' --line 12", results,
+ "Didn't record the resolved command")
+
+def do_bang_N_test(self):
 interp = self.dbg.GetCommandInterpreter()
 result = lldb.SBCommandReturnObject()
 interp.HandleCommand("session history", result, True)
Index: lldb/source/Commands/CommandObjectRegexCommand.cpp
===
--- lldb/source/Commands/CommandObjectRegexCommand.cpp
+++ lldb/source/Commands/CommandObjectRegexCommand.cpp
@@ -75,7 +75,7 @@
   // should have set up the context appropriately, we shouldn't have to
   // redo that.
   return m_interpreter.HandleCommand(new_command->c_str(),
- eLazyBoolCalculate, result);
+ eLazyBoolNo, result);
 }
   }
   result.SetStatus(eReturnStatusFailed);


Index: lldb/test/API/functionalities/history/TestHistoryRecall.py
===
--- lldb/test/API/functionalities/history/TestHistoryRecall.py
+++ lldb/test/API/functionalities/history/TestHistoryRecall.py
@@ -1,5 +1,5 @@
 """
-Make sure the !N and !-N commands work properly.
+Test some features of "session history" and history recall.
 """
 
 
@@ -20,9 +20,25 @@
 
 def test_history_recall(self):
 """Test the !N and !-N functionality of the command interpreter."""
-self.sample_test()
+self.do_bang_N_test()
 
-def sample_test(self):
+def test_regex_history(self):
+"""Test the regex commands don't add two elements to the history"""
+self.do_regex_history_test()
+
+def do_regex_history_test(self):
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+command = "_regexp-break foo.c:12"
+self.runCmd(command, msg="Run the regex break command", inHistory = True)
+interp.HandleCommand("session history", result, True)
+self

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

2022-06-01 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

Note, the original code tries to finesse this by passing eLazyBoolCalculate, 
which then resolves to a check against the current command depth.  But that 
check was wrong for SBDebugger.HandleCommand, which is considered at depth 0, 
and anyway, I don't think it ever makes sense to have the regex command add the 
resolved command to the history.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126789

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

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

2022-06-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere 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/D126789/new/

https://reviews.llvm.org/D126789

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

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

2022-06-01 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added a comment.

thank you for fixing!

Q: why does this test have this thin `test_X`/`do_x_test` separation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126789

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

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

2022-06-01 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added a comment.

I noticed that the example python command and this feature don't support 
specifying breakpoint locations. Should it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126513

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

[Lldb-commits] [lldb] 62b4482 - Revert "Adapt LLDB for D120540."

2022-06-01 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2022-06-01T10:33:53-07:00
New Revision: 62b448217595c33788693f4b682ea5a84d9e2005

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

LOG: Revert "Adapt LLDB for D120540."

This reverts commit ca73de43744503a557b1f3709c0ff4751798702f.

That patch was just hiding the problem, instead of fixing it.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
index a75671049f61d..38dd55bc76d36 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -604,6 +604,7 @@ ClangModulesDeclVendor::Create(Target &target) {
   "clang",
   "-fmodules",
   "-fimplicit-module-maps",
+  "-fcxx-modules",
   "-fsyntax-only",
   "-femit-all-decls",
   "-target",



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

[Lldb-commits] [PATCH] D126596: [lldb, test] Fix typos in the lldb tests

2022-06-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Interesting insight in which tests were clearly copied from the sample test.

Thanks, LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126596

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

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

2022-06-01 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectProcess.cpp:668
+  [this, old_sync]() {
+this->GetDebugger().SetAsyncExecution(!old_sync);
+  });

this should be `old_sync` (no `!`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126513

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

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

2022-06-01 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

In D126789#3550572 , @kastiglione 
wrote:

> thank you for fixing!
>
> Q: why does this test have this thin `test_X`/`do_x_test` separation?

We did that separation originally (and the sample test reflects that) so that 
you could see the list of tests in a file (with hopefully suggestive names) 
easily at the top, and then all the gritty details would be in the do_ methods 
below.  It's also good to remind yourself that for unittest all the "test_" 
methods are special, you can't use that name for a helper function...

This format is not unique to this test, many of the tests (particularly older 
ones or ones that copied the sample test) do it that way.  With tiny actual 
test bodies like these, it's unclear how much you gain by this, but the test 
was already mostly following that separation (except the test implementation 
was just copied over from the sample test) I followed the practice of the file.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126789

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

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

2022-06-01 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: lldb/source/Commands/CommandObjectRegexCommand.cpp:74-76
   // Pass in true for "no context switching".  The command that called us
   // should have set up the context appropriately, we shouldn't have to
   // redo that.

Is this comment out of date now? It mentions passing in "true"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126789

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

[Lldb-commits] [PATCH] D126770: [lldb] [Process/FreeBSD] Do not send SIGSTOP to stopped process

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

[Lldb-commits] [lldb] 62b4482 - Revert "Adapt LLDB for D120540."

[Lldb-commits] [PATCH] D126596: [lldb, test] Fix typos in the lldb tests

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history

11 matches

Site Navigation

Mail list logo

Footer information