[Lldb-commits] [PATCH] D86261: Add hashing of the .text section to ProcessMinidump.

2020-08-24 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.

thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86261

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


[Lldb-commits] [PATCH] D86416: [lldb] -stdlib=libc++ for linking with lldb lib also if LLVM_ENABLE_LIBCXX

2020-08-24 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a reviewer: JDevlieghere.
teemperor added a comment.

Thanks for working on this! This looks good to me, but I wonder if doing this 
with a dedicated flag instead of an environment variable would be better. But 
I'll leave that to the others who have a better idea how the dotest flags 
should work.

Out of curiosity, will this also fix the TestPluginCommands.py ?




Comment at: lldb/test/API/lit.site.cfg.py.in:23
 config.python_executable = "@Python3_EXECUTABLE@"
+config.libcxx_used = @LLVM_LIBCXX_USED@
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/API/dotest.py"

Maybe move this below the `config.llvm_use_sanitizer` line, as this is 
currently in the middle of the python/dotest.py-related config flags.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D86416

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


[Lldb-commits] [PATCH] D86416: [lldb] -stdlib=libc++ for linking with lldb lib also if LLVM_ENABLE_LIBCXX

2020-08-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D86416#2232966 , @teemperor wrote:

> This looks good to me, but I wonder if doing this with a dedicated flag 
> instead of an environment variable would be better. But I'll leave that to 
> the others who have a better idea how the dotest flags should work.

Yeah, please make this a dedicated flag and store its value in the 
configuration. We've been slowly (but steadily) moving away from passing around 
things trough the environment.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D86416

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


[Lldb-commits] [PATCH] D86416: [lldb] -stdlib=libc++ for linking with lldb lib also if LLVM_ENABLE_LIBCXX

2020-08-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I actually think tests relying on this should be converted to c++ unit tests. 
The logic to compile and link against liblldb takes up a considerable chunk of 
our test buildsystem. Why recreate that logic, when cmake knows how to do that 
already? TestPluginCommands might be more involved, but converting 
`api/command-return-object`  should be trivial using the api unit test 
framework we already have (unittests/API)...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D86416

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


[Lldb-commits] [PATCH] D86375: Load correct module for linux and android when duplicates exist in minidump.

2020-08-24 Thread Pavel Labath via Phabricator via lldb-commits
labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.

If I correctly understand what this is doing, then I don't think it's a good 
idea. The base of an (elf) shared library does not have to be mapped 
executable. These are the mappings I get for a trivial hello world program (no 
mmapping of libraries or anything) on my linux machine:

  0040-00401000 r--p  fd:01 2838574
/tmp/l/a.out
  00401000-00402000 r-xp 1000 fd:01 2838574
/tmp/l/a.out
  00402000-00403000 r--p 2000 fd:01 2838574
/tmp/l/a.out
  00403000-00404000 r--p 2000 fd:01 2838574
/tmp/l/a.out
  00404000-00405000 rw-p 3000 fd:01 2838574
/tmp/l/a.out
  020fb000-0211c000 rw-p  00:00 0  
[heap]
  7fe4f5d87000-7fe4f5da9000 r--p  fd:01 2738932
/lib64/libc-2.31.so
  7fe4f5da9000-7fe4f5ef3000 r-xp 00022000 fd:01 2738932
/lib64/libc-2.31.so
  7fe4f5ef3000-7fe4f5f3d000 r--p 0016c000 fd:01 2738932
/lib64/libc-2.31.so
  7fe4f5f3d000-7fe4f5f41000 r--p 001b5000 fd:01 2738932
/lib64/libc-2.31.so
  7fe4f5f41000-7fe4f5f43000 rw-p 001b9000 fd:01 2738932
/lib64/libc-2.31.so
  ...

Here, the correct base of a.out is 0x0040 and the libc base is 
0x7fe4f5d87000. But this patch would make them be detected as 0x00401000 and 
0x7fe4f5da9000, respectively.

This behavior is controlled by the `-z (no)separate-code`. My machine has 
`separate-code` as default, but that setting may not be universal, so you may 
need to pass this flag explicitly to reproduce this. For reference, these are 
the mappings I get when compiling a.out with `-z noseparate-code` (libc 
mappings remain unchanged, of course):

  0040-00401000 r-xp  fd:01 2838574
/tmp/l/a.out
  00401000-00402000 r--p  fd:01 2838574
/tmp/l/a.out
  00402000-00403000 rw-p 1000 fd:01 2838574
/tmp/l/a.out

It sounds like we need a better heuristic. How about "the number of consecutive 
mappings with the same name"? User mmapping code is likely going to map the 
library in a single chunk, but the dynamic linker will typically create 
multiple mappings (even a trivial executable can have five), so it seems like 
picking the longest sequence could work...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86375

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


[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

2020-08-24 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: aprantl, shafik.
Herald added a project: LLDB.
labath requested review of this revision.
Herald added a subscriber: JDevlieghere.

The function was returning an incorrect (empty) value on the first
invocation. Given that this only affected the first invocation, this
bug/typo went mostly unaffected. DW_AT_const_value were particularly
badly affected by this as the GetByteSize call is
SymbolFileDWARF::ParseVariableDIE is likely to be the first call of this
function, and its effects cannot be undone by retrying.

Depends on D86348 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86436

Files:
  lldb/source/Symbol/Type.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s


Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
@@ -5,10 +5,10 @@
 
 # RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
 # RUN: %lldb %t \
-# RUN:   -o "target variable udata data1 data2 data4 data8 string strp ref4" \
+# RUN:   -o "target variable udata data1 data2 data4 data8 string strp ref4 
udata_ptr" \
 # RUN:   -o exit | FileCheck %s
 
-# CHECK-LABEL: target variable udata data1 data2 data4 data8 string strp ref4
+# CHECK-LABEL: target variable
 ## Variable specified via DW_FORM_udata. This is typical for clang (10).
 # CHECK: (unsigned long) udata = 4742474247424742
 ## Variables specified via fixed-size forms. This is typical for gcc (9).
@@ -22,6 +22,8 @@
 # CHECK: (char [7]) strp = "strp"
 ## Bogus attribute form. Let's make sure we don't crash at least.
 # CHECK: (char [7]) ref4 = 
+## A variable of pointer type.
+# CHECK: (unsigned long *) udata_ptr = 0xdeadbeefbaadf00d
 
 .section.debug_abbrev,"",@progbits
 .byte   1   # Abbreviation Code
@@ -33,6 +35,13 @@
 .byte   8   # DW_FORM_string
 .byte   0   # EOM(1)
 .byte   0   # EOM(2)
+.byte   2   # Abbreviation Code
+.byte   15  # DW_TAG_pointer_type
+.byte   0   # DW_CHILDREN_no
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
 .byte   4   # Abbreviation Code
 .byte   1   # DW_TAG_array_type
 .byte   1   # DW_CHILDREN_yes
@@ -109,6 +118,9 @@
 .asciz  "unsigned long" # DW_AT_name
 .byte   8   # DW_AT_byte_size
 .byte   7   # DW_AT_encoding
+.Lulong_ptr:
+.byte   2   # Abbrev DW_TAG_pointer_type
+.long   .Lulong-.Lcu_begin0 # DW_AT_type
 
 .byte   10  # Abbrev DW_TAG_variable
 .asciz  "udata" # DW_AT_name
@@ -150,6 +162,11 @@
 .long   .Lchar_arr-.Lcu_begin0  # DW_AT_type
 .long   .Lulong-.Lcu_begin0 # DW_AT_const_value
 
+.byte   10  # Abbrev DW_TAG_variable
+.asciz  "udata_ptr" # DW_AT_name
+.long   .Lulong_ptr-.Lcu_begin0 # DW_AT_type
+.uleb128 0xdeadbeefbaadf00d # DW_AT_const_value
+
 .byte   0   # End Of Children Mark
 .Ldebug_info_end0:
 
Index: lldb/source/Symbol/Type.cpp
===
--- lldb/source/Symbol/Type.cpp
+++ lldb/source/Symbol/Type.cpp
@@ -375,6 +375,7 @@
   if (ArchSpec arch = m_symbol_file->GetObjectFile()->GetArchitecture()) {
 m_byte_size = arch.GetAddressByteSize();
 m_byte_size_has_value = true;
+return m_byte_size;
   }
 } break;
   }


Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
@@ -5,10 +5,10 @@
 
 # RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
 # RUN: %lldb %t \
-# RUN:   -o "target variable udata data1 data2 data4 data8 string strp ref4" \
+# RUN:   -o "target variable udata data1 data2 data4 data8 string strp ref4 udata_ptr" \
 # RUN:   -o exit | FileCheck %s
 
-# CHECK-LABEL: target variable udata data1 data2 data4 data8 string strp ref4
+# CHECK-LABEL: target variable
 ## Variable specified via DW_FORM_udata. This is typical for clang (10).
 # CHECK: (unsigned long) udata = 4742474247424742
 ## Variables specified via fixed-size forms. This is typical for gcc (9).
@@ -22,6 +22,8 @@
 # CHECK: (char [7]) strp = "strp"
 #

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

2020-08-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D86348#2231094 , @aprantl wrote:

> This looks nice! I'm somewhat suspicious that the new test doesn't 
> specifically test the union case of the old test, but I'm assuming that would 
> still work and your simpler tests covers the same code?

For a while I did want to just delete that union+bitfield test, but I 
eventually concluded that it is interesting to keep it. This patch keeps the 
original tests, only it renames it to a less generic name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

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


[Lldb-commits] [lldb] 0e301fd - [lldb/Utility] Remove some Scalar type accessors

2020-08-24 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-08-24T11:45:30+02:00
New Revision: 0e301fd02386e01ca93a28e8c0484447fbb440a1

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

LOG: [lldb/Utility] Remove some Scalar type accessors

Now that the number of Scalar "types" has been reduced, these don't make
sense anymore.

Added: 


Modified: 
lldb/include/lldb/Utility/Scalar.h
lldb/source/Utility/Scalar.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Scalar.h 
b/lldb/include/lldb/Utility/Scalar.h
index 4e0505e669dd..9d3a20c93898 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -75,11 +75,7 @@ class Scalar {
 llvm::APFloat::rmNearestTiesToEven, &ignore);
   }
   Scalar(llvm::APInt v)
-  : m_type(GetBestTypeForBitSize(v.getBitWidth(), true)),
-m_integer(std::move(v)), m_float(0.0f) {}
-
-  /// Return the most efficient Scalar::Type for the requested bit size.
-  static Type GetBestTypeForBitSize(size_t bit_size, bool sign);
+  : m_type(e_sint), m_integer(std::move(v)), m_float(0.0f) {}
 
   bool SignExtend(uint32_t bit_pos);
 
@@ -126,12 +122,6 @@ class Scalar {
 
   static const char *GetValueTypeAsCString(Scalar::Type value_type);
 
-  static Scalar::Type
-  GetValueTypeForSignedIntegerWithByteSize(size_t byte_size);
-
-  static Scalar::Type
-  GetValueTypeForUnsignedIntegerWithByteSize(size_t byte_size);
-
   // All operators can benefits from the implicit conversions that will happen
   // automagically by the compiler, so no temporary objects will need to be
   // created. As a result, we currently don't need a variety of overloaded set

diff  --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 1a27808068d6..e5a1454561f2 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -197,13 +197,9 @@ void Scalar::GetValue(Stream *s, bool show_type) const {
   }
 }
 
-Scalar::Type Scalar::GetBestTypeForBitSize(size_t bit_size, bool sign) {
-  return sign ? e_sint : e_uint;
-}
-
 void Scalar::TruncOrExtendTo(uint16_t bits, bool sign) {
   m_integer = sign ? m_integer.sextOrTrunc(bits) : m_integer.zextOrTrunc(bits);
-  m_type = GetBestTypeForBitSize(bits, sign);
+  m_type = sign ? e_sint : e_uint;
 }
 
 bool Scalar::IntegralPromote(uint16_t bits, bool sign) {
@@ -262,16 +258,6 @@ const char *Scalar::GetValueTypeAsCString(Scalar::Type 
type) {
   return "???";
 }
 
-Scalar::Type
-Scalar::GetValueTypeForSignedIntegerWithByteSize(size_t byte_size) {
-  return e_sint;
-}
-
-Scalar::Type
-Scalar::GetValueTypeForUnsignedIntegerWithByteSize(size_t byte_size) {
-  return e_uint;
-}
-
 bool Scalar::MakeSigned() {
   bool success = false;
 
@@ -768,12 +754,7 @@ Status Scalar::SetValueFromData(const DataExtractor &data,
   case lldb::eEncodingSint: {
 if (data.GetByteSize() < byte_size)
   return Status("insufficient data");
-Type type = GetBestTypeForBitSize(byte_size*8, encoding == 
lldb::eEncodingSint);
-if (type == e_void) {
-  return Status("unsupported integer byte size: %" PRIu64 "",
-static_cast(byte_size));
-}
-m_type = type;
+m_type = encoding == lldb::eEncodingSint ? e_sint : e_uint;
 if (data.GetByteOrder() == endian::InlHostByteOrder()) {
   m_integer = APInt::getNullValue(8 * byte_size);
   llvm::LoadIntFromMemory(m_integer, data.GetDataStart(), byte_size);



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


[Lldb-commits] [PATCH] D82853: [LLDB] Support custom expedited register set in gdb-remote

2020-08-24 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid updated this revision to Diff 287323.
omjavaid added a comment.

This reworks previous implementation by returning a vector containing register 
numbers to be expedited. Default case is minimal set of generic registers or 
complete register set 0.

In the child rev D82855  for 
NativeRegisterContextLinux_arm64 we call the base function and push_back vg reg 
num into register number vector before return.


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

https://reviews.llvm.org/D82853

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -504,30 +504,16 @@
   json::Object register_object;
 
 #ifdef LLDB_JTHREADSINFO_FULL_REGISTER_SET
-  // Expedite all registers in the first register set (i.e. should be GPRs)
-  // that are not contained in other registers.
-  const RegisterSet *reg_set_p = reg_ctx_sp->GetRegisterSet(0);
-  if (!reg_set_p)
-return llvm::make_error("failed to get registers",
-   llvm::inconvertibleErrorCode());
-  for (const uint32_t *reg_num_p = reg_set_p->registers;
-   *reg_num_p != LLDB_INVALID_REGNUM; ++reg_num_p) {
-uint32_t reg_num = *reg_num_p;
+  const auto expedited_regs =
+  reg_ctx.GetExpeditedRegisters(ExpeditedRegs::Full);
 #else
-  // Expedite only a couple of registers until we figure out why sending
-  // registers is expensive.
-  static const uint32_t k_expedited_registers[] = {
-  LLDB_REGNUM_GENERIC_PC, LLDB_REGNUM_GENERIC_SP, LLDB_REGNUM_GENERIC_FP,
-  LLDB_REGNUM_GENERIC_RA, LLDB_INVALID_REGNUM};
-
-  for (const uint32_t *generic_reg_p = k_expedited_registers;
-   *generic_reg_p != LLDB_INVALID_REGNUM; ++generic_reg_p) {
-uint32_t reg_num = reg_ctx.ConvertRegisterKindToRegisterNumber(
-eRegisterKindGeneric, *generic_reg_p);
-if (reg_num == LLDB_INVALID_REGNUM)
-  continue; // Target does not support the given register.
+  const auto expedited_regs =
+  reg_ctx.GetExpeditedRegisters(ExpeditedRegs::Minimal);
 #endif
-
+  if (expedited_regs.empty())
+return llvm::make_error("failed to get registers",
+   llvm::inconvertibleErrorCode());
+  for (auto ®_num : expedited_regs) {
 const RegisterInfo *const reg_info_p =
 reg_ctx.GetRegisterInfoAtIndex(reg_num);
 if (reg_info_p == nullptr) {
@@ -806,35 +792,19 @@
 
   // Grab the register context.
   NativeRegisterContext& reg_ctx = thread->GetRegisterContext();
-  // Expedite all registers in the first register set (i.e. should be GPRs)
-  // that are not contained in other registers.
-  const RegisterSet *reg_set_p;
-  if (reg_ctx.GetRegisterSetCount() > 0 &&
-  ((reg_set_p = reg_ctx.GetRegisterSet(0)) != nullptr)) {
-LLDB_LOGF(log,
-  "GDBRemoteCommunicationServerLLGS::%s expediting registers "
-  "from set '%s' (registers set count: %zu)",
-  __FUNCTION__, reg_set_p->name ? reg_set_p->name : "",
-  reg_set_p->num_registers);
+  const auto expedited_regs =
+  reg_ctx.GetExpeditedRegisters(ExpeditedRegs::Full);
 
-for (const uint32_t *reg_num_p = reg_set_p->registers;
- *reg_num_p != LLDB_INVALID_REGNUM; ++reg_num_p) {
+  if (!expedited_regs.empty()) {
+for (auto ®_num : expedited_regs) {
   const RegisterInfo *const reg_info_p =
-  reg_ctx.GetRegisterInfoAtIndex(*reg_num_p);
-  if (reg_info_p == nullptr) {
-LLDB_LOGF(log,
-  "GDBRemoteCommunicationServerLLGS::%s failed to get "
-  "register info for register set '%s', register index "
-  "%" PRIu32,
-  __FUNCTION__,
-  reg_set_p->name ? reg_set_p->name : "",
-  *reg_num_p);
-  } else if (reg_info_p->value_regs == nullptr) {
-// Only expediate registers that are not contained in other registers.
+  reg_ctx.GetRegisterInfoAtIndex(reg_num);
+  // Only expediate registers that are not contained in other registers.
+  if (reg_info_p != nullptr && reg_info_p->value_regs == nullptr) {
 RegisterValue reg_value;
 Status error = reg_ctx.ReadRegister(reg_info_p, reg_value);
 if (error.Success()) {
-  response.Printf("%.02x:", *reg_num_p);
+  response.Printf("%.02x:", reg_num);
   WriteRegisterValueInHexFixedWidth(response, reg_ctx, *reg_info_p,
 ®_value, lldb::eByteOrderBig);
   response.PutChar(';');
@@ -8

[Lldb-commits] [PATCH] D82855: [LLDB] Send SVE vg register in custom expedited registerset

2020-08-24 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid updated this revision to Diff 287325.
omjavaid added a comment.

Updated after changes to parent rev D82853 


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

https://reviews.llvm.org/D82855

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h


Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -101,6 +101,7 @@
   uint32_t GetRegNumSVEFFR() const;
   uint32_t GetRegNumFPCR() const;
   uint32_t GetRegNumFPSR() const;
+  uint32_t GetRegNumSVEVG() const;
 
 private:
   typedef std::map>
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -341,3 +341,5 @@
 uint32_t RegisterInfoPOSIX_arm64::GetRegNumFPCR() const { return fpu_fpcr; }
 
 uint32_t RegisterInfoPOSIX_arm64::GetRegNumFPSR() const { return fpu_fpsr; }
+
+uint32_t RegisterInfoPOSIX_arm64::GetRegNumSVEVG() const { return sve_vg; }
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -44,6 +44,9 @@
 
   void InvalidateAllRegisters() override;
 
+  std::vector
+  GetExpeditedRegisters(ExpeditedRegs expType) const override;
+
   // Hardware breakpoints/watchpoint management functions
 
   uint32_t NumSupportedHardwareBreakpoints() override;
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -1127,4 +1127,14 @@
   return m_sve_ptrace_payload.data();
 }
 
+std::vector NativeRegisterContextLinux_arm64::GetExpeditedRegisters(
+ExpeditedRegs expType) const {
+  std::vector expedited_reg_nums =
+  NativeRegisterContext::GetExpeditedRegisters(expType);
+  if (m_sve_state == SVEState::FPSIMD || m_sve_state == SVEState::Full)
+expedited_reg_nums.push_back(GetRegisterInfo().GetRegNumSVEVG());
+
+  return expedited_reg_nums;
+}
+
 #endif // defined (__arm64__) || defined (__aarch64__)


Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -101,6 +101,7 @@
   uint32_t GetRegNumSVEFFR() const;
   uint32_t GetRegNumFPCR() const;
   uint32_t GetRegNumFPSR() const;
+  uint32_t GetRegNumSVEVG() const;
 
 private:
   typedef std::map>
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -341,3 +341,5 @@
 uint32_t RegisterInfoPOSIX_arm64::GetRegNumFPCR() const { return fpu_fpcr; }
 
 uint32_t RegisterInfoPOSIX_arm64::GetRegNumFPSR() const { return fpu_fpsr; }
+
+uint32_t RegisterInfoPOSIX_arm64::GetRegNumSVEVG() const { return sve_vg; }
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -44,6 +44,9 @@
 
   void InvalidateAllRegisters() override;
 
+  std::vector
+  GetExpeditedRegisters(ExpeditedRegs expType) const override;
+
   // Hardware breakpoints/watchpoint management functions
 
   uint32_t NumSupportedHardwareBreakpoints() override;
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -1127,4 +1127,14 @@
   return m_sve_ptrace_payload.data();
 }
 
+std::vector NativeRegisterContextLinux_arm64::GetExpeditedRegisters(
+ExpeditedRegs expType) const {
+  std::vector expedited_reg_nums =
+  NativeRegisterContext::GetExpeditedRegisters(e

[Lldb-commits] [PATCH] D82866: [LLDB] Test SVE dynamic resize with multiple threads

2020-08-24 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid updated this revision to Diff 287333.
omjavaid added a comment.

Updated after changes to TestSVERegister.py for detecting SVE support using 
/proc/cpuinfo


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

https://reviews.llvm.org/D82866

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
@@ -0,0 +1,57 @@
+#include 
+#include 
+
+void *thread_func(void *x_void_ptr) {
+  void *return_ptr = NULL;
+  if (*((int *)x_void_ptr) == 0) {
+prctl(PR_SVE_SET_VL, 8 * 4);
+asm volatile("ptrue p0.s\n\t");
+asm volatile("fcpy  z30.s, p0/m, #1.\n\t");
+asm volatile("fcpy  z31.s, p0/m, #5.\n\t");
+return_ptr = (void *)x_void_ptr; // Thread X breakpoint 1
+asm volatile("ptrue p0.s\n\t");
+asm volatile("fcpy  z0.s, p0/m, #1.\n\t");
+asm volatile("fcpy  z1.s, p0/m, #5.\n\t");
+return return_ptr; // Thread X breakpoint 2
+  }
+
+  if (*((int *)x_void_ptr) == 1) {
+prctl(PR_SVE_SET_VL, 8 * 2);
+asm volatile("ptrue p0.s\n\t");
+asm volatile("fcpy  z30.s, p0/m, #5.\n\t");
+asm volatile("fcpy  z31.s, p0/m, #1.\n\t");
+return_ptr = x_void_ptr; // Thread Y breakpoint 1
+asm volatile("ptrue p0.s\n\t");
+asm volatile("fcpy  z1.s, p0/m, #1.\n\t");
+asm volatile("fcpy  z0.s, p0/m, #5.\n\t");
+return return_ptr; // Thread Y breakpoint 2
+  }
+  return return_ptr;
+}
+
+int main() {
+  prctl(PR_SVE_SET_VL, 8);
+
+  /* this variable is our reference to the second thread */
+  pthread_t x_thread, y_thread;
+
+  int x = 0, y = 1;
+
+  /* create a second thread which executes with argument x */
+  if (pthread_create(&x_thread, NULL, thread_func, &x)) // Break in main thread
+return 1;
+
+  /* create a second thread which executes with argument y */
+  if (pthread_create(&y_thread, NULL, thread_func, &y))
+return 1;
+
+  /* wait for the first thread to finish */
+  if (pthread_join(x_thread, NULL))
+return 2;
+
+  /* wait for the second thread to finish */
+  if (pthread_join(y_thread, NULL))
+return 2;
+
+  return 0;
+}
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -0,0 +1,165 @@
+"""
+Test the AArch64 SVE registers dynamic resize with multiple threads.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class RegisterCommandsTestCase(TestBase):
+
+def targetHasSVE(self):
+triple = self.dbg.GetSelectedPlatform().GetTriple()
+
+# TODO other platforms, please implement this function
+if not re.match(".*-.*-linux", triple):
+return False
+
+# Need to do something different for non-Linux/Android targets
+cpuinfo_path = self.getBuildArtifact("cpuinfo")
+if configuration.lldb_platform_name:
+self.runCmd('platform get-file "/proc/cpuinfo" ' + cpuinfo_path)
+else:
+cpuinfo_path = "/proc/cpuinfo"
+
+try:
+f = open(cpuinfo_path, 'r')
+cpuinfo = f.read()
+f.close()
+except:
+return False
+
+return " sve " in cpuinfo
+
+def check_sve_register_size_and_offset(self, set, name, expected_size,
+   expected_offset):
+reg_value = set.GetChildMemberWithName(name)
+
+self.assertTrue(reg_value.IsValid(),
+'Verify we have a register named "%s"' % (name))
+self.assertEqual(reg_value.GetByteSize(), expected_size,
+ 'Verify "%s" size == %i' % (name, expected_size))
+self.assertEqual(reg_value.GetByteOffset(), expected_offset,
+ 'Verify "%s" == %i' % (name, expected_offset))
+
+def check_sve_registers_config(self, set, vg_test_value):
+vg_reg_value = set.GetChildMemberWithName("vg").GetValueAsUnsigned()
+offset = set.GetChildMemberWithName("z0").GetByteOffset()
+
+z_reg_size = vg_reg_value * 8
+p_reg_size = z_reg_size / 8
+
+for i in range(32):
+self.check_sve_register_size_and_offset(
+set, 's%i' % (i), 4,

[Lldb-commits] [PATCH] D80700: [lldb] common completion for process pids and process names

2020-08-24 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG19311f5c3e9a: [lldb] common completion for process pids and 
process names (authored by MrHate, committed by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80700

Files:
  lldb/include/lldb/Interpreter/CommandCompletions.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py
  lldb/test/API/functionalities/completion/main.cpp

Index: lldb/test/API/functionalities/completion/main.cpp
===
--- lldb/test/API/functionalities/completion/main.cpp
+++ lldb/test/API/functionalities/completion/main.cpp
@@ -1,3 +1,5 @@
+#include 
+
 class Foo
 {
 public:
@@ -11,14 +13,16 @@
 
 struct Container { int MemberVar; };
 
-int main()
-{
-Foo fooo;
-Foo *ptr_fooo = &fooo;
-fooo.Bar(1, 2);
+int main(int argc, char *argv[]) {
+  if (argc > 1 && std::string(argv[1]) == "-x")
+std::cin.get();
+
+  Foo fooo;
+  Foo *ptr_fooo = &fooo;
+  fooo.Bar(1, 2);
 
-Container container;
-Container *ptr_container = &container;
-int q = Quux();
-return container.MemberVar = 3; // Break here
+  Container container;
+  Container *ptr_container = &container;
+  int q = Quux();
+  return container.MemberVar = 3; // Break here
 }
Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -5,6 +5,8 @@
 
 
 import os
+from multiprocessing import Process
+import psutil
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -117,6 +119,27 @@
 self.complete_from_to('process ' + subcommand + ' mac',
   'process ' + subcommand + ' mach-o-core')
 
+@skipIfRemote
+def test_common_completion_process_pid_and_name(self):
+# The LLDB process itself and the process already attached to are both
+# ignored by the process discovery mechanism, thus we need a process known 
+# to us here.
+self.build()
+server = self.spawnSubprocess(
+self.getBuildArtifact("a.out"),
+["-x"], # Arg "-x" makes the subprocess wait for input thus it won't be terminated too early
+install_remote=False)
+self.assertIsNotNone(server)
+pid = server.pid
+
+self.complete_from_to('process attach -p ', [str(pid)])
+self.complete_from_to('platform process attach -p ', [str(pid)])
+self.complete_from_to('platform process info ', [str(pid)])
+
+pname = psutil.Process(pid).name()  # FIXME: psutil doesn't work for remote
+self.complete_from_to('process attach -n ', [str(pname)])
+self.complete_from_to('platform process attach -n ', [str(pname)])
+
 def test_process_signal(self):
 # The tab completion for "process signal"  won't work without a running process.
 self.complete_from_to('process signal ',
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -1078,9 +1078,9 @@
 { eArgTypePath, "path", CommandCompletions::eDiskFileCompletion, { nullptr, false }, "Path." },
 { eArgTypePermissionsNumber, "perms-numeric", CommandCompletions::eNoCompletion, { nullptr, false }, "Permissions given as an octal number (e.g. 755)." },
 { eArgTypePermissionsString, "perms=string", CommandCompletions::eNoCompletion, { nullptr, false }, "Permissions given as a string value (e.g. rw-r-xr--)." },
-{ eArgTypePid, "pid", CommandCompletions::eNoCompletion, { nullptr, false }, "The process ID number." },
+{ eArgTypePid, "pid", CommandCompletions::eProcessIDCompletion, { nullptr, false }, "The process ID number." },
 { eArgTypePlugin, "plugin", CommandCompletions::eProcessPluginCompletion, { nullptr, false }, "Help text goes here." },
-{ eArgTypeProcessName, "process-name", CommandCompletions::eNoCompletion, { nullptr, false }, "The name of the process." },
+{ eArgTypeProcessName, "process-name", CommandCompletions::eProcessNameCompletion, { nullptr, false }, "The name of the process." },
 { eArgTypePythonClass, "python-class", CommandCompletions::eNoCompletion, { nullptr, false }, "The name of a Python class." },
 { eArgTypePythonFunction, "python-function", CommandCompletions::eNoCompletion, { nullptr, false }, "The name of a Python function." },
 { eArgTypePython

[Lldb-commits] [lldb] 19311f5 - [lldb] common completion for process pids and process names

2020-08-24 Thread Raphael Isemann via lldb-commits

Author: Gongyu Deng
Date: 2020-08-24T17:30:43+02:00
New Revision: 19311f5c3e9ada9d445e49feb7a2ae00ddaee2fa

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

LOG: [lldb] common completion for process pids and process names

1. Added two common completions: `ProcessIDs` and `ProcessNames`, which are
refactored from their original dedicated option completions;
2. Removed the dedicated option completion functions of `process attach` and
`platform process attach`, so that they can use arg-type-bound common
completions instead;
3. Bound `eArgTypePid` to the pid completion, `eArgTypeProcessName` to the
process name completion in `CommandObject.cpp`;
4. Added a related test case.

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandCompletions.h
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/test/API/functionalities/completion/TestCompletion.py
lldb/test/API/functionalities/completion/main.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandCompletions.h 
b/lldb/include/lldb/Interpreter/CommandCompletions.h
index 1d8972e0ca03..0392eaed62c5 100644
--- a/lldb/include/lldb/Interpreter/CommandCompletions.h
+++ b/lldb/include/lldb/Interpreter/CommandCompletions.h
@@ -45,10 +45,12 @@ class CommandCompletions {
 eThreadIndexCompletion = (1u << 17),
 eWatchPointIDCompletion = (1u << 18),
 eBreakpointNameCompletion = (1u << 19),
+eProcessIDCompletion = (1u << 20),
+eProcessNameCompletion = (1u << 21),
 // This item serves two purposes.  It is the last element in the enum, so
 // you can add custom enums starting from here in your Option class. Also
 // if you & in this bit the base code will not process the option.
-eCustomCompletion = (1u << 20)
+eCustomCompletion = (1u << 22)
   };
 
   static bool InvokeCommonCompletionCallbacks(
@@ -110,6 +112,12 @@ class CommandCompletions {
  CompletionRequest &request,
  SearchFilter *searcher);
 
+  static void ProcessIDs(CommandInterpreter &interpreter,
+ CompletionRequest &request, SearchFilter *searcher);
+
+  static void ProcessNames(CommandInterpreter &interpreter,
+   CompletionRequest &request, SearchFilter *searcher);
+
   static void DisassemblyFlavors(CommandInterpreter &interpreter,
  CompletionRequest &request,
  SearchFilter *searcher);

diff  --git a/lldb/source/Commands/CommandCompletions.cpp 
b/lldb/source/Commands/CommandCompletions.cpp
index 109613e223c7..9e74d8ad26cd 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -71,6 +71,8 @@ bool CommandCompletions::InvokeCommonCompletionCallbacks(
   {eThreadIndexCompletion, CommandCompletions::ThreadIndexes},
   {eWatchPointIDCompletion, CommandCompletions::WatchPointIDs},
   {eBreakpointNameCompletion, CommandCompletions::BreakpointNames},
+  {eProcessIDCompletion, CommandCompletions::ProcessIDs},
+  {eProcessNameCompletion, CommandCompletions::ProcessNames},
   {eNoCompletion, nullptr} // This one has to be last in the list.
   };
 
@@ -649,6 +651,33 @@ void 
CommandCompletions::DisassemblyFlavors(CommandInterpreter &interpreter,
   }
 }
 
+void CommandCompletions::ProcessIDs(CommandInterpreter &interpreter,
+CompletionRequest &request,
+SearchFilter *searcher) {
+  lldb::PlatformSP platform_sp(interpreter.GetPlatform(true));
+  if (!platform_sp)
+return;
+  ProcessInstanceInfoList process_infos;
+  ProcessInstanceInfoMatch match_info;
+  platform_sp->FindProcesses(match_info, process_infos);
+  for (const ProcessInstanceInfo &info : process_infos)
+request.TryCompleteCurrentArg(std::to_string(info.GetProcessID()),
+  info.GetNameAsStringRef());
+}
+
+void CommandCompletions::ProcessNames(CommandInterpreter &interpreter,
+  CompletionRequest &request,
+  SearchFilter *searcher) {
+  lldb::PlatformSP platform_sp(interpreter.GetPlatform(true));
+  if (!platform_sp)
+return;
+  ProcessInstanceInfoList process_infos;
+  ProcessInstanceInfoMatch match_info;
+  platform_sp->FindProcesses(match_info, process_infos);
+  for (const ProcessInstanceInfo &info : process_infos)
+request.TryCompleteCurrentArg(info.GetNameAsStringRef());
+}
+
 void CommandCompleti

[Lldb-commits] [lldb] 3cd8d7b - [lldb] Remote disk file/directory completion for platform commands

2020-08-24 Thread Raphael Isemann via lldb-commits

Author: Gongyu Deng
Date: 2020-08-24T17:55:54+02:00
New Revision: 3cd8d7b1727f06a701f41764c1109e5d321284b3

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

LOG: [lldb] Remote disk file/directory completion for platform commands

1. Extended the gdb-remote communication related classes with disk 
file/directory
   completion functions;
2. Added two common completion functions RemoteDiskFiles and
   RemoteDiskDirectories based on the functions above;
3. Added completion for these commands:
   A. platform get-file  ;
   B. platform put-file  ;
   C. platform get-size ;
   D. platform settings -w ;
   E. platform open file .
4. Added related tests for client and server;
5. Updated docs/lldb-platform-packets.txt.

Reviewed By: labath

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

Added: 

lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
lldb/test/API/tools/lldb-server/TestGdbRemoteCompletion.py

Modified: 
lldb/docs/lldb-platform-packets.txt
lldb/include/lldb/Interpreter/CommandCompletions.h
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Removed: 




diff  --git a/lldb/docs/lldb-platform-packets.txt 
b/lldb/docs/lldb-platform-packets.txt
index 23d1cacc5f7e..8d3fed7ab341 100644
--- a/lldb/docs/lldb-platform-packets.txt
+++ b/lldb/docs/lldb-platform-packets.txt
@@ -237,6 +237,27 @@ incompatible with the flags that gdb specifies.
 //  Continues to return the results of the qfProcessInfo.  Once all matches
 //  have been sent, Exx is returned to indicate end of matches.
 
+//--
+// qPathComplete
+//
+// BRIEF
+//   Get a list of matched disk files/directories by passing a boolean flag
+//   and a partial path.
+//
+// EXAMPLE
+//
+//   receive: qPathComplete:0,6d61696e
+//   send:M6d61696e2e637070
+//   receive: qPathComplete:1,746573
+//   send:M746573742f,74657374732f
+//
+//   If the first argument is zero, the result should contain all
+//   files (including directories) starting with the given path. If the
+//   argument is one, the result should contain only directories.
+//
+//   The result should be a comma-separated list of hex-encoded paths.
+//   Paths denoting a directory should end with a directory separator ('/' or 
'\').
+
 //--
 // vFile:size:
 //

diff  --git a/lldb/include/lldb/Interpreter/CommandCompletions.h 
b/lldb/include/lldb/Interpreter/CommandCompletions.h
index 0392eaed62c5..b90e81cb95a8 100644
--- a/lldb/include/lldb/Interpreter/CommandCompletions.h
+++ b/lldb/include/lldb/Interpreter/CommandCompletions.h
@@ -47,10 +47,12 @@ class CommandCompletions {
 eBreakpointNameCompletion = (1u << 19),
 eProcessIDCompletion = (1u << 20),
 eProcessNameCompletion = (1u << 21),
+eRemoteDiskFileCompletion = (1u << 22),
+eRemoteDiskDirectoryCompletion = (1u << 23),
 // This item serves two purposes.  It is the last element in the enum, so
 // you can add custom enums starting from here in your Option class. Also
 // if you & in this bit the base code will not process the option.
-eCustomCompletion = (1u << 22)
+eCustomCompletion = (1u << 24)
   };
 
   static bool InvokeCommonCompletionCallbacks(
@@ -72,6 +74,14 @@ class CommandCompletions {
   StringList &matches,
   TildeExpressionResolver &Resolver);
 
+  static void RemoteDiskFiles(CommandInterpreter &interpreter,
+  CompletionRequest &request,
+  SearchFilter *searcher);
+
+  static void RemoteDiskDirectories(CommandInterpreter &interpreter,
+CompletionRequest &request,
+SearchFilter *searcher);
+
   static void SourceFiles(CommandInterpreter &interpreter,
   CompletionRequest &request, SearchFilter *searcher);
 

diff  --git a/lldb/include/lldb/Target/Platform.h 

[Lldb-commits] [PATCH] D85284: [lldb] Remote disk file/directory completion for platform commands

2020-08-24 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3cd8d7b1727f: [lldb] Remote disk file/directory completion 
for platform commands (authored by MrHate, committed by teemperor).
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D85284?vs=286611&id=287414#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85284

Files:
  lldb/docs/lldb-platform-packets.txt
  lldb/include/lldb/Interpreter/CommandCompletions.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteCompletion.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteCompletion.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteCompletion.py
@@ -0,0 +1,63 @@
+import tempfile
+import gdbremote_testcase
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbgdbserverutils import *
+
+class GdbRemoteCompletionTestCase(gdbremote_testcase.GdbRemoteTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def init_lldb_server(self):
+self.debug_monitor_exe = get_lldb_server_exe()
+if not self.debug_monitor_exe:
+self.skipTest("lldb-server exe not found")
+port_file = tempfile.NamedTemporaryFile().name
+commandline_args = [
+"platform",
+"--listen",
+"*:0",
+"--socket-file",
+port_file
+]
+server = self.spawnSubprocess(
+get_lldb_server_exe(),
+commandline_args,
+install_remote=False)
+self.assertIsNotNone(server)
+self.stub_hostname = "localhost"
+self.port = int(lldbutil.wait_for_file_on_target(self, port_file))
+self.sock = self.create_socket()
+
+self.add_no_ack_remote_stream()
+
+def generate_hex_path(self, target):
+return str(os.path.join(self.getBuildDir(), target)).encode().hex()
+
+@skipIfDarwinEmbedded #  lldb-server tests not updated to work on ios etc yet
+@llgs_test
+def test_autocomplete_path(self):
+self.build()
+self.init_lldb_server()
+
+# Test file-included completion when flag is set to 0.
+self.test_sequence.add_log_lines(
+["read packet: $qPathComplete:0,{}#00".format(
+self.generate_hex_path("main")),
+ "send packet: $M{},{}#00".format(
+self.generate_hex_path("main.d"),
+self.generate_hex_path("main.o"))
+],
+True)
+
+# Test directory-only completion when flag is set to 1.
+os.makedirs(os.path.join(self.getBuildDir(), "test"))
+self.test_sequence.add_log_lines(
+["read packet: $qPathComplete:1,{}#00".format(
+self.generate_hex_path("tes")),
+ "send packet: $M{}{}#00".format(
+self.generate_hex_path("test"),
+os.path.sep.encode().hex()) # "test/" or "test\".
+],
+True)
+
+self.expect_gdbremote_sequence()
Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -178,6 +178,8 @@
 return self.qsProcessInfo()
 if packet.startswith("qfProcessInfo"):
 return self.qfProcessInfo(packet)
+if packet.startswith("qPathComplete:"):
+return self.qPathComplete()
 
 return self.other(packet)
 
@@ -282,6 +284,9 @@
 def qMemoryRegionInfo(self):
 return ""
 
+def qPathComplete(self):
+return ""
+
 """
 Raised when we receive a packet for which there is no default action.
 Override the responder class to implement behavior suitable for the test at
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
=

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

2020-08-24 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

I came to the same conclusion when analyzing 
https://bugs.llvm.org/show_bug.cgi?id=47257 (but you beat me to the punch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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


[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

2020-08-24 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/source/Symbol/Type.cpp:378
 m_byte_size_has_value = true;
+return m_byte_size;
   }

Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this fix 
shows this having this additional state we carry around is error prone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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


[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

2020-08-24 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Sounds good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

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


[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

2020-08-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Symbol/Type.cpp:378
 m_byte_size_has_value = true;
+return m_byte_size;
   }

shafik wrote:
> Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this fix 
> shows this having this additional state we carry around is error prone.
IIRC, this was done this way to reduce sizeof(Type). Note that the external 
interface is Optional and the only place that knows (should know?) 
about this encoding is this function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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


[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

2020-08-24 Thread Davide Italiano via Phabricator via lldb-commits
davide added inline comments.



Comment at: lldb/source/Symbol/Type.cpp:378
 m_byte_size_has_value = true;
+return m_byte_size;
   }

shafik wrote:
> Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this fix 
> shows this having this additional state we carry around is error prone.
This is a much larger project, probably worth considering.
With that in mind, it wouldn't have prevented this bug from happening, as this 
falls through and returns an `Optional` anyway. 
Yay fuzz testing (that found this), I would say.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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


[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

2020-08-24 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/source/Symbol/Type.cpp:378
 m_byte_size_has_value = true;
+return m_byte_size;
   }

labath wrote:
> davide wrote:
> > shafik wrote:
> > > Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this 
> > > fix shows this having this additional state we carry around is error 
> > > prone.
> > This is a much larger project, probably worth considering.
> > With that in mind, it wouldn't have prevented this bug from happening, as 
> > this falls through and returns an `Optional` anyway. 
> > Yay fuzz testing (that found this), I would say.
> IIRC, this was done this way to reduce sizeof(Type). Note that the external 
> interface is Optional and the only place that knows (should know?) 
> about this encoding is this function.
It would have prevented this bug as you just return `m_byte_size` no matter 
what.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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


[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

2020-08-24 Thread Davide Italiano via Phabricator via lldb-commits
davide added a subscriber: teemperor.
davide added a comment.

@labath something we noticed when finding this (and related bugs) is that 
`frame var` carries a decent diagnostic

  (int *) l_125 = 

and the expression parser returns just returns something not particularly 
useful:

  (lldb) p l_125 
  error: :43:31: no member named 'l_125' in namespace 
'$__lldb_local_vars'
  using $__lldb_local_vars::l_125;
^
  error: :1:1: use of undeclared identifier 'l_125'
  l_125

From my testing infrastructure/fuzzing perspective the two are 
indistinguishable, as the script I've written chokes on both, but it would be 
better from an ergonomics point of view if `p` would return something 
meaningful, if possible (even if there's a bug in lldb). Do you think it's 
worth filing a PR? (also, cc: @teemperor for ideas as he spent a fair amount of 
time working on the expression parser)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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


[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

2020-08-24 Thread Davide Italiano via Phabricator via lldb-commits
davide added inline comments.



Comment at: lldb/source/Symbol/Type.cpp:378
 m_byte_size_has_value = true;
+return m_byte_size;
   }

shafik wrote:
> labath wrote:
> > davide wrote:
> > > shafik wrote:
> > > > Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this 
> > > > fix shows this having this additional state we carry around is error 
> > > > prone.
> > > This is a much larger project, probably worth considering.
> > > With that in mind, it wouldn't have prevented this bug from happening, as 
> > > this falls through and returns an `Optional` anyway. 
> > > Yay fuzz testing (that found this), I would say.
> > IIRC, this was done this way to reduce sizeof(Type). Note that the external 
> > interface is Optional and the only place that knows (should 
> > know?) about this encoding is this function.
> It would have prevented this bug as you just return `m_byte_size` no matter 
> what.
If you think it's an improvement, please consider submitting a review, I would 
be eager to look at it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86436

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


[Lldb-commits] [PATCH] D84124: [lldb] type category name common completion

2020-08-24 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG188f1ac301c5: [lldb] type category name common completion 
(authored by MrHate, committed by teemperor).
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D84124?vs=279085&id=287448#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84124

Files:
  lldb/include/lldb/Interpreter/CommandCompletions.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py

Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -468,6 +468,12 @@
 for subcommand in subcommands:
 self.complete_from_to('thread ' + subcommand + ' ', ['1'])
 
+def test_common_completion_type_category_name(self):
+subcommands = ['delete', 'list', 'enable', 'disable', 'define']
+for subcommand in subcommands:
+self.complete_from_to('type category ' + subcommand + ' ', ['default'])
+self.complete_from_to('type filter add -w ', ['default'])
+
 def test_command_argument_completion(self):
 """Test completion of command arguments"""
 self.complete_from_to("watchpoint set variable -", ["-w", "-s"])
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -1068,7 +1068,7 @@
 { eArgTypeLogCategory, "log-category", CommandCompletions::eNoCompletion, { nullptr, false }, "The name of a category within a log channel, e.g. all (try \"log list\" to see a list of all channels and their categories." },
 { eArgTypeLogChannel, "log-channel", CommandCompletions::eNoCompletion, { nullptr, false }, "The name of a log channel, e.g. process.gdb-remote (try \"log list\" to see a list of all channels and their categories)." },
 { eArgTypeMethod, "method", CommandCompletions::eNoCompletion, { nullptr, false }, "A C++ method name." },
-{ eArgTypeName, "name", CommandCompletions::eNoCompletion, { nullptr, false }, "Help text goes here." },
+{ eArgTypeName, "name", CommandCompletions::eTypeCategoryNameCompletion, { nullptr, false }, "Help text goes here." },
 { eArgTypeNewPathPrefix, "new-path-prefix", CommandCompletions::eNoCompletion, { nullptr, false }, "Help text goes here." },
 { eArgTypeNumLines, "num-lines", CommandCompletions::eNoCompletion, { nullptr, false }, "The number of lines to use." },
 { eArgTypeNumberPerLine, "number-per-line", CommandCompletions::eNoCompletion, { nullptr, false }, "The number of items per line to display." },
Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -1769,6 +1769,14 @@
 
   ~CommandObjectTypeCategoryDefine() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(),
+CommandCompletions::eTypeCategoryNameCompletion, request, nullptr);
+  }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 const size_t argc = command.GetArgumentCount();
@@ -1865,6 +1873,14 @@
 
   ~CommandObjectTypeCategoryEnable() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(),
+CommandCompletions::eTypeCategoryNameCompletion, request, nullptr);
+  }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 const size_t argc = command.GetArgumentCount();
@@ -1927,6 +1943,14 @@
 
   ~CommandObjectTypeCategoryDelete() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(),
+CommandCompletions::eTypeCategoryNameCompletion, request, nullptr);
+  }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 const size_t argc = command.GetArgumentCount();
@@ -2032,6 +2056,14 @@
 
   ~CommandObjectTypeCategoryDisable(

[Lldb-commits] [lldb] 188f1ac - [lldb] type category name common completion

2020-08-24 Thread Raphael Isemann via lldb-commits

Author: Gongyu Deng
Date: 2020-08-24T19:54:23+02:00
New Revision: 188f1ac301c5c6da6d2f5697952510fc39cbdd43

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

LOG: [lldb] type category name common completion

1. Added a new common completion TypeCategoryNames to provide a list of 
category names for completion;
2. Applied the completion to these commands: type category 
delete/enable/disable/list/define;
3. Added a related test case;
4. Bound the completion to the arguments of the type 'eArgTypeName'.

Reviewed By: teemperor, JDevlieghere

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandCompletions.h
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectType.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandCompletions.h 
b/lldb/include/lldb/Interpreter/CommandCompletions.h
index b90e81cb95a8..c80bde0e719b 100644
--- a/lldb/include/lldb/Interpreter/CommandCompletions.h
+++ b/lldb/include/lldb/Interpreter/CommandCompletions.h
@@ -49,6 +49,7 @@ class CommandCompletions {
 eProcessNameCompletion = (1u << 21),
 eRemoteDiskFileCompletion = (1u << 22),
 eRemoteDiskDirectoryCompletion = (1u << 23),
+eTypeCategoryNameCompletion = (1u << 24),
 // This item serves two purposes.  It is the last element in the enum, so
 // you can add custom enums starting from here in your Option class. Also
 // if you & in this bit the base code will not process the option.
@@ -146,6 +147,10 @@ class CommandCompletions {
 
   static void WatchPointIDs(CommandInterpreter &interpreter,
 CompletionRequest &request, SearchFilter 
*searcher);
+
+  static void TypeCategoryNames(CommandInterpreter &interpreter,
+CompletionRequest &request,
+SearchFilter *searcher);
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Commands/CommandCompletions.cpp 
b/lldb/source/Commands/CommandCompletions.cpp
index 9221830b6460..0ea6d4288169 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/DataFormatters/DataVisualization.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/CommandCompletions.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -74,7 +75,9 @@ bool CommandCompletions::InvokeCommonCompletionCallbacks(
   {eProcessIDCompletion, CommandCompletions::ProcessIDs},
   {eProcessNameCompletion, CommandCompletions::ProcessNames},
   {eRemoteDiskFileCompletion, CommandCompletions::RemoteDiskFiles},
-  {eRemoteDiskDirectoryCompletion, 
CommandCompletions::RemoteDiskDirectories},
+  {eRemoteDiskDirectoryCompletion,
+   CommandCompletions::RemoteDiskDirectories},
+  {eTypeCategoryNameCompletion, CommandCompletions::TypeCategoryNames},
   {eNoCompletion, nullptr} // This one has to be last in the list.
   };
 
@@ -780,3 +783,14 @@ void CommandCompletions::WatchPointIDs(CommandInterpreter 
&interpreter,
   strm.GetString());
   }
 }
+
+void CommandCompletions::TypeCategoryNames(CommandInterpreter &interpreter,
+   CompletionRequest &request,
+   SearchFilter *searcher) {
+  DataVisualization::Categories::ForEach(
+  [&request](const lldb::TypeCategoryImplSP &category_sp) {
+request.TryCompleteCurrentArg(category_sp->GetName(),
+  category_sp->GetDescription());
+return true;
+  });
+}

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 3cd4ad88afc7..b6af481090d7 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4725,8 +4725,6 @@ class CommandObjectTargetStopHookDelete : public 
CommandObjectParsed {
   void
   HandleArgumentCompletion(CompletionRequest &request,
OptionElementVector &opt_element_vector) override {
-if (request.GetCursorIndex())
-  return;
 CommandCompletions::InvokeCommonCompletionCallbacks(
 GetCommandInterpreter(), CommandCompletions::eStopHookIDCompletion,
 request, nullptr);

diff  --git a/lldb/source/Commands/CommandObjectType.cpp 
b/lldb/source/Commands/CommandObjectType.cpp
index b23f91de0ce6..d820e7abd21f 100644
--- a/l

[Lldb-commits] [PATCH] D86416: [lldb] -stdlib=libc++ for linking with lldb lib also if LLVM_ENABLE_LIBCXX

2020-08-24 Thread Luboš Luňák via Phabricator via lldb-commits
llunak updated this revision to Diff 287451.
llunak added a comment.

Updated to not use environment variable.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D86416

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/lit.site.cfg.py.in

Index: lldb/test/API/lit.site.cfg.py.in
===
--- lldb/test/API/lit.site.cfg.py.in
+++ lldb/test/API/lit.site.cfg.py.in
@@ -12,6 +12,7 @@
 config.lldb_src_root = "@LLDB_SOURCE_DIR@"
 config.lldb_libs_dir = "@LLDB_LIBS_DIR@"
 config.cmake_cxx_compiler = "@CMAKE_CXX_COMPILER@"
+config.libcxx_used = @LLVM_LIBCXX_USED@
 config.host_os = "@HOST_OS@"
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.shared_libs = @LLVM_ENABLE_SHARED_LIBS@
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -188,6 +188,9 @@
 if config.lldb_libs_dir:
   dotest_cmd += ['--lldb-libs-dir', config.lldb_libs_dir]
 
+if config.libcxx_used:
+  dotest_cmd += ['--libcxx-used']
+
 if 'lldb-repro-capture' in config.available_features or \
 'lldb-repro-replay' in config.available_features:
   dotest_cmd += ['--skip-category=lldb-vscode', '--skip-category=std-module']
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1476,6 +1476,8 @@
 stdlibflag = "-stdlib=libc++"
 else:  # this includes NetBSD
 stdlibflag = ""
+if configuration.libcxx_used:
+stdlibflag = "-stdlib=libc++"
 return stdlibflag
 
 def getstdFlag(self):
@@ -1532,12 +1534,13 @@
 """Platform specific way to build a default library. """
 
 stdflag = self.getstdFlag()
+stdlibflag = self.getstdlibFlag()
 
 lib_dir = configuration.lldb_libs_dir
 if self.hasDarwinFramework():
 d = {'DYLIB_CXX_SOURCES': sources,
  'DYLIB_NAME': lib_name,
- 'CFLAGS_EXTRAS': "%s -stdlib=libc++" % stdflag,
+ 'CFLAGS_EXTRAS': "%s %s" % (stdflag, stdlibflag),
  'FRAMEWORK_INCLUDES': "-F%s" % self.framework_dir,
  'LD_EXTRAS': "%s -Wl,-rpath,%s -dynamiclib" % (self.dsym, self.framework_dir),
  }
@@ -1545,19 +1548,21 @@
 d = {
 'DYLIB_CXX_SOURCES': sources,
 'DYLIB_NAME': lib_name,
-'CFLAGS_EXTRAS': "%s -I%s " % (stdflag,
-   os.path.join(
-   os.environ["LLDB_SRC"],
-   "include")),
+'CFLAGS_EXTRAS': "%s %s -I%s " % (stdflag,
+  stdlibflag,
+  os.path.join(
+  os.environ["LLDB_SRC"],
+  "include")),
 'LD_EXTRAS': "-shared -l%s\liblldb.lib" % self.os.environ["LLDB_IMPLIB_DIR"]}
 else:
 d = {
 'DYLIB_CXX_SOURCES': sources,
 'DYLIB_NAME': lib_name,
-'CFLAGS_EXTRAS': "%s -I%s -fPIC" % (stdflag,
-os.path.join(
-os.environ["LLDB_SRC"],
-"include")),
+'CFLAGS_EXTRAS': "%s %s -I%s -fPIC" % (stdflag,
+   stdlibflag,
+   os.path.join(
+   os.environ["LLDB_SRC"],
+   "include")),
 'LD_EXTRAS': "-shared -L%s -llldb -Wl,-rpath,%s" % (lib_dir, lib_dir)}
 if self.TraceOn():
 print(
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -177,6 +177,11 @@
 dest='lldb_libs_dir',
 metavar='path',
 help='The path to LLDB library directory (containing liblldb)')
+group.add_argument(
+'--libcxx-used',
+dest='libcxx_used',
+metavar='libc++ used',
+help='Whether to force use of libc++ when using liblldb')
 group.add

[Lldb-commits] [PATCH] D86416: [lldb] -stdlib=libc++ for linking with lldb lib also if LLVM_ENABLE_LIBCXX

2020-08-24 Thread Luboš Luňák via Phabricator via lldb-commits
llunak marked an inline comment as done.
llunak added a comment.

In D86416#2232966 , @teemperor wrote:

> Out of curiosity, will this also fix the TestPluginCommands.py ?

Yes.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D86416

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


[Lldb-commits] [lldb] 0e6c9a6 - Add hashing of the .text section to ProcessMinidump.

2020-08-24 Thread Greg Clayton via lldb-commits

Author: Greg Clayton
Date: 2020-08-24T11:43:50-07:00
New Revision: 0e6c9a6e7940a2f8ee624358d828acffdb9ccca5

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

LOG: Add hashing of the .text section to ProcessMinidump.

Breakpad will always have a UUID for binaries when it creates minidump files. 
If an ELF files has a GNU build ID, it will use that. If it doesn't, it will 
create one by hashing up to the first 4096 bytes of the .text section. LLDB was 
not able to load these binaries even when we had the right binary because the 
UUID didn't match. LLDB will use the GNU build ID first as the main UUID for a 
binary and fallback onto a 8 byte CRC if a binary doesn't have one. With this 
fix, we will check for the Breakpad hash or the Facebook hash (a modified 
version of the breakpad hash that collides a bit less) and accept binaries when 
these hashes match.

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

Added: 

lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad-overflow.yaml
lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml

lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-breakpad-uuid-match.yaml

lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-facebook-uuid-match.yaml

Modified: 
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp 
b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
index fc8ee346f449..af378ea7741f 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -121,6 +121,72 @@ class PlaceholderObjectFile : public ObjectFile {
   lldb::addr_t m_base;
   lldb::addr_t m_size;
 };
+
+/// Duplicate the HashElfTextSection() from the breakpad sources.
+///
+/// Breakpad, a Google crash log reporting tool suite, creates minidump files
+/// for many 
diff erent architectures. When using Breakpad to create ELF
+/// minidumps, it will check for a GNU build ID when creating a minidump file
+/// and if one doesn't exist in the file, it will say the UUID of the file is a
+/// checksum of up to the first 4096 bytes of the .text section. Facebook also
+/// uses breakpad and modified this hash to avoid collisions so we can
+/// calculate and check for this as well.
+///
+/// The breakpad code might end up hashing up to 15 bytes that immediately
+/// follow the .text section in the file, so this code must do exactly what it
+/// does so we can get an exact match for the UUID.
+///
+/// \param[in] module_sp The module to grab the .text section from.
+///
+/// \param[in/out] breakpad_uuid A vector that will receive the calculated
+///breakpad .text hash.
+///
+/// \param[in/out] facebook_uuid A vector that will receive the calculated
+///facebook .text hash.
+///
+void HashElfTextSection(ModuleSP module_sp, std::vector 
&breakpad_uuid,
+std::vector &facebook_uuid) {
+  SectionList *sect_list = module_sp->GetSectionList();
+  if (sect_list == nullptr)
+return;
+  SectionSP sect_sp = sect_list->FindSectionByName(ConstString(".text"));
+  if (!sect_sp)
+return;
+  constexpr size_t kMDGUIDSize = 16;
+  constexpr size_t kBreakpadPageSize = 4096;
+  // The breakpad code has a bug where it might access beyond the end of a
+  // .text section by up to 15 bytes, so we must ensure we round up to the
+  // next kMDGUIDSize byte boundary.
+  DataExtractor data;
+  const size_t text_size = sect_sp->GetFileSize();
+  const size_t read_size = std::min(
+  llvm::alignTo(text_size, kMDGUIDSize), kBreakpadPageSize);
+  sect_sp->GetObjectFile()->GetData(sect_sp->GetFileOffset(), read_size, data);
+
+  breakpad_uuid.assign(kMDGUIDSize, 0);
+  facebook_uuid.assign(kMDGUIDSize, 0);
+
+  // The only 
diff erence between the breakpad hash and the facebook hash is the
+  // hashing of the text section size into the hash prior to hashing the .text
+  // contents.
+  for (size_t i = 0; i < kMDGUIDSize; i++)
+facebook_uuid[i] ^= text_size % 255;
+
+  // This code carefully duplicates how the hash was created in Breakpad
+  // sources, including the error where it might has an extra 15 bytes past the
+  // end of the .text section if the .text section is less than a page size in
+  // length.
+  const uint8_t *ptr = data.GetDataStart();
+  const uint8_t *ptr_end = data.GetDataEnd();
+  while (ptr < ptr_end) {
+for (unsigned i = 0; i < kMDGUIDSize; i++) {
+  breakpad_uuid[i] ^= ptr[i];
+  facebook_uuid[i] ^= ptr[i];
+}
+ptr += kMDGUIDSize;
+  }
+}
+
 } // namespace
 
 ConstString Proce

[Lldb-commits] [PATCH] D86261: Add hashing of the .text section to ProcessMinidump.

2020-08-24 Thread Greg Clayton via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e6c9a6e7940: Add hashing of the .text section to 
ProcessMinidump. (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86261

Files:
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
  
lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad-overflow.yaml
  lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml
  
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-breakpad-uuid-match.yaml
  
lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-facebook-uuid-match.yaml

Index: lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-facebook-uuid-match.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-facebook-uuid-match.yaml
@@ -0,0 +1,15 @@
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  ARM
+Platform ID: Linux
+CSD Version: '15E216'
+CPU:
+  CPUID:   0x
+  - Type:ModuleList
+Modules:
+  - Base of Image:   0x1000
+Size of Image:   0x1000
+Module Name: '/invalid/path/on/current/system/libbreakpad.so'
+CodeView Record: 52534453141010100410101013101010575e451000
+...
Index: lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-breakpad-uuid-match.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/minidump-new/linux-arm-breakpad-uuid-match.yaml
@@ -0,0 +1,15 @@
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  ARM
+Platform ID: Linux
+CSD Version: '15E216'
+CPU:
+  CPUID:   0x
+  - Type:ModuleList
+Modules:
+  - Base of Image:   0x1000
+Size of Image:   0x1000
+Module Name: '/invalid/path/on/current/system/libbreakpad.so'
+CodeView Record: 52534453040014000300474e55
+...
Index: lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml
@@ -0,0 +1,15 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_DYN
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+Sections:
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x0001
+AddressAlign:0x0004
+Content: 040014000300474E5500
Index: lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad-overflow.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad-overflow.yaml
@@ -0,0 +1,21 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_DYN
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+Sections:
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x0001
+AddressAlign:0x0001
+Content: 04
+  - Name:.data
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_WRITE ]
+Address: 0x00010001
+AddressAlign:0x0001
+Content: 0014000300474E5500
Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -179,6 +179,69 @@
"/invalid/path/on/current/system/libuuidmismatch.so",
"7295E17C-6668-9E05-CBB5-DEE5003865D5")
 
+def test_breakpad_hash_match(self):
+"""
+Breakpad creates minidump files using CvRecord in each module whose
+signature is set to PDB70 where the UUID is a hash generated by
+breakpad of the .text section. This is only done when the
+executable has no ELF build ID.
+
+This test verifies that if we have a minidump with a 16 byte UUID,
+that we are able to associat

[Lldb-commits] [PATCH] D86375: Load correct module for linux and android when duplicates exist in minidump.

2020-08-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In D86375#2233054 , @labath wrote:

> If I correctly understand what this is doing, then I don't think it's a good 
> idea. The base of an (elf) shared library does not have to be mapped 
> executable. These are the mappings I get for a trivial hello world program 
> (no mmapping of libraries or anything) on my linux machine:
>
>   0040-00401000 r--p  fd:01 2838574
> /tmp/l/a.out
>   00401000-00402000 r-xp 1000 fd:01 2838574
> /tmp/l/a.out
>   00402000-00403000 r--p 2000 fd:01 2838574
> /tmp/l/a.out
>   00403000-00404000 r--p 2000 fd:01 2838574
> /tmp/l/a.out
>   00404000-00405000 rw-p 3000 fd:01 2838574
> /tmp/l/a.out
>   020fb000-0211c000 rw-p  00:00 0  
> [heap]
>   7fe4f5d87000-7fe4f5da9000 r--p  fd:01 2738932
> /lib64/libc-2.31.so
>   7fe4f5da9000-7fe4f5ef3000 r-xp 00022000 fd:01 2738932
> /lib64/libc-2.31.so
>   7fe4f5ef3000-7fe4f5f3d000 r--p 0016c000 fd:01 2738932
> /lib64/libc-2.31.so
>   7fe4f5f3d000-7fe4f5f41000 r--p 001b5000 fd:01 2738932
> /lib64/libc-2.31.so
>   7fe4f5f41000-7fe4f5f43000 rw-p 001b9000 fd:01 2738932
> /lib64/libc-2.31.so
>   ...
>
> Here, the correct base of a.out is 0x0040 and the libc base is 
> 0x7fe4f5d87000. But this patch would make them be detected as 0x00401000 and 
> 0x7fe4f5da9000, respectively.

It actually will do the right thing here. We only prefer the executable mapping 
for multiple entries in the minidump modules list. The minidump will have only 
one load address per module and that address will always be the first mapping 
in the linux proc maps file.

So the only entry for "/tmp/l/a.out" in the minidump would be the one for 
0040, and the only mapping for /lib64/libc-2.31.so would be 7fe4f5d87000.

If we have multiple module entries _and_ if you have separate code, the old 
behavior will continue and it will prefer the mapping with the lowest address 
because neither of the binaries would have the base address marked as 
executable. If we have multiple entries for the exact same path, but with 
different addresses, we will prefer the one that _is_ marked executable 
correctly (which happens on Android for us all the time, can could easily 
happen on Linux with "-z no-separate-code".

> This behavior is controlled by the `-z (no)separate-code`. My machine has 
> `separate-code` as default, but that setting may not be universal, so you may 
> need to pass this flag explicitly to reproduce this. For reference, these are 
> the mappings I get when compiling a.out with `-z noseparate-code` (libc 
> mappings remain unchanged, of course):
>
>   0040-00401000 r-xp  fd:01 2838574
> /tmp/l/a.out
>   00401000-00402000 r--p  fd:01 2838574
> /tmp/l/a.out
>   00402000-00403000 rw-p 1000 fd:01 2838574
> /tmp/l/a.out
>
> It sounds like we need a better heuristic. How about "the number of 
> consecutive mappings with the same name"? User mmapping code is likely going 
> to map the library in a single chunk, but the dynamic linker will typically 
> create multiple mappings (even a trivial executable can have five), so it 
> seems like picking the longest sequence could work...

So this heuristic will work like it did before for "-z separate-code", and will 
fix multiple mappings if "-z no-separate-code" is used. So this is only a win 
in my book since we only check the start address for a shared library _and_ we 
only check for the execute part if we have multiple module entries for the same 
file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86375

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


[Lldb-commits] [PATCH] D86388: Fix use-after-free in ThreadPlan, and add test.

2020-08-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

I'm confused as to how this patch actually fixes the problem.  When the thread 
gets removed from the thread list, it should get Destroy called on it - which 
should set m_destroy_called, causing IsValid to return false..  So I am not 
clear under what circumstances FindThreadByID will fail, but the cached thread 
shared pointer's IsValid is still true?  If IsValid is holding true over the 
thread's removal from the thread list, then I'm worried that this change will 
keep us using the old ThreadSP that was reported the next time we stopped and 
this thread ID was represented by a different ThreadSP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86388

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


[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

2020-08-24 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik updated this revision to Diff 287482.
shafik marked 5 inline comments as done.
shafik added a comment.

-Add REQUIRES x86

- Fix-up the comments in the test


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

https://reviews.llvm.org/D86311

Files:
  lldb/source/Core/ValueObjectVariable.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
@@ -0,0 +1,409 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+
+# CHECK: (lldb) target variable constant
+# CHECK: (U) constant = {
+# CHECK:   raw = 1688469761
+# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
+# CHECK: }
+
+# Test we are able to display a variable whose value is given by DW_AT_const_value.
+# Compiling at -O1 allows us to capture this case. Below is the code used
+# to generate the assembly:
+#
+# typedef union
+# {
+#   unsigned raw;
+#   struct
+#   {
+# unsigned a : 8;
+# unsigned b : 8;
+# unsigned c : 6;
+# unsigned d : 2;
+# unsigned e : 6;
+# unsigned f : 2;
+#   } ;
+# } U;
+#
+# static U __attribute__((used)) _type_anchor;
+# static const int constant = 0x64A40101;
+#
+# int g() { return constant; }
+#
+# int main() {
+#   U u;
+#   u.raw = 0x64A40101;
+# }
+#
+# Compiled as follows:
+#
+#   clang -gdwarf-4 -O1 dw_at_const_value_bug.c -S -o dw_at_const_value_bug.s
+#
+# I was not able to obtain a global of type U with DW_AT_const_value but was able
+# to using int. This required modifying the DW_AT_type of constant to be type
+# U. After that stripping as much of the assembly as possible to give us a
+# smaller reproducer.
+
+
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+	.no_dead_strip	__type_anchor
+	.section	__DWARF,__debug_str,regular,debug
+Linfo_string:
+  .zero 90
+	.asciz	"constant"  ## string offset=90
+	.asciz	"int"   ## string offset=99
+	.asciz	"_type_anchor"  ## string offset=103
+	.asciz	"U" ## string offset=116
+	.asciz	"raw"   ## string offset=118
+	.asciz	"unsigned int"  ## string offset=122
+	.asciz	"a" ## string offset=135
+	.asciz	"b" ## string offset=137
+	.asciz	"c" ## string offset=139
+	.asciz	"d" ## string offset=141
+	.asciz	"e" ## string offset=143
+	.asciz	"f" ## string offset=145
+	.asciz	"g" ## string offset=147
+	.asciz	"main"  ## string offset=149
+	.asciz	"u" ## string offset=154
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	66  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\264B" ## DW_AT_GNU_pubnames
+	.byte	25  ## DW_FORM_flag_present
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.byte	17  ## DW_AT_low_pc
+	.byte	1   ## DW_FORM_addr
+	.byte	18  ## DW_AT_high_pc
+	.byte	6   ## DW_FORM_data4
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	52  ## DW_TAG_variable
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	28  ## DW_AT_const_value
+	.byte	15  ## DW_FORM_udata
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	38  ## DW_TAG_const_type
+	.byte	0   ## DW_CHILDREN_no
+	.byte	73 

[Lldb-commits] [PATCH] D86389: [lldb] Add a SymbolFileProvider to record and replay calls to dsymForUUID

2020-08-24 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

Looks good!


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

https://reviews.llvm.org/D86389

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


[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

2020-08-24 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/source/Core/ValueObjectVariable.cpp:136
+if (expr.GetExpressionData(m_data)) {
+   if (m_data.GetDataStart() && m_data.GetByteSize())
+m_value.SetBytes(m_data.GetDataStart(), m_data.GetByteSize());

labath wrote:
> I doubt this check is necessary -- surely we should be able to rely on 
> `GetExpressionData` setting the data extractor to something reasonable if it 
> returns success.
> 
> If anything, it would be nice to make a test with an empty DW_AT_const_value, 
> which checks that lldb does something reasonable. It shouldn't that hard -- 
> copy the DW_TAG_variable abbreviation, change DW_AT_const_value to 
> DW_FORM_block, and make a new variable DIE with an empty block.
I see what you are saying, let me verify this in a separate PR and if that 
bears out then we can remove this check.


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

https://reviews.llvm.org/D86311

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


[Lldb-commits] [PATCH] D86389: [lldb] Add a SymbolFileProvider to record and replay calls to dsymForUUID

2020-08-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa842950b62b6: [lldb] Add a SymbolFileProvider to record and 
replay calls to dsymForUUID (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D86389?vs=287164&id=287508#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86389

Files:
  lldb/include/lldb/Utility/Reproducer.h
  lldb/include/lldb/Utility/ReproducerProvider.h
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Symbol/LocateSymbolFile.cpp
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
  lldb/source/Utility/ReproducerProvider.cpp
  lldb/test/Shell/Reproducer/Inputs/core
  lldb/test/Shell/Reproducer/Inputs/dsymforuuid.sh
  lldb/test/Shell/Reproducer/TestDebugSymbols.test

Index: lldb/test/Shell/Reproducer/TestDebugSymbols.test
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/TestDebugSymbols.test
@@ -0,0 +1,14 @@
+# REQUIRES: system-darwin
+
+# RUN: rm -rf %t.repro
+# RUN: env LLDB_APPLE_DSYMFORUUID_EXECUTABLE=%S/Inputs/dsymforuuid.sh %lldb --capture --capture-path %t.repro -c %S/Inputs/core -o 'reproducer generate'
+
+# RUN: cat %t.repro/symbol-files.yaml | FileCheck %s --check-prefix YAML
+# YAML: AD52358C-94F8-3796-ADD6-B20FFAC00E5C
+# YAML: /path/to/unstripped/executable
+# YAML: /path/to/foo.dSYM/Contents/Resources/DWARF/foo
+
+# RUN: %lldb -b -o 'reproducer dump -p symbol-files -f %t.repro' | FileCheck %s --check-prefix DUMP
+# DUMP: uuid: AD52358C-94F8-3796-ADD6-B20FFAC00E5C
+# DUMP-NEXT: module path: /path/to/unstripped/executable
+# DUMP-NEXT: symbol path: /path/to/foo.dSYM/Contents/Resources/DWARF/foo
Index: lldb/test/Shell/Reproducer/Inputs/dsymforuuid.sh
===
--- /dev/null
+++ lldb/test/Shell/Reproducer/Inputs/dsymforuuid.sh
@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+
+echo ""
+echo "http://www.apple.com/DTDs/PropertyList-1.0.dtd\";>"
+echo ""
+echo ""
+echo "AD52358C-94F8-3796-ADD6-B20FFAC00E5C"
+echo ""
+echo "DBGArchitecture"
+echo "x86_64"
+echo "DBGBuildSourcePath"
+echo "/path/to/build/sources"
+echo "DBGSourcePath"
+echo "/path/to/actual/sources"
+echo "DBGDSYMPath"
+echo "/path/to/foo.dSYM/Contents/Resources/DWARF/foo"
+echo "DBGSymbolRichExecutable"
+echo "/path/to/unstripped/executable"
+echo ""
+echo ""
+echo ""
Index: lldb/source/Utility/ReproducerProvider.cpp
===
--- lldb/source/Utility/ReproducerProvider.cpp
+++ lldb/source/Utility/ReproducerProvider.cpp
@@ -105,6 +105,61 @@
   m_os.flush();
 }
 
+void SymbolFileProvider::AddSymbolFile(const UUID *uuid,
+   const FileSpec &module_file,
+   const FileSpec &symbol_file) {
+  if (!uuid || (!module_file && !symbol_file))
+return;
+  m_symbol_files.emplace_back(uuid->GetAsString(), module_file.GetPath(),
+  symbol_file.GetPath());
+}
+
+void SymbolFileProvider::Keep() {
+  FileSpec file = this->GetRoot().CopyByAppendingPathComponent(Info::file);
+  std::error_code ec;
+  llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text);
+  if (ec)
+return;
+
+  // Remove duplicates.
+  llvm::sort(m_symbol_files.begin(), m_symbol_files.end());
+  m_symbol_files.erase(
+  std::unique(m_symbol_files.begin(), m_symbol_files.end()),
+  m_symbol_files.end());
+
+  llvm::yaml::Output yout(os);
+  yout << m_symbol_files;
+}
+
+SymbolFileLoader::SymbolFileLoader(Loader *loader) {
+  if (!loader)
+return;
+
+  FileSpec file = loader->GetFile();
+  if (!file)
+return;
+
+  auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
+  if (auto err = error_or_file.getError())
+return;
+
+  llvm::yaml::Input yin((*error_or_file)->getBuffer());
+  yin >> m_symbol_files;
+}
+
+std::pair
+SymbolFileLoader::GetPaths(const UUID *uuid) const {
+  if (!uuid)
+return {};
+
+  auto it = std::lower_bound(m_symbol_files.begin(), m_symbol_files.end(),
+ SymbolFileProvider::Entry(uuid->GetAsString()));
+  if (it == m_symbol_files.end())
+return {};
+  return std::make_pair(FileSpec(it->module_path),
+FileSpec(it->symbol_path));
+}
+
 void ProviderBase::anchor() {}
 char CommandProvider::ID = 0;
 char FileProvider::ID = 0;
@@ -113,6 +168,7 @@
 char WorkingDirectoryProvider::ID = 0;
 char HomeDirectoryProvider::ID = 0;
 char ProcessInfoProvider::ID = 0;
+char SymbolFileProvider::ID = 0;
 const char *CommandProvider::Info::file = "command-interpreter.yaml";
 const char *CommandProvider::Info::name = "command-interpreter";
 const char *FileProvider::Info::file = "files.yaml"

[Lldb-commits] [lldb] a842950 - [lldb] Add a SymbolFileProvider to record and replay calls to dsymForUUID

2020-08-24 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-08-24T15:09:08-07:00
New Revision: a842950b62b6d029a392c3c312c6495d6368c2a4

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

LOG: [lldb] Add a SymbolFileProvider to record and replay calls to dsymForUUID

When replaying a reproducer captured from a core file, we always use
dsymForUUID for the kernel binary. When enabled, we also use it to find
kexts. Since these files are already contained in the reproducer,
there's no reason to call out to an external tool. If the tool returns a
different result, e.g. because the dSYM got garbage collected, it will
break reproducer replay. The SymbolFileProvider solves the issue by
mapping UUIDs to module and symbol paths in the reproducer.

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

Added: 
lldb/test/Shell/Reproducer/Inputs/core
lldb/test/Shell/Reproducer/Inputs/dsymforuuid.sh
lldb/test/Shell/Reproducer/TestDebugSymbols.test

Modified: 
lldb/include/lldb/Utility/Reproducer.h
lldb/include/lldb/Utility/ReproducerProvider.h
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/source/Symbol/LocateSymbolFile.cpp
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
lldb/source/Utility/ReproducerProvider.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Reproducer.h 
b/lldb/include/lldb/Utility/Reproducer.h
index 4dc6ddd51394..d3d6589a7ef8 100644
--- a/lldb/include/lldb/Utility/Reproducer.h
+++ b/lldb/include/lldb/Utility/Reproducer.h
@@ -22,6 +22,7 @@
 #include 
 
 namespace lldb_private {
+class UUID;
 namespace repro {
 
 class Reproducer;

diff  --git a/lldb/include/lldb/Utility/ReproducerProvider.h 
b/lldb/include/lldb/Utility/ReproducerProvider.h
index b84b8a67c4ca..abb13f0edd43 100644
--- a/lldb/include/lldb/Utility/ReproducerProvider.h
+++ b/lldb/include/lldb/Utility/ReproducerProvider.h
@@ -12,6 +12,7 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/ProcessInfo.h"
 #include "lldb/Utility/Reproducer.h"
+#include "lldb/Utility/UUID.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileCollector.h"
@@ -205,6 +206,41 @@ class HomeDirectoryProvider : public 
DirectoryProvider {
   static char ID;
 };
 
+/// Provider for mapping UUIDs to symbol and executable files.
+class SymbolFileProvider : public Provider {
+public:
+  SymbolFileProvider(const FileSpec &directory)
+  : Provider(directory), m_symbol_files() {}
+
+  void AddSymbolFile(const UUID *uuid, const FileSpec &module_path,
+ const FileSpec &symbol_path);
+  void Keep() override;
+
+  struct Entry {
+Entry() = default;
+Entry(std::string uuid) : uuid(std::move(uuid)) {}
+Entry(std::string uuid, std::string module_path, std::string symbol_path)
+: uuid(std::move(uuid)), module_path(std::move(module_path)),
+  symbol_path(std::move(symbol_path)) {}
+
+bool operator==(const Entry &rhs) const { return uuid == rhs.uuid; }
+bool operator<(const Entry &rhs) const { return uuid < rhs.uuid; }
+
+std::string uuid;
+std::string module_path;
+std::string symbol_path;
+  };
+
+  struct Info {
+static const char *name;
+static const char *file;
+  };
+  static char ID;
+
+private:
+  std::vector m_symbol_files;
+};
+
 /// The MultiProvider is a provider that hands out recorder which can be used
 /// to capture data for 
diff erent instances of the same object. The recorders
 /// can be passed around or stored as an instance member.
@@ -345,6 +381,16 @@ template  class MultiLoader {
   unsigned m_index = 0;
 };
 
+class SymbolFileLoader {
+public:
+  SymbolFileLoader(Loader *loader);
+  std::pair GetPaths(const UUID *uuid) const;
+
+private:
+  // Sorted list of UUID to path mappings.
+  std::vector m_symbol_files;
+};
+
 /// Helper to read directories written by the DirectoryProvider.
 template 
 llvm::Expected GetDirectoryFrom(repro::Loader *loader) {
@@ -357,4 +403,20 @@ llvm::Expected GetDirectoryFrom(repro::Loader 
*loader) {
 } // namespace repro
 } // namespace lldb_private
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::repro::SymbolFileProvider::Entry)
+
+namespace llvm {
+namespace yaml {
+template <>
+struct MappingTraits {
+  static void mapping(IO &io,
+  lldb_private::repro::SymbolFileProvider::Entry &entry) {
+io.mapRequired("uuid", entry.uuid);
+io.mapRequired("module-path", entry.module_path);
+io.mapRequired("symbol-path", entry.symbol_path);
+  }
+};
+} // namespace yaml
+} // namespace llvm
+
 #endif // LLDB_UTILITY_REPRODUCER_PROVIDER_H

diff  --git a/lldb/source/Commands/CommandObjectReproducer.cpp 
b/lldb/source/Commands/CommandObjectReproducer.cpp
index 9add2df52985..da2d9ca5a901 100644
--- a/lldb/source/Commands/CommandObjectReproduc

[Lldb-commits] [lldb] 93b2551 - [LLDB] Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

2020-08-24 Thread via lldb-commits

Author: shafik
Date: 2020-08-24T15:17:27-07:00
New Revision: 93b255142bb7025f62cf83dd5b7d3b04aab5445b

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

LOG: [LLDB] Fix how ValueObjectVariable handles DW_AT_const_value when the 
DWARFExpression holds the data that represents a constant value

In some cases when we have a DW_AT_const_value and the data can be found in the
DWARFExpression then ValueObjectVariable does not handle it properly and we end
up with an extracting data from value failed error.

The test is a very stripped down assembly file since reproducing this relies on 
the results of compiling with -O1 which may not be stable over time.

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s

Modified: 
lldb/source/Core/ValueObjectVariable.cpp

Removed: 




diff  --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index ab67e3038cf0..43e888a68725 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -132,8 +132,11 @@ bool ValueObjectVariable::UpdateValue() {
   if (variable->GetLocationIsConstantValueData()) {
 // expr doesn't contain DWARF bytes, it contains the constant variable
 // value bytes themselves...
-if (expr.GetExpressionData(m_data))
+if (expr.GetExpressionData(m_data)) {
+   if (m_data.GetDataStart() && m_data.GetByteSize())
+m_value.SetBytes(m_data.GetDataStart(), m_data.GetByteSize());
   m_value.SetContext(Value::eContextTypeVariable, variable);
+}
 else
   m_error.SetErrorString("empty constant data");
 // constant bytes can't be edited - sorry

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s 
b/lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
new file mode 100644
index ..09c369ff083a
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
@@ -0,0 +1,409 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+
+# CHECK: (lldb) target variable constant
+# CHECK: (U) constant = {
+# CHECK:   raw = 1688469761
+# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
+# CHECK: }
+
+# Test we are able to display a variable whose value is given by 
DW_AT_const_value.
+# Compiling at -O1 allows us to capture this case. Below is the code used
+# to generate the assembly:
+#
+# typedef union
+# {
+#   unsigned raw;
+#   struct
+#   {
+# unsigned a : 8;
+# unsigned b : 8;
+# unsigned c : 6;
+# unsigned d : 2;
+# unsigned e : 6;
+# unsigned f : 2;
+#   } ;
+# } U;
+#
+# static U __attribute__((used)) _type_anchor;
+# static const int constant = 0x64A40101;
+#
+# int g() { return constant; }
+#
+# int main() {
+#   U u;
+#   u.raw = 0x64A40101;
+# }
+#
+# Compiled as follows:
+#
+#   clang -gdwarf-4 -O1 dw_at_const_value_bug.c -S -o dw_at_const_value_bug.s
+#
+# I was not able to obtain a global of type U with DW_AT_const_value but was 
able
+# to using int. This required modifying the DW_AT_type of constant to be type
+# U. After that stripping as much of the assembly as possible to give us a
+# smaller reproducer.
+
+
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+   .no_dead_strip  __type_anchor
+   .section__DWARF,__debug_str,regular,debug
+Linfo_string:
+  .zero 90
+   .asciz  "constant"  ## string offset=90
+   .asciz  "int"   ## string offset=99
+   .asciz  "_type_anchor"  ## string offset=103
+   .asciz  "U" ## string offset=116
+   .asciz  "raw"   ## string offset=118
+   .asciz  "unsigned int"  ## string offset=122
+   .asciz  "a" ## string offset=135
+   .asciz  "b" ## string offset=137
+   .asciz  "c" ## string offset=139
+   .asciz  "d" ## string offset=141
+   .asciz  "e" ## string offset=143
+   .asciz  "f" ## string offset=145
+   .asciz  "g" ## string offset=147
+   .asciz  "main"  ## string offset=149
+   .asciz  "u" ## string offset=154
+   .section__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+   .byte   1   ## Abbreviation Code
+   .byte   17  ## DW_TAG_compile_unit
+   .byte   1   ## DW_CHILDREN_yes
+   .byte   37  ## DW_AT_producer
+   .byte   14  ## DW_FORM_strp
+   .byte   19  

[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

2020-08-24 Thread Shafik Yaghmour via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG93b255142bb7: [LLDB] Fix how ValueObjectVariable handles 
DW_AT_const_value when the… (authored by shafik).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86311

Files:
  lldb/source/Core/ValueObjectVariable.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
@@ -0,0 +1,409 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+
+# CHECK: (lldb) target variable constant
+# CHECK: (U) constant = {
+# CHECK:   raw = 1688469761
+# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
+# CHECK: }
+
+# Test we are able to display a variable whose value is given by DW_AT_const_value.
+# Compiling at -O1 allows us to capture this case. Below is the code used
+# to generate the assembly:
+#
+# typedef union
+# {
+#   unsigned raw;
+#   struct
+#   {
+# unsigned a : 8;
+# unsigned b : 8;
+# unsigned c : 6;
+# unsigned d : 2;
+# unsigned e : 6;
+# unsigned f : 2;
+#   } ;
+# } U;
+#
+# static U __attribute__((used)) _type_anchor;
+# static const int constant = 0x64A40101;
+#
+# int g() { return constant; }
+#
+# int main() {
+#   U u;
+#   u.raw = 0x64A40101;
+# }
+#
+# Compiled as follows:
+#
+#   clang -gdwarf-4 -O1 dw_at_const_value_bug.c -S -o dw_at_const_value_bug.s
+#
+# I was not able to obtain a global of type U with DW_AT_const_value but was able
+# to using int. This required modifying the DW_AT_type of constant to be type
+# U. After that stripping as much of the assembly as possible to give us a
+# smaller reproducer.
+
+
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+	.no_dead_strip	__type_anchor
+	.section	__DWARF,__debug_str,regular,debug
+Linfo_string:
+  .zero 90
+	.asciz	"constant"  ## string offset=90
+	.asciz	"int"   ## string offset=99
+	.asciz	"_type_anchor"  ## string offset=103
+	.asciz	"U" ## string offset=116
+	.asciz	"raw"   ## string offset=118
+	.asciz	"unsigned int"  ## string offset=122
+	.asciz	"a" ## string offset=135
+	.asciz	"b" ## string offset=137
+	.asciz	"c" ## string offset=139
+	.asciz	"d" ## string offset=141
+	.asciz	"e" ## string offset=143
+	.asciz	"f" ## string offset=145
+	.asciz	"g" ## string offset=147
+	.asciz	"main"  ## string offset=149
+	.asciz	"u" ## string offset=154
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	66  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\264B" ## DW_AT_GNU_pubnames
+	.byte	25  ## DW_FORM_flag_present
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.byte	17  ## DW_AT_low_pc
+	.byte	1   ## DW_FORM_addr
+	.byte	18  ## DW_AT_high_pc
+	.byte	6   ## DW_FORM_data4
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	52  ## DW_TAG_variable
+	.byte	0   ## DW_CHILDREN_no
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	28  ## DW_AT_const_value
+	.byte	15  ## DW_FORM_udata
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	38   

[Lldb-commits] [PATCH] D86493: Remove unused/misnamed SetObjectModificationTime

2020-08-24 Thread Dave Lee via Phabricator via lldb-commits
kastiglione created this revision.
kastiglione added reviewers: clayborg, friss, JDevlieghere, aprantl.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
kastiglione requested review of this revision.

Remove `SetObjectModificationTime` which is not currently used, and assigns to 
the wrong member.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86493

Files:
  lldb/include/lldb/Core/Module.h


Index: lldb/include/lldb/Core/Module.h
===
--- lldb/include/lldb/Core/Module.h
+++ lldb/include/lldb/Core/Module.h
@@ -506,10 +506,6 @@
 return m_object_mod_time;
   }
 
-  void SetObjectModificationTime(const llvm::sys::TimePoint<> &mod_time) {
-m_mod_time = mod_time;
-  }
-
   /// This callback will be called by SymbolFile implementations when
   /// parsing a compile unit that contains SDK information.
   /// \param sysroot will be added to the path remapping dictionary.


Index: lldb/include/lldb/Core/Module.h
===
--- lldb/include/lldb/Core/Module.h
+++ lldb/include/lldb/Core/Module.h
@@ -506,10 +506,6 @@
 return m_object_mod_time;
   }
 
-  void SetObjectModificationTime(const llvm::sys::TimePoint<> &mod_time) {
-m_mod_time = mod_time;
-  }
-
   /// This callback will be called by SymbolFile implementations when
   /// parsing a compile unit that contains SDK information.
   /// \param sysroot will be added to the path remapping dictionary.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

2020-08-24 Thread walter erquinigo via Phabricator via lldb-commits
wallace marked 12 inline comments as done.
wallace added inline comments.



Comment at: lldb/source/Target/Trace.cpp:109
+  StringRef load_address_str;
+  if (!module->GetValueForKeyAsString("loadAddress", load_address_str))
+return SetMissingFieldError(error, "loadAddress", "string", *module);

wallace wrote:
> clayborg wrote:
> > clayborg wrote:
> > > does JSON typically use camel case? Should this be "load-address"?
> > Should load address be encoded as an integer to begin with? I know it is 
> > less readable as an integer since we can't use hex numbers It would 
> > simplify the logic here. If we want to use strings, we should make a helper 
> > function that decodes an address from a key's value in a dictionary so we 
> > can re-use elsewhere.
> yes, it's camel case typically
The way this is implemented is that it's expecting a string number in any 
radix.  You can pass "123123" or "0xFFAABBFF" for example. You cannot pass it 
directly as a simple number because JSON doesn't support 64-bit integers



Comment at: lldb/source/Target/Trace.cpp:109-110
+  StringRef load_address_str;
+  if (!module->GetValueForKeyAsString("loadAddress", load_address_str))
+return SetMissingFieldError(error, "loadAddress", "string", *module);
+  addr_t load_address;

clayborg wrote:
> clayborg wrote:
> > does JSON typically use camel case? Should this be "load-address"?
> Should load address be encoded as an integer to begin with? I know it is less 
> readable as an integer since we can't use hex numbers It would simplify the 
> logic here. If we want to use strings, we should make a helper function that 
> decodes an address from a key's value in a dictionary so we can re-use 
> elsewhere.
yes, it's camel case typically


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

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


[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

2020-08-24 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 287518.
wallace added a comment.

- Addressed comments.
- Added schemas for both the base class and the implementation plugins.
- Added two tests checking the error messages along with the schema when 
parsing failed.
- Made some general clean up of the code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectTrace.h
  lldb/source/Commands/Options.td
  lldb/source/Core/PluginManager.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/Trace/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Trace.cpp
  lldb/source/Utility/StructuredData.cpp
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/intelpt-trace/3842849.trace
  lldb/test/API/commands/trace/intelpt-trace/a.out
  lldb/test/API/commands/trace/intelpt-trace/main.cpp
  lldb/test/API/commands/trace/intelpt-trace/trace.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
@@ -0,0 +1,12 @@
+{
+  "plugin": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": []
+}
Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad.json
@@ -0,0 +1,15 @@
+{
+  "plugin": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "triple": "x86_64-*-linux",
+  "processes": [
+123
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace/trace.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace.json
@@ -0,0 +1,30 @@
+{
+  "plugin": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "triple": "x86_64-*-linux",
+  "processes": [
+{
+  "pid": 1234,
+  "threads": [
+{
+  "tid": 5678,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/main.cpp
@@ -0,0 +1,8 @@
+int main() {
+  int ret = 0;
+
+  for (int i = 0; i < 4; i++)
+ret ^= 1;
+
+  return ret;
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -0,0 +1,126 @@
+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 testLoadTrace(self):
+src_dir = self.getSourceDir()
+trace_definition_file = os.path.join(src_dir, "intelpt-trace", "trace.json")
+self.expect("trace load -v " + trace_definition_file, substrs=["intel-pt"])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertEqual(process.GetProcessID(), 1234)
+
+self.assertEqual(process.GetNumThreads(), 1)
+self.assertEqual(process.GetThreadAtIndex(0).GetThreadID(), 5678)
+
+self.assertEqual(target.GetNumModules(), 1)
+module = target.GetModuleAtIndex(0)
+path = module.GetFileSpec()
+self.assertEqual(path.fullpath, os.path.join(src_dir, "intelpt-trace", "a.out"))
+self.assertGreater(module.GetNumSections(), 0)
+self.assertEqual

[Lldb-commits] [lldb] 4283320 - [LLDB] Fix SVE offset calculation in NativeRegisterContextLinux_arm64

2020-08-24 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2020-08-25T03:54:41+05:00
New Revision: 4283320b7286dc94367b22df09499dc934e1fbf9

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

LOG: [LLDB] Fix SVE offset calculation in NativeRegisterContextLinux_arm64

There was typo left from changes in CalculateSVEOffset where we moved
FPSR/FPCR offset calculation into WriteRegister and ReadRegister.

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

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index f7b398ce620d..0aef36c7e231 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -118,7 +118,7 @@ NativeRegisterContextLinux_arm64::ReadRegister(const 
RegisterInfo *reg_info,
: "");
 
   uint8_t *src;
-  uint32_t offset;
+  uint32_t offset = LLDB_INVALID_INDEX32;
   uint64_t sve_vg;
   std::vector sve_reg_non_live;
 
@@ -172,7 +172,6 @@ NativeRegisterContextLinux_arm64::ReadRegister(const 
RegisterInfo *reg_info,
 offset = CalculateSVEOffset(GetRegisterInfoAtIndex(sve_reg_num));
   }
 
-  offset = CalculateSVEOffset(reg_info);
   assert(offset < GetSVEBufferSize());
   src = (uint8_t *)GetSVEBuffer() + offset;
 }
@@ -234,7 +233,7 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
: "");
 
   uint8_t *dst;
-  uint32_t offset;
+  uint32_t offset = LLDB_INVALID_INDEX32;
   std::vector sve_reg_non_live;
 
   if (IsGPR(reg)) {
@@ -291,7 +290,6 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
 offset = CalculateSVEOffset(GetRegisterInfoAtIndex(sve_reg_num));
   }
 
-  offset = CalculateSVEOffset(reg_info);
   assert(offset < GetSVEBufferSize());
   dst = (uint8_t *)GetSVEBuffer() + offset;
   ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);



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


[Lldb-commits] [PATCH] D86497: [lldb] Add reproducer verifier

2020-08-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: LLDB, labath.
Herald added subscribers: llvm-commits, dang, hiraditya.
Herald added a project: LLVM.
JDevlieghere requested review of this revision.

Add a reproducer verifier that catches:

- Missing or invalid home directory
- Missing or invalid working directory
- Missing or invalid module/symbol paths
- Missing files from the VFS


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D86497

Files:
  lldb/include/lldb/Utility/ReproducerProvider.h
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/Options.td
  lldb/source/Utility/ReproducerProvider.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp

Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -1159,6 +1159,17 @@
   return ExternalContentsPrefixDir;
 }
 
+void RedirectingFileSystem::setFallthrough(bool Fallthrough) {
+  IsFallthrough = Fallthrough;
+}
+
+std::vector RedirectingFileSystem::getRoots() const {
+  std::vector R;
+  for (const auto &Root : Roots)
+R.push_back(Root->getName());
+  return R;
+}
+
 void RedirectingFileSystem::dump(raw_ostream &OS) const {
   for (const auto &Root : Roots)
 dumpEntry(OS, Root.get());
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -749,6 +749,10 @@
 
   StringRef getExternalContentsPrefixDir() const;
 
+  void setFallthrough(bool Fallthrough);
+
+  std::vector getRoots() const;
+
   void dump(raw_ostream &OS) const;
   void dumpEntry(raw_ostream &OS, Entry *E, int NumSpaces = 0) const;
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
Index: lldb/source/Utility/ReproducerProvider.cpp
===
--- lldb/source/Utility/ReproducerProvider.cpp
+++ lldb/source/Utility/ReproducerProvider.cpp
@@ -9,6 +9,7 @@
 #include "lldb/Utility/ReproducerProvider.h"
 #include "lldb/Utility/ProcessInfo.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace lldb_private;
@@ -160,6 +161,87 @@
 FileSpec(it->symbol_path));
 }
 
+void Verifier::Verify(
+std::function error_callback,
+std::function warning_callback) const {
+  if (!m_loader) {
+error_callback("invalid loader");
+return;
+  }
+
+  FileSpec vfs_mapping = m_loader->GetFile();
+  ErrorOr> buffer =
+  vfs::getRealFileSystem()->getBufferForFile(vfs_mapping.GetPath());
+  if (!buffer) {
+error_callback("unable to read files: " + buffer.getError().message());
+return;
+  }
+
+  IntrusiveRefCntPtr vfs = vfs::getVFSFromYAML(
+  std::move(buffer.get()), nullptr, vfs_mapping.GetPath());
+  if (!vfs) {
+error_callback("unable to initialize the virtual file system");
+return;
+  }
+
+  auto &redirecting_vfs = static_cast(*vfs);
+  redirecting_vfs.setFallthrough(false);
+
+  llvm::Expected working_dir =
+  GetDirectoryFrom(m_loader);
+  if (working_dir) {
+if (!vfs->exists(*working_dir))
+  warning_callback("working directory '" + *working_dir + "' not in VFS");
+vfs->setCurrentWorkingDirectory(*working_dir);
+  } else {
+warning_callback("no working directory in reproducer: " +
+ toString(working_dir.takeError()));
+  }
+
+  llvm::Expected home_dir =
+  GetDirectoryFrom(m_loader);
+  if (home_dir) {
+if (!vfs->exists(*working_dir))
+  warning_callback("home directory '" + *working_dir + "' not in VFS");
+  } else {
+warning_callback("no home directory in reproducer: " +
+ toString(working_dir.takeError()));
+  }
+
+  Expected symbol_files =
+  m_loader->LoadBuffer();
+  if (symbol_files) {
+std::vector entries;
+llvm::yaml::Input yin(*symbol_files);
+yin >> entries;
+for (const auto &entry : entries) {
+  if (!entry.module_path.empty() && !vfs->exists(entry.module_path)) {
+warning_callback("'" + entry.module_path + "': module path for " +
+ entry.uuid + " not in VFS");
+  }
+  if (!entry.symbol_path.empty() && !vfs->exists(entry.symbol_path)) {
+warning_callback("'" + entry.symbol_path + "': symbol path for " +
+ entry.uuid + " not in VFS");
+  }
+}
+  } else {
+llvm::consumeError(symbol_files.takeError());
+  }
+
+  std::vector roots = redirecting_vfs.getRoots();
+  for (llvm::StringRef root : roots) {
+std::error_code ec;
+vfs::recursive_directory_iterator iter(*vfs, root, ec);
+vfs::recursive_directory_iterator end;
+for (; iter != end && !ec; iter.increment(ec)) {
+  ErrorOr status = vfs->status(ite

[Lldb-commits] [PATCH] D86497: [lldb] Add reproducer verifier

2020-08-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

This will require a test and maybe some code deduplication in 
`CommandObjectReproducer` but I already wanted to put the patch up in case I 
can't get to that today.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D86497

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


[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

2020-08-24 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 287527.
wallace added a comment.

- Added a command that shows the schema of a given trace plugin.
- Added a test for such command


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectTrace.h
  lldb/source/Commands/Options.td
  lldb/source/Core/PluginManager.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/Trace/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Trace.cpp
  lldb/source/Utility/StructuredData.cpp
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/TestTraceSchema.py
  lldb/test/API/commands/trace/intelpt-trace/3842849.trace
  lldb/test/API/commands/trace/intelpt-trace/a.out
  lldb/test/API/commands/trace/intelpt-trace/main.cpp
  lldb/test/API/commands/trace/intelpt-trace/trace.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
@@ -0,0 +1,12 @@
+{
+  "plugin": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": []
+}
Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad.json
@@ -0,0 +1,15 @@
+{
+  "plugin": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "triple": "x86_64-*-linux",
+  "processes": [
+123
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace/trace.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace.json
@@ -0,0 +1,30 @@
+{
+  "plugin": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "triple": "x86_64-*-linux",
+  "processes": [
+{
+  "pid": 1234,
+  "threads": [
+{
+  "tid": 5678,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/main.cpp
@@ -0,0 +1,8 @@
+int main() {
+  int ret = 0;
+
+  for (int i = 0; i < 4; i++)
+ret ^= 1;
+
+  return ret;
+}
Index: lldb/test/API/commands/trace/TestTraceSchema.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceSchema.py
@@ -0,0 +1,52 @@
+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 testSchema(self):
+schema = """{
+  "plugin": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel" | "unknown",
+  "family": integer,
+  "model": integer,
+  "stepping": integer
+}
+  },
+  "triple": string, // llvm-triple
+  "processes": [
+{
+  "pid": integer,
+  "threads": [
+{
+  "tid": integer,
+  "traceFile": string // path to trace file relative to the settings file
+}
+  ],
+  "modules": [
+{
+  "file": string, // path to trace file relative to the settings file
+  "loadAddress": string, // address in hex or decimal form
+  "uuid"?: string,
+}
+  ]
+}
+  ]
+}"""
+self.expect("trace schema intel-pt", substrs=[schema])
+
+def testInvalidPluginSchema(self):
+self.expect("trace schema invalid-plugin", error=True,
+ 

[Lldb-commits] [PATCH] D82866: [LLDB] Test SVE dynamic resize with multiple threads

2020-08-24 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid updated this revision to Diff 287526.
omjavaid added a comment.

This update changes this test;s dependence on register offset API.


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

https://reviews.llvm.org/D82866

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
@@ -0,0 +1,102 @@
+#include 
+#include 
+
+static inline void write_sve_registers() {
+  asm volatile("setffr\n\t");
+  asm volatile("ptrue p0.b\n\t");
+  asm volatile("ptrue p1.h\n\t");
+  asm volatile("ptrue p2.s\n\t");
+  asm volatile("ptrue p3.d\n\t");
+  asm volatile("pfalse p4.b\n\t");
+  asm volatile("ptrue p5.b\n\t");
+  asm volatile("ptrue p6.h\n\t");
+  asm volatile("ptrue p7.s\n\t");
+  asm volatile("ptrue p8.d\n\t");
+  asm volatile("pfalse p9.b\n\t");
+  asm volatile("ptrue p10.b\n\t");
+  asm volatile("ptrue p11.h\n\t");
+  asm volatile("ptrue p12.s\n\t");
+  asm volatile("ptrue p13.d\n\t");
+  asm volatile("pfalse p14.b\n\t");
+  asm volatile("ptrue p15.b\n\t");
+
+  asm volatile("cpy  z0.b, p0/z, #1\n\t");
+  asm volatile("cpy  z1.b, p5/z, #2\n\t");
+  asm volatile("cpy  z2.b, p10/z, #3\n\t");
+  asm volatile("cpy  z3.b, p15/z, #4\n\t");
+  asm volatile("cpy  z4.b, p0/z, #5\n\t");
+  asm volatile("cpy  z5.b, p5/z, #6\n\t");
+  asm volatile("cpy  z6.b, p10/z, #7\n\t");
+  asm volatile("cpy  z7.b, p15/z, #8\n\t");
+  asm volatile("cpy  z8.b, p0/z, #9\n\t");
+  asm volatile("cpy  z9.b, p5/z, #10\n\t");
+  asm volatile("cpy  z10.b, p10/z, #11\n\t");
+  asm volatile("cpy  z11.b, p15/z, #12\n\t");
+  asm volatile("cpy  z12.b, p0/z, #13\n\t");
+  asm volatile("cpy  z13.b, p5/z, #14\n\t");
+  asm volatile("cpy  z14.b, p10/z, #15\n\t");
+  asm volatile("cpy  z15.b, p15/z, #16\n\t");
+  asm volatile("cpy  z16.b, p0/z, #17\n\t");
+  asm volatile("cpy  z17.b, p5/z, #18\n\t");
+  asm volatile("cpy  z18.b, p10/z, #19\n\t");
+  asm volatile("cpy  z19.b, p15/z, #20\n\t");
+  asm volatile("cpy  z20.b, p0/z, #21\n\t");
+  asm volatile("cpy  z21.b, p5/z, #22\n\t");
+  asm volatile("cpy  z22.b, p10/z, #23\n\t");
+  asm volatile("cpy  z23.b, p15/z, #24\n\t");
+  asm volatile("cpy  z24.b, p0/z, #25\n\t");
+  asm volatile("cpy  z25.b, p5/z, #26\n\t");
+  asm volatile("cpy  z26.b, p10/z, #27\n\t");
+  asm volatile("cpy  z27.b, p15/z, #28\n\t");
+  asm volatile("cpy  z28.b, p0/z, #29\n\t");
+  asm volatile("cpy  z29.b, p5/z, #30\n\t");
+  asm volatile("cpy  z30.b, p10/z, #31\n\t");
+  asm volatile("cpy  z31.b, p15/z, #32\n\t");
+}
+
+void *thread_func(void *x_void_ptr) {
+  if (*((int *)x_void_ptr) == 0) {
+prctl(PR_SVE_SET_VL, 8 * 4);
+write_sve_registers();
+write_sve_registers(); // Thread X breakpoint 1
+return NULL;   // Thread X breakpoint 2
+  }
+
+  if (*((int *)x_void_ptr) == 1) {
+prctl(PR_SVE_SET_VL, 8 * 2);
+write_sve_registers();
+write_sve_registers(); // Thread Y breakpoint 1
+return NULL;   // Thread Y breakpoint 2
+  }
+
+  return NULL;
+}
+
+int main() {
+  /* this variable is our reference to the second thread */
+  pthread_t x_thread, y_thread;
+
+  int x = 0, y = 1;
+
+  /* Set vector length to 8 and write SVE registers values */
+  prctl(PR_SVE_SET_VL, 8 * 8);
+  write_sve_registers();
+
+  /* create a second thread which executes with argument x */
+  if (pthread_create(&x_thread, NULL, thread_func, &x)) // Break in main thread
+return 1;
+
+  /* create a second thread which executes with argument y */
+  if (pthread_create(&y_thread, NULL, thread_func, &y))
+return 1;
+
+  /* wait for the first thread to finish */
+  if (pthread_join(x_thread, NULL))
+return 2;
+
+  /* wait for the second thread to finish */
+  if (pthread_join(y_thread, NULL))
+return 2;
+
+  return 0;
+}
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -0,0 +1,161 @@
+"""
+Test the AArch64 SVE registers dynamic resize with multiple threads.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class RegisterCommandsTestCase(TestBase):
+
+def targetHasSVE(self):
+triple = self.dbg.GetSelectedPlatform().GetTriple()
+
+# TODO other platforms, please implem

[Lldb-commits] [PATCH] D82865: [LLDB] Add GetByteOffset to SBValue interface for reading register offset

2020-08-24 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment.

In D82865#2225978 , @labath wrote:

> The new API looks fairly confusing. This doesn't sound like something that 
> the SB users should generally know or care about. What would the 
> GetByteOffset function return in the generic case?
> I don't find the explanation in the summary convincing. Presumably, you 
> should be able to verify the proper behavior by checking the data (and its 
> size) that the SBValue object holds. If that returns the right thing, who 
> cares about the value of the offset field?

This was something which helped during development so I thought about 
implementing tests based on this but as you highlighted I wasnt sure if there 
is a proper client for this API function. I am gonna abandon this change and 
have updated test implementation removed dependence on GetByteOffset.


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

https://reviews.llvm.org/D82865

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


[Lldb-commits] [PATCH] D86497: [lldb] Add reproducer verifier

2020-08-24 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectReproducer.cpp:670
+auto error_callback = [&](llvm::StringRef error) {
+  result.AppendError(error);
+};

Is this lambda missing an assignment to `errors`?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D86497

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


[Lldb-commits] [PATCH] D82064: [ARM64] Add QEMU testing environment setup guide for SVE testing

2020-08-24 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment.

In D82064#2223991 , @JDevlieghere 
wrote:

> I didn't look at the scripts in detail but I have a few general (high level) 
> comments:
>
> - I also feel this should go under `lldb/scripts`.
> - Can we put the documentations as RST under `lldb/docs` so it's rendered on 
> the website? We already have bunch of developer resources there. That will 
> certainly help with discovery.
> - LLVM generally encourages scripts to be written in Python. While it doesn't 
> look like this will work out of the box on Windows, I can imagine someone 
> might be interested in that down the line. I also think the bar is lower for 
> new contributors to write Python than bash. I'm kind of on the fence here 
> because there's a reasonable balance between just running commands and 
> control flow, but then I've been scarred by the `build-script-impl` disaster 
> in Swift so I lean towards better safe than sorry.

These scripts fairly optional and are more like developer help for someone who 
wants to give a quick test to any of LLVM tools on some target in absence of 
hardware and I agree someone may wanna use them on windows and quite likely I 
might be that someone in very near future. But I ll hold moving to python for 
now. I ll write an rst doc and arrange the scripts under lldb/scripts as 
suggested.


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

https://reviews.llvm.org/D82064

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


[Lldb-commits] [PATCH] D86497: [lldb] Add reproducer verifier

2020-08-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/source/Commands/CommandObjectReproducer.cpp:670
+auto error_callback = [&](llvm::StringRef error) {
+  result.AppendError(error);
+};

kastiglione wrote:
> Is this lambda missing an assignment to `errors`?
Good catch, it is indeed. 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D86497

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


[Lldb-commits] [lldb] 2501e91 - [lldb] Don't depend on psutil in TestCompletion.py

2020-08-24 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-08-25T08:30:33+02:00
New Revision: 2501e911a5a174fc1a07a2a1ac687a2bf0f05ef3

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

LOG: [lldb] Don't depend on psutil in TestCompletion.py

psutil isn't reall a dependency of the test suite so this shouldn't be
unconditionally be imported here. Instead just check for the process name
by looking for the "a.out" string to get the bots green again.

Added: 


Modified: 
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py 
b/lldb/test/API/functionalities/completion/TestCompletion.py
index e2b003219a00..b80594b7568b 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -6,7 +6,6 @@
 
 import os
 from multiprocessing import Process
-import psutil
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -119,6 +118,18 @@ def test_process_plugin_completion(self):
 self.complete_from_to('process ' + subcommand + ' mac',
   'process ' + subcommand + ' mach-o-core')
 
+def completions_contain_str(self, input, needle):
+interp = self.dbg.GetCommandInterpreter()
+match_strings = lldb.SBStringList()
+num_matches = interp.HandleCompletion(input, len(input), 0, -1, 
match_strings)
+found_needle = False
+for match in match_strings:
+  if needle in match:
+found_needle = True
+break
+self.assertTrue(found_needle, "Returned completions: " + 
"\n".join(match_strings))
+
+
 @skipIfRemote
 def test_common_completion_process_pid_and_name(self):
 # The LLDB process itself and the process already attached to are both
@@ -136,9 +147,8 @@ def test_common_completion_process_pid_and_name(self):
 self.complete_from_to('platform process attach -p ', [str(pid)])
 self.complete_from_to('platform process info ', [str(pid)])
 
-pname = psutil.Process(pid).name()  # FIXME: psutil doesn't work for 
remote
-self.complete_from_to('process attach -n ', [str(pname)])
-self.complete_from_to('platform process attach -n ', [str(pname)])
+self.completions_contain_str('process attach -n ', "a.out")
+self.completions_contain_str('platform process attach -n ', "a.out")
 
 def test_process_signal(self):
 # The tab completion for "process signal"  won't work without a 
running process.



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