Re: [Lldb-commits] [PATCH] D24255: Fix for rL280668, Intel(R) Memory Protection Extensions (Intel(R) MPX) support.

[Valentina Giusti via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL280942: Fix for rL280668, Intel(R) Memory Protection 
Extensions (Intel(R) MPX) support. (authored by valentinagiusti).

Changed prior to commit:
  https://reviews.llvm.org/D24255?vs=70695=70698#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24255

Files:
  lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/TestRegisters.py
  lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/a.cpp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/TestMPXRegisters.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/main.cpp
  lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/main.cpp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/register_command/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/register_command/TestRegisters.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/register_command/a.cpp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/register/register_command/main.cpp
  lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
  lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  lldb/trunk/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h
  lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
  lldb/trunk/source/Plugins/Process/Utility/lldb-x86-register-enums.h
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1592,6 +1592,9 @@
   case eFormatVectorOfFloat32:
 response.PutCString("format:vector-float32;");
 break;
+  case eFormatVectorOfUInt64:
+response.PutCString("format:vector-uint64;");
+break;
   case eFormatVectorOfUInt128:
 response.PutCString("format:vector-uint128;");
 break;
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -547,6 +547,7 @@
   .Case("vector-sint32", eFormatVectorOfSInt32)
   .Case("vector-uint32", eFormatVectorOfUInt32)
   .Case("vector-float32", eFormatVectorOfFloat32)
+  .Case("vector-uint64", eFormatVectorOfUInt64)
   .Case("vector-uint128", eFormatVectorOfUInt128)
   .Default(eFormatInvalid);
 }
@@ -4178,6 +4179,8 @@
   reg_info.format = eFormatVectorOfUInt32;
 else if (value == "vector-float32")
   reg_info.format = eFormatVectorOfFloat32;
+else if (value == "vector-uint64")
+  reg_info.format = eFormatVectorOfUInt64;
 else if (value == "vector-uint128")
   reg_info.format = eFormatVectorOfUInt128;
   } else if (name == "group_id") {
Index: lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h
===
--- lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h
@@ -34,6 +34,16 @@
LLVM_EXTENSION offsetof(FXSAVE, xmm[7]) + sizeof(XMMReg) +  \
(32 * reg_index))
 
+#define BNDR_OFFSET(reg_index) \
+(LLVM_EXTENSION offsetof(UserArea, i387) + \
+ LLVM_EXTENSION offsetof(FPR, xstate) + \
+ LLVM_EXTENSION offsetof(XSAVE, mpxr[reg_index]))
+
+#define BNDC_OFFSET(reg_index) \
+(LLVM_EXTENSION offsetof(UserArea, i387) + \
+ LLVM_EXTENSION offsetof(FPR, xstate) + \
+ LLVM_EXTENSION offsetof(XSAVE, mpxc[reg_index]))
+
 // Number of bytes needed to represent a FPR.
 #if !defined(FPR_SIZE)
 #define FPR_SIZE(reg) sizeof(((FXSAVE *)NULL)->reg)
@@ -48,6 +58,10 @@
 // Number of bytes needed to represent a YMM register.
 #define YMM_SIZE sizeof(YMMReg)
 
+// Number of bytes needed to represent MPX registers.
+#define BNDR_SIZE sizeof(MPXReg)
+#define BNDC_SIZE 

Re: [Lldb-commits] [PATCH] D24255: Fix for rL280668, Intel(R) Memory Protection Extensions (Intel(R) MPX) support.

[Valentina Giusti via lldb-commits]

valentinagiusti added inline comments.


Comment at: 
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/Makefile:7
@@ +6,2 @@
+
+include $(LEVEL)/Makefile.rules

Np, thanks for the review and explanations ;)


https://reviews.llvm.org/D24255



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

Re: [Lldb-commits] [PATCH] D24255: Fix for rL280668, Intel(R) Memory Protection Extensions (Intel(R) MPX) support.

[Valentina Giusti via lldb-commits]

valentinagiusti updated this revision to Diff 70695.
valentinagiusti added a comment.

Improved MPX test Makefile and removed workaround for unnamed register sets, 
and rebased according to the new coding style.


https://reviews.llvm.org/D24255

Files:
  packages/Python/lldbsuite/test/functionalities/register/Makefile
  packages/Python/lldbsuite/test/functionalities/register/TestRegisters.py
  packages/Python/lldbsuite/test/functionalities/register/a.cpp
  
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/Makefile
  
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/TestMPXRegisters.py
  
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/main.cpp
  packages/Python/lldbsuite/test/functionalities/register/main.cpp
  
packages/Python/lldbsuite/test/functionalities/register/register_command/Makefile
  
packages/Python/lldbsuite/test/functionalities/register/register_command/TestRegisters.py
  packages/Python/lldbsuite/test/functionalities/register/register_command/a.cpp
  
packages/Python/lldbsuite/test/functionalities/register/register_command/main.cpp
  source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
  source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  source/Plugins/Process/Utility/RegisterContext_x86.h
  source/Plugins/Process/Utility/RegisterInfos_i386.h
  source/Plugins/Process/Utility/RegisterInfos_x86_64.h
  source/Plugins/Process/Utility/lldb-x86-register-enums.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -547,6 +547,7 @@
   .Case("vector-sint32", eFormatVectorOfSInt32)
   .Case("vector-uint32", eFormatVectorOfUInt32)
   .Case("vector-float32", eFormatVectorOfFloat32)
+  .Case("vector-uint64", eFormatVectorOfUInt64)
   .Case("vector-uint128", eFormatVectorOfUInt128)
   .Default(eFormatInvalid);
 }
@@ -4178,6 +4179,8 @@
   reg_info.format = eFormatVectorOfUInt32;
 else if (value == "vector-float32")
   reg_info.format = eFormatVectorOfFloat32;
+else if (value == "vector-uint64")
+  reg_info.format = eFormatVectorOfUInt64;
 else if (value == "vector-uint128")
   reg_info.format = eFormatVectorOfUInt128;
   } else if (name == "group_id") {
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1592,6 +1592,9 @@
   case eFormatVectorOfFloat32:
 response.PutCString("format:vector-float32;");
 break;
+  case eFormatVectorOfUInt64:
+response.PutCString("format:vector-uint64;");
+break;
   case eFormatVectorOfUInt128:
 response.PutCString("format:vector-uint128;");
 break;
Index: source/Plugins/Process/Utility/lldb-x86-register-enums.h
===
--- source/Plugins/Process/Utility/lldb-x86-register-enums.h
+++ source/Plugins/Process/Utility/lldb-x86-register-enums.h
@@ -104,6 +104,18 @@
   lldb_ymm7_i386,
   k_last_avx_i386 = lldb_ymm7_i386,
 
+  k_first_mpxr_i386,
+  lldb_bnd0_i386 = k_first_mpxr_i386,
+  lldb_bnd1_i386,
+  lldb_bnd2_i386,
+  lldb_bnd3_i386,
+  k_last_mpxr = lldb_bnd3_i386,
+
+  k_first_mpxc_i386,
+  lldb_bndcfgu_i386 = k_first_mpxc_i386,
+  lldb_bndstatus_i386,
+  k_last_mpxc_i386 = lldb_bndstatus_i386,
+
   lldb_dr0_i386,
   lldb_dr1_i386,
   lldb_dr2_i386,
@@ -117,9 +129,11 @@
   k_num_gpr_registers_i386 = k_last_gpr_i386 - k_first_gpr_i386 + 1,
   k_num_fpr_registers_i386 = k_last_fpr_i386 - k_first_fpr_i386 + 1,
   k_num_avx_registers_i386 = k_last_avx_i386 - k_first_avx_i386 + 1,
+  k_num_mpx_registers_i386 = k_last_mpxc_i386 - k_first_mpxr_i386 + 1,
   k_num_user_registers_i386 = k_num_gpr_registers_i386 +
   k_num_fpr_registers_i386 +
-  k_num_avx_registers_i386,
+  k_num_avx_registers_i386 +
+  k_num_mpx_registers_i386,
 };
 
 //---
@@ -273,6 +287,18 @@
   lldb_ymm15_x86_64,
   k_last_avx_x86_64 = lldb_ymm15_x86_64,
 
+  k_first_mpxr_x86_64,
+  lldb_bnd0_x86_64 = k_first_mpxr_x86_64,
+  lldb_bnd1_x86_64,
+  

Re: [Lldb-commits] [PATCH] D24255: Fix for rL280668, Intel(R) Memory Protection Extensions (Intel(R) MPX) support.

[Pavel Labath via lldb-commits]

labath added a comment.

I think we're approaching the end now. If you could upload the new version 
(you'll have to reformat the changes), I'll give it a final pass.



Comment at: 
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/Makefile:6
@@ +5,3 @@
+ifeq "$(ARCH)" "i386"
+   CFLAGS_EXTRAS += -mmpx -fcheck-pointer-bounds -fuse-ld=bfd -m32
+   LD_EXTRAS += -mmpx -fcheck-pointer-bounds -fuse-ld=bfd -m32

valentinagiusti wrote:
> You are right, if I use CFLAGS_EXTRAS I don't have to check for the arch or 
> use LD_EXTRAS. I wish I knew this from the beginning...
Yeah, I don't blame you. :) The test suite is not an easy thing to wrap your 
head around. I am sorry if some of my comments sounded a bit harsh.


https://reviews.llvm.org/D24255



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

Re: [Lldb-commits] [PATCH] D24255: Fix for rL280668, Intel(R) Memory Protection Extensions (Intel(R) MPX) support.

[Valentina Giusti via lldb-commits]

valentinagiusti marked an inline comment as done.


Comment at: 
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/Makefile:6
@@ +5,3 @@
+ifeq "$(ARCH)" "i386"
+   CFLAGS_EXTRAS += -mmpx -fcheck-pointer-bounds -fuse-ld=bfd -m32
+   LD_EXTRAS += -mmpx -fcheck-pointer-bounds -fuse-ld=bfd -m32

You are right, if I use CFLAGS_EXTRAS I don't have to check for the arch or use 
LD_EXTRAS. I wish I knew this from the beginning...


https://reviews.llvm.org/D24255



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

Re: [Lldb-commits] [PATCH] D24255: Fix for rL280668, Intel(R) Memory Protection Extensions (Intel(R) MPX) support.

[Pavel Labath via lldb-commits]

labath added inline comments.


Comment at: 
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/Makefile:5
@@ +4,3 @@
+
+ifeq "$(ARCH)" "i386"
+   CXXFLAGS += -mmpx -fcheck-pointer-bounds -fuse-ld=bfd -m32

valentinagiusti wrote:
> labath wrote:
> > This should not be necessary. Makefile.rules already correctly appends -m32 
> > when needed. Maybe CFLAGS_EXTRAS would work instead (?)
> Unfortunately it doesn't append -m32 to all the instances when also a linker 
> is needed in the build process. In fact, in the test logs it shows that only 
> the first call of the g++ command has such a flag, and therefore the inferior 
> code build ends with an error.
> If there is a better way to do the same with CFLAGS_EXTRAS please let me know!
> 
I don't see how that is possible. I just made the following test:
```
$ cat Makefile
LEVEL = ../../../make

CXX_SOURCES := main.cpp

CFLAGS_EXTRAS += -Wzero-as-null-pointer-constant

include $(LEVEL)/Makefile.rules
 $ make
g++  -std=c++11  -g -O0 -fno-builtin -m64  -Wzero-as-null-pointer-constant 
-I/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/make/../../../../../include
 -include 
/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/make/test_common.h
  -c -o main.o main.cpp
g++  main.o -g -O0 -fno-builtin -m64  -Wzero-as-null-pointer-constant 
-I/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/make/../../../../../include
 -include 
/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/make/test_common.h
-o "a.out"
$ make ARCH=i386
g++  -std=c++11  -g -O0 -fno-builtin -m32  -Wzero-as-null-pointer-constant 
-I/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/make/../../../../../include
 -include 
/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/make/test_common.h
  -c -o main.o main.cpp
g++  main.o -g -O0 -fno-builtin -m32  -Wzero-as-null-pointer-constant 
-I/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/make/../../../../../include
 -include 
/usr/local/google/home/labath/ll/lldb/packages/Python/lldbsuite/test/make/test_common.h
-o "a.out"
```
As you see CFLAGS_EXTRAS was added to both compiler and linker command lines 
for both 32-bit and 64-bit cases (so, setting LD_EXTRAS is not necessary). 
Also, Makefile.rules already correctly appended the correct -m32/64 switch to 
all compiler and linker executions (so, you should not need to set it yourself).

If this does not work for you, then we need to figure out why and fix it. I 
don't believe we should be adding complex rules to these Makefiles, as it 
increases the maintenance burden down the line. 


Comment at: 
packages/Python/lldbsuite/test/functionalities/register/register_command/TestRegisters.py:297
@@ -294,4 +296,3 @@
 for registerSet in registerSets:
-if 'advanced vector extensions' in 
registerSet.GetName().lower():
-has_avx = True
-break
+if registerSet.GetName():
+if 'advanced vector extensions' in 
registerSet.GetName().lower():

valentinagiusti wrote:
> labath wrote:
> > Do we want to allow a register set with no name? It looks like the root of 
> > the problem is elsewhere.
> These lines of code are just to detect if there are AVX or MPX register sets, 
> so I don't think there is the need to do anything about nameless sets here. 
> If you don't like this solution, I think an alternative is to just check if 
> there are the register names that belong to one set or the other, it just 
> takes a bit longer - or I could just look for the first register in the set.
I was referring to the addition of the `if registerSet.GetName()` pre-check. I 
know in the previous version of the commit this test failed here due to 
register set name being `None`. So this also implicitly checked that each 
register set has a name. Now, you've circumvented that check. This seems to be 
unnecessary because the test passes even when I remove that, so I think we 
should do that. I think it's a reasonable  expectation that each register set 
has a name.


https://reviews.llvm.org/D24255



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

Re: [Lldb-commits] [PATCH] D24255: Fix for rL280668, Intel(R) Memory Protection Extensions (Intel(R) MPX) support.

[Valentina Giusti via lldb-commits]

valentinagiusti updated this revision to Diff 70507.
valentinagiusti added a comment.

Improved TestMPXRegisters.py and Makefile according to review.


https://reviews.llvm.org/D24255

Files:
  packages/Python/lldbsuite/test/functionalities/register/Makefile
  packages/Python/lldbsuite/test/functionalities/register/TestRegisters.py
  packages/Python/lldbsuite/test/functionalities/register/a.cpp
  
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/Makefile
  
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/TestMPXRegisters.py
  
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/main.cpp
  packages/Python/lldbsuite/test/functionalities/register/main.cpp
  
packages/Python/lldbsuite/test/functionalities/register/register_command/Makefile
  
packages/Python/lldbsuite/test/functionalities/register/register_command/TestRegisters.py
  packages/Python/lldbsuite/test/functionalities/register/register_command/a.cpp
  
packages/Python/lldbsuite/test/functionalities/register/register_command/main.cpp
  source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
  source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  source/Plugins/Process/Utility/RegisterContext_x86.h
  source/Plugins/Process/Utility/RegisterInfos_i386.h
  source/Plugins/Process/Utility/RegisterInfos_x86_64.h
  source/Plugins/Process/Utility/lldb-x86-register-enums.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -610,6 +610,7 @@
   .Case("vector-sint32", eFormatVectorOfSInt32)
   .Case("vector-uint32", eFormatVectorOfUInt32)
   .Case("vector-float32", eFormatVectorOfFloat32)
+  .Case("vector-uint64", eFormatVectorOfUInt64)
   .Case("vector-uint128", eFormatVectorOfUInt128)
   .Default(eFormatInvalid);
 }
@@ -4545,6 +4546,8 @@
 reg_info.format = eFormatVectorOfUInt32;
 else if (value == "vector-float32")
 reg_info.format = eFormatVectorOfFloat32;
+else if (value == "vector-uint64")
+reg_info.format = eFormatVectorOfUInt64;
 else if (value == "vector-uint128")
 reg_info.format = eFormatVectorOfUInt128;
 }
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1566,6 +1566,7 @@
 case eFormatVectorOfSInt32:  response.PutCString ("format:vector-sint32;"); break;
 case eFormatVectorOfUInt32:  response.PutCString ("format:vector-uint32;"); break;
 case eFormatVectorOfFloat32: response.PutCString ("format:vector-float32;"); break;
+case eFormatVectorOfUInt64:  response.PutCString ("format:vector-uint64;"); break;
 case eFormatVectorOfUInt128: response.PutCString ("format:vector-uint128;"); break;
 default: break;
 };
Index: source/Plugins/Process/Utility/lldb-x86-register-enums.h
===
--- source/Plugins/Process/Utility/lldb-x86-register-enums.h
+++ source/Plugins/Process/Utility/lldb-x86-register-enums.h
@@ -106,6 +106,18 @@
 lldb_ymm7_i386,
 k_last_avx_i386 = lldb_ymm7_i386,
 
+k_first_mpxr_i386,
+lldb_bnd0_i386 = k_first_mpxr_i386,
+lldb_bnd1_i386,
+lldb_bnd2_i386,
+lldb_bnd3_i386,
+k_last_mpxr = lldb_bnd3_i386,
+
+k_first_mpxc_i386,
+lldb_bndcfgu_i386 = k_first_mpxc_i386,
+lldb_bndstatus_i386,
+k_last_mpxc_i386 = lldb_bndstatus_i386,
+
 lldb_dr0_i386,
 lldb_dr1_i386,
 lldb_dr2_i386,
@@ -119,7 +131,8 @@
 k_num_gpr_registers_i386 = k_last_gpr_i386 - k_first_gpr_i386 + 1,
 k_num_fpr_registers_i386 = k_last_fpr_i386 - k_first_fpr_i386 + 1,
 k_num_avx_registers_i386 = k_last_avx_i386 - k_first_avx_i386 + 1,
-k_num_user_registers_i386 = k_num_gpr_registers_i386 + k_num_fpr_registers_i386 + k_num_avx_registers_i386,
+k_num_mpx_registers_i386 = k_last_mpxc_i386 - 

Re: [Lldb-commits] [PATCH] D24255: Fix for rL280668, Intel(R) Memory Protection Extensions (Intel(R) MPX) support.

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.

Just switch to using CFLAGS_EXTRAS and LD_EXTRAS and this is good to go.



Comment at: 
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/Makefile:6-11
@@ +5,8 @@
+ifeq "$(ARCH)" "i386"
+   CXXFLAGS += -mmpx -fcheck-pointer-bounds -fuse-ld=bfd -m32
+   LDFLAGS += -mmpx -fcheck-pointer-bounds -fuse-ld=bfd -m32
+endif
+ifeq "$(ARCH)" "x86_64"
+   CXXFLAGS += -mmpx -fcheck-pointer-bounds -fuse-ld=bfd
+   LDFLAGS += -mmpx -fcheck-pointer-bounds -fuse-ld=bfd
+endif

It would be nice to not modify the CXXFLAGS or LDFLAGS directly. You should 
modify CFLAGS_EXTRAS and LD_EXTRAS. I am not sure if there is a 
CXXFLAGS_EXTRAS, but feel free to add it if you can't use CFLAGS_EXTRAS


https://reviews.llvm.org/D24255



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

Re: [Lldb-commits] [PATCH] D24255: Fix for rL280668, Intel(R) Memory Protection Extensions (Intel(R) MPX) support.

[Valentina Giusti via lldb-commits]

valentinagiusti added a comment.

Hi, inline there are my other replies.



Comment at: 
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/Makefile:5
@@ +4,3 @@
+
+ifeq "$(ARCH)" "i386"
+   CXXFLAGS += -mmpx -fcheck-pointer-bounds -fuse-ld=bfd -m32

labath wrote:
> This should not be necessary. Makefile.rules already correctly appends -m32 
> when needed. Maybe CFLAGS_EXTRAS would work instead (?)
Unfortunately it doesn't append -m32 to all the instances when also a linker is 
needed in the build process. In fact, in the test logs it shows that only the 
first call of the g++ command has such a flag, and therefore the inferior code 
build ends with an error.
If there is a better way to do the same with CFLAGS_EXTRAS please let me know!



Comment at: 
packages/Python/lldbsuite/test/functionalities/register/register_command/TestRegisters.py:297
@@ -294,4 +296,3 @@
 for registerSet in registerSets:
-if 'advanced vector extensions' in 
registerSet.GetName().lower():
-has_avx = True
-break
+if registerSet.GetName():
+if 'advanced vector extensions' in 
registerSet.GetName().lower():

labath wrote:
> Do we want to allow a register set with no name? It looks like the root of 
> the problem is elsewhere.
These lines of code are just to detect if there are AVX or MPX register sets, 
so I don't think there is the need to do anything about nameless sets here. If 
you don't like this solution, I think an alternative is to just check if there 
are the register names that belong to one set or the other, it just takes a bit 
longer - or I could just look for the first register in the set.


https://reviews.llvm.org/D24255



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

[Lldb-commits] [PATCH] D24255: Fix for rL280668, Intel(R) Memory Protection Extensions (Intel(R) MPX) support.

[Valentina Giusti via lldb-commits]

valentinagiusti created this revision.
valentinagiusti added a subscriber: lldb-commits.

Signed-off-by: Valentina Giusti 

https://reviews.llvm.org/D24255

Files:
  packages/Python/lldbsuite/test/functionalities/register/Makefile
  packages/Python/lldbsuite/test/functionalities/register/TestRegisters.py
  packages/Python/lldbsuite/test/functionalities/register/a.cpp
  
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/Makefile
  
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/TestMPXRegisters.py
  
packages/Python/lldbsuite/test/functionalities/register/intel_xtended_registers/main.cpp
  packages/Python/lldbsuite/test/functionalities/register/main.cpp
  
packages/Python/lldbsuite/test/functionalities/register/register_command/Makefile
  
packages/Python/lldbsuite/test/functionalities/register/register_command/TestRegisters.py
  packages/Python/lldbsuite/test/functionalities/register/register_command/a.cpp
  
packages/Python/lldbsuite/test/functionalities/register/register_command/main.cpp
  source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
  source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  source/Plugins/Process/Utility/RegisterContext_x86.h
  source/Plugins/Process/Utility/RegisterInfos_i386.h
  source/Plugins/Process/Utility/RegisterInfos_x86_64.h
  source/Plugins/Process/Utility/lldb-x86-register-enums.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -610,6 +610,7 @@
   .Case("vector-sint32", eFormatVectorOfSInt32)
   .Case("vector-uint32", eFormatVectorOfUInt32)
   .Case("vector-float32", eFormatVectorOfFloat32)
+  .Case("vector-uint64", eFormatVectorOfUInt64)
   .Case("vector-uint128", eFormatVectorOfUInt128)
   .Default(eFormatInvalid);
 }
@@ -4545,6 +4546,8 @@
 reg_info.format = eFormatVectorOfUInt32;
 else if (value == "vector-float32")
 reg_info.format = eFormatVectorOfFloat32;
+else if (value == "vector-uint64")
+reg_info.format = eFormatVectorOfUInt64;
 else if (value == "vector-uint128")
 reg_info.format = eFormatVectorOfUInt128;
 }
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1566,6 +1566,7 @@
 case eFormatVectorOfSInt32:  response.PutCString ("format:vector-sint32;"); break;
 case eFormatVectorOfUInt32:  response.PutCString ("format:vector-uint32;"); break;
 case eFormatVectorOfFloat32: response.PutCString ("format:vector-float32;"); break;
+case eFormatVectorOfUInt64:  response.PutCString ("format:vector-uint64;"); break;
 case eFormatVectorOfUInt128: response.PutCString ("format:vector-uint128;"); break;
 default: break;
 };
Index: source/Plugins/Process/Utility/lldb-x86-register-enums.h
===
--- source/Plugins/Process/Utility/lldb-x86-register-enums.h
+++ source/Plugins/Process/Utility/lldb-x86-register-enums.h
@@ -106,6 +106,18 @@
 lldb_ymm7_i386,
 k_last_avx_i386 = lldb_ymm7_i386,
 
+k_first_mpxr_i386,
+lldb_bnd0_i386 = k_first_mpxr_i386,
+lldb_bnd1_i386,
+lldb_bnd2_i386,
+lldb_bnd3_i386,
+k_last_mpxr = lldb_bnd3_i386,
+
+k_first_mpxc_i386,
+lldb_bndcfgu_i386 = k_first_mpxc_i386,
+lldb_bndstatus_i386,
+k_last_mpxc_i386 = lldb_bndstatus_i386,
+
 lldb_dr0_i386,
 lldb_dr1_i386,
 lldb_dr2_i386,
@@ -119,7 +131,8 @@
 k_num_gpr_registers_i386 = k_last_gpr_i386 - k_first_gpr_i386 + 1,
 k_num_fpr_registers_i386 = k_last_fpr_i386 - k_first_fpr_i386 + 1,
 k_num_avx_registers_i386 = k_last_avx_i386 - k_first_avx_i386 + 1,
-k_num_user_registers_i386 = k_num_gpr_registers_i386 + k_num_fpr_registers_i386 + k_num_avx_registers_i386,
+k_num_mpx_registers_i386 = k_last_mpxc_i386 -