[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 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 with two small nits. Comment at: lldb/include/lldb/API/SBWatchpoint.h:14 +#include "lldb/API/SBType.h" +#include No longer necessary? =

[Lldb-commits] [PATCH] D144904: [Linux] Add kernel.yama.ptrace_scope note when ptrace fails to attach.

2023-02-28 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht added inline comments. Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:235 +std::error_code(errno, std::generic_category()), +"The current value of ptrace_scope is %d, which can cause ptrace to " +"fail to attach to a running

[Lldb-commits] [PATCH] D144904: [Linux] Add kernel.yama.ptrace_scope note when ptrace fails to attach.

2023-02-28 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht updated this revision to Diff 501367. rupprecht marked an inline comment as done. rupprecht added a comment. - Fix comment typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144904/new/ https://reviews.llvm.org/D144904 Files: lldb/sour

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib accepted this revision. mib added a comment. LGTM! Thanks for your patch @delcypher Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144937/new/ https://reviews.llvm.org/D144937 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Dan Liew via Phabricator via lldb-commits
delcypher marked an inline comment as done. delcypher added a comment. @JDevlieghere @mib @bulbazord Thanks for all the feedback. This patch is ready for another review pass. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144937/new/ https://review

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Dan Liew via Phabricator via lldb-commits
delcypher marked an inline comment as done. delcypher added inline comments. Comment at: lldb/source/API/SBWatchpoint.cpp:319 + } + return false; +} bulbazord wrote: > delcypher wrote: > > @bulbazord What I don't like about my implementation here is that the us

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Dan Liew via Phabricator via lldb-commits
delcypher updated this revision to Diff 501356. delcypher added a comment. - Add `WatchpointValueKind` enum - Use `WatchpointValueKind` for new `GetWatchValueKind()` method (previously named `SBWatchpoint::IsWatchVariable`) - Add `IsWatchingsRead()` method - Add `IsWatchingWrites()` method - Remo

[Lldb-commits] [lldb] a92f783 - Fix the run locker setting for async launches that don't stop at the

2023-02-28 Thread Jim Ingham via lldb-commits
Author: Jim Ingham Date: 2023-02-28T17:34:49-08:00 New Revision: a92f7832f35c6c4792d8693e724c19802da75b36 URL: https://github.com/llvm/llvm-project/commit/a92f7832f35c6c4792d8693e724c19802da75b36 DIFF: https://github.com/llvm/llvm-project/commit/a92f7832f35c6c4792d8693e724c19802da75b36.diff LO

[Lldb-commits] [PATCH] D144665: Use Resume not PrivateResume when asynchronously continuing after the start at stop

2023-02-28 Thread Jim Ingham via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGa92f7832f35c: Fix the run locker setting for async launches that don't stop at the (authored by jingham). Repository: r

[Lldb-commits] [lldb] 00b2c33 - Fix typos in the test command for D144929

2023-02-28 Thread Jim Ingham via lldb-commits
Author: Jim Ingham Date: 2023-02-28T16:59:19-08:00 New Revision: 00b2c33c9359e1f75dd29d5083fac66bfd52db6e URL: https://github.com/llvm/llvm-project/commit/00b2c33c9359e1f75dd29d5083fac66bfd52db6e DIFF: https://github.com/llvm/llvm-project/commit/00b2c33c9359e1f75dd29d5083fac66bfd52db6e.diff LO

[Lldb-commits] [PATCH] D145020: Hoist debugserver arch-dep sources out of a side CMakeLists into the debugserver main CMakeLists

2023-02-28 Thread Jason Molenda via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf136ca848309: Put the arch-dep debugserver files in main CMakeLists.txt (authored by jasonmolenda). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145020/new/

[Lldb-commits] [lldb] f136ca8 - Put the arch-dep debugserver files in main CMakeLists.txt

2023-02-28 Thread Jason Molenda via lldb-commits
Author: Jason Molenda Date: 2023-02-28T16:57:11-08:00 New Revision: f136ca84830942451fc20f8980dbc36028520920 URL: https://github.com/llvm/llvm-project/commit/f136ca84830942451fc20f8980dbc36028520920 DIFF: https://github.com/llvm/llvm-project/commit/f136ca84830942451fc20f8980dbc36028520920.diff

[Lldb-commits] [PATCH] D145020: Hoist debugserver arch-dep sources out of a side CMakeLists into the debugserver main CMakeLists

2023-02-28 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/D145020/new/ https://reviews.llvm.org/D145020 ___

[Lldb-commits] [PATCH] D145020: Hoist debugserver arch-dep sources out of a side CMakeLists into the debugserver main CMakeLists

2023-02-28 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda created this revision. jasonmolenda added a reviewer: JDevlieghere. jasonmolenda added a project: LLDB. Herald added a subscriber: kristof.beyls. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits. There was historical

[Lldb-commits] [PATCH] D144664: Preserve SBValue's that only have an Error state

2023-02-28 Thread Jim Ingham via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGe8a2fd5e7be2: An SBValue whose underlying ValueObject has no valid value, but does (authored by jingham). Repository: r

[Lldb-commits] [lldb] e8a2fd5 - An SBValue whose underlying ValueObject has no valid value, but does

2023-02-28 Thread Jim Ingham via lldb-commits
Author: Jim Ingham Date: 2023-02-28T16:41:20-08:00 New Revision: e8a2fd5e7be23e4087e7ff58efa354dfae1a17c4 URL: https://github.com/llvm/llvm-project/commit/e8a2fd5e7be23e4087e7ff58efa354dfae1a17c4 DIFF: https://github.com/llvm/llvm-project/commit/e8a2fd5e7be23e4087e7ff58efa354dfae1a17c4.diff LO

[Lldb-commits] [PATCH] D144929: Add SBCommandInterpreter::UserCommandExists

2023-02-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment. LGTM! Sorry I forgot about this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144929/new/ https://reviews.llvm.org/D144929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D144929: Add SBCommandInterpreter::UserCommandExists

2023-02-28 Thread Jim Ingham via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG4eb694e35dd5: Add SBCommandInterpreter::UserCommandExists parallel to CommandExists. (authored by jingham). Repository:

[Lldb-commits] [lldb] 4eb694e - Add SBCommandInterpreter::UserCommandExists parallel to CommandExists.

2023-02-28 Thread Jim Ingham via lldb-commits
Author: Jim Ingham Date: 2023-02-28T15:58:14-08:00 New Revision: 4eb694e35dd514395819b375b31a80bd6d58378e URL: https://github.com/llvm/llvm-project/commit/4eb694e35dd514395819b375b31a80bd6d58378e DIFF: https://github.com/llvm/llvm-project/commit/4eb694e35dd514395819b375b31a80bd6d58378e.diff LO

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added inline comments. Comment at: lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py:68 +self.runCmd('watchpoint set variable -w read_write -- global') +watchpoint = target.GetWatchpointAtIndex(0) +self.assertTrue(watchpoint.IsWatchV

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Dan Liew via Phabricator via lldb-commits
delcypher added inline comments. Comment at: lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py:66 +if variable_watchpoint: +# FIXME: There should probably be an API to create a variable watchpoint +self.runCmd('watchpoint set variable -w re

[Lldb-commits] [lldb] b22dcaf - Update debugserver xcode proj to build with c++17

2023-02-28 Thread Jason Molenda via lldb-commits
Author: Jason Molenda Date: 2023-02-28T13:37:03-08:00 New Revision: b22dcaf113280278293639a02207edf78d103fb8 URL: https://github.com/llvm/llvm-project/commit/b22dcaf113280278293639a02207edf78d103fb8 DIFF: https://github.com/llvm/llvm-project/commit/b22dcaf113280278293639a02207edf78d103fb8.diff

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments. Comment at: lldb/source/API/SBWatchpoint.cpp:319 + } + return false; +} delcypher wrote: > @bulbazord What I don't like about my implementation here is that the user > can't tell the difference between > > 1. there's no shared

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Dan Liew via Phabricator via lldb-commits
delcypher added inline comments. Comment at: lldb/source/API/SBWatchpoint.cpp:319 + } + return false; +} @bulbazord What I don't like about my implementation here is that the user can't tell the difference between 1. there's no shared pointer AND 2. watchpoin

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment. Seems like you've already got feedback about breaking ABI. Small nit: Comment at: lldb/bindings/interface/SBWatchpointDocstrings.i:30-31 +%feature("docstring", " +Returns true if the watchpoint is a variable watchpoint. Otherwise the +watchpoi

[Lldb-commits] [lldb] 35d17c1 - [lldb] Remove const qualifier on bool argument passed by value

2023-02-28 Thread Med Ismail Bennani via lldb-commits
Author: Med Ismail Bennani Date: 2023-02-28T12:52:32-08:00 New Revision: 35d17c17a6702c70e877c63e9f7c4ce3e35c1bb9 URL: https://github.com/llvm/llvm-project/commit/35d17c17a6702c70e877c63e9f7c4ce3e35c1bb9 DIFF: https://github.com/llvm/llvm-project/commit/35d17c17a6702c70e877c63e9f7c4ce3e35c1bb9.

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Dan Liew via Phabricator via lldb-commits
delcypher added a comment. In D144937#4159324 , @jasonmolenda wrote: > Hi Dan, I hadn't looked this over very closely but one thing that jumped out > is that you're adding a member to SBWatchpoint, and we can't do that, it's an > API breaking change.

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere requested changes to this revision. JDevlieghere added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/API/SBWatchpoint.cpp:329-341 +// We can't return `watchpoint_sp->GetWatchSpec().c_str()` +// because the temporary s

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

2023-02-28 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. You're passing `is_callback` by value so the `const` is close to redundant. I think there's an "Effective C++" chapter dedicated to this. LLVM is pretty conservative in marking this as `const` and while LLDB uses it a bit more this isn't common at all. The current

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment. In D144937#4159324 , @jasonmolenda wrote: > In cases where we need to store additional information than a weak pointer to > an lldb private object, we traditionally add an Impl class which has the > additional member(s) an

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

2023-02-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9a9fce1fed6d: [lldb] Fix {break,watch}point command function stopp

[Lldb-commits] [lldb] 9a9fce1 - [lldb] Fix {break, watch}point command function stopping behaviour

2023-02-28 Thread Med Ismail Bennani via lldb-commits
Author: Med Ismail Bennani Date: 2023-02-28T11:39:58-08:00 New Revision: 9a9fce1fed6d2af3fb6d1d7073c4a0b3e00bc515 URL: https://github.com/llvm/llvm-project/commit/9a9fce1fed6d2af3fb6d1d7073c4a0b3e00bc515 DIFF: https://github.com/llvm/llvm-project/commit/9a9fce1fed6d2af3fb6d1d7073c4a0b3e00bc515.

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

2023-02-28 Thread Alex Langford via Phabricator via lldb-commits
bulbazord accepted this revision. bulbazord added a comment. LGTM thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144688/new/ https://reviews.llvm.org/D144688 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.

[Lldb-commits] [PATCH] D144688: [lldb] Fix {break, watch}point command function stopping behaviour

2023-02-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 501240. mib marked an inline comment as done. mib added a comment. Address @delcypher comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144688/new/ https://reviews.llvm.org/D144688 Files: lldb/include/lldb/Interpreter/ScriptInterpreter.h lldb/

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment. Hi Dan, I hadn't looked this over very closely but one thing that jumped out is that you're adding a member to SBWatchpoint, and we can't do that, it's an API breaking change. In cases where we need to store additional information than a weak pointer to an lldb pr

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added inline comments. Comment at: lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py:83 +self.assertFalse(watchpoint.IsWatchVariable()) +self.assertEqual(watchpoint.GetWatchSpec(), '') + mib wrote: > unrelated to this patch, the

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added inline comments. Comment at: lldb/test/API/python_api/watchpoint/TestSetWatchpoint.py:66 +if variable_watchpoint: +# FIXME: There should probably be an API to create a variable watchpoint +self.runCmd('watchpoint set variable -w read_wri

[Lldb-commits] [PATCH] D144937: [LLDB] Expose several methods in SBWatchpoint

2023-02-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment. Most of this looks fine to me, besides the WatchSpec cache string in the SB class. I don't think the spec changes after the creation of the watchpoint, so may be this WatchSpec object should be a `lldb_private::ConstString` and from there you'll be able to get your `c_strin

[Lldb-commits] [PATCH] D144904: [Linux] Add kernel.yama.ptrace_scope note when ptrace fails to attach.

2023-02-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. And testing would require the test to `sudo ...` so I don't think this can be tested. Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:235 +std::error_code(errno, std::generic_category()), +"The current value o

[Lldb-commits] [PATCH] D144904: [Linux] Add kernel.yama.ptrace_scope note when ptrace fails to attach.

2023-02-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/unittests/Process/Linux/ProcfsTests.cpp:113 + + // At this point we shouldn't fail parsing the core ids + Expected ptrace_scope = GetPtraceScope(); What do you mean by core ids? Repository: rG LLVM Githu