[Lldb-commits] [PATCH] D108554: [lldb] Support querying registers via generic names without alt_names

2021-09-05 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 370785.
mgorny added a comment.

Preserve the prior (probably accidental) behavior of generic names taking 
precedence over later registers with the same name (e.g. generic `sp` alias 
taking precedence over 16-bit `sp` pseudo-register).


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

https://reviews.llvm.org/D108554

Files:
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Target/RegisterContext.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -20,8 +20,16 @@
   i386:x86-64
   GNU/Linux
   
+
+
+
+
+
 
+
+
 
+
 
 
 
@@ -59,8 +67,16 @@
 
 def readRegisters(self):
 return (
-"0102030405060708"  # rsp
-"1112131415161718"  # rip
+"0102030405060708"  # rcx
+"1112131415161718"  # rdx
+"2122232425262728"  # rsi
+"3132333435363738"  # rdi
+"4142434445464748"  # rbp
+"5152535455565758"  # rsp
+"6162636465666768"  # r8
+"7172737475767778"  # r9
+"8182838485868788"  # rip
+"91929394"  # eflags
 "0102030405060708090a"  # st0
 "1112131415161718191a" +  # st1
 "2122232425262728292a" * 6 +  # st2..st7
@@ -82,9 +98,31 @@
 
 # rsp and rip should be displayed as uints
 self.match("register read rsp",
-   ["rsp = 0x0807060504030201"])
+   ["rsp = 0x5857565554535251"])
 self.match("register read rip",
-   ["rip = 0x1817161514131211"])
+   ["rip = 0x8887868584838281"])
+
+# test generic aliases
+self.match("register read arg4",
+   ["rcx = 0x0807060504030201"])
+self.match("register read arg3",
+   ["rdx = 0x1817161514131211"])
+self.match("register read arg2",
+   ["rsi = 0x2827262524232221"])
+self.match("register read arg1",
+   ["rdi = 0x3837363534333231"])
+self.match("register read fp",
+   ["rbp = 0x4847464544434241"])
+self.match("register read sp",
+   ["rsp = 0x5857565554535251"])
+self.match("register read arg5",
+   ["r8 = 0x6867666564636261"])
+self.match("register read arg6",
+   ["r9 = 0x7877767574737271"])
+self.match("register read pc",
+   ["rip = 0x8887868584838281"])
+self.match("register read flags",
+   ["eflags = 0x94939291"])
 
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
Index: lldb/source/Target/RegisterContext.cpp
===
--- lldb/source/Target/RegisterContext.cpp
+++ lldb/source/Target/RegisterContext.cpp
@@ -54,6 +54,17 @@
   if (reg_name.empty())
 return nullptr;
 
+  // Try matching generic register names first.  This is consistent with
+  // how aliases work, esp. wrt "sp" generic alias taking precedence over
+  // "sp" pseudo-register on x86_64.
+  uint32_t generic_reg = Args::StringToGenericRegister(reg_name);
+  if (generic_reg != LLDB_INVALID_REGNUM) {
+const RegisterInfo *reg_info =
+GetRegisterInfo(eRegisterKindGeneric, generic_reg);
+if (reg_info)
+  return reg_info;
+  }
+
   const uint32_t num_registers = GetRegisterCount();
   for (uint32_t reg = start_idx; reg < num_registers; ++reg) {
 const RegisterInfo *reg_info = GetRegisterInfoAtIndex(reg);
@@ -62,6 +73,7 @@
 reg_name.equals_insensitive(reg_info->alt_name))
   return reg_info;
   }
+
   return nullptr;
 }
 
Index: lldb/source/Host/common/NativeRegisterContext.cpp
===
--- lldb/source/Host/common/NativeRegisterContext.cpp
+++ lldb/source/Host/common/NativeRegisterContext.cpp
@@ -56,6 +56,17 @@
   if (reg_name.empty())
 return nullptr;
 
+  // Try matching generic register names first.  This is consistent with
+  // how aliases

[Lldb-commits] [PATCH] D108831: [lldb] [gdb-remote] Add x86_64 pseudo-registers when using gdbserver

2021-09-05 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 370786.
mgorny added a comment.

Restore the `sp` test.


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

https://reviews.llvm.org/D108831

Files:
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86_64.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -9,8 +9,32 @@
 
 @skipIfXmlSupportMissing
 @skipIfRemote
-def test_x86_64_vec_regs(self):
-"""Test rendering of x86_64 vector registers from gdbserver."""
+def test_x86_64_regs(self):
+"""Test grabbing various x86_64 registers from gdbserver."""
+reg_data = [
+"0102030405060708",  # rcx
+"1112131415161718",  # rdx
+"2122232425262728",  # rsi
+"3132333435363738",  # rdi
+"4142434445464748",  # rbp
+"5152535455565758",  # rsp
+"6162636465666768",  # r8
+"7172737475767778",  # r9
+"8182838485868788",  # rip
+"91929394",  # eflags
+"0102030405060708090a",  # st0
+"1112131415161718191a",  # st1
+] + 6 * [
+"2122232425262728292a"  # st2..st7
+] + [
+"8182838485868788898a8b8c8d8e8f90",  # xmm0
+"9192939495969798999a9b9c9d9e9fa0",  # xmm1
+] + 14 * [
+"a1a2a3a4a5a6a7a8a9aaabacadaeafb0",  # xmm2..xmm15
+] + [
+"",  # mxcsr
+]
+
 class MyResponder(MockGDBServerResponder):
 def qXferRead(self, obj, annex, offset, length):
 if annex == "target.xml":
@@ -66,25 +90,10 @@
 return ""
 
 def readRegisters(self):
-return (
-"0102030405060708"  # rcx
-"1112131415161718"  # rdx
-"2122232425262728"  # rsi
-"3132333435363738"  # rdi
-"4142434445464748"  # rbp
-"5152535455565758"  # rsp
-"6162636465666768"  # r8
-"7172737475767778"  # r9
-"8182838485868788"  # rip
-"91929394"  # eflags
-"0102030405060708090a"  # st0
-"1112131415161718191a" +  # st1
-"2122232425262728292a" * 6 +  # st2..st7
-"8182838485868788898a8b8c8d8e8f90"  # xmm0
-"9192939495969798999a9b9c9d9e9fa0"  # xmm1
-"a1a2a3a4a5a6a7a8a9aaabacadaeafb0" * 14 +  # xmm2..xmm15
-""  # mxcsr
-)
+return "".join(reg_data)
+
+def writeRegisters(self, reg_hex):
+return "OK"
 
 def haltReason(self):
 return "T02thread:1ff0d;threads:1ff0d;thread-pcs:00010001bc00;07:0102030405060708;10:1112131415161718;"
@@ -124,6 +133,26 @@
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read ecx",
+   ["ecx = 0x04030201"])
+self.match("register read cx",
+   ["cx = 0x0201"])
+self.match("register read ch",
+   ["ch = 0x02"])
+self.match("register read cl",
+   ["cl = 0x01"])
+self.match("register read r8d",
+   ["r8d = 0x64636261"])
+self.match("register read r8w",
+   ["r8w = 0x6261"])
+self.match("register read r8l",
+   ["r8l = 0x61"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
@@ -135,3 +164,44 @@
 self.match("register read xmm1",
["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
 "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
+
+# test writing into pseudo-registers
+self.runCmd("register write ecx 0xfffefdfc")
+reg_data[0] = "fcfdfeff05060708"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read rcx",
+   ["rcx = 0x08070605fffefdfc"])
+
+

[Lldb-commits] [PATCH] D109281: [lldb] [gdb-remote] Extend x86 pseudo-regs to i386

2021-09-05 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 370787.
mgorny added a comment.

Add `sp` test.


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

https://reviews.llvm.org/D109281

Files:
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86_64.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -389,11 +389,112 @@
 # test generic aliases
 self.match("register read fp",
["ebp = 0x54535251"])
+self.match("register read sp",
+   ["esp = 0x44434241"])
 self.match("register read pc",
["eip = 0x84838281"])
 self.match("register read flags",
["eflags = 0x94939291"])
 
+# test pseudo-registers
+self.match("register read cx",
+   ["cx = 0x1211"])
+self.match("register read ch",
+   ["ch = 0x12"])
+self.match("register read cl",
+   ["cl = 0x11"])
+self.match("register read mm0",
+   ["mm0 = 0x0807060504030201"])
+self.match("register read mm1",
+   ["mm1 = 0x1817161514131211"])
+
+# both stX and xmmX should be displayed as vectors
+self.match("register read st0",
+   ["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
+self.match("register read st1",
+   ["st1 = {0x11 0x12 0x13 0x14 0x15 0x16 0x17 0x18 0x19 0x1a}"])
+self.match("register read xmm0",
+   ["xmm0 = {0x81 0x82 0x83 0x84 0x85 0x86 0x87 0x88 "
+"0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
+self.match("register read xmm1",
+   ["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
+"0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])
+
+# test recombining ymmX
+self.match("register read ymm0h",
+   ["ymm0h = {0xb1 0xb2 0xb3 0xb4 0xb5 0xb6 0xb7 0xb8 "
+"0xb9 0xba 0xbb 0xbc 0xbd 0xbe 0xbf 0xc0}"])
+self.match("register read ymm0",
+   ["ymm0 = {0x81 0x82 0x83 0x84 0x85 0x86 0x87 0x88 0x89 0x8a "
+"0x8b 0x8c 0x8d 0x8e 0x8f 0x90 0xb1 0xb2 0xb3 0xb4 0xb5 "
+"0xb6 0xb7 0xb8 0xb9 0xba 0xbb 0xbc 0xbd 0xbe 0xbf 0xc0}"])
+self.match("register read ymm1h",
+   ["ymm1h = {0xc1 0xc2 0xc3 0xc4 0xc5 0xc6 0xc7 0xc8 "
+"0xc9 0xca 0xcb 0xcc 0xcd 0xce 0xcf 0xd0}"])
+self.match("register read ymm1",
+   ["ymm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 0x99 0x9a "
+"0x9b 0x9c 0x9d 0x9e 0x9f 0xa0 0xc1 0xc2 0xc3 0xc4 0xc5 "
+"0xc6 0xc7 0xc8 0xc9 0xca 0xcb 0xcc 0xcd 0xce 0xcf 0xd0}"])
+
+# test writing into pseudo-registers
+self.runCmd("register write cx 0xfbfa")
+reg_data[1] = "fafb1314"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ecx",
+   ["ecx = 0x1413fbfa"])
+
+self.runCmd("register write ch 0xf9")
+reg_data[1] = "faf91314"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9fa"])
+self.match("register read ecx",
+   ["ecx = 0x1413f9fa"])
+
+self.runCmd("register write cl 0xf8")
+reg_data[1] = "f8f91314"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read cx",
+   ["cx = 0xf9f8"])
+self.match("register read ecx",
+   ["ecx = 0x1413f9f8"])
+
+self.runCmd("register write mm0 0xfffefdfcfbfaf9f8")
+reg_data[10] = "f8f9fafbfcfdfeff090a"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read st0",
+   ["st0 = {0xf8 0xf9 0xfa 0xfb 0xfc 0xfd 0xfe 0xff 0x09 0x0a}"])
+
+self.runCmd("register write xmm0 \"{0xff 0xfe 0xfd 0xfc 0xfb 0xfa 0xf9 "
+"0xf8 0xf7 0xf6 0xf5 0xf4 0xf3 0xf2 0xf1 0xf0}\"")
+reg_data[18] = "fffefdfcfbfaf9f8f7f6f5f4f3f2f1f0"
+self.assertPacketLogContains(["G" + "".join(reg_data)])
+self.match("register read ymm0",
+   ["ymm0 = {0xff 0xfe 0xfd 0xfc 0xfb 0xfa 0xf9 0xf8 0xf7 0xf6 "
+   

[Lldb-commits] [PATCH] D107717: [LLVM][CMake][NFC] Resolve FIXME: Rename LLVM_CMAKE_PATH to LLVM_CMAKE_DIR throughout the project

2021-09-05 Thread Alf via Phabricator via lldb-commits
gAlfonso-bit added a comment.

@ldionne update?


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

https://reviews.llvm.org/D107717

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


[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-09-05 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment.

In D97786#2974202 , @pfaffe wrote:

> In D97786#2973868 , @dblaikie wrote:
>
>> This doesn't solve all use cases/it's not a terribly general purpose 
>> situation - using -fdebug-prefix-map/-fdebug-compilation-dir requires 
>> knowledge of these limitations/how to use these features together. (though I 
>> agree that changes in this area should apply to all relative comp_dir 
>> lookups - source and dwo files alike, rather than handling one group 
>> differently from the other when they're all defined as comp_dir relative)
>
> My main concern is that even with knowledge of how the debug-compilation-dir 
> features work, there are use cases that stop working after this change and 
> are unfixable on the user's end. Anything that involves linking shared 
> objects into multiple executables for example.

Not fixable? Not sure I follow. In the case of cwd-relative (old behavior) you 
can fix the behavior by changing where you invoke the debugger from, and in the 
case of exe-relative you can fix the behavior by moving the executable? (once 
both source and dwo lookup is implemented consistently, at least)

Do you know anyone using the feature that would be challenged by this change in 
behavior? The only place I know using the feature is Chromium. (Google's 
internal build system uses -fdebug-compilation-dir=/proc/cwd so it really gets 
cwd-relative always, even with this change in lldb (but it's not a portable 
solution, since the path is unix-ish-specific, whereas Chromium probably needs 
a more portable solution that works on Windows, etc) - and then some local 
patches to gdb I think to do change the search paths so comp_dir gets ignored 
anyway, I think? not 100% sure, it's all a bit arcane)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97786

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


[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-09-05 Thread Philip Pfaffe via Phabricator via lldb-commits
pfaffe added a comment.

In D97786#2974879 , @dblaikie wrote:

> Not fixable? Not sure I follow. In the case of cwd-relative (old behavior) 
> you can fix the behavior by changing where you invoke the debugger from, and 
> in the case of exe-relative you can fix the behavior by moving the 
> executable? (once both source and dwo lookup is implemented consistently, at 
> least)

Thanks, I totally missed the fact that the patch intended to keep cwd-relative 
lookup working! Kudos for that! I overlooked it being mentioned in the 
discussion, and it's not in the commit message and the implementation is quite 
subtle. That eliminates most of my concerns!

That being said the implementation looks like it only does this partially. If I 
understand this correctly, the cwd-relative check happens in line 1646, but 
that only checks if the `dwo_name` exists. Below, `comp_dir/dwo_name` is 
however not checked against cwd. That means that we only get the old behavior 
if comp_dir happens to be `.`; any non-trivial relative comp_dir will behave 
differently, in that I need to cd into the comp_dir manually first to make 
things work. That's much better than I originally feared (chromium, e.g., uses 
`.`, so they're fine), but I still think we should retain the old behavior for 
non-trivial comp_dirs as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97786

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


[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-09-05 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment.

In D97786#2976549 , @pfaffe wrote:

> In D97786#2974879 , @dblaikie wrote:
>
>> Not fixable? Not sure I follow. In the case of cwd-relative (old behavior) 
>> you can fix the behavior by changing where you invoke the debugger from, and 
>> in the case of exe-relative you can fix the behavior by moving the 
>> executable? (once both source and dwo lookup is implemented consistently, at 
>> least)
>
> Thanks, I totally missed the fact that the patch intended to keep 
> cwd-relative lookup working!

Hmm, sorry, I didn't mean to imply that/didn't know the patch did that (perhaps 
it only does by accident, I'm not sure).

What I meant was - no matter which interpretation (binary relative or cwd 
relative) is provided by a DWARF consumer, there's a way to make it work 
(assuming the paths in the DWARF/across all the CUs are consistently relative 
to the same thing) - either you move the binary, or you move where you invoke 
the binary from/your cwd. So I don't think either interpretation makes 
something unworkable/unusable, depending on where you define the constraints of 
the problem (if you define it as "there's something you can do when 
compiling/linking the binary to get a certain behavior to work" (ie: you can 
always get cwd-relative if that's what you want), then yes, there's no solution 
if the change is made to make it binary-relative - but if you define the 
problem as "there's a way to debug this binary given these relative paths" - 
then yes, I think there's an answer either way: move the binary, or move the 
cwd). Then it's a question of backwards compatibility (I don't know of anyone 
relying on the cwd-relative behavior, do you? the only place using this 
functionality that I know of is Chromium) and a general "which is the better 
behavior", which, again, if it's basically only Chromium using it/built for 
that situation, I think it's fair to say they probably get to say which is more 
suitable for their use case.

(all this said: Chromium already had a debugger configuration (at least for 
gdb?) to set the `directories` property ( 
https://sourceware.org/gdb/onlinedocs/gdb/Source-Path.html ) - which basically 
overrides/implements the comp_dir relative searching (by initializing the 
`directories` property, by default, with `$cdir;$cwd` (so it searches a given 
DWARF-specified-comp_dir-relative-path relative to DW_AT_comp_dir, and if that 
fails, searches relative to $cwd) - which meant that it really didn't matter 
what DW_AT_comp_dir they used, it could've been complete garbage and 
wouldn't've mattered) - but that hadn't extended to dwo searching, so they had 
a problem with dwo searching, which, imho, the right solution would've been to 
extend gdb's use of its `directories` property to be used also for dwo 
searching, since it's the mechanism for comp_dir relative path resolution and 
dwo paths are comp_dir relative - then there wouldn't've been much need to 
change anything, so far as I know... (and some similar functionality for lldb 
would be nice too))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97786

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


Re: [Lldb-commits] New feature suggestion: Tacking variable update

2021-09-05 Thread Jyothi Krishna V S via lldb-commits
Thanks a lot Jim.

Our motivation was from the idea that the user might not be knowing which
variable might be causing the issue and might need multiple runs to finally
zero upon the problem. Another one is if the variable in question is
updated inside a loop. here we need not track all updates, just the last
iteration. However the overhead and multithreading are going to be a
challenge and we are looking at viable solutions.


On the command model we are tracking variable "a" , we are trying to track
and list all the updates to variable a up until that point in the
code(possibly a breakpoint).

Thank you for pointing out the thread plan tracer. Our model is also
similar to this. We will definitely take a deeper look into it and see if
we can suppress some of the undesirable features/effects. :)



On Tue, Aug 31, 2021 at 11:41 PM Jim Ingham  wrote:

> BTW, if you do want to follow this approach, then you should look at the
> "thread plan tracer" bits built into lldb, if you haven't already seen
> them.  The idea is that you can inject an automatic tracer into the
> machinery that governs how the program is executed.  The tracer can either
> follow the normal execution of thread plans or it can override the "step
> mode" so that it will force all "continues" to be a series of instruction
> single steps.  Your proposal is just adding a tracer that forces single
> step and does some printing.  So a more general version of your proposed
> feature would allow the user to set the tracer on, and register a tracer
> callback.  Running programs in single step mode is both very slow and
> fragile (some instructions don't behave quite right in single step mode) so
> we never extended this feature to the user level.  But if you want this as
> a pervasive feature, that would be the way to implement it.
>
> Jim
>
>
> > On Aug 31, 2021, at 10:50 AM, Jim Ingham via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> > From what I can tell, you are suggesting using instruction single
> stepping and printing out the variable value if it has changed.  Is that
> right?
> >
> > That seems much more expensive than watchpoints, and it isn't clear to
> me what the benefits would be.  I would be interested to hear more about
> the problems you had with watchpoints that lead you away from using them.
> You just say "it breaks at every update" but I'm not sure what that means.
> Watchpoints seem the natural way to do the job you are suggesting, so it
> seems the first task should be fixing whatever is rendering them unfit for
> one of their main purposes...
> >
> > If you are planning on using single stepping, how were you planning to
> also track modifications of the variable on other threads?  If you don't
> also observe them step by step you will miss one of the hardest to find
> source of modifications, the very ones a facility like this should help to
> uncover.
> >
> > Anyway, if your strategy was to use single-stepping on only one of the
> threads with an "is changed" check & subsequent printing, that does not
> require a separate command.  You can do that pretty easily with a scripted
> thread plan.  The SimpleStep in the scripted_step.py example is half way
> there, you would just have to remove the range check and add the printing.
> We don't currently ship any of the scripted thread plans with lldb by
> default, but if we come up with some useful ones we can certainly start
> doing so.
> >
> > I think your command model is a little confused as well.  From the way
> you were suggesting using it, your "track a" is an operation that sets the
> target running.  What in your model would happen if the program hit a
> breakpoint and stopped? Am I expected to say "track a" again to set it
> going in track mode, or would "step" continue the tracking?  If the latter,
> then "track a" is both a mode setting command and a continue command, which
> is confusing.  If you were going to do it as a mode command, then there
> should be:
> >
> > (lldb) track add variable "a"
> > 1: Tracking variable a
> > (lldb) track add variable "b"
> > 2: Tracking variable b
> > (lldb) track list
> > 1: Tracking variable a
> > 2: Tracking variable b
> > (lldb) track disable 1
> > Disabling tracking of variable a
> > (lldb) continue
> >
> > AddressFilename  Value  ...
> >
> > If you were going to do it as a variant of "continue" then you don't
> need enable and disable.
> >
> > And I'm not sure that writing this out as a table with header & entries
> will end up being very workable either.  In a real debugging session you
> would be using this feature in consort with hitting breakpoints, stepping,
> etc.  So you'll either have to reprint the header at every restart or the
> table entries and header will quickly get separated.
> >
> > Anyway, if you can indeed do what you contemplate with a straightforward
> scripted step plan, I'm not in favor of adding this as a dedicated
> command.  The approach seems likely to be so slow as not to be terrib

[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-09-05 Thread Philip Pfaffe via Phabricator via lldb-commits
pfaffe added a comment.

In D97786#2973868 , @dblaikie wrote:

> This doesn't solve all use cases/it's not a terribly general purpose 
> situation - using -fdebug-prefix-map/-fdebug-compilation-dir requires 
> knowledge of these limitations/how to use these features together. (though I 
> agree that changes in this area should apply to all relative comp_dir lookups 
> - source and dwo files alike, rather than handling one group differently from 
> the other when they're all defined as comp_dir relative)

My main concern is that even with knowledge of how the debug-compilation-dir 
features work, there are use cases that stop working after this change and are 
unfixable on the user's end. Anything that involves linking shared objects into 
multiple executables for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97786

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


[Lldb-commits] [PATCH] D101285: [lldb] [llgs server] Support creating core dumps on NetBSD

2021-09-05 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 370823.
mgorny added a comment.

Rebased.


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

https://reviews.llvm.org/D101285

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
@@ -0,0 +1,52 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import binascii
+import os
+
+class TestGdbSaveCore(gdbremote_testcase.GdbRemoteTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def coredump_test(self, core_path=None, expect_path=None):
+self.build()
+self.set_inferior_startup_attach()
+procs = self.prep_debug_monitor_and_inferior()
+self.add_qSupported_packets()
+ret = self.expect_gdbremote_sequence()
+self.assertIn("qSaveCore+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+packet = "$qSaveCore"
+if core_path is not None:
+packet += ";path-hint:{}".format(
+binascii.b2a_hex(core_path.encode()).decode())
+
+self.test_sequence.add_log_lines([
+"read packet: {}#00".format(packet),
+{"direction": "send", "regex": "[$]core-path:([0-9a-f]+)#.*",
+ "capture": {1: "path"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+out_path = binascii.a2b_hex(ret["path"].encode()).decode()
+if expect_path is not None:
+self.assertEqual(out_path, expect_path)
+
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(out_path)
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
+
+@skipUnlessPlatform(oslist=["netbsd"])
+def test_netbsd_path(self):
+core = lldbutil.append_to_process_working_directory(self, "core")
+self.coredump_test(core, core)
+
+@skipUnlessPlatform(oslist=["netbsd"])
+def test_netbsd_no_path(self):
+self.coredump_test()
+
+@skipUnlessPlatform(oslist=["netbsd"])
+def test_netbsd_bad_path(self):
+self.coredump_test("/dev/null/cantwritehere")
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -260,6 +260,8 @@
   break;
 
 case 'S':
+  if (PACKET_STARTS_WITH("qSaveCore"))
+return eServerPacketType_qLLDBSaveCore;
   if (PACKET_STARTS_WITH("qSpeedTest:"))
 return eServerPacketType_qSpeedTest;
   if (PACKET_MATCHES("qShlibInfoAddr"))
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -214,6 +214,8 @@
 
   PacketResult Handle_QPassSignals(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_qSaveCore(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_g(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qMemTags(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -226,6 +226,10 @@
   quit = true;
   return this->Handle_k(packet);
 });
+
+  RegisterMemberFunctionHandler(
+  StringExtractorGDBRemote::eServerPacketType_qLLDBSaveCore,
+  &GDBRemoteCommunicationServerLLGS::Handle_qSaveCore);
 }
 
 void GDBRemoteCommunicationServerLLGS::SetLaunchInfo(const ProcessLaunchInfo &info) {
@@ -3604,6 +3608,41 @@
   return status.Success() ? SendOKResponse() : SendErrorResponse(1);
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_qSaveCore(
+StringExtractorGDBRemote &packet) {
+  // Fail if we do