[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

2020-11-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I like this. Some comments in the tests -- trying to reduce the amount of 
preprocessor magic..




Comment at: lldb/unittests/Process/FreeBSD/CMakeLists.txt:1
+add_lldb_unittest(RegisterContextFreeBSDTests
+  RegisterContextFreeBSDTests.cpp

I'd put this in `unittests/Process/Utility`, to match the location of the code 
being tested.



Comment at: lldb/unittests/Process/FreeBSD/CMakeLists.txt:8-9
+
+target_include_directories(RegisterContextFreeBSDTests PRIVATE
+  ${LLDB_SOURCE_DIR}/source/Plugins/Process/Utility)

Drop this, and include the file as `"Plugins/Process/Utility/..."`



Comment at: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp:16
+
+#if defined(__x86_64__) || defined(__i386__)
+#include "lldb-x86-register-enums.h"

These don't really have to be ifdefed, do they?



Comment at: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp:26-31
+#define ASSERT_REG(regname, offset, size)  
\
+  {
\
+const RegisterInfo *reg_info = ®_ctx.GetRegisterInfo()[lldb_##regname]; 
\
+EXPECT_EQ(reg_info->byte_offset, offset);  
\
+EXPECT_EQ(reg_info->byte_size, size);  
\
+  }

```
pair GetRegParams(const RegisterContext &ctx, unsigned reg) {
  const RegisterInfo &info = reg_ctx.GetRegisterInfo()[reg];
  return {info.byte_offset, info.byte_size};
}
```



Comment at: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp:35
+
+#define ASSERT_GPR_X86_64(regname) 
\
+  ASSERT_REG(regname##_x86_64, offsetof(reg, r_##regname), 
\

```
#define ASSERT_GPR_X86_64(regname)
  ASSERT_THAT(GetRegParams(reg_ctx, regname##_x86_64, 
testing::Pair(offsetof(reg, r_##regname), sizeof(reg::r_##regname))
```



Comment at: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp:72-74
+#if defined(__i386__)
+#define reg32 reg
+#endif

Inside the test function:
```
#ifdef __i386__
using native_i386_regs = ::reg32;
#else
using native_i386_regs = ::reg;
#endif
```


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

https://reviews.llvm.org/D91216

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


[Lldb-commits] [PATCH] D91264: [lldb] [test] Add a minimal test for x86 dbreg reading

2020-11-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I have a feeling we discussed this already, but I don't remember what was the 
conclusion.

Was exposing debug registers to the user a conscious choice? Other OSes don't 
actually do that. Neither does gdb, afaict...

In think that allowing the user to see dr contents is a good idea -- allows him 
to check what the debugger has done. I'm not convinced about the writing aspect 
though -- seems like an easy way to confuse the debugger and make it crash...




Comment at: lldb/test/Shell/Register/x86-db-read.test:2
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64) && can-set-dbregs
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t

Who sets `can-set-dbregs` ?


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

https://reviews.llvm.org/D91264

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


[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

2020-11-12 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:615
+  if (offset == LLDB_INVALID_XSAVE_OFFSET)
+return false;
+

When does this return false? When the ymm regs are not available?



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h:83
   size_t GetDBROffset() const;
+  bool GetYMMSplitReg(uint32_t reg, void **xmm, void **ymm_hi);
 };

`void *&`


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

https://reviews.llvm.org/D91293

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


[Lldb-commits] [PATCH] D91264: [lldb] [test] Add a minimal test for x86 dbreg reading

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

In D91264#2390699 , @labath wrote:

> I have a feeling we discussed this already, but I don't remember what was the 
> conclusion.
>
> Was exposing debug registers to the user a conscious choice? Other OSes don't 
> actually do that. Neither does gdb, afaict...
>
> In think that allowing the user to see dr contents is a good idea -- allows 
> him to check what the debugger has done. I'm not convinced about the writing 
> aspect though -- seems like an easy way to confuse the debugger and make it 
> crash...

I agree with this. Hence, I've covered only the reading part. I don't have a 
strong opinion whether we should ban writes.




Comment at: lldb/test/Shell/Register/x86-db-read.test:2
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64) && can-set-dbregs
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t

labath wrote:
> Who sets `can-set-dbregs` ?
https://github.com/llvm/llvm-project/blob/master/lldb/test/Shell/lit.cfg.py#L123


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

https://reviews.llvm.org/D91264

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


[Lldb-commits] [PATCH] D91264: [lldb] [test] Add a minimal test for x86 dbreg reading

2020-11-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.






Comment at: lldb/test/Shell/Register/x86-db-read.test:2
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64) && can-set-dbregs
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t

mgorny wrote:
> labath wrote:
> > Who sets `can-set-dbregs` ?
> https://github.com/llvm/llvm-project/blob/master/lldb/test/Shell/lit.cfg.py#L123
The feature name is `dbregs-set`, is it not ?


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

https://reviews.llvm.org/D91264

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


[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:615
+  if (offset == LLDB_INVALID_XSAVE_OFFSET)
+return false;
+

labath wrote:
> When does this return false? When the ymm regs are not available?
Yes. Note that this change brings us closer to being able to disable whole 
regsets — I just need to figure out how to do it ;-).



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h:83
   size_t GetDBROffset() const;
+  bool GetYMMSplitReg(uint32_t reg, void **xmm, void **ymm_hi);
 };

labath wrote:
> `void *&`
Are you sure about this? I feel like the whole `&` deal is quite confusing 
(read: shouldn't have happened). You don't know whether the method modifies the 
variables passed to it unless you look at the method prototype.


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

https://reviews.llvm.org/D91293

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


[Lldb-commits] [PATCH] D91264: [lldb] [test] Add a minimal test for x86 dbreg reading

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 304751.
mgorny added a comment.

Corrected `REQUIRES`.


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

https://reviews.llvm.org/D91264

Files:
  lldb/test/Shell/Register/Inputs/x86-db-read.cpp
  lldb/test/Shell/Register/x86-db-read.test


Index: lldb/test/Shell/Register/x86-db-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-db-read.test
@@ -0,0 +1,35 @@
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64) && dbregs-set
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{[0-9]+}} stopped
+
+watchpoint set variable -w write g_8w
+# CHECK: Watchpoint created: Watchpoint 1: addr = 0x{{[0-9a-f]*}} size = 1 
state = enabled type = w
+watchpoint set variable -w read_write g_16rw
+# CHECK: Watchpoint created: Watchpoint 2: addr = 0x{{[0-9a-f]*}} size = 2 
state = enabled type = rw
+watchpoint set variable -w write g_32w
+# CHECK: Watchpoint created: Watchpoint 3: addr = 0x{{[0-9a-f]*}} size = 4 
state = enabled type = w
+watchpoint set variable -w read_write g_32rw
+# CHECK: Watchpoint created: Watchpoint 4: addr = 0x{{[0-9a-f]*}} size = 4 
state = enabled type = rw
+
+print &g_8w
+# CHECK: (uint8_t *) $0 = [[VAR8W:0x[0-9a-f]*]]
+print &g_16rw
+# CHECK: (uint16_t *) $1 = [[VAR16RW:0x[0-9a-f]*]]
+print &g_32w
+# CHECK: (uint32_t *) $2 = [[VAR32W:0x[0-9a-f]*]]
+print &g_32rw
+# CHECK: (uint32_t *) $3 = [[VAR64RW:0x[0-9a-f]*]]
+
+register read --all
+# CHECK-DAG: dr0 = [[VAR8W]]
+# CHECK-DAG: dr1 = [[VAR16RW]]
+# CHECK-DAG: dr2 = [[VAR32W]]
+# CHECK-DAG: dr3 = [[VAR64RW]]
+# CHECK-DAG: dr6 = 0x{{()?}}0ff0
+# CHECK-DAG: dr7 = 0x{{()?}}fd7104aa
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/Inputs/x86-db-read.cpp
===
--- /dev/null
+++ lldb/test/Shell/Register/Inputs/x86-db-read.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+
+uint8_t g_8w;
+uint16_t g_16rw;
+uint32_t g_32w;
+uint32_t g_32rw;
+
+int main() {
+  ::raise(SIGSTOP);
+  return 0;
+}


Index: lldb/test/Shell/Register/x86-db-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-db-read.test
@@ -0,0 +1,35 @@
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64) && dbregs-set
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{[0-9]+}} stopped
+
+watchpoint set variable -w write g_8w
+# CHECK: Watchpoint created: Watchpoint 1: addr = 0x{{[0-9a-f]*}} size = 1 state = enabled type = w
+watchpoint set variable -w read_write g_16rw
+# CHECK: Watchpoint created: Watchpoint 2: addr = 0x{{[0-9a-f]*}} size = 2 state = enabled type = rw
+watchpoint set variable -w write g_32w
+# CHECK: Watchpoint created: Watchpoint 3: addr = 0x{{[0-9a-f]*}} size = 4 state = enabled type = w
+watchpoint set variable -w read_write g_32rw
+# CHECK: Watchpoint created: Watchpoint 4: addr = 0x{{[0-9a-f]*}} size = 4 state = enabled type = rw
+
+print &g_8w
+# CHECK: (uint8_t *) $0 = [[VAR8W:0x[0-9a-f]*]]
+print &g_16rw
+# CHECK: (uint16_t *) $1 = [[VAR16RW:0x[0-9a-f]*]]
+print &g_32w
+# CHECK: (uint32_t *) $2 = [[VAR32W:0x[0-9a-f]*]]
+print &g_32rw
+# CHECK: (uint32_t *) $3 = [[VAR64RW:0x[0-9a-f]*]]
+
+register read --all
+# CHECK-DAG: dr0 = [[VAR8W]]
+# CHECK-DAG: dr1 = [[VAR16RW]]
+# CHECK-DAG: dr2 = [[VAR32W]]
+# CHECK-DAG: dr3 = [[VAR64RW]]
+# CHECK-DAG: dr6 = 0x{{()?}}0ff0
+# CHECK-DAG: dr7 = 0x{{()?}}fd7104aa
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/Inputs/x86-db-read.cpp
===
--- /dev/null
+++ lldb/test/Shell/Register/Inputs/x86-db-read.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+
+uint8_t g_8w;
+uint16_t g_16rw;
+uint32_t g_32w;
+uint32_t g_32rw;
+
+int main() {
+  ::raise(SIGSTOP);
+  return 0;
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

2020-11-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h:83
   size_t GetDBROffset() const;
+  bool GetYMMSplitReg(uint32_t reg, void **xmm, void **ymm_hi);
 };

mgorny wrote:
> labath wrote:
> > `void *&`
> Are you sure about this? I feel like the whole `&` deal is quite confusing 
> (read: shouldn't have happened). You don't know whether the method modifies 
> the variables passed to it unless you look at the method prototype.
well.. there certainly are code styles which prohibit non-const reference 
arguments. However, llvm is not one of them. Also lldb has a issue with 
compulsive null checks, so I think it's important to communicate the fact the 
two arguments cannot be null, which reference arguments do.

Another possibility would be to just make this a real return value (pair ?). The downside there is that it's not as self-documenting. Pick 
your poison...


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

https://reviews.llvm.org/D91293

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


[Lldb-commits] [PATCH] D91264: [lldb] [test] Add a minimal test for x86 dbreg reading

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: lldb/test/Shell/Register/x86-db-read.test:2
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64) && can-set-dbregs
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > Who sets `can-set-dbregs` ?
> > https://github.com/llvm/llvm-project/blob/master/lldb/test/Shell/lit.cfg.py#L123
> The feature name is `dbregs-set`, is it not ?
Indeed it is. Silly me.


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

https://reviews.llvm.org/D91264

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


[Lldb-commits] [PATCH] D91264: [lldb] [test] Add a minimal test for x86 dbreg reading

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 304772.
mgorny added a comment.

Restrict the test to FreeBSD only.


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

https://reviews.llvm.org/D91264

Files:
  lldb/test/Shell/Register/Inputs/x86-db-read.cpp
  lldb/test/Shell/Register/x86-db-read.test


Index: lldb/test/Shell/Register/x86-db-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-db-read.test
@@ -0,0 +1,36 @@
+# Debug registers are currently printed only on FreeBSD.
+# REQUIRES: system-freebsd
+# REQUIRES: native && (target-x86 || target-x86_64) && dbregs-set
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{[0-9]+}} stopped
+
+watchpoint set variable -w write g_8w
+# CHECK: Watchpoint created: Watchpoint 1: addr = 0x{{[0-9a-f]*}} size = 1 
state = enabled type = w
+watchpoint set variable -w read_write g_16rw
+# CHECK: Watchpoint created: Watchpoint 2: addr = 0x{{[0-9a-f]*}} size = 2 
state = enabled type = rw
+watchpoint set variable -w write g_32w
+# CHECK: Watchpoint created: Watchpoint 3: addr = 0x{{[0-9a-f]*}} size = 4 
state = enabled type = w
+watchpoint set variable -w read_write g_32rw
+# CHECK: Watchpoint created: Watchpoint 4: addr = 0x{{[0-9a-f]*}} size = 4 
state = enabled type = rw
+
+print &g_8w
+# CHECK: (uint8_t *) $0 = [[VAR8W:0x[0-9a-f]*]]
+print &g_16rw
+# CHECK: (uint16_t *) $1 = [[VAR16RW:0x[0-9a-f]*]]
+print &g_32w
+# CHECK: (uint32_t *) $2 = [[VAR32W:0x[0-9a-f]*]]
+print &g_32rw
+# CHECK: (uint32_t *) $3 = [[VAR64RW:0x[0-9a-f]*]]
+
+register read --all
+# CHECK-DAG: dr0 = [[VAR8W]]
+# CHECK-DAG: dr1 = [[VAR16RW]]
+# CHECK-DAG: dr2 = [[VAR32W]]
+# CHECK-DAG: dr3 = [[VAR64RW]]
+# CHECK-DAG: dr6 = 0x{{()?}}0ff0
+# CHECK-DAG: dr7 = 0x{{()?}}fd7104aa
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/Inputs/x86-db-read.cpp
===
--- /dev/null
+++ lldb/test/Shell/Register/Inputs/x86-db-read.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+
+uint8_t g_8w;
+uint16_t g_16rw;
+uint32_t g_32w;
+uint32_t g_32rw;
+
+int main() {
+  ::raise(SIGSTOP);
+  return 0;
+}


Index: lldb/test/Shell/Register/x86-db-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-db-read.test
@@ -0,0 +1,36 @@
+# Debug registers are currently printed only on FreeBSD.
+# REQUIRES: system-freebsd
+# REQUIRES: native && (target-x86 || target-x86_64) && dbregs-set
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{[0-9]+}} stopped
+
+watchpoint set variable -w write g_8w
+# CHECK: Watchpoint created: Watchpoint 1: addr = 0x{{[0-9a-f]*}} size = 1 state = enabled type = w
+watchpoint set variable -w read_write g_16rw
+# CHECK: Watchpoint created: Watchpoint 2: addr = 0x{{[0-9a-f]*}} size = 2 state = enabled type = rw
+watchpoint set variable -w write g_32w
+# CHECK: Watchpoint created: Watchpoint 3: addr = 0x{{[0-9a-f]*}} size = 4 state = enabled type = w
+watchpoint set variable -w read_write g_32rw
+# CHECK: Watchpoint created: Watchpoint 4: addr = 0x{{[0-9a-f]*}} size = 4 state = enabled type = rw
+
+print &g_8w
+# CHECK: (uint8_t *) $0 = [[VAR8W:0x[0-9a-f]*]]
+print &g_16rw
+# CHECK: (uint16_t *) $1 = [[VAR16RW:0x[0-9a-f]*]]
+print &g_32w
+# CHECK: (uint32_t *) $2 = [[VAR32W:0x[0-9a-f]*]]
+print &g_32rw
+# CHECK: (uint32_t *) $3 = [[VAR64RW:0x[0-9a-f]*]]
+
+register read --all
+# CHECK-DAG: dr0 = [[VAR8W]]
+# CHECK-DAG: dr1 = [[VAR16RW]]
+# CHECK-DAG: dr2 = [[VAR32W]]
+# CHECK-DAG: dr3 = [[VAR64RW]]
+# CHECK-DAG: dr6 = 0x{{()?}}0ff0
+# CHECK-DAG: dr7 = 0x{{()?}}fd7104aa
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/Inputs/x86-db-read.cpp
===
--- /dev/null
+++ lldb/test/Shell/Register/Inputs/x86-db-read.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+
+uint8_t g_8w;
+uint16_t g_16rw;
+uint32_t g_32w;
+uint32_t g_32rw;
+
+int main() {
+  ::raise(SIGSTOP);
+  return 0;
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] da121ff - [lldb] Introduce a LLDB printing policy for Clang type names that suppressed inline namespaces

2020-11-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-11-12T14:00:33+01:00
New Revision: da121fff1184267a405f81a87f7314df2d474e1c

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

LOG: [lldb] Introduce a LLDB printing policy for Clang type names that 
suppressed inline namespaces

Commit 5f12f4ff9078455cad9d4806da01f570553a5bf9 made suppressing inline 
namespaces
when printing typenames default to true. As we're using the inline namespaces
in LLDB to construct internal type names (which need internal namespaces in them
to, for example, differentiate libc++'s std::__1::string from the std::string
from libstdc++), this broke most of the type formatting logic.

Added: 


Modified: 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 6a5c5cb69ac6..6e27386a233c 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1991,6 +1991,24 @@ TypeSystemClang::GetDeclarationName(llvm::StringRef name,
   return getASTContext().DeclarationNames.getCXXOperatorName(op_kind);
 }
 
+PrintingPolicy TypeSystemClang::GetTypePrintingPolicy() {
+  clang::PrintingPolicy printing_policy(getASTContext().getPrintingPolicy());
+  printing_policy.SuppressTagKeyword = true;
+  // Inline namespaces are important for some type formatters (e.g., libc++
+  // and libstdc++ are 
diff erentiated by their inline namespaces).
+  printing_policy.SuppressInlineNamespace = false;
+  printing_policy.SuppressUnwrittenScope = false;
+  return printing_policy;
+}
+
+std::string TypeSystemClang::GetTypeNameForDecl(const NamedDecl *named_decl) {
+  clang::PrintingPolicy printing_policy = GetTypePrintingPolicy();
+  std::string result;
+  llvm::raw_string_ostream os(result);
+  named_decl->printQualifiedName(os, printing_policy);
+  return result;
+}
+
 FunctionDecl *TypeSystemClang::CreateFunctionDeclaration(
 clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module,
 llvm::StringRef name, const CompilerType &function_clang_type,
@@ -3652,12 +3670,10 @@ ConstString 
TypeSystemClang::GetTypeName(lldb::opaque_compiler_type_t type) {
   // For a typedef just return the qualified name.
   if (const auto *typedef_type = qual_type->getAs()) {
 const clang::TypedefNameDecl *typedef_decl = typedef_type->getDecl();
-return ConstString(typedef_decl->getQualifiedNameAsString());
+return ConstString(GetTypeNameForDecl(typedef_decl));
   }
 
-  clang::PrintingPolicy printing_policy(getASTContext().getPrintingPolicy());
-  printing_policy.SuppressTagKeyword = true;
-  return ConstString(qual_type.getAsString(printing_policy));
+  return ConstString(qual_type.getAsString(GetTypePrintingPolicy()));
 }
 
 ConstString
@@ -3670,6 +3686,7 @@ 
TypeSystemClang::GetDisplayTypeName(lldb::opaque_compiler_type_t type) {
   printing_policy.SuppressTagKeyword = true;
   printing_policy.SuppressScope = false;
   printing_policy.SuppressUnwrittenScope = true;
+  printing_policy.SuppressInlineNamespace = true;
   return ConstString(qual_type.getAsString(printing_policy));
 }
 
@@ -8934,8 +8951,7 @@ void 
TypeSystemClang::DumpTypeDescription(lldb::opaque_compiler_type_t type,
   if (level == eDescriptionLevelVerbose)
 typedef_decl->dump(llvm_ostrm);
   else {
-std::string clang_typedef_name(
-typedef_decl->getQualifiedNameAsString());
+std::string clang_typedef_name(GetTypeNameForDecl(typedef_decl));
 if (!clang_typedef_name.empty()) {
   s->PutCString("typedef ");
   s->PutCString(clang_typedef_name);
@@ -9412,8 +9428,7 @@ TypeSystemClang::DeclContextGetScopeQualifiedName(void 
*opaque_decl_ctx) {
 clang::NamedDecl *named_decl =
 llvm::dyn_cast((clang::DeclContext 
*)opaque_decl_ctx);
 if (named_decl)
-  return ConstString(
-  llvm::StringRef(named_decl->getQualifiedNameAsString()));
+  return ConstString(GetTypeNameForDecl(named_decl));
   }
   return ConstString();
 }

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 061e6cb8eb16..5eb492f0dbc2 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1063,6 +1063,13 @@ class TypeSystemClang : public TypeSystem {
   }
 
 private:
+  /// Returns the PrintingPolicy used when generating the internal type names.
+  /// These type names are 

[Lldb-commits] [lldb] 1115d1d - Revert "Generalize regex matching std::string variants to compensate for recent"

2020-11-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-11-12T14:01:22+01:00
New Revision: 1115d1d08302e246789b3c3915e65f3147888e47

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

LOG: Revert "Generalize regex matching std::string variants to compensate for 
recent"

This reverts commit 856fd98a176240470dcc2b8ad54b5c17ef6a75b3. The type 
formatters
use inline namespaces to find the formatter that fits the type ABI, so they
can't just ignore the inline namespaces.

The failing tests should be fixed by da121fff1184267a405f81a87f7314df2d474e1c .

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 7bc5b194514a..83a140ebd5e7 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -461,52 +461,52 @@ static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP 
cpp_category_sp) {
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxStringSummaryProviderASCII,
 "std::string summary provider",
-ConstString("^std::(__[[:alnum:]]+::)?string$"), 
stl_summary_flags,
+ConstString("^std::__[[:alnum:]]+::string$"), 
stl_summary_flags,
 true);
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxStringSummaryProviderASCII,
 "std::string summary provider",
-ConstString("^std::(__[[:alnum:]]+::)?basic_string, "
-"std::(__[[:alnum:]]+::)?allocator )?>$"),
+ConstString("^std::__[[:alnum:]]+::basic_string, "
+"std::__[[:alnum:]]+::allocator >$"),
 stl_summary_flags, true);
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxStringSummaryProviderASCII,
 "std::string summary provider",
-ConstString("^std::(__[[:alnum:]]+::)?basic_string, "
-"std::(__[[:alnum:]]+::)?allocator 
)?>$"),
+ConstString("^std::__[[:alnum:]]+::basic_string, "
+"std::__[[:alnum:]]+::allocator 
>$"),
 stl_summary_flags, true);
 
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxStringSummaryProviderUTF16,
 "std::u16string summary provider",
 ConstString(
-"^std::(__[[:alnum:]]+::)?basic_string, "
-"std::(__[[:alnum:]]+::)?allocator )?>$"),
+"^std::__[[:alnum:]]+::basic_string, "
+"std::__[[:alnum:]]+::allocator >$"),
 stl_summary_flags, true);
 
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxStringSummaryProviderUTF32,
 "std::u32string summary provider",
 ConstString(
-"^std::(__[[:alnum:]]+::)?basic_string, "
-"std::(__[[:alnum:]]+::)?allocator )?>$"),
+"^std::__[[:alnum:]]+::basic_string, "
+"std::__[[:alnum:]]+::allocator >$"),
 stl_summary_flags, true);
 
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxWStringSummaryProvider,
 "std::wstring summary provider",
-ConstString("^std::(__[[:alnum:]]+::)?wstring$"),
+ConstString("^std::__[[:alnum:]]+::wstring$"),
 stl_summary_flags, true);
   AddCXXSummary(cpp_category_sp,
 lldb_private::formatters::LibcxxWStringSummaryProvider,
 "std::wstring summary provider",
-ConstString("^std::(__[[:alnum:]]+::)?basic_string, "
-"std::(__[[:alnum:]]+::)?allocator )?>$"),
+ConstString("^std::__[[:alnum:]]+::basic_string, "
+"std::__[[:alnum:]]+::allocator >$"),
 stl_summary_flags, true);
 
   SyntheticChildren::Flags stl_synth_flags;
@@ -787,10 +787,6 @@ static void 
LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   ConstString("std::__cxx11::basic_string, "
   "std::allocator >"),
   cxx11_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::basic_string, "
-  "std::allocator >"),
-  cxx11_string_summary_sp);
   cpp_category_sp->GetTypeSummariesContainer()->Add(
   ConstString("std::__cxx11::basic_string, "
   "std::allocator >"),



__

[Lldb-commits] [lldb] f37834c - [lldb] [test] Add a minimal test for x86 dbreg reading

2020-11-12 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-12T14:09:03+01:00
New Revision: f37834c7dcbe69405bf3e182d2b3e3227cc4a569

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

LOG: [lldb] [test] Add a minimal test for x86 dbreg reading

Add a test verifying that after the 'watchpoint' command, new values
of x86 debug registers can be read back correctly.  The primary purpose
of this test is to catch broken DRn reading and help debugging it.

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

Added: 
lldb/test/Shell/Register/Inputs/x86-db-read.cpp
lldb/test/Shell/Register/x86-db-read.test

Modified: 


Removed: 




diff  --git a/lldb/test/Shell/Register/Inputs/x86-db-read.cpp 
b/lldb/test/Shell/Register/Inputs/x86-db-read.cpp
new file mode 100644
index ..7aa957e10db8
--- /dev/null
+++ b/lldb/test/Shell/Register/Inputs/x86-db-read.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+
+uint8_t g_8w;
+uint16_t g_16rw;
+uint32_t g_32w;
+uint32_t g_32rw;
+
+int main() {
+  ::raise(SIGSTOP);
+  return 0;
+}

diff  --git a/lldb/test/Shell/Register/x86-db-read.test 
b/lldb/test/Shell/Register/x86-db-read.test
new file mode 100644
index ..f8380440c627
--- /dev/null
+++ b/lldb/test/Shell/Register/x86-db-read.test
@@ -0,0 +1,36 @@
+# Debug registers are currently printed only on FreeBSD.
+# REQUIRES: system-freebsd
+# REQUIRES: native && (target-x86 || target-x86_64) && dbregs-set
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{[0-9]+}} stopped
+
+watchpoint set variable -w write g_8w
+# CHECK: Watchpoint created: Watchpoint 1: addr = 0x{{[0-9a-f]*}} size = 1 
state = enabled type = w
+watchpoint set variable -w read_write g_16rw
+# CHECK: Watchpoint created: Watchpoint 2: addr = 0x{{[0-9a-f]*}} size = 2 
state = enabled type = rw
+watchpoint set variable -w write g_32w
+# CHECK: Watchpoint created: Watchpoint 3: addr = 0x{{[0-9a-f]*}} size = 4 
state = enabled type = w
+watchpoint set variable -w read_write g_32rw
+# CHECK: Watchpoint created: Watchpoint 4: addr = 0x{{[0-9a-f]*}} size = 4 
state = enabled type = rw
+
+print &g_8w
+# CHECK: (uint8_t *) $0 = [[VAR8W:0x[0-9a-f]*]]
+print &g_16rw
+# CHECK: (uint16_t *) $1 = [[VAR16RW:0x[0-9a-f]*]]
+print &g_32w
+# CHECK: (uint32_t *) $2 = [[VAR32W:0x[0-9a-f]*]]
+print &g_32rw
+# CHECK: (uint32_t *) $3 = [[VAR64RW:0x[0-9a-f]*]]
+
+register read --all
+# CHECK-DAG: dr0 = [[VAR8W]]
+# CHECK-DAG: dr1 = [[VAR16RW]]
+# CHECK-DAG: dr2 = [[VAR32W]]
+# CHECK-DAG: dr3 = [[VAR64RW]]
+# CHECK-DAG: dr6 = 0x{{()?}}0ff0
+# CHECK-DAG: dr7 = 0x{{()?}}fd7104aa
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0



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


[Lldb-commits] [lldb] a8bfee2 - [lldb] [Process/Utility] Fix DR offsets for FreeBSD

2020-11-12 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-12T14:09:03+01:00
New Revision: a8bfee2a356a2e70f854bf6d0b364798f4183795

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

LOG: [lldb] [Process/Utility] Fix DR offsets for FreeBSD

Fix Debug Register offsets to be specified relatively to UserArea
on FreeBSD/amd64 and FreeBSD/i386, and add them to UserArea on i386.
This fixes overlapping GPRs and DRs in gdb-remote protocol, making it
impossible to correctly get and set debug registers from the LLDB
client.

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
index 10d346a3cb7e..acebe9d53568 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
@@ -35,7 +35,7 @@ struct GPR {
   uint32_t gs;
 };
 
-struct dbreg {
+struct DBG {
   uint32_t dr[8]; /* debug registers */
   /* Index 0-3: debug address registers */
   /* Index 4-5: reserved */
@@ -48,10 +48,13 @@ using FPR_i386 = FXSAVE;
 struct UserArea {
   GPR gpr;
   FPR_i386 i387;
+  DBG dbg;
 };
 
 #define DR_SIZE sizeof(uint32_t)
-#define DR_OFFSET(reg_index) (LLVM_EXTENSION offsetof(dbreg, dr[reg_index]))
+#define DR_OFFSET(reg_index)   
\
+  (LLVM_EXTENSION offsetof(UserArea, dbg) +
\
+   LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
 
 // Include RegisterInfos_i386 to declare our g_register_infos_i386 structure.
 #define DECLARE_REGISTER_INFOS_I386_STRUCT

diff  --git 
a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
index c1f390ade9b9..e0f3971c6e27 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
@@ -59,7 +59,9 @@ struct UserArea {
   DBG dbg;
 };
 
-#define DR_OFFSET(reg_index) (LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
+#define DR_OFFSET(reg_index)   
\
+  (LLVM_EXTENSION offsetof(UserArea, dbg) +
\
+   LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
 
 // Include RegisterInfos_x86_64 to declare our g_register_infos_x86_64
 // structure.



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


[Lldb-commits] [PATCH] D91264: [lldb] [test] Add a minimal test for x86 dbreg reading

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf37834c7dcbe: [lldb] [test] Add a minimal test for x86 dbreg 
reading (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91264

Files:
  lldb/test/Shell/Register/Inputs/x86-db-read.cpp
  lldb/test/Shell/Register/x86-db-read.test


Index: lldb/test/Shell/Register/x86-db-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-db-read.test
@@ -0,0 +1,36 @@
+# Debug registers are currently printed only on FreeBSD.
+# REQUIRES: system-freebsd
+# REQUIRES: native && (target-x86 || target-x86_64) && dbregs-set
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{[0-9]+}} stopped
+
+watchpoint set variable -w write g_8w
+# CHECK: Watchpoint created: Watchpoint 1: addr = 0x{{[0-9a-f]*}} size = 1 
state = enabled type = w
+watchpoint set variable -w read_write g_16rw
+# CHECK: Watchpoint created: Watchpoint 2: addr = 0x{{[0-9a-f]*}} size = 2 
state = enabled type = rw
+watchpoint set variable -w write g_32w
+# CHECK: Watchpoint created: Watchpoint 3: addr = 0x{{[0-9a-f]*}} size = 4 
state = enabled type = w
+watchpoint set variable -w read_write g_32rw
+# CHECK: Watchpoint created: Watchpoint 4: addr = 0x{{[0-9a-f]*}} size = 4 
state = enabled type = rw
+
+print &g_8w
+# CHECK: (uint8_t *) $0 = [[VAR8W:0x[0-9a-f]*]]
+print &g_16rw
+# CHECK: (uint16_t *) $1 = [[VAR16RW:0x[0-9a-f]*]]
+print &g_32w
+# CHECK: (uint32_t *) $2 = [[VAR32W:0x[0-9a-f]*]]
+print &g_32rw
+# CHECK: (uint32_t *) $3 = [[VAR64RW:0x[0-9a-f]*]]
+
+register read --all
+# CHECK-DAG: dr0 = [[VAR8W]]
+# CHECK-DAG: dr1 = [[VAR16RW]]
+# CHECK-DAG: dr2 = [[VAR32W]]
+# CHECK-DAG: dr3 = [[VAR64RW]]
+# CHECK-DAG: dr6 = 0x{{()?}}0ff0
+# CHECK-DAG: dr7 = 0x{{()?}}fd7104aa
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/Inputs/x86-db-read.cpp
===
--- /dev/null
+++ lldb/test/Shell/Register/Inputs/x86-db-read.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+
+uint8_t g_8w;
+uint16_t g_16rw;
+uint32_t g_32w;
+uint32_t g_32rw;
+
+int main() {
+  ::raise(SIGSTOP);
+  return 0;
+}


Index: lldb/test/Shell/Register/x86-db-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-db-read.test
@@ -0,0 +1,36 @@
+# Debug registers are currently printed only on FreeBSD.
+# REQUIRES: system-freebsd
+# REQUIRES: native && (target-x86 || target-x86_64) && dbregs-set
+# RUN: %clangxx_host -g %p/Inputs/x86-db-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{[0-9]+}} stopped
+
+watchpoint set variable -w write g_8w
+# CHECK: Watchpoint created: Watchpoint 1: addr = 0x{{[0-9a-f]*}} size = 1 state = enabled type = w
+watchpoint set variable -w read_write g_16rw
+# CHECK: Watchpoint created: Watchpoint 2: addr = 0x{{[0-9a-f]*}} size = 2 state = enabled type = rw
+watchpoint set variable -w write g_32w
+# CHECK: Watchpoint created: Watchpoint 3: addr = 0x{{[0-9a-f]*}} size = 4 state = enabled type = w
+watchpoint set variable -w read_write g_32rw
+# CHECK: Watchpoint created: Watchpoint 4: addr = 0x{{[0-9a-f]*}} size = 4 state = enabled type = rw
+
+print &g_8w
+# CHECK: (uint8_t *) $0 = [[VAR8W:0x[0-9a-f]*]]
+print &g_16rw
+# CHECK: (uint16_t *) $1 = [[VAR16RW:0x[0-9a-f]*]]
+print &g_32w
+# CHECK: (uint32_t *) $2 = [[VAR32W:0x[0-9a-f]*]]
+print &g_32rw
+# CHECK: (uint32_t *) $3 = [[VAR64RW:0x[0-9a-f]*]]
+
+register read --all
+# CHECK-DAG: dr0 = [[VAR8W]]
+# CHECK-DAG: dr1 = [[VAR16RW]]
+# CHECK-DAG: dr2 = [[VAR32W]]
+# CHECK-DAG: dr3 = [[VAR64RW]]
+# CHECK-DAG: dr6 = 0x{{()?}}0ff0
+# CHECK-DAG: dr7 = 0x{{()?}}fd7104aa
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Register/Inputs/x86-db-read.cpp
===
--- /dev/null
+++ lldb/test/Shell/Register/Inputs/x86-db-read.cpp
@@ -0,0 +1,12 @@
+#include 
+#include 
+
+uint8_t g_8w;
+uint16_t g_16rw;
+uint32_t g_32w;
+uint32_t g_32rw;
+
+int main() {
+  ::raise(SIGSTOP);
+  return 0;
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D91254: [lldb] [Process/Utility] Fix DR offsets for FreeBSD

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa8bfee2a356a: [lldb] [Process/Utility] Fix DR offsets for 
FreeBSD (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91254

Files:
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp


Index: lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
@@ -59,7 +59,9 @@
   DBG dbg;
 };
 
-#define DR_OFFSET(reg_index) (LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
+#define DR_OFFSET(reg_index)   
\
+  (LLVM_EXTENSION offsetof(UserArea, dbg) +
\
+   LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
 
 // Include RegisterInfos_x86_64 to declare our g_register_infos_x86_64
 // structure.
Index: lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
@@ -35,7 +35,7 @@
   uint32_t gs;
 };
 
-struct dbreg {
+struct DBG {
   uint32_t dr[8]; /* debug registers */
   /* Index 0-3: debug address registers */
   /* Index 4-5: reserved */
@@ -48,10 +48,13 @@
 struct UserArea {
   GPR gpr;
   FPR_i386 i387;
+  DBG dbg;
 };
 
 #define DR_SIZE sizeof(uint32_t)
-#define DR_OFFSET(reg_index) (LLVM_EXTENSION offsetof(dbreg, dr[reg_index]))
+#define DR_OFFSET(reg_index)   
\
+  (LLVM_EXTENSION offsetof(UserArea, dbg) +
\
+   LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
 
 // Include RegisterInfos_i386 to declare our g_register_infos_i386 structure.
 #define DECLARE_REGISTER_INFOS_I386_STRUCT


Index: lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
@@ -59,7 +59,9 @@
   DBG dbg;
 };
 
-#define DR_OFFSET(reg_index) (LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
+#define DR_OFFSET(reg_index)   \
+  (LLVM_EXTENSION offsetof(UserArea, dbg) +\
+   LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
 
 // Include RegisterInfos_x86_64 to declare our g_register_infos_x86_64
 // structure.
Index: lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_i386.cpp
@@ -35,7 +35,7 @@
   uint32_t gs;
 };
 
-struct dbreg {
+struct DBG {
   uint32_t dr[8]; /* debug registers */
   /* Index 0-3: debug address registers */
   /* Index 4-5: reserved */
@@ -48,10 +48,13 @@
 struct UserArea {
   GPR gpr;
   FPR_i386 i387;
+  DBG dbg;
 };
 
 #define DR_SIZE sizeof(uint32_t)
-#define DR_OFFSET(reg_index) (LLVM_EXTENSION offsetof(dbreg, dr[reg_index]))
+#define DR_OFFSET(reg_index)   \
+  (LLVM_EXTENSION offsetof(UserArea, dbg) +\
+   LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
 
 // Include RegisterInfos_i386 to declare our g_register_infos_i386 structure.
 #define DECLARE_REGISTER_INFOS_I386_STRUCT
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

2020-11-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

This looks pretty straightforward, modulo the inline comment about the 
centralization of the offset computations




Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:534
 
+  // On AArch64 architecture we set offsets on client side in accordance with
+  // GDB g/G packet standard in sequence of increasing register numbers.

This part seems dubious. It would be better if it were not tied to the 
architecture, but to the fact that the stub did not provide the register offset 
for these registers. It would also be better if it was placed together with the 
rest of the code for computing the register offsets.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:571
+} else
+  reg_offset += reg_info.byte_size;
+

Which, I guess, means here.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4438
+} else
+  reg_offset += reg_info.byte_size;
+

And here (feel free to factor this into some kind of a utility function).


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

https://reviews.llvm.org/D91241

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


[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

2020-11-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/test/API/linux/aarch64/mte_memory_region/main.c:9-14
+  if (!(getauxval(AT_HWCAP2) & HWCAP2_MTE))
+return 1;
+
+  int got = prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0);
+  if (got)
+return 1;

DavidSpickett wrote:
> labath wrote:
> > Instead of duplicating these checks in dotest, maybe we could use the 
> > result of the inferior as a indication to skip the test. Like, if, instead 
> > of hitting the breakpoint, the inferior exits with code 47, we know that 
> > the required cpu feature is not supported?
> Sounds good to me. 
> 
> That would mean defining things like PROT_MTE in the test file, for 
> toolchains that won't have it. I assume that's ok to do.
> (I'll probably need to do that for lldb-server code later anyway)
Depends... How likely is the system to support memory tagging if the relevant 
headers don't define the constants? Do you want to support systems like those?

Maybe you could do something like this:
```
int main() {
#ifdef HWCAP2_MTE
  // do stuff
#else
  return 47;
#endif
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

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


[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

2020-11-12 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment.

In D91241#2391072 , @labath wrote:

> This looks pretty straightforward, modulo the inline comment about the 
> centralization of the offset computations

So the current approach of finalizing offsets of pseudo registers after 
receiving/processing all registers is because in case of SVE we define FPU 
register set first containing V, D, S  pseudo registers first and then define 
SVE registers set where their primary register Z is defined.

We have choice of either documenting that all pseudo register should come after 
their corresponding primary register are defined and make this change to SVE 
register definitions as well where we are not enforcing this principle mainly 
due to consistency with non-SVE vs SVE register sets..


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

https://reviews.llvm.org/D91241

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


[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

2020-11-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

That's a good point. Maybe this does need to be a two-pass algorithm (first 
compute the offsets of primary registers, then fill out subregs). But that 
doesn't mean the two passes should be in two completely separate files. It 
would still be better if that was done in a single place. Maybe we can move all 
the offset computation out of the parsing loop and into a separate helper 
function. Doing the computation inside the loop is not really correct anyway, 
as the registers in the target.xml don't have to come in ascending regnum order 
(all stubs probably send them that way, but I don't think they _have_ to do 
that).


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

https://reviews.llvm.org/D91241

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


[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

2020-11-12 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment.

In D91241#2391126 , @labath wrote:

> That's a good point. Maybe this does need to be a two-pass algorithm (first 
> compute the offsets of primary registers, then fill out subregs). But that 
> doesn't mean the two passes should be in two completely separate files. It 
> would still be better if that was done in a single place. Maybe we can move 
> all the offset computation out of the parsing loop and into a separate helper 
> function. Doing the computation inside the loop is not really correct anyway, 
> as the registers in the target.xml don't have to come in ascending regnum 
> order (all stubs probably send them that way, but I don't think they _have_ 
> to do that).

I guess GDB standard does enforce ascending order, here is what it says about 
regnum:

"The register’s number. If omitted, a register’s number is one greater than 
that of the previous register (either in the current feature or in a preceding 
feature); the first register in the target description defaults to zero. This 
register number is used to read or write the register; e.g. it is used in the 
remote p and P packets, and registers appear in the g and G packets in order of 
increasing register number."

https://sourceware.org/gdb/current/onlinedocs/gdb/Target-Description-Format.html


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

https://reviews.llvm.org/D91241

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


[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp:26-31
+#define ASSERT_REG(regname, offset, size)  
\
+  {
\
+const RegisterInfo *reg_info = ®_ctx.GetRegisterInfo()[lldb_##regname]; 
\
+EXPECT_EQ(reg_info->byte_offset, offset);  
\
+EXPECT_EQ(reg_info->byte_size, size);  
\
+  }

labath wrote:
> ```
> pair GetRegParams(const RegisterContext &ctx, unsigned 
> reg) {
>   const RegisterInfo &info = reg_ctx.GetRegisterInfo()[reg];
>   return {info.byte_offset, info.byte_size};
> }
> ```
Fun fact: `RegisterContextFreeBSD_*` is not a `RegisterContext&` but a 
`RegisterInfoInterface&` ;-).


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

https://reviews.llvm.org/D91216

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


[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 304813.
mgorny added a comment.

Implemented all the requested changes.


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

https://reviews.llvm.org/D91216

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp

Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp
@@ -0,0 +1,99 @@
+//===-- RegisterContextFreeBSDTests.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
+//
+//===--===//
+
+// clang-format off
+#include 
+#include 
+// clang-format on
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "Plugins/Process/Utility/RegisterContextFreeBSD_i386.h"
+#include "Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+std::pair GetRegParams(RegisterInfoInterface &ctx, uint32_t reg) {
+const RegisterInfo &info = ctx.GetRegisterInfo()[reg];
+return {info.byte_offset, info.byte_size};
+}
+
+#if defined(__x86_64__)
+
+#define ASSERT_GPR_X86_64(regname) \
+  EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname##_x86_64), \
+  ::testing::Pair(offsetof(reg, r_##regname), sizeof(reg::r_##regname)))
+
+TEST(RegisterContextFreeBSDTest, x86_64) {
+  ArchSpec arch{"x86_64-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_x86_64 reg_ctx{arch};
+
+  ASSERT_GPR_X86_64(r15);
+  ASSERT_GPR_X86_64(r14);
+  ASSERT_GPR_X86_64(r13);
+  ASSERT_GPR_X86_64(r12);
+  ASSERT_GPR_X86_64(r11);
+  ASSERT_GPR_X86_64(r10);
+  ASSERT_GPR_X86_64(r9);
+  ASSERT_GPR_X86_64(r8);
+  ASSERT_GPR_X86_64(rdi);
+  ASSERT_GPR_X86_64(rsi);
+  ASSERT_GPR_X86_64(rbp);
+  ASSERT_GPR_X86_64(rbx);
+  ASSERT_GPR_X86_64(rdx);
+  ASSERT_GPR_X86_64(rcx);
+  ASSERT_GPR_X86_64(rax);
+  ASSERT_GPR_X86_64(fs);
+  ASSERT_GPR_X86_64(gs);
+  ASSERT_GPR_X86_64(es);
+  ASSERT_GPR_X86_64(ds);
+  ASSERT_GPR_X86_64(rip);
+  ASSERT_GPR_X86_64(cs);
+  ASSERT_GPR_X86_64(rflags);
+  ASSERT_GPR_X86_64(rsp);
+  ASSERT_GPR_X86_64(ss);
+}
+#endif
+
+#if defined(__i386__) || defined(__x86_64__)
+
+#define ASSERT_GPR_I386(regname)   \
+  EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname##_i386), \
+  ::testing::Pair(offsetof(native_i386_regs, r_##regname), sizeof(native_i386_regs::r_##regname)))
+
+TEST(RegisterContextFreeBSDTest, i386) {
+  ArchSpec arch{"i686-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_i386 reg_ctx{arch};
+
+#if defined(__i386__)
+  using native_i386_regs = ::reg;
+#else
+  using native_i386_regs = ::reg32;
+#endif
+
+  ASSERT_GPR_I386(fs);
+  ASSERT_GPR_I386(es);
+  ASSERT_GPR_I386(ds);
+  ASSERT_GPR_I386(edi);
+  ASSERT_GPR_I386(esi);
+  ASSERT_GPR_I386(ebp);
+  ASSERT_GPR_I386(ebx);
+  ASSERT_GPR_I386(edx);
+  ASSERT_GPR_I386(ecx);
+  ASSERT_GPR_I386(eax);
+  ASSERT_GPR_I386(eip);
+  ASSERT_GPR_I386(cs);
+  ASSERT_GPR_I386(eflags);
+  ASSERT_GPR_I386(esp);
+  ASSERT_GPR_I386(ss);
+  ASSERT_GPR_I386(gs);
+}
+#endif
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -0,0 +1,8 @@
+if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
+  add_lldb_unittest(RegisterContextFreeBSDTests
+RegisterContextFreeBSDTests.cpp
+
+LINK_LIBS
+  lldbPluginProcessUtility
+)
+endif()
Index: lldb/unittests/Process/CMakeLists.txt
===
--- lldb/unittests/Process/CMakeLists.txt
+++ lldb/unittests/Process/CMakeLists.txt
@@ -3,6 +3,7 @@
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
 endif()
+add_subdirectory(Utility)
 add_subdirectory(minidump)
 
 add_lldb_unittest(ProcessEventDataTests
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -18,6 +18,8 @@
 #include 
 // clang-format on
 
+#include 
+
 #include "Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h"
 #include "Plugins/Process/Utility/RegisterContext_x86.h"
 #include "Plugins/Process/

[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

2020-11-12 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments.



Comment at: lldb/test/API/linux/aarch64/mte_memory_region/main.c:9-14
+  if (!(getauxval(AT_HWCAP2) & HWCAP2_MTE))
+return 1;
+
+  int got = prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0);
+  if (got)
+return 1;

labath wrote:
> DavidSpickett wrote:
> > labath wrote:
> > > Instead of duplicating these checks in dotest, maybe we could use the 
> > > result of the inferior as a indication to skip the test. Like, if, 
> > > instead of hitting the breakpoint, the inferior exits with code 47, we 
> > > know that the required cpu feature is not supported?
> > Sounds good to me. 
> > 
> > That would mean defining things like PROT_MTE in the test file, for 
> > toolchains that won't have it. I assume that's ok to do.
> > (I'll probably need to do that for lldb-server code later anyway)
> Depends... How likely is the system to support memory tagging if the relevant 
> headers don't define the constants? Do you want to support systems like those?
> 
> Maybe you could do something like this:
> ```
> int main() {
> #ifdef HWCAP2_MTE
>   // do stuff
> #else
>   return 47;
> #endif
> }
> ```
No I don't think so, if you want to run this test the toolchain should have the 
constants. I'll do what you suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

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


[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

2020-11-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D91241#2391156 , @omjavaid wrote:

> I guess GDB standard does enforce ascending order, here is what it says about 
> regnum:
>
> "The register’s number. If omitted, a register’s number is one greater than 
> that of the previous register (either in the current feature or in a 
> preceding feature); the first register in the target description defaults to 
> zero. This register number is used to read or write the register; e.g. it is 
> used in the remote p and P packets, and registers appear in the g and G 
> packets in order of increasing register number."
>
> https://sourceware.org/gdb/current/onlinedocs/gdb/Target-Description-Format.html

I've just read that paragraph before writing that, but that's not how I would 
interpret it. What I think that says is:

- in the `g` packet, registers appear in the increasing register number order. 
(I think we agree on that part)
- **if the register number is omitted**, the registers are assigned increasing 
register numbers. Here, I think the bold part is very important, as it means 
the rest of the sentence describes default/fallback behavior. If the stub 
specifies the register number explicitly, then I'd say it's free to send order 
the register descriptions any way it likes. It can even combine things and set 
explicit numbers for some  register, but not others...


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

https://reviews.llvm.org/D91241

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


[Lldb-commits] [lldb] b4b8365 - [lldb][NFC] Move OptionDefinition from lldb-private-types.h to its own Utility header

2020-11-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-11-12T15:30:26+01:00
New Revision: b4b836563ae3603b601b57d8992f2d5fe60f02f8

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

LOG: [lldb][NFC] Move OptionDefinition from lldb-private-types.h to its own 
Utility header

Also moves the curious isprint8 function (which was used to check whether we 
have a
valid short option) into the struct and documents it.

Added: 
lldb/include/lldb/Utility/OptionDefinition.h

Modified: 
lldb/include/lldb/Interpreter/Options.h
lldb/include/lldb/lldb-private-types.h
lldb/source/Host/common/OptionParser.cpp
lldb/source/Interpreter/Options.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/Options.h 
b/lldb/include/lldb/Interpreter/Options.h
index ebceaea8383d..9738cce2f7a1 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -14,6 +14,7 @@
 
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/CompletionRequest.h"
+#include "lldb/Utility/OptionDefinition.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-private.h"
@@ -40,12 +41,6 @@ struct OptionArgElement {
 
 typedef std::vector OptionElementVector;
 
-static inline bool isprint8(int ch) {
-  if (ch & 0xff00u)
-return false;
-  return llvm::isPrint(ch);
-}
-
 /// \class Options Options.h "lldb/Interpreter/Options.h"
 /// A command line option parsing protocol class.
 ///

diff  --git a/lldb/include/lldb/Utility/OptionDefinition.h 
b/lldb/include/lldb/Utility/OptionDefinition.h
new file mode 100644
index ..725e0904f159
--- /dev/null
+++ b/lldb/include/lldb/Utility/OptionDefinition.h
@@ -0,0 +1,55 @@
+//===-- OptionDefinition.h --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_UTILITY_OPTIONDEFINITION_H
+#define LLDB_UTILITY_OPTIONDEFINITION_H
+
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private-types.h"
+#include "llvm/ADT/StringExtras.h"
+#include 
+
+namespace lldb_private {
+struct OptionDefinition {
+  /// Used to mark options that can be used together.  If
+  /// `(1 << n & usage_mask) != 0` then this option belongs to option set n.
+  uint32_t usage_mask;
+  /// This option is required (in the current usage level).
+  bool required;
+  /// Full name for this option.
+  const char *long_option;
+  /// Single character for this option. If the option doesn't use a short
+  /// option character, this has to be a integer value that is not a printable
+  /// ASCII code point and also unique in the used set of options.
+  /// @see OptionDefinition::HasShortOption
+  int short_option;
+  /// no_argument, required_argument or optional_argument
+  int option_has_arg;
+  /// If non-NULL, option is valid iff |validator->IsValid()|, otherwise
+  /// always valid.
+  OptionValidator *validator;
+  /// If not empty, an array of enum values.
+  OptionEnumValues enum_values;
+  /// The kind of completion for this option.
+  /// Contains values of the CommandCompletions::CommonCompletionTypes enum.
+  uint32_t completion_type;
+  /// Type of argument this option takes.
+  lldb::CommandArgumentType argument_type;
+  /// Full text explaining what this options does and what (if any) argument to
+  /// pass it.
+  const char *usage_text;
+
+  /// Whether this has a short option character.
+  bool HasShortOption() const {
+// See the short_option documentation for more.
+return llvm::isPrint(short_option);
+  }
+};
+} // namespace lldb_private
+
+#endif // LLDB_UTILITY_OPTIONDEFINITION_H

diff  --git a/lldb/include/lldb/lldb-private-types.h 
b/lldb/include/lldb/lldb-private-types.h
index fb8c2db2e21c..c7e652650da7 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -107,33 +107,6 @@ struct OptionValidator {
   virtual const char *LongConditionString() const = 0;
 };
 
-struct OptionDefinition {
-  /// Used to mark options that can be used together.  If
-  /// `(1 << n & usage_mask) != 0` then this option belongs to option set n.
-  uint32_t usage_mask;
-  /// This option is required (in the current usage level).
-  bool required;
-  /// Full name for this option.
-  const char *long_option;
-  /// Single character for this option.
-  int short_option;
-  /// no_argument, required_argument or optional_argument
-  int option_has_arg;
-  /// If non-NULL, option is valid iff |validator->IsValid()|, otherwise
-  /// always valid.
-  OptionValidator *validator;
-  /// If 

[Lldb-commits] [PATCH] D90450: [lldb] Add expect_var_path to test variable path results

2020-11-12 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd85cc03c9c4c: [lldb] Add expect_var_path to test variable 
path results (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90450

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  
lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py
  lldb/test/API/functionalities/archives/TestBSDArchives.py
  lldb/test/API/lang/cpp/wchar_t/TestCxxWCharT.py

Index: lldb/test/API/lang/cpp/wchar_t/TestCxxWCharT.py
===
--- lldb/test/API/lang/cpp/wchar_t/TestCxxWCharT.py
+++ lldb/test/API/lang/cpp/wchar_t/TestCxxWCharT.py
@@ -20,20 +20,16 @@
 lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp"))
 
 # Check that we correctly report templates on wchar_t
-self.expect("frame variable foo_y",
-substrs=['(Foo) foo_y = '])
+self.expect_var_path("foo_y", type="Foo")
 
 # Check that we correctly report templates on int
-self.expect("frame variable foo_x",
-substrs=['(Foo) foo_x = '])
+self.expect_var_path("foo_x", type="Foo")
 
 # Check that we correctly report wchar_t
-self.expect("frame variable foo_y.object",
-substrs=['(wchar_t) foo_y.object = '])
+self.expect_var_path("foo_y.object", type="wchar_t")
 
 # Check that we correctly report int
-self.expect("frame variable foo_x.object",
-substrs=['(int) foo_x.object = '])
+self.expect_var_path("foo_x.object", type="int")
 
 # Check that we can run expressions that return wchar_t
 self.expect("expression L'a'", substrs=['(wchar_t) $', "L'a'"])
Index: lldb/test/API/functionalities/archives/TestBSDArchives.py
===
--- lldb/test/API/functionalities/archives/TestBSDArchives.py
+++ lldb/test/API/functionalities/archives/TestBSDArchives.py
@@ -43,8 +43,7 @@
 # Break at a(int) first.
 self.expect("frame variable", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=['(int) arg = 1'])
-self.expect("frame variable __a_global", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=['(int) __a_global = 1'])
+self.expect_var_path("__a_global", type="int", value="1")
 
 # Set breakpoint for b() next.
 lldbutil.run_break_set_by_symbol(
@@ -57,5 +56,4 @@
  'stop reason = breakpoint'])
 self.expect("frame variable", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=['(int) arg = 2'])
-self.expect("frame variable __b_global", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=['(int) __b_global = 2'])
+self.expect_var_path("__b_global", type="int", value="2")
Index: lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py
===
--- lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py
+++ lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py
@@ -60,21 +60,14 @@
 stop_reason == lldb.eStopReasonBreakpoint,
 "Thread in process stopped in 'main' should have a stop reason of eStopReasonBreakpoint")
 
-self.expect("frame variable argv[1]", substrs=['file1.txt'])
-self.expect("frame variable argv[2]", substrs=['file2.txt'])
-self.expect("frame variable argv[3]", substrs=['file3.txt'])
-self.expect("frame variable argv[4]", substrs=['file4.txy'])
-self.expect("frame variable argv[5]", substrs=['()'])
-self.expect("frame variable argv[6]", substrs=['>'])
-self.expect("frame variable argv[7]", substrs=['<'])
-self.expect(
-"frame variable argv[5]",
-substrs=['file5.tyx'],
-matching=False)
-self.expect(
-"frame variable argv[8]",
-substrs=['file5.tyx'],
-matching=False)
+self.expect_var_path("argv[1]", summary='"file1.txt"')
+self.expect_var_path("argv[2]", summary='"file2.txt"')
+self.expect_var_path("argv[3]", summary='"file3.txt"')
+self.expect_var_path("argv[4]", summary='"file4.txy"')
+self.expect_var_path("argv[5]", summary='"()"')
+self.expect_var_path("argv[6]", summary='">"')
+self.expect_var_path("argv[7]", summary='"<"')
+self.expect_var_path("argc", value='8')
 
 self.runCmd("process kill")
 
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Py

[Lldb-commits] [lldb] d85cc03 - [lldb] Add expect_var_path to test variable path results

2020-11-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-11-12T16:14:48+01:00
New Revision: d85cc03c9c4cc44c0281320558abc440575ae1d4

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

LOG: [lldb] Add expect_var_path to test variable path results

This adds `expect_var_path` to test variable paths so we no longer have to
use `frame var` and find substrs in the command output. The behaviour
is identical with `expect_expr` (and it also uses the same checking backend),
but it instead calls `GetValueForVariablePath` to evaluate the string as a 
variable
path.

Also rewrites a few of the tests that previously used `frame variable` to use
`expect_var_path`.

Reviewed By: labath

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py

lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py
lldb/test/API/functionalities/archives/TestBSDArchives.py
lldb/test/API/lang/cpp/wchar_t/TestCxxWCharT.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 89c25eb55516..a02c445a937a 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2592,6 +2592,35 @@ def expect_expr(
 value_check.check_value(self, eval_result, str(eval_result))
 return eval_result
 
+def expect_var_path(
+self,
+var_path,
+summary=None,
+value=None,
+type=None,
+children=None
+):
+"""
+Evaluates the given variable path and verifies the result.
+See also 'frame variable' and SBFrame.GetValueForVariablePath.
+:param var_path: The variable path as a string.
+:param summary: The summary that the variable should have. None if the 
summary should not be checked.
+:param value: The value that the variable should have. None if the 
value should not be checked.
+:param type: The type that the variable result should have. None if 
the type should not be checked.
+:param children: The expected children of the variable  as a list of 
ValueChecks.
+ None if the children shouldn't be checked.
+"""
+self.assertTrue(var_path.strip() == var_path,
+"Expression contains trailing/leading whitespace: '" + 
var_path + "'")
+
+frame = self.frame()
+eval_result = frame.GetValueForVariablePath(var_path)
+
+value_check = ValueCheck(type=type, value=value,
+ summary=summary, children=children)
+value_check.check_value(self, eval_result, str(eval_result))
+return eval_result
+
 def invoke(self, obj, name, trace=False):
 """Use reflection to call a method dynamically with no argument."""
 trace = (True if traceAlways else trace)

diff  --git 
a/lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py
 
b/lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py
index 8602f7f56b15..9ade3dba12c5 100644
--- 
a/lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py
+++ 
b/lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py
@@ -60,21 +60,14 @@ def test(self):
 stop_reason == lldb.eStopReasonBreakpoint,
 "Thread in process stopped in 'main' should have a stop reason of 
eStopReasonBreakpoint")
 
-self.expect("frame variable argv[1]", substrs=['file1.txt'])
-self.expect("frame variable argv[2]", substrs=['file2.txt'])
-self.expect("frame variable argv[3]", substrs=['file3.txt'])
-self.expect("frame variable argv[4]", substrs=['file4.txy'])
-self.expect("frame variable argv[5]", substrs=['()'])
-self.expect("frame variable argv[6]", substrs=['>'])
-self.expect("frame variable argv[7]", substrs=['<'])
-self.expect(
-"frame variable argv[5]",
-substrs=['file5.tyx'],
-matching=False)
-self.expect(
-"frame variable argv[8]",
-substrs=['file5.tyx'],
-matching=False)
+self.expect_var_path("argv[1]", summary='"file1.txt"')
+self.expect_var_path("argv[2]", summary='"file2.txt"')
+self.expect_var_path("argv[3]", summary='"file3.txt"')
+self.expect_var_path("argv[4]", summary='"file4.txy"')
+self.expect_var_path("argv[5]", summary='"()"')
+self.expect_var_path("argv[6]", summary='">"')
+self.expect_var_path("argv[7]", summary='"<"')
+self.expect_var_path("argc", value='8')
 
 se

[Lldb-commits] [PATCH] D91118: Fix handling of bit-fields in a union

2020-11-12 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


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

https://reviews.llvm.org/D91118

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


[Lldb-commits] [lldb] d4b08cc - [lldb] Replace TestAbortExitCode with a debugserver specific test

2020-11-12 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-11-12T17:33:21+01:00
New Revision: d4b08ccb87944ec6c647f02b536a40922ec2dd73

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

LOG: [lldb] Replace TestAbortExitCode with a debugserver specific test

When I added TestAbortExitCode I actually planned this to be a generic test for 
the
exit code functionality on POSIX systems. However due to all the different test 
setups we
can have I don't think this worked out. Right now the test had to be made so 
permissive
that it pretty much can't fail.

Just to summarize, we would need to support the following situations:
1. ToT debugserver (on macOS)
2. lldb-server (on other platforms)
3. Any old debugserver version when using the system debugserver (on macOS)

This patch is removing TestAbortExitCode and adds a ToT debugserver specific 
test
that checks the patch that motivated the whole exit code testing. There is 
already
an exit-code test for lldb-server from what I can see and 3) is pretty much 
untestable
as we don't know anything about the system debugserver.

Reviewed By: kastiglione

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

Added: 
lldb/test/API/macosx/debugserver-exit-code/Makefile
lldb/test/API/macosx/debugserver-exit-code/TestDebugServerExitCode.py
lldb/test/API/macosx/debugserver-exit-code/main.c

Modified: 


Removed: 
lldb/test/Shell/Process/Inputs/abort.c
lldb/test/Shell/Process/TestAbortExitCode.test



diff  --git a/lldb/test/API/macosx/debugserver-exit-code/Makefile 
b/lldb/test/API/macosx/debugserver-exit-code/Makefile
new file mode 100644
index ..10495940055b
--- /dev/null
+++ b/lldb/test/API/macosx/debugserver-exit-code/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/macosx/debugserver-exit-code/TestDebugServerExitCode.py 
b/lldb/test/API/macosx/debugserver-exit-code/TestDebugServerExitCode.py
new file mode 100644
index ..d80c2dd4eba9
--- /dev/null
+++ b/lldb/test/API/macosx/debugserver-exit-code/TestDebugServerExitCode.py
@@ -0,0 +1,27 @@
+"""
+Tests the exit code/description coming from the debugserver.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+@skipUnlessDarwin
+@skipIfOutOfTreeDebugserver
+def test_abort(self):
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+process = target.LaunchSimple(None, None, None)
+# Continue until process is terminated.
+process.Continue()
+# Test for the abort signal code.
+self.assertEqual(process.GetExitStatus(), 6)
+# Test for the exit code description.
+self.assertEqual(process.GetExitDescription(),
+ "Terminated due to signal 6")

diff  --git a/lldb/test/Shell/Process/Inputs/abort.c 
b/lldb/test/API/macosx/debugserver-exit-code/main.c
similarity index 100%
rename from lldb/test/Shell/Process/Inputs/abort.c
rename to lldb/test/API/macosx/debugserver-exit-code/main.c

diff  --git a/lldb/test/Shell/Process/TestAbortExitCode.test 
b/lldb/test/Shell/Process/TestAbortExitCode.test
deleted file mode 100644
index 746bc915897e..
--- a/lldb/test/Shell/Process/TestAbortExitCode.test
+++ /dev/null
@@ -1,6 +0,0 @@
-UNSUPPORTED: system-windows
-
-RUN: %clang_host %p/Inputs/abort.c -o %t
-RUN: %lldb %t -o run -o continue | FileCheck %s
-
-CHECK: {{status = 6 \(0x0006\)|status = 0 \(0x\) Terminated due to 
signal 6}}



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


[Lldb-commits] [PATCH] D89305: [lldb] Replace TestAbortExitCode with a debugserver specific test

2020-11-12 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4b08ccb8794: [lldb] Replace TestAbortExitCode with a 
debugserver specific test (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89305

Files:
  lldb/test/API/macosx/debugserver-exit-code/Makefile
  lldb/test/API/macosx/debugserver-exit-code/TestDebugServerExitCode.py
  lldb/test/API/macosx/debugserver-exit-code/main.c
  lldb/test/Shell/Process/Inputs/abort.c
  lldb/test/Shell/Process/TestAbortExitCode.test


Index: lldb/test/Shell/Process/TestAbortExitCode.test
===
--- lldb/test/Shell/Process/TestAbortExitCode.test
+++ /dev/null
@@ -1,6 +0,0 @@
-UNSUPPORTED: system-windows
-
-RUN: %clang_host %p/Inputs/abort.c -o %t
-RUN: %lldb %t -o run -o continue | FileCheck %s
-
-CHECK: {{status = 6 \(0x0006\)|status = 0 \(0x\) Terminated due to 
signal 6}}
Index: lldb/test/API/macosx/debugserver-exit-code/TestDebugServerExitCode.py
===
--- /dev/null
+++ lldb/test/API/macosx/debugserver-exit-code/TestDebugServerExitCode.py
@@ -0,0 +1,27 @@
+"""
+Tests the exit code/description coming from the debugserver.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+@skipUnlessDarwin
+@skipIfOutOfTreeDebugserver
+def test_abort(self):
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+process = target.LaunchSimple(None, None, None)
+# Continue until process is terminated.
+process.Continue()
+# Test for the abort signal code.
+self.assertEqual(process.GetExitStatus(), 6)
+# Test for the exit code description.
+self.assertEqual(process.GetExitDescription(),
+ "Terminated due to signal 6")
Index: lldb/test/API/macosx/debugserver-exit-code/Makefile
===
--- /dev/null
+++ lldb/test/API/macosx/debugserver-exit-code/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules


Index: lldb/test/Shell/Process/TestAbortExitCode.test
===
--- lldb/test/Shell/Process/TestAbortExitCode.test
+++ /dev/null
@@ -1,6 +0,0 @@
-UNSUPPORTED: system-windows
-
-RUN: %clang_host %p/Inputs/abort.c -o %t
-RUN: %lldb %t -o run -o continue | FileCheck %s
-
-CHECK: {{status = 6 \(0x0006\)|status = 0 \(0x\) Terminated due to signal 6}}
Index: lldb/test/API/macosx/debugserver-exit-code/TestDebugServerExitCode.py
===
--- /dev/null
+++ lldb/test/API/macosx/debugserver-exit-code/TestDebugServerExitCode.py
@@ -0,0 +1,27 @@
+"""
+Tests the exit code/description coming from the debugserver.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+@skipUnlessDarwin
+@skipIfOutOfTreeDebugserver
+def test_abort(self):
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+process = target.LaunchSimple(None, None, None)
+# Continue until process is terminated.
+process.Continue()
+# Test for the abort signal code.
+self.assertEqual(process.GetExitStatus(), 6)
+# Test for the exit code description.
+self.assertEqual(process.GetExitDescription(),
+ "Terminated due to signal 6")
Index: lldb/test/API/macosx/debugserver-exit-code/Makefile
===
--- /dev/null
+++ lldb/test/API/macosx/debugserver-exit-code/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 304859.
mgorny added a comment.

clang-format


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

https://reviews.llvm.org/D91216

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp

Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp
@@ -0,0 +1,102 @@
+//===-- RegisterContextFreeBSDTests.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
+//
+//===--===//
+
+// clang-format off
+#include 
+#include 
+// clang-format on
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "Plugins/Process/Utility/RegisterContextFreeBSD_i386.h"
+#include "Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+std::pair GetRegParams(RegisterInfoInterface &ctx,
+   uint32_t reg) {
+  const RegisterInfo &info = ctx.GetRegisterInfo()[reg];
+  return {info.byte_offset, info.byte_size};
+}
+
+#if defined(__x86_64__)
+
+#define ASSERT_GPR_X86_64(regname) \
+  EXPECT_THAT( \
+  GetRegParams(reg_ctx, lldb_##regname##_x86_64),  \
+  ::testing::Pair(offsetof(reg, r_##regname), sizeof(reg::r_##regname)))
+
+TEST(RegisterContextFreeBSDTest, x86_64) {
+  ArchSpec arch{"x86_64-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_x86_64 reg_ctx{arch};
+
+  ASSERT_GPR_X86_64(r15);
+  ASSERT_GPR_X86_64(r14);
+  ASSERT_GPR_X86_64(r13);
+  ASSERT_GPR_X86_64(r12);
+  ASSERT_GPR_X86_64(r11);
+  ASSERT_GPR_X86_64(r10);
+  ASSERT_GPR_X86_64(r9);
+  ASSERT_GPR_X86_64(r8);
+  ASSERT_GPR_X86_64(rdi);
+  ASSERT_GPR_X86_64(rsi);
+  ASSERT_GPR_X86_64(rbp);
+  ASSERT_GPR_X86_64(rbx);
+  ASSERT_GPR_X86_64(rdx);
+  ASSERT_GPR_X86_64(rcx);
+  ASSERT_GPR_X86_64(rax);
+  ASSERT_GPR_X86_64(fs);
+  ASSERT_GPR_X86_64(gs);
+  ASSERT_GPR_X86_64(es);
+  ASSERT_GPR_X86_64(ds);
+  ASSERT_GPR_X86_64(rip);
+  ASSERT_GPR_X86_64(cs);
+  ASSERT_GPR_X86_64(rflags);
+  ASSERT_GPR_X86_64(rsp);
+  ASSERT_GPR_X86_64(ss);
+}
+#endif
+
+#if defined(__i386__) || defined(__x86_64__)
+
+#define ASSERT_GPR_I386(regname)   \
+  EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname##_i386),\
+  ::testing::Pair(offsetof(native_i386_regs, r_##regname), \
+  sizeof(native_i386_regs::r_##regname)))
+
+TEST(RegisterContextFreeBSDTest, i386) {
+  ArchSpec arch{"i686-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_i386 reg_ctx{arch};
+
+#if defined(__i386__)
+  using native_i386_regs = ::reg;
+#else
+  using native_i386_regs = ::reg32;
+#endif
+
+  ASSERT_GPR_I386(fs);
+  ASSERT_GPR_I386(es);
+  ASSERT_GPR_I386(ds);
+  ASSERT_GPR_I386(edi);
+  ASSERT_GPR_I386(esi);
+  ASSERT_GPR_I386(ebp);
+  ASSERT_GPR_I386(ebx);
+  ASSERT_GPR_I386(edx);
+  ASSERT_GPR_I386(ecx);
+  ASSERT_GPR_I386(eax);
+  ASSERT_GPR_I386(eip);
+  ASSERT_GPR_I386(cs);
+  ASSERT_GPR_I386(eflags);
+  ASSERT_GPR_I386(esp);
+  ASSERT_GPR_I386(ss);
+  ASSERT_GPR_I386(gs);
+}
+#endif
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -0,0 +1,8 @@
+if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
+  add_lldb_unittest(RegisterContextFreeBSDTests
+RegisterContextFreeBSDTests.cpp
+
+LINK_LIBS
+  lldbPluginProcessUtility
+)
+endif()
Index: lldb/unittests/Process/CMakeLists.txt
===
--- lldb/unittests/Process/CMakeLists.txt
+++ lldb/unittests/Process/CMakeLists.txt
@@ -3,6 +3,7 @@
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
 endif()
+add_subdirectory(Utility)
 add_subdirectory(minidump)
 
 add_lldb_unittest(ProcessEventDataTests
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -18,6 +18,8 @

[Lldb-commits] [PATCH] D91248: [lldb] [Process/FreeBSDRemote] Access FPR via RegisterInfo offsets

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 304861.
mgorny added a comment.

Update to match changes in GPR patch.


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

https://reviews.llvm.org/D91248

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp

Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp
@@ -27,6 +27,10 @@
   return {info.byte_offset, info.byte_size};
 }
 
+#define ASSERT_OFF(regname, offset, size)  \
+  EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname),   \
+  ::testing::Pair(offset + base_offset, size))
+
 #if defined(__x86_64__)
 
 #define ASSERT_GPR_X86_64(regname) \
@@ -62,6 +66,57 @@
   ASSERT_GPR_X86_64(rflags);
   ASSERT_GPR_X86_64(rsp);
   ASSERT_GPR_X86_64(ss);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t base_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_x86_64].byte_offset;
+
+  // assert against FXSAVE struct
+  ASSERT_OFF(fctrl_x86_64, 0x00, 2);
+  ASSERT_OFF(fstat_x86_64, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  ASSERT_OFF(ftag_x86_64, 0x04, 2);
+  ASSERT_OFF(fop_x86_64, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
+  ASSERT_OFF(fioff_x86_64, 0x08, 4);
+  ASSERT_OFF(fiseg_x86_64, 0x0C, 4);
+  ASSERT_OFF(fooff_x86_64, 0x10, 4);
+  ASSERT_OFF(foseg_x86_64, 0x14, 4);
+  ASSERT_OFF(mxcsr_x86_64, 0x18, 4);
+  ASSERT_OFF(mxcsrmask_x86_64, 0x1C, 4);
+  ASSERT_OFF(st0_x86_64, 0x20, 10);
+  ASSERT_OFF(st1_x86_64, 0x30, 10);
+  ASSERT_OFF(st2_x86_64, 0x40, 10);
+  ASSERT_OFF(st3_x86_64, 0x50, 10);
+  ASSERT_OFF(st4_x86_64, 0x60, 10);
+  ASSERT_OFF(st5_x86_64, 0x70, 10);
+  ASSERT_OFF(st6_x86_64, 0x80, 10);
+  ASSERT_OFF(st7_x86_64, 0x90, 10);
+  ASSERT_OFF(mm0_x86_64, 0x20, 8);
+  ASSERT_OFF(mm1_x86_64, 0x30, 8);
+  ASSERT_OFF(mm2_x86_64, 0x40, 8);
+  ASSERT_OFF(mm3_x86_64, 0x50, 8);
+  ASSERT_OFF(mm4_x86_64, 0x60, 8);
+  ASSERT_OFF(mm5_x86_64, 0x70, 8);
+  ASSERT_OFF(mm6_x86_64, 0x80, 8);
+  ASSERT_OFF(mm7_x86_64, 0x90, 8);
+  ASSERT_OFF(xmm0_x86_64, 0xA0, 16);
+  ASSERT_OFF(xmm1_x86_64, 0xB0, 16);
+  ASSERT_OFF(xmm2_x86_64, 0xC0, 16);
+  ASSERT_OFF(xmm3_x86_64, 0xD0, 16);
+  ASSERT_OFF(xmm4_x86_64, 0xE0, 16);
+  ASSERT_OFF(xmm5_x86_64, 0xF0, 16);
+  ASSERT_OFF(xmm6_x86_64, 0x100, 16);
+  ASSERT_OFF(xmm7_x86_64, 0x110, 16);
+  ASSERT_OFF(xmm8_x86_64, 0x120, 16);
+  ASSERT_OFF(xmm9_x86_64, 0x130, 16);
+  ASSERT_OFF(xmm10_x86_64, 0x140, 16);
+  ASSERT_OFF(xmm11_x86_64, 0x150, 16);
+  ASSERT_OFF(xmm12_x86_64, 0x160, 16);
+  ASSERT_OFF(xmm13_x86_64, 0x170, 16);
+  ASSERT_OFF(xmm14_x86_64, 0x180, 16);
+  ASSERT_OFF(xmm15_x86_64, 0x190, 16);
 }
 #endif
 
@@ -98,5 +153,48 @@
   ASSERT_GPR_I386(esp);
   ASSERT_GPR_I386(ss);
   ASSERT_GPR_I386(gs);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t base_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_i386].byte_offset;
+
+  // assert against FXSAVE struct
+  ASSERT_OFF(fctrl_i386, 0x00, 2);
+  ASSERT_OFF(fstat_i386, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  ASSERT_OFF(ftag_i386, 0x04, 2);
+  ASSERT_OFF(fop_i386, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
+  ASSERT_OFF(fioff_i386, 0x08, 4);
+  ASSERT_OFF(fiseg_i386, 0x0C, 4);
+  ASSERT_OFF(fooff_i386, 0x10, 4);
+  ASSERT_OFF(foseg_i386, 0x14, 4);
+  ASSERT_OFF(mxcsr_i386, 0x18, 4);
+  ASSERT_OFF(mxcsrmask_i386, 0x1C, 4);
+  ASSERT_OFF(st0_i386, 0x20, 10);
+  ASSERT_OFF(st1_i386, 0x30, 10);
+  ASSERT_OFF(st2_i386, 0x40, 10);
+  ASSERT_OFF(st3_i386, 0x50, 10);
+  ASSERT_OFF(st4_i386, 0x60, 10);
+  ASSERT_OFF(st5_i386, 0x70, 10);
+  ASSERT_OFF(st6_i386, 0x80, 10);
+  ASSERT_OFF(st7_i386, 0x90, 10);
+  ASSERT_OFF(mm0_i386, 0x20, 8);
+  ASSERT_OFF(mm1_i386, 0x30, 8);
+  ASSERT_OFF(mm2_i386, 0x40, 8);
+  ASSERT_OFF(mm3_i386, 0x50, 8);
+  ASSERT_OFF(mm4_i386, 0x60, 8);
+  ASSERT_OFF(mm5_i386, 0x70, 8);
+  ASSERT_OFF(mm6_i386, 0x80, 8);
+  ASSERT_OFF(mm7_i386, 0x90, 8);
+  ASSERT_OFF(xmm0_i386, 0xA0, 16);
+  ASSERT_OFF(xmm1_i386, 0xB0, 16);
+  ASSERT_OFF(xmm2_i386, 0xC0, 16);
+  ASSERT_OFF(xmm3_i386, 0xD0, 16);
+  ASSERT_OFF(xmm4_i386, 0xE0, 16);
+  ASSERT_OFF(xmm5_i386, 0xF0, 16);
+  ASSERT_OFF(xmm6_i386, 0x100, 16);
+  ASSERT_OFF(xmm7_i386

[Lldb-commits] [PATCH] D91268: [lldb] [Process/FreeBSDRemote] Access debug registers via offsets

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 304863.
mgorny added a comment.

Update following the changes in GPR patch.


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

https://reviews.llvm.org/D91268

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp

Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp
@@ -37,6 +37,8 @@
   EXPECT_THAT( \
   GetRegParams(reg_ctx, lldb_##regname##_x86_64),  \
   ::testing::Pair(offsetof(reg, r_##regname), sizeof(reg::r_##regname)))
+#define ASSERT_DBR_X86_64(num) \
+  ASSERT_OFF(dr##num##_x86_64, offsetof(dbreg, dr[num]), sizeof(dbreg::dr[num]))
 
 TEST(RegisterContextFreeBSDTest, x86_64) {
   ArchSpec arch{"x86_64-unknown-freebsd12.2"};
@@ -117,6 +119,16 @@
   ASSERT_OFF(xmm13_x86_64, 0x170, 16);
   ASSERT_OFF(xmm14_x86_64, 0x180, 16);
   ASSERT_OFF(xmm15_x86_64, 0x190, 16);
+
+  base_offset = reg_ctx.GetRegisterInfo()[lldb_dr0_x86_64].byte_offset;
+  ASSERT_DBR_X86_64(0);
+  ASSERT_DBR_X86_64(1);
+  ASSERT_DBR_X86_64(2);
+  ASSERT_DBR_X86_64(3);
+  ASSERT_DBR_X86_64(4);
+  ASSERT_DBR_X86_64(5);
+  ASSERT_DBR_X86_64(6);
+  ASSERT_DBR_X86_64(7);
 }
 #endif
 
@@ -126,6 +138,9 @@
   EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname##_i386),\
   ::testing::Pair(offsetof(native_i386_regs, r_##regname), \
   sizeof(native_i386_regs::r_##regname)))
+#define ASSERT_DBR_I386(num)   \
+  ASSERT_OFF(dr##num##_i386, offsetof(dbreg32, dr[num]),   \
+ sizeof(dbreg32::dr[num]))
 
 TEST(RegisterContextFreeBSDTest, i386) {
   ArchSpec arch{"i686-unknown-freebsd12.2"};
@@ -196,5 +211,15 @@
   ASSERT_OFF(xmm5_i386, 0xF0, 16);
   ASSERT_OFF(xmm6_i386, 0x100, 16);
   ASSERT_OFF(xmm7_i386, 0x110, 16);
+
+  base_offset = reg_ctx.GetRegisterInfo()[lldb_dr0_i386].byte_offset;
+  ASSERT_DBR_I386(0);
+  ASSERT_DBR_I386(1);
+  ASSERT_DBR_I386(2);
+  ASSERT_DBR_I386(3);
+  ASSERT_DBR_I386(4);
+  ASSERT_DBR_I386(5);
+  ASSERT_DBR_I386(6);
+  ASSERT_DBR_I386(7);
 }
 #endif
Index: lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
===
--- lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
+++ lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
@@ -40,7 +40,6 @@
 
   uint32_t NumSupportedHardwareWatchpoints() override;
 
-private:
   const RegisterInfo *GetDR(int num) const;
 };
 
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -71,17 +71,17 @@
   // Private member variables.
   std::array m_gpr;
   std::array m_fpr; // FXSAVE
-  struct dbreg m_dbr;
+  std::array m_dbr;
   std::vector m_xsave;
   std::array m_xsave_offsets;
 
   llvm::Optional GetSetForNativeRegNum(int reg_num) const;
-  int GetDR(int num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 
   size_t GetFPROffset() const;
+  size_t GetDBROffset() const;
 };
 
 } // namespace process_freebsd
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -247,8 +247,7 @@
 NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextRegisterInfo(
-  native_thread, CreateRegisterInfoInterface(target_arch)),
-  m_dbr() {
+  native_thread, CreateRegisterInfoInterface(target_arch)) {
   assert(m_gpr.size() == GetRegisterInfoInterface().GetGPRSize());
 }
 
@@ -296,15 +295,6 @@
 return lldb_bndcfgu_x86_64;
   case lldb_bndstatus_i386:
 return lldb_bndstatus_x86_64;
-  case lldb_dr0_i386:
-  case lldb_dr1_i386:
-  case lldb_dr2_i386:
-  case lldb_dr3_i386:
-  case lldb_dr4_i386:
-  case lldb_dr5_i386:
-  case lldb_dr6_i386:
-  cas

[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 304865.
mgorny added a comment.

Changed prototype to use pointer referenec.


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

https://reviews.llvm.org/D91293

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -58,15 +58,13 @@
 private:
   // Private member types.
   enum RegSetKind {
+YMMRegSet,
+MaxXSaveSet = YMMRegSet,
+
 GPRegSet,
 FPRegSet,
-XSaveRegSet,
 DBRegSet,
   };
-  enum {
-YMMXSaveSet,
-MaxXSaveSet = YMMXSaveSet,
-  };
 
   // Private member variables.
   std::array m_gpr;
@@ -82,6 +80,7 @@
 
   size_t GetFPROffset() const;
   size_t GetDBROffset() const;
+  bool GetYMMSplitReg(uint32_t reg, void *&xmm, void *&ymm_hi);
 };
 
 } // namespace process_freebsd
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -275,31 +275,6 @@
   }
 }
 
-static constexpr int RegNumX86ToX86_64(int regnum) {
-  switch (regnum) {
-  case lldb_ymm0_i386:
-  case lldb_ymm1_i386:
-  case lldb_ymm2_i386:
-  case lldb_ymm3_i386:
-  case lldb_ymm4_i386:
-  case lldb_ymm5_i386:
-  case lldb_ymm6_i386:
-  case lldb_ymm7_i386:
-return lldb_ymm0_x86_64 + regnum - lldb_ymm0_i386;
-  case lldb_bnd0_i386:
-  case lldb_bnd1_i386:
-  case lldb_bnd2_i386:
-  case lldb_bnd3_i386:
-return lldb_bnd0_x86_64 + regnum - lldb_bnd0_i386;
-  case lldb_bndcfgu_i386:
-return lldb_bndcfgu_x86_64;
-  case lldb_bndstatus_i386:
-return lldb_bndstatus_x86_64;
-  default:
-llvm_unreachable("Unhandled i386 register.");
-  }
-}
-
 llvm::Optional
 NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(int reg_num) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
@@ -309,7 +284,7 @@
 if (reg_num >= k_first_fpr_i386 && reg_num <= k_last_fpr_i386)
   return FPRegSet;
 if (reg_num >= k_first_avx_i386 && reg_num <= k_last_avx_i386)
-  return XSaveRegSet; // AVX
+  return YMMRegSet;
 if (reg_num >= k_first_mpxr_i386 && reg_num <= k_last_mpxr_i386)
   return llvm::None; // MPXR
 if (reg_num >= k_first_mpxc_i386 && reg_num <= k_last_mpxc_i386)
@@ -323,7 +298,7 @@
 if (reg_num >= k_first_fpr_x86_64 && reg_num <= k_last_fpr_x86_64)
   return FPRegSet;
 if (reg_num >= k_first_avx_x86_64 && reg_num <= k_last_avx_x86_64)
-  return XSaveRegSet; // AVX
+  return YMMRegSet;
 if (reg_num >= k_first_mpxr_x86_64 && reg_num <= k_last_mpxr_x86_64)
   return llvm::None; // MPXR
 if (reg_num >= k_first_mpxc_x86_64 && reg_num <= k_last_mpxc_x86_64)
@@ -354,7 +329,7 @@
   case DBRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_GETDBREGS, m_thread.GetID(),
m_dbr.data());
-  case XSaveRegSet: {
+  case YMMRegSet: {
 struct ptrace_xstate_info info;
 Status ret = NativeProcessFreeBSD::PtraceWrapper(
 PT_GETXSTATE_INFO, GetProcessPid(), &info, sizeof(info));
@@ -364,10 +339,10 @@
 assert(info.xsave_mask & XFEATURE_ENABLED_X87);
 assert(info.xsave_mask & XFEATURE_ENABLED_SSE);
 
-m_xsave_offsets[YMMXSaveSet] = LLDB_INVALID_XSAVE_OFFSET;
+m_xsave_offsets[YMMRegSet] = LLDB_INVALID_XSAVE_OFFSET;
 if (info.xsave_mask & XFEATURE_ENABLED_YMM_HI128) {
   uint32_t eax, ecx, edx;
-  __get_cpuid_count(0x0D, 2, &eax, &m_xsave_offsets[YMMXSaveSet], &ecx,
+  __get_cpuid_count(0x0D, 2, &eax, &m_xsave_offsets[YMMRegSet], &ecx,
 &edx);
 }
 
@@ -395,7 +370,7 @@
   case DBRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_SETDBREGS, m_thread.GetID(),
m_dbr.data());
-  case XSaveRegSet:
+  case YMMRegSet:
 // ReadRegisterSet() must always be called before WriteRegisterSet().
 assert(m_xsave.size() > 0);
 return NativeProcessFreeBSD::PtraceWrapper(PT_SETXSTATE, GetProcessPid(),
@@ -442,66 +417,27 @@
   case GPRegSet:
 reg_value.SetBytes(m_gpr.data() + reg_info->byte_offset,
reg_info->byte_size, endian::InlHostByteOrder());
-return error;
+break;
   case FPRegSet:
 reg_value.SetBytes(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
reg_info->byte_size, endian::InlHostByteOrder());
-return error;
+bre

[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands

2020-11-12 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 304867.
wallace added a comment.

improve error messages

The diff is ready for review again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90729

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Interpreter/CommandObjectMultiword.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/ProcessTrace.h
  lldb/include/lldb/Target/RecordedProcess.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectMultiword.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectThreadUtil.cpp
  lldb/source/Commands/CommandObjectThreadUtil.h
  lldb/source/Core/PluginManager.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Target/Process.cpp
  lldb/source/Target/ProcessTrace.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceStartStop.py

Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -0,0 +1,73 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceLoad(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+TestBase.setUp(self)
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The intel-pt test plugin is not enabled")
+
+def expectGenericHelpMessageForStartCommand(self):
+self.expect("help thread trace start",
+substrs=["Syntax: thread trace start []"])
+
+def testStartStopSessionFileThreads(self):
+# it should fail for processes from json session files
+self.expect("trace load -v " + os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"))
+self.expect("thread trace start", error=True,
+substrs=["error: Tracing is not supported. Can't trace a non-live process"])
+
+# the help command should be the generic one, as it's not a live process
+self.expectGenericHelpMessageForStartCommand()
+
+# this should fail because 'trace stop' is not yet implemented
+self.expect("thread trace stop", error=True,
+substrs=["error: Failed stopping thread 3842849"])
+
+@skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+def testStartStopLiveThreads(self):
+# The help command should be the generic one if there's no process running
+self.expectGenericHelpMessageForStartCommand()
+
+self.expect("thread trace start", error=True,
+substrs=["error: Process not available"])
+
+self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+
+self.expect("thread trace start", error=True,
+substrs=["error: Process not available"])
+
+# The help command should be the generic one if there's still no process running
+self.expectGenericHelpMessageForStartCommand()
+
+self.expect("r")
+
+# the help command should be the intel-pt one now
+self.expect("help thread trace start",
+substrs=["Start tracing one or more threads with intel-pt.",
+ "Syntax: thread trace start [  ...] []"])
+
+self.expect("thread trace start",
+patterns=["would trace tid .* with size_in_kb 4 and custom_config 0"])
+
+self.expect("thread trace start --size 20 --custom-config 1",
+patterns=["would trace tid .* with size_in_kb 20 and custom_config 1"])
+
+# This fails because "trace stop" is not yet implemented.
+self.expect("thread trace stop", error=True,
+substrs=["error: Process is not being traced"])
+
+self.expect("c")
+# Now the process has finished, so the commands should fail
+self.expect("thread trace start", error=True,
+substrs=["error: Process must be launched"])
+
+  

[Lldb-commits] [lldb] bae9aed - [LLDB] Fix handling of bit-fields in a union

2020-11-12 Thread via lldb-commits

Author: shafik
Date: 2020-11-12T14:09:27-08:00
New Revision: bae9aedb341c5f4eceafba2ee1fec7c05d842c97

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

LOG: [LLDB] Fix handling of bit-fields in a union

When parsing DWARF and laying out bit-fields we don't properly take into 
account when they are in a union, they will all have a zero offset.

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
lldb/test/API/lang/cpp/bitfields/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 2003a24c04fa..6633acd70a50 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2581,7 +2581,11 @@ void DWARFASTParserClang::ParseSingleMember(
   // The ObjC runtime knows the byte offset but we still need to 
provide
   // the bit-offset in the layout. It just means something 
diff erent then
   // what it does in C and C++. So we skip this check for ObjC types.
+  //
+  // We also skip this for fields of a union since they will all have a
+  // zero offset.
   if (!TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type) 
&&
+  !(parent_die.Tag() == DW_TAG_union_type && 
this_field_info.bit_offset == 0) &&
   ((this_field_info.bit_offset >= parent_bit_size) ||
(last_field_info.IsBitfield() &&
 !last_field_info.NextBitfieldOffsetIsValid(

diff  --git a/lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py 
b/lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
index 7320ce2b816e..0585cf60951b 100644
--- a/lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
+++ b/lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
@@ -46,6 +46,16 @@ def test_and_run_command(self):
 self.expect("expr (clang_example.f.a)", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=['uint64_t', '1'])
 
+self.expect("expr uwbf",
+substrs=['a = 255',
+'b = 65535',
+'c = 4294967295',
+'x = 4294967295'] )
+
+self.expect("expr uwubf",
+substrs=['a = 16777215',
+'x = 4294967295'] )
+
 self.expect(
 "frame variable --show-types lba",
 VARIABLES_DISPLAYED_CORRECTLY,

diff  --git a/lldb/test/API/lang/cpp/bitfields/main.cpp 
b/lldb/test/API/lang/cpp/bitfields/main.cpp
index 986b7cb947ed..f9015b758c72 100644
--- a/lldb/test/API/lang/cpp/bitfields/main.cpp
+++ b/lldb/test/API/lang/cpp/bitfields/main.cpp
@@ -57,7 +57,7 @@ int main(int argc, char const *argv[]) {
   f.i = 1;
   f.j = 0;
   f.k = 1;
-} 
+}
   } clang_example;
 
   class B {
@@ -70,6 +70,18 @@ int main(int argc, char const *argv[]) {
 uint32_t d_a : 1;
   } derived;
 
+  union union_with_bitfields {
+  unsigned int a : 8;
+  unsigned int b : 16;
+  unsigned int c : 32;
+  unsigned int x;
+  } uwbf;
+
+  union union_with_unnamed_bitfield {
+   unsigned int : 16, a : 24;
+   unsigned int x;
+  } uwubf;
+
   lba.a = 2;
 
   lbb.a = 1;
@@ -89,5 +101,8 @@ int main(int argc, char const *argv[]) {
   derived.b_a = 2;
   derived.d_a = 1;
 
+  uwbf.x = 0x;
+  uwubf.x = 0x;
+
   return 0; // Set break point at this line.
 }



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


[Lldb-commits] [PATCH] D91118: Fix handling of bit-fields in a union

2020-11-12 Thread Shafik Yaghmour via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbae9aedb341c: [LLDB] Fix handling of bit-fields in a union 
(authored by shafik).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91118

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
  lldb/test/API/lang/cpp/bitfields/main.cpp


Index: lldb/test/API/lang/cpp/bitfields/main.cpp
===
--- lldb/test/API/lang/cpp/bitfields/main.cpp
+++ lldb/test/API/lang/cpp/bitfields/main.cpp
@@ -57,7 +57,7 @@
   f.i = 1;
   f.j = 0;
   f.k = 1;
-} 
+}
   } clang_example;
 
   class B {
@@ -70,6 +70,18 @@
 uint32_t d_a : 1;
   } derived;
 
+  union union_with_bitfields {
+  unsigned int a : 8;
+  unsigned int b : 16;
+  unsigned int c : 32;
+  unsigned int x;
+  } uwbf;
+
+  union union_with_unnamed_bitfield {
+   unsigned int : 16, a : 24;
+   unsigned int x;
+  } uwubf;
+
   lba.a = 2;
 
   lbb.a = 1;
@@ -89,5 +101,8 @@
   derived.b_a = 2;
   derived.d_a = 1;
 
+  uwbf.x = 0x;
+  uwubf.x = 0x;
+
   return 0; // Set break point at this line.
 }
Index: lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
===
--- lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
+++ lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
@@ -46,6 +46,16 @@
 self.expect("expr (clang_example.f.a)", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=['uint64_t', '1'])
 
+self.expect("expr uwbf",
+substrs=['a = 255',
+'b = 65535',
+'c = 4294967295',
+'x = 4294967295'] )
+
+self.expect("expr uwubf",
+substrs=['a = 16777215',
+'x = 4294967295'] )
+
 self.expect(
 "frame variable --show-types lba",
 VARIABLES_DISPLAYED_CORRECTLY,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2581,7 +2581,11 @@
   // The ObjC runtime knows the byte offset but we still need to 
provide
   // the bit-offset in the layout. It just means something different 
then
   // what it does in C and C++. So we skip this check for ObjC types.
+  //
+  // We also skip this for fields of a union since they will all have a
+  // zero offset.
   if (!TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type) 
&&
+  !(parent_die.Tag() == DW_TAG_union_type && 
this_field_info.bit_offset == 0) &&
   ((this_field_info.bit_offset >= parent_bit_size) ||
(last_field_info.IsBitfield() &&
 !last_field_info.NextBitfieldOffsetIsValid(


Index: lldb/test/API/lang/cpp/bitfields/main.cpp
===
--- lldb/test/API/lang/cpp/bitfields/main.cpp
+++ lldb/test/API/lang/cpp/bitfields/main.cpp
@@ -57,7 +57,7 @@
   f.i = 1;
   f.j = 0;
   f.k = 1;
-} 
+}
   } clang_example;
 
   class B {
@@ -70,6 +70,18 @@
 uint32_t d_a : 1;
   } derived;
 
+  union union_with_bitfields {
+  unsigned int a : 8;
+  unsigned int b : 16;
+  unsigned int c : 32;
+  unsigned int x;
+  } uwbf;
+
+  union union_with_unnamed_bitfield {
+   unsigned int : 16, a : 24;
+   unsigned int x;
+  } uwubf;
+
   lba.a = 2;
 
   lbb.a = 1;
@@ -89,5 +101,8 @@
   derived.b_a = 2;
   derived.d_a = 1;
 
+  uwbf.x = 0x;
+  uwubf.x = 0x;
+
   return 0; // Set break point at this line.
 }
Index: lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
===
--- lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
+++ lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py
@@ -46,6 +46,16 @@
 self.expect("expr (clang_example.f.a)", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=['uint64_t', '1'])
 
+self.expect("expr uwbf",
+substrs=['a = 255',
+'b = 65535',
+'c = 4294967295',
+'x = 4294967295'] )
+
+self.expect("expr uwubf",
+substrs=['a = 16777215',
+'x = 4294967295'] )
+
 self.expect(
 "frame variable --show-types lba",
 VARIABLES_DISPLAYED_CORRECTLY,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/Sym

[Lldb-commits] [PATCH] D91130: [crashlog] Implement parser for JSON encoded crashlogs

2020-11-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Thanks for adding the extra tests!


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

https://reviews.llvm.org/D91130

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


[Lldb-commits] [lldb] 406ad18 - [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-12 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-11-12T15:24:06-08:00
New Revision: 406ad187486b4277fc82a2c0714ae53396e47928

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

LOG: [lldb/DataFormatters] Display null C++ pointers as nullptr

Display null pointer as `nullptr`, `nil` and `NULL` for C++,
Objective-C/Objective-C++ and C respectively. The original motivation
for this patch was to display a null std::string pointer as nullptr
instead of "", but the fix seemed generic enough to be done for all
summary providers.

Differential revision: https://reviews.llvm.org/D77153

Added: 
lldb/test/API/lang/objcxx/objc-builtin-types/Makefile
lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp

Modified: 
lldb/include/lldb/Target/Language.h
lldb/source/DataFormatters/ValueObjectPrinter.cpp
lldb/source/Expression/UserExpression.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h

lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
lldb/test/API/lang/c/anonymous/TestAnonymous.py

Removed: 
lldb/test/API/lang/objc/objc-builtin-types/Makefile
lldb/test/API/lang/objc/objc-builtin-types/TestObjCBuiltinTypes.py
lldb/test/API/lang/objc/objc-builtin-types/main.cpp



diff  --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 9dc9df363d79..6368828e36da 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -211,6 +211,10 @@ class Language : public PluginInterface {
   // nil/null object, this method returns true
   virtual bool IsNilReference(ValueObject &valobj);
 
+  /// Returns the summary string for ValueObjects for which IsNilReference() is
+  /// true.
+  virtual llvm::StringRef GetNilReferenceSummaryString() { return {}; }
+
   // for a ValueObject of some "reference type", if the language provides a
   // technique to decide whether the reference has ever been assigned to some
   // object, this method will return true if such detection is possible, and if

diff  --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index c8a306334cf5..082ad344d2d1 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -355,22 +355,33 @@ void ValueObjectPrinter::GetValueSummaryError(std::string 
&value,
   if (err_cstr)
 error.assign(err_cstr);
 
-  if (ShouldPrintValueObject()) {
-if (IsNil())
-  summary.assign("nil");
-else if (IsUninitialized())
-  summary.assign("");
-else if (m_options.m_omit_summary_depth == 0) {
-  TypeSummaryImpl *entry = GetSummaryFormatter();
-  if (entry)
-m_valobj->GetSummaryAsCString(entry, summary,
-  m_options.m_varformat_language);
-  else {
-const char *sum_cstr =
-m_valobj->GetSummaryAsCString(m_options.m_varformat_language);
-if (sum_cstr)
-  summary.assign(sum_cstr);
-  }
+  if (!ShouldPrintValueObject())
+return;
+
+  if (IsNil()) {
+lldb::LanguageType lang_type =
+(m_options.m_varformat_language == lldb::eLanguageTypeUnknown)
+? m_valobj->GetPreferredDisplayLanguage()
+: m_options.m_varformat_language;
+if (Language *lang_plugin = Language::FindPlugin(lang_type)) {
+  summary.assign(lang_plugin->GetNilReferenceSummaryString().str());
+} else {
+  // We treat C as the fallback language rather than as a separate Language
+  // plugin.
+  summary.assign("NULL");
+}
+  } else if (IsUninitialized()) {
+summary.assign("");
+  } else if (m_options.m_omit_summary_depth == 0) {
+TypeSummaryImpl *entry = GetSummaryFormatter();
+if (entry) {
+  m_valobj->GetSummaryAsCString(entry, summary,
+m_options.m_varformat_language);
+} else {
+  const char *sum_cstr =
+  m_valobj->GetSummaryAsCString(m_options.m_varformat_language);
+  if (sum_cstr)
+summary.assign(sum_cstr);
 }
   }
 }
@@ -403,7 +414,9 @@ bool ValueObjectPrinter::PrintValueAndSummaryIfNeeded(bool 
&value_printed,
   // this thing is nil (but show the value if the user passes a format
   // explicitly

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-11-12 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG406ad187486b: [lldb/DataFormatters] Display null C++ 
pointers as nullptr (authored by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D77153?vs=304614&id=304983#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  
lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/TestForwardDeclFromStdModule.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  lldb/test/API/lang/c/anonymous/TestAnonymous.py
  lldb/test/API/lang/objc/objc-builtin-types/Makefile
  lldb/test/API/lang/objc/objc-builtin-types/TestObjCBuiltinTypes.py
  lldb/test/API/lang/objc/objc-builtin-types/main.cpp
  lldb/test/API/lang/objcxx/objc-builtin-types/Makefile
  lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
  lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp

Index: lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
===
--- lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
+++ lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
@@ -1,7 +1,5 @@
 """Test that the expression parser doesn't get confused by 'id' and 'Class'"""
 
-
-
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -21,7 +19,6 @@
 self.main_source, '// Set breakpoint here.')
 
 @add_test_categories(['pyapi'])
-# [regression] Can't print ivar value: error: reference to 'id' is ambiguous
 def test_with_python_api(self):
 """Test expression parser respect for ObjC built-in types."""
 self.build()
@@ -57,4 +54,7 @@
 
 self.expect("expr (foo)", patterns=["\(ns::id\) \$.* = 0"])
 
-self.expect("expr id my_id = 0; my_id", patterns=["\(id\) \$.* = nil"])
+self.expect("expr --language Objective-C++ -- id my_id = 0; my_id",
+patterns=["\(id\) \$.* = nil"])
+self.expect("expr --language C++ -- id my_id = 0; my_id",
+patterns=["\(id\) \$.* = nullptr"])
Index: lldb/test/API/lang/c/anonymous/TestAnonymous.py
===
--- lldb/test/API/lang/c/anonymous/TestAnonymous.py
+++ lldb/test/API/lang/c/anonymous/TestAnonymous.py
@@ -62,7 +62,7 @@
 
 # These should display correctly.
 self.expect("expression pz", VARIABLES_DISPLAYED_CORRECTLY,
-substrs=["(type_z *) $", " = 0x"])
+substrs=["(type_z *) $", " = NULL"])
 
 self.expect("expression z.y", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["(type_y) $", "dummy = 2"])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -74,6 +74,7 @@
 std::u32string u32_string(U"🍄🍅🍆🍌");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -80,6 +80,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -117,6 +118,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-

[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

2020-11-12 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 304990.
mgorny added a comment.

Use a `struct`-based return type.


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

https://reviews.llvm.org/D91293

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -32,6 +32,11 @@
 
 class NativeProcessFreeBSD;
 
+struct YMMSplitPtr {
+  void *xmm;
+  void *ymm_hi;
+};
+
 class NativeRegisterContextFreeBSD_x86_64
 : public NativeRegisterContextFreeBSD,
   public NativeRegisterContextWatchpoint_x86 {
@@ -58,15 +63,13 @@
 private:
   // Private member types.
   enum RegSetKind {
+YMMRegSet,
+MaxXSaveSet = YMMRegSet,
+
 GPRegSet,
 FPRegSet,
-XSaveRegSet,
 DBRegSet,
   };
-  enum {
-YMMXSaveSet,
-MaxXSaveSet = YMMXSaveSet,
-  };
 
   // Private member variables.
   std::array m_gpr;
@@ -82,6 +85,7 @@
 
   size_t GetFPROffset() const;
   size_t GetDBROffset() const;
+  llvm::Optional GetYMMSplitReg(uint32_t reg);
 };
 
 } // namespace process_freebsd
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -275,31 +275,6 @@
   }
 }
 
-static constexpr int RegNumX86ToX86_64(int regnum) {
-  switch (regnum) {
-  case lldb_ymm0_i386:
-  case lldb_ymm1_i386:
-  case lldb_ymm2_i386:
-  case lldb_ymm3_i386:
-  case lldb_ymm4_i386:
-  case lldb_ymm5_i386:
-  case lldb_ymm6_i386:
-  case lldb_ymm7_i386:
-return lldb_ymm0_x86_64 + regnum - lldb_ymm0_i386;
-  case lldb_bnd0_i386:
-  case lldb_bnd1_i386:
-  case lldb_bnd2_i386:
-  case lldb_bnd3_i386:
-return lldb_bnd0_x86_64 + regnum - lldb_bnd0_i386;
-  case lldb_bndcfgu_i386:
-return lldb_bndcfgu_x86_64;
-  case lldb_bndstatus_i386:
-return lldb_bndstatus_x86_64;
-  default:
-llvm_unreachable("Unhandled i386 register.");
-  }
-}
-
 llvm::Optional
 NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(int reg_num) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
@@ -309,7 +284,7 @@
 if (reg_num >= k_first_fpr_i386 && reg_num <= k_last_fpr_i386)
   return FPRegSet;
 if (reg_num >= k_first_avx_i386 && reg_num <= k_last_avx_i386)
-  return XSaveRegSet; // AVX
+  return YMMRegSet;
 if (reg_num >= k_first_mpxr_i386 && reg_num <= k_last_mpxr_i386)
   return llvm::None; // MPXR
 if (reg_num >= k_first_mpxc_i386 && reg_num <= k_last_mpxc_i386)
@@ -323,7 +298,7 @@
 if (reg_num >= k_first_fpr_x86_64 && reg_num <= k_last_fpr_x86_64)
   return FPRegSet;
 if (reg_num >= k_first_avx_x86_64 && reg_num <= k_last_avx_x86_64)
-  return XSaveRegSet; // AVX
+  return YMMRegSet;
 if (reg_num >= k_first_mpxr_x86_64 && reg_num <= k_last_mpxr_x86_64)
   return llvm::None; // MPXR
 if (reg_num >= k_first_mpxc_x86_64 && reg_num <= k_last_mpxc_x86_64)
@@ -354,7 +329,7 @@
   case DBRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_GETDBREGS, m_thread.GetID(),
m_dbr.data());
-  case XSaveRegSet: {
+  case YMMRegSet: {
 struct ptrace_xstate_info info;
 Status ret = NativeProcessFreeBSD::PtraceWrapper(
 PT_GETXSTATE_INFO, GetProcessPid(), &info, sizeof(info));
@@ -364,10 +339,10 @@
 assert(info.xsave_mask & XFEATURE_ENABLED_X87);
 assert(info.xsave_mask & XFEATURE_ENABLED_SSE);
 
-m_xsave_offsets[YMMXSaveSet] = LLDB_INVALID_XSAVE_OFFSET;
+m_xsave_offsets[YMMRegSet] = LLDB_INVALID_XSAVE_OFFSET;
 if (info.xsave_mask & XFEATURE_ENABLED_YMM_HI128) {
   uint32_t eax, ecx, edx;
-  __get_cpuid_count(0x0D, 2, &eax, &m_xsave_offsets[YMMXSaveSet], &ecx,
+  __get_cpuid_count(0x0D, 2, &eax, &m_xsave_offsets[YMMRegSet], &ecx,
 &edx);
 }
 
@@ -395,7 +370,7 @@
   case DBRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_SETDBREGS, m_thread.GetID(),
m_dbr.data());
-  case XSaveRegSet:
+  case YMMRegSet:
 // ReadRegisterSet() must always be called before WriteRegisterSet().
 assert(m_xsave.size() > 0);
 return NativeProcessFreeBSD::PtraceWrapper(PT_SETXSTATE, GetProcessPid(),
@@ -442,66 +417,27 @@
   case GPRegSet:
 reg_value.SetBytes(m_gpr.data() + reg_info->byte_offset,
reg_info->byte_size, endian::InlHostByteOrder());
-r

[Lldb-commits] [PATCH] D91238: Recognize __builtin_debugtrap on arm64, advance pc past it so users can continue easily

2020-11-12 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda updated this revision to Diff 305040.
jasonmolenda added a comment.

Minor update to the patch to move the test case under API/macosx for now, 
because it depends on debugserver behavior.  Add comments to note that we're 
all in agreement this should really be handled up in lldb - but that's more a 
TODO for a future day, for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91238

Files:
  lldb/test/API/macosx/builtin-debugtrap/Makefile
  lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py
  lldb/test/API/macosx/builtin-debugtrap/main.cpp
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp

Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -524,6 +524,28 @@
 
   return true;
 }
+// detect a __builtin_debugtrap instruction pattern ("brk #0xf000")
+// and advance the $pc past it, so that the user can continue execution.
+// Generally speaking, this knowledge should be centralized in lldb, 
+// recognizing the builtin_trap instruction and knowing how to advance
+// the pc past it, so that continue etc work.
+if (exc.exc_data.size() == 2 && exc.exc_data[0] == EXC_ARM_BREAKPOINT) {
+  nub_addr_t pc = GetPC(INVALID_NUB_ADDRESS);
+  if (pc != INVALID_NUB_ADDRESS && pc > 0) {
+DNBBreakpoint *bp =
+m_thread->Process()->Breakpoints().FindByAddress(pc);
+if (bp == nullptr) {
+  uint8_t insnbuf[4];
+  if (m_thread->Process()->ReadMemory(pc, 4, insnbuf) == 4) {
+uint8_t builtin_debugtrap_insn[4] = {0x00, 0x00, 0x3e,
+ 0xd4}; // brk #0xf000
+if (memcmp(insnbuf, builtin_debugtrap_insn, 4) == 0) {
+  SetPC(pc + 4);
+}
+  }
+}
+  }
+}
 break;
   }
   return false;
Index: lldb/test/API/macosx/builtin-debugtrap/main.cpp
===
--- /dev/null
+++ lldb/test/API/macosx/builtin-debugtrap/main.cpp
@@ -0,0 +1,11 @@
+#include 
+int global = 0;
+int main()
+{
+  global = 5; // Set a breakpoint here
+  __builtin_debugtrap();
+  global = 10;
+  __builtin_trap();
+  global = 15;
+  return global;
+}
Index: lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py
===
--- /dev/null
+++ lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py
@@ -0,0 +1,69 @@
+"""
+Test that lldb can continue past a __builtin_debugtrap, but not a __builtin_trap
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class BuiltinDebugTrapTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+# Currently this depends on behavior in debugserver to
+# advance the pc past __builtin_trap instructions so that
+# continue works.  Everyone is in agreement about how this
+# should be moved up into lldb.
+@skipUnlessDarwin
+
+def test(self):
+self.build()
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "// Set a breakpoint here", lldb.SBFileSpec("main.cpp"))
+
+# Continue to __builtin_debugtrap()
+process.Continue()
+if self.TraceOn():
+self.runCmd("f")
+self.runCmd("bt")
+self.runCmd("ta v global")
+
+self.assertEqual(process.GetSelectedThread().GetStopReason(), 
+ lldb.eStopReasonException)
+
+list = target.FindGlobalVariables("global", 1, lldb.eMatchTypeNormal)
+self.assertEqual(list.GetSize(), 1)
+global_value = list.GetValueAtIndex(0)
+
+self.assertEqual(global_value.GetValueAsUnsigned(), 5)
+
+# Continue to the __builtin_trap() -- we should be able to 
+# continue past __builtin_debugtrap.
+process.Continue()
+if self.TraceOn():
+self.runCmd("f")
+self.runCmd("bt")
+self.runCmd("ta v global")
+
+self.assertEqual(process.GetSelectedThread().GetStopReason(), 
+ lldb.eStopReasonException)
+
+# "global" is now 10.
+self.assertEqual(global_value.GetValueAsUnsigned(), 10)
+
+# We should be at the same point as before -- cannot advance
+# past a __builtin_trap().
+process.Continue()
+if self.TraceOn():
+self.runCmd("f")
+self.runCmd("bt")
+self.runCmd("ta v global")
+
+self.assertEqual(process.GetSelectedThread().GetStopReason(), 
+  

[Lldb-commits] [PATCH] D91238: Recognize __builtin_debugtrap on arm64, advance pc past it so users can continue easily

2020-11-12 Thread Jason Molenda 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 rG92b036dea24b: debugserver should advance pc past 
builtin_debugtrap insn (authored by jasonmolenda).

Changed prior to commit:
  https://reviews.llvm.org/D91238?vs=305040&id=305042#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91238

Files:
  lldb/test/API/macosx/builtin-debugtrap/Makefile
  lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py
  lldb/test/API/macosx/builtin-debugtrap/main.cpp
  lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp

Index: lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -524,6 +524,28 @@
 
   return true;
 }
+// detect a __builtin_debugtrap instruction pattern ("brk #0xf000")
+// and advance the $pc past it, so that the user can continue execution.
+// Generally speaking, this knowledge should be centralized in lldb, 
+// recognizing the builtin_trap instruction and knowing how to advance
+// the pc past it, so that continue etc work.
+if (exc.exc_data.size() == 2 && exc.exc_data[0] == EXC_ARM_BREAKPOINT) {
+  nub_addr_t pc = GetPC(INVALID_NUB_ADDRESS);
+  if (pc != INVALID_NUB_ADDRESS && pc > 0) {
+DNBBreakpoint *bp =
+m_thread->Process()->Breakpoints().FindByAddress(pc);
+if (bp == nullptr) {
+  uint8_t insnbuf[4];
+  if (m_thread->Process()->ReadMemory(pc, 4, insnbuf) == 4) {
+uint8_t builtin_debugtrap_insn[4] = {0x00, 0x00, 0x3e,
+ 0xd4}; // brk #0xf000
+if (memcmp(insnbuf, builtin_debugtrap_insn, 4) == 0) {
+  SetPC(pc + 4);
+}
+  }
+}
+  }
+}
 break;
   }
   return false;
Index: lldb/test/API/macosx/builtin-debugtrap/main.cpp
===
--- /dev/null
+++ lldb/test/API/macosx/builtin-debugtrap/main.cpp
@@ -0,0 +1,11 @@
+#include 
+int global = 0;
+int main()
+{
+  global = 5; // Set a breakpoint here
+  __builtin_debugtrap();
+  global = 10;
+  __builtin_trap();
+  global = 15;
+  return global;
+}
Index: lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py
===
--- /dev/null
+++ lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py
@@ -0,0 +1,70 @@
+"""
+Test that lldb can continue past a __builtin_debugtrap, but not a __builtin_trap
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class BuiltinDebugTrapTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+# Currently this depends on behavior in debugserver to
+# advance the pc past __builtin_trap instructions so that
+# continue works.  Everyone is in agreement that this
+# should be moved up into lldb instead of depending on the
+# remote stub rewriting the pc values.
+@skipUnlessDarwin
+
+def test(self):
+self.build()
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "// Set a breakpoint here", lldb.SBFileSpec("main.cpp"))
+
+# Continue to __builtin_debugtrap()
+process.Continue()
+if self.TraceOn():
+self.runCmd("f")
+self.runCmd("bt")
+self.runCmd("ta v global")
+
+self.assertEqual(process.GetSelectedThread().GetStopReason(), 
+ lldb.eStopReasonException)
+
+list = target.FindGlobalVariables("global", 1, lldb.eMatchTypeNormal)
+self.assertEqual(list.GetSize(), 1)
+global_value = list.GetValueAtIndex(0)
+
+self.assertEqual(global_value.GetValueAsUnsigned(), 5)
+
+# Continue to the __builtin_trap() -- we should be able to 
+# continue past __builtin_debugtrap.
+process.Continue()
+if self.TraceOn():
+self.runCmd("f")
+self.runCmd("bt")
+self.runCmd("ta v global")
+
+self.assertEqual(process.GetSelectedThread().GetStopReason(), 
+ lldb.eStopReasonException)
+
+# "global" is now 10.
+self.assertEqual(global_value.GetValueAsUnsigned(), 10)
+
+# We should be at the same point as before -- cannot advance
+# past a __builtin_trap().
+process.Continue()
+if self.TraceOn():
+self.runCmd("f")
+self.runCmd("bt

[Lldb-commits] [lldb] 92b036d - debugserver should advance pc past builtin_debugtrap insn

2020-11-12 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2020-11-12T23:31:14-08:00
New Revision: 92b036dea24b6e7ebfd950facdbf5543135eda05

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

LOG: debugserver should advance pc past builtin_debugtrap insn

On x86_64, when you hit a __builtin_debugtrap instruction, you
can continue past this in the debugger.  This patch has debugserver
recognize the specific instruction used for __builtin_debugtrap
and advance the pc past it, so that the user can continue execution
once they've hit one of these.

In the patch discussion, we were in agreement that it would be better
to have this knowledge up in lldb instead of depending on each
stub rewriting the pc behind the debugger's back, but that's a
larger scale change for another day.


Differential revision: https://reviews.llvm.org/D91238

Added: 
lldb/test/API/macosx/builtin-debugtrap/Makefile
lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py
lldb/test/API/macosx/builtin-debugtrap/main.cpp

Modified: 
lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp

Removed: 




diff  --git a/lldb/test/API/macosx/builtin-debugtrap/Makefile 
b/lldb/test/API/macosx/builtin-debugtrap/Makefile
new file mode 100644
index ..8b20bcb0
--- /dev/null
+++ b/lldb/test/API/macosx/builtin-debugtrap/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git a/lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py 
b/lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py
new file mode 100644
index ..d645da460df4
--- /dev/null
+++ b/lldb/test/API/macosx/builtin-debugtrap/TestBuiltinDebugTrap.py
@@ -0,0 +1,70 @@
+"""
+Test that lldb can continue past a __builtin_debugtrap, but not a 
__builtin_trap
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class BuiltinDebugTrapTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+# Currently this depends on behavior in debugserver to
+# advance the pc past __builtin_trap instructions so that
+# continue works.  Everyone is in agreement that this
+# should be moved up into lldb instead of depending on the
+# remote stub rewriting the pc values.
+@skipUnlessDarwin
+
+def test(self):
+self.build()
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "// Set a breakpoint here", lldb.SBFileSpec("main.cpp"))
+
+# Continue to __builtin_debugtrap()
+process.Continue()
+if self.TraceOn():
+self.runCmd("f")
+self.runCmd("bt")
+self.runCmd("ta v global")
+
+self.assertEqual(process.GetSelectedThread().GetStopReason(), 
+ lldb.eStopReasonException)
+
+list = target.FindGlobalVariables("global", 1, lldb.eMatchTypeNormal)
+self.assertEqual(list.GetSize(), 1)
+global_value = list.GetValueAtIndex(0)
+
+self.assertEqual(global_value.GetValueAsUnsigned(), 5)
+
+# Continue to the __builtin_trap() -- we should be able to 
+# continue past __builtin_debugtrap.
+process.Continue()
+if self.TraceOn():
+self.runCmd("f")
+self.runCmd("bt")
+self.runCmd("ta v global")
+
+self.assertEqual(process.GetSelectedThread().GetStopReason(), 
+ lldb.eStopReasonException)
+
+# "global" is now 10.
+self.assertEqual(global_value.GetValueAsUnsigned(), 10)
+
+# We should be at the same point as before -- cannot advance
+# past a __builtin_trap().
+process.Continue()
+if self.TraceOn():
+self.runCmd("f")
+self.runCmd("bt")
+self.runCmd("ta v global")
+
+self.assertEqual(process.GetSelectedThread().GetStopReason(), 
+ lldb.eStopReasonException)
+
+# "global" is still 10.
+self.assertEqual(global_value.GetValueAsUnsigned(), 10)

diff  --git a/lldb/test/API/macosx/builtin-debugtrap/main.cpp 
b/lldb/test/API/macosx/builtin-debugtrap/main.cpp
new file mode 100644
index ..2cbe2a48b503
--- /dev/null
+++ b/lldb/test/API/macosx/builtin-debugtrap/main.cpp
@@ -0,0 +1,11 @@
+#include 
+int global = 0;
+int main()
+{
+  global = 5; // Set a breakpoint here
+  __builtin_debugtrap();
+  global = 10;
+  __builtin_trap();
+  global = 15;
+  return global;
+}

diff  --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp 
b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index 7e1af7a75021..ab1623ba4217 100644
--- a/lldb/tools/debugserver/so