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

2020-09-01 Thread David Zarzycki via Phabricator via lldb-commits 
davezarzycki added a comment.

Hello. I have an auto-bisecting multi-stage bot that has identified this change 
as breaking release (without assertions) testing on Fedora 33 x86-64. Can we 
get a quick fix or revert this change for now?

  FAIL: lldb-shell :: Reproducer/Functionalities/TestImageList.test (68782 of 
69683)
   TEST 'lldb-shell :: 
Reproducer/Functionalities/TestImageList.test' FAILED 
  Script:
  --
  : 'RUN: at line 6';   /tmp/_update_lc/r/bin/clang 
--target=specify-a-target-or-use-a-_host-substitution 
--target=x86_64-unknown-linux-gnu -pthread 
-fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell
 
/home/dave/ro_s/lp/lldb/test/Shell/Reproducer/Functionalities/Inputs/stepping.c 
-g -o 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.out
  : 'RUN: at line 8';   rm -rf 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
  : 'RUN: at line 10';   echo "CAPTURE" >> 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
  : 'RUN: at line 11';   /tmp/_update_lc/r/bin/lldb --no-lldbinit -S 
/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init -x -b  --capture 
--capture-path 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.repro
 -o 'run' -o 'image list' -o 'reproducer generate' 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.out
 >> 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
 2>&1
  : 'RUN: at line 17';   echo "REPLAY" >> 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
  : 'RUN: at line 18';   /tmp/_update_lc/r/bin/lldb --no-lldbinit -S 
/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init -x -b --replay 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.repro
 >> 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
 2>&1
  : 'RUN: at line 20';   cat 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
 | /tmp/_update_lc/r/bin/FileCheck 
/home/dave/ro_s/lp/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  clang-12: warning: argument unused during compilation: 
'-fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell'
 [-Wunused-command-line-argument]
  
  --
  
  
  Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
  
  Failed Tests (1):
lldb-shell :: Reproducer/Functionalities/TestImageList.test
  
  
  Testing Time: 125.54s
Unsupported  : 10774
Passed   : 58806
Expectedly Failed:   102
Failed   : 1
  FAILED: CMakeFiles/check-all


Repository:
  rG LLVM Github Monorepo

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] 7c80f2d - Revert "[lldb] Add reproducer verifier"

2020-09-01 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-09-01T12:21:44+02:00
New Revision: 7c80f2da812e45bbdfa3c8f9ab24440f8ef3362a

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

LOG: Revert "[lldb] Add reproducer verifier"

This reverts commit 297f69afac58fc9dc13897857a5e70131c5adc85. It broke
the Fedora 33 x86-64 bot. See the review for more info.

Added: 


Modified: 
lldb/include/lldb/API/SBReproducer.h
lldb/include/lldb/Utility/Reproducer.h
lldb/source/API/SBReproducer.cpp
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/source/Commands/Options.td
lldb/source/Utility/Reproducer.cpp
lldb/source/Utility/ReproducerProvider.cpp
lldb/test/Shell/Reproducer/TestDebugSymbols.test
lldb/tools/driver/Driver.cpp
lldb/tools/driver/Options.td
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp

Removed: 
lldb/test/Shell/Reproducer/TestVerify.test



diff  --git a/lldb/include/lldb/API/SBReproducer.h 
b/lldb/include/lldb/API/SBReproducer.h
index 5578162412c8..78044e9acbc3 100644
--- a/lldb/include/lldb/API/SBReproducer.h
+++ b/lldb/include/lldb/API/SBReproducer.h
@@ -11,32 +11,8 @@
 
 #include "lldb/API/SBDefines.h"
 
-namespace lldb_private {
-namespace repro {
-struct ReplayOptions;
-}
-} // namespace lldb_private
-
 namespace lldb {
 
-class LLDB_API SBReplayOptions {
-public:
-  SBReplayOptions();
-  SBReplayOptions(const SBReplayOptions &rhs);
-  ~SBReplayOptions();
-
-  SBReplayOptions &operator=(const SBReplayOptions &rhs);
-
-  void SetVerify(bool verify);
-  bool GetVerify() const;
-
-  void SetCheckVersion(bool check);
-  bool GetCheckVersion() const;
-
-private:
-  std::unique_ptr m_opaque_up;
-};
-
 /// The SBReproducer class is special because it bootstraps the capture and
 /// replay of SB API calls. As a result we cannot rely on any other SB objects
 /// in the interface or implementation of this class.
@@ -46,7 +22,6 @@ class LLDB_API SBReproducer {
   static const char *Capture(const char *path);
   static const char *Replay(const char *path);
   static const char *Replay(const char *path, bool skip_version_check);
-  static const char *Replay(const char *path, const SBReplayOptions &options);
   static const char *PassiveReplay(const char *path);
   static const char *GetPath();
   static bool SetAutoGenerate(bool b);

diff  --git a/lldb/include/lldb/Utility/Reproducer.h 
b/lldb/include/lldb/Utility/Reproducer.h
index 7e5591493d71..d6cde4485090 100644
--- a/lldb/include/lldb/Utility/Reproducer.h
+++ b/lldb/include/lldb/Utility/Reproducer.h
@@ -227,22 +227,6 @@ class Reproducer {
   mutable std::mutex m_mutex;
 };
 
-class Verifier {
-public:
-  Verifier(Loader *loader) : m_loader(loader) {}
-  void Verify(llvm::function_ref error_callback,
-  llvm::function_ref warning_callback,
-  llvm::function_ref note_callback) const;
-
-private:
-  Loader *m_loader;
-};
-
-struct ReplayOptions {
-  bool verify = true;
-  bool check_version = true;
-};
-
 } // namespace repro
 } // namespace lldb_private
 

diff  --git a/lldb/source/API/SBReproducer.cpp 
b/lldb/source/API/SBReproducer.cpp
index 233e0b5b..7d08a88fe9e3 100644
--- a/lldb/source/API/SBReproducer.cpp
+++ b/lldb/source/API/SBReproducer.cpp
@@ -30,33 +30,6 @@ using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::repro;
 
-SBReplayOptions::SBReplayOptions()
-: m_opaque_up(std::make_unique()){};
-
-SBReplayOptions::SBReplayOptions(const SBReplayOptions &rhs)
-: m_opaque_up(std::make_unique(*rhs.m_opaque_up)) {}
-
-SBReplayOptions::~SBReplayOptions() = default;
-
-SBReplayOptions &SBReplayOptions::operator=(const SBReplayOptions &rhs) {
-  if (this == &rhs)
-return *this;
-  *m_opaque_up = *rhs.m_opaque_up;
-  return *this;
-}
-
-void SBReplayOptions::SetVerify(bool verify) { m_opaque_up->verify = verify; }
-
-bool SBReplayOptions::GetVerify() const { return m_opaque_up->verify; }
-
-void SBReplayOptions::SetCheckVersion(bool check) {
-  m_opaque_up->check_version = check;
-}
-
-bool SBReplayOptions::GetCheckVersion() const {
-  return m_opaque_up->check_version;
-}
-
 SBRegistry::SBRegistry() {
   Registry &R = *this;
 
@@ -190,18 +163,10 @@ const char *SBReproducer::PassiveReplay(const char *path) 
{
 }
 
 const char *SBReproducer::Replay(const char *path) {
-  SBReplayOptions options;
-  return SBReproducer::Replay(path, options);
+  return SBReproducer::Replay(path, false);
 }
 
 const char *SBReproducer::Replay(const char *path, bool skip_version_check) {
-  SBReplayOptions options;
-  options.SetCheckVersion(!skip_version_check);
-  return SBReproducer::Replay(path, options);
-}
-
-const char *SBReproducer::Replay(const char *path,
- const SBReplayOptions &

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

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

Don't see an obvious fix, so I reverted for Jonas in 
7c80f2da812e45bbdfa3c8f9ab24440f8ef3362a 



Repository:
  rG LLVM Github Monorepo

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] D86497: [lldb] Add reproducer verifier

2020-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D86497#2248934 , @davezarzycki 
wrote:

> Hello. I have an auto-bisecting multi-stage bot that has identified this 
> change as breaking release (without assertions) testing on Fedora 33 x86-64. 
> Can we get a quick fix or revert this change for now?
>
>   FAIL: lldb-shell :: Reproducer/Functionalities/TestImageList.test (68782 of 
> 69683)
>    TEST 'lldb-shell :: 
> Reproducer/Functionalities/TestImageList.test' FAILED 
>   Script:
>   --
>   : 'RUN: at line 6';   /tmp/_update_lc/r/bin/clang 
> --target=specify-a-target-or-use-a-_host-substitution 
> --target=x86_64-unknown-linux-gnu -pthread 
> -fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell
>  
> /home/dave/ro_s/lp/lldb/test/Shell/Reproducer/Functionalities/Inputs/stepping.c
>  -g -o 
> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.out
>   : 'RUN: at line 8';   rm -rf 
> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
>   : 'RUN: at line 10';   echo "CAPTURE" >> 
> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
>   : 'RUN: at line 11';   /tmp/_update_lc/r/bin/lldb --no-lldbinit -S 
> /tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init -x -b  --capture 
> --capture-path 
> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.repro
>  -o 'run' -o 'image list' -o 'reproducer generate' 
> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.out
>  >> 
> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
>  2>&1
>   : 'RUN: at line 17';   echo "REPLAY" >> 
> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
>   : 'RUN: at line 18';   /tmp/_update_lc/r/bin/lldb --no-lldbinit -S 
> /tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init -x -b --replay 
> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.repro
>  >> 
> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
>  2>&1
>   : 'RUN: at line 20';   cat 
> /tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
>  | /tmp/_update_lc/r/bin/FileCheck 
> /home/dave/ro_s/lp/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
>   --
>   Exit Code: 1
>   
>   Command Output (stderr):
>   --
>   clang-12: warning: argument unused during compilation: 
> '-fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell'
>  [-Wunused-command-line-argument]
>   
>   --
>   
>   
>   Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
>   
>   Failed Tests (1):
> lldb-shell :: Reproducer/Functionalities/TestImageList.test
>   
>   
>   Testing Time: 125.54s
> Unsupported  : 10774
> Passed   : 58806
> Expectedly Failed:   102
> Failed   : 1
>   FAILED: CMakeFiles/check-all

Hey Dave, I cannot repro this, neither on macOS nor Linux when building in 
release without assertions. Could you rerun the test manually and copy the 
output of the different commands? The lit output is really abysmal :(


Repository:
  rG LLVM Github Monorepo

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] D86962: [LLDB] Move NativeRegisterContextLinux/RegisterContextPOSIX*_arm to RegisterInfoAndSetInterface

2020-09-01 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added subscribers: llvm-commits, danielkiss, hiraditya, kristof.beyls.
Herald added a project: LLVM.
omjavaid requested review of this revision.

This patch removes register set definitions and other redundant code from 
NativeRegisterContextLinux/RegisterContextPOSIX*_arm.
Register sets are now moved under RegisterInfosPOSIX_arm which now uses 
RegisterInfoAndSetInterface. This is similar to what we earlier did for AArch64.


https://reviews.llvm.org/D86962

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp

Index: llvm/lib/Transforms/IPO/OpenMPOpt.cpp
===
--- llvm/lib/Transforms/IPO/OpenMPOpt.cpp
+++ llvm/lib/Transforms/IPO/OpenMPOpt.cpp
@@ -407,6 +407,10 @@
 return true;
   }
 
+  static const unsigned BasePtrsArgNum = 2;
+  static const unsigned PtrsArgNum = 3;
+  static const unsigned SizesArgNum = 4;
+
 private:
   /// Traverses the BasicBlock where \p Array is, collecting the stores made to
   /// \p Array, leaving StoredValues with the values stored before the
@@ -705,14 +709,12 @@
 // offload arrays, offload_baseptrs, offload_ptrs, offload_sizes.
 // Therefore:
 // i8** %offload_baseptrs.
-const unsigned BasePtrsArgNum = 2;
-Value *BasePtrsArg = RuntimeCall.getArgOperand(BasePtrsArgNum);
+Value *BasePtrsArg =
+RuntimeCall.getArgOperand(OffloadArray::BasePtrsArgNum);
 // i8** %offload_ptrs.
-const unsigned PtrsArgNum = 3;
-Value *PtrsArg = RuntimeCall.getArgOperand(PtrsArgNum);
+Value *PtrsArg = RuntimeCall.getArgOperand(OffloadArray::PtrsArgNum);
 // i8** %offload_sizes.
-const unsigned SizesArgNum = 4;
-Value *SizesArg = RuntimeCall.getArgOperand(SizesArgNum);
+Value *SizesArg = RuntimeCall.getArgOperand(OffloadArray::SizesArgNum);
 
 // Get values stored in **offload_baseptrs.
 auto *V = getUnderlyingObject(BasePtrsArg);
Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -83,9 +83,6 @@
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
 break;
-  case llvm::Triple::arm:
-reg_interface = new RegisterInfoPOSIX_arm(arch);
-break;
   case llvm::Triple::ppc:
 reg_interface = new RegisterContextFreeBSD_powerpc32(arch);
 break;
@@ -122,9 +119,6 @@
 
 case llvm::Triple::Linux: {
   switch (arch.GetMachine()) {
-  case llvm::Triple::arm:
-reg_interface = new RegisterInfoPOSIX_arm(arch);
-break;
   case llvm::Triple::aarch64:
 break;
   case llvm::Triple::mipsel:
@@ -157,9 +151,6 @@
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
 break;
-  case llvm::Triple::arm:
-reg_interface = new RegisterInfoPOSIX_arm(arch);
-break;
   case llvm::Triple::x86:
 reg_interface = new RegisterContextOpenBSD_i386(arch);
 break;
@@ -176,7 +167,8 @@
   break;
 }
 
-if (!reg_interface && arch.GetMachine() != llvm::Triple::aarch64) {
+if (!reg_interface && arch.GetMachine() != llvm::Triple::aarch64 &&
+arch.GetMachine() != llvm::Triple::arm) {
   LLDB_LOGF(log, "elf-core::%s:: Architecture(%d) or OS(%d) not supported",
 __FUNCTION__, arch.GetMachine(), arch.GetTriple().getOS());
   assert(false && "Architecture or OS not supported");
@@ -190,7 +182,8 @@
   break;
 case llvm::Triple::arm:
   m_thread_reg_ctx_sp = std::make_shared(
-  *this, reg_interface, m_gpregset_data, m_notes);
+  *this, std::make_unique(arch), m_gpregset_data,
+  m_notes);
   break;
 case llvm::Triple::mipsel:
 case llvm::Triple::mips:
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
@@ -18,7 +18,7 @@
 public:
   RegisterContextCorePOSIX_arm(
   lldb_private::Thread &thread,
-  lldb_private::RegisterInfoInterface *register_info,
+  std::unique_ptr r

[Lldb-commits] [PATCH] D86962: [LLDB] Move NativeRegisterContextLinux/RegisterContextPOSIX*_arm to RegisterInfoAndSetInterface

2020-09-01 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid added a comment.

NativeRegisterContextLinux_arm still uses 
"Plugins/Process/Utility/lldb-arm-register-enums.h" which I intend to deal in a 
later patch.


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

https://reviews.llvm.org/D86962

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

[Lldb-commits] [PATCH] D86962: [LLDB] Move NativeRegisterContextLinux/RegisterContextPOSIX*_arm to RegisterInfoAndSetInterface

2020-09-01 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid updated this revision to Diff 289218.
omjavaid added a comment.

Fixed last diff it contained an invalid change.


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

https://reviews.llvm.org/D86962

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp

Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -83,9 +83,6 @@
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
 break;
-  case llvm::Triple::arm:
-reg_interface = new RegisterInfoPOSIX_arm(arch);
-break;
   case llvm::Triple::ppc:
 reg_interface = new RegisterContextFreeBSD_powerpc32(arch);
 break;
@@ -122,9 +119,6 @@
 
 case llvm::Triple::Linux: {
   switch (arch.GetMachine()) {
-  case llvm::Triple::arm:
-reg_interface = new RegisterInfoPOSIX_arm(arch);
-break;
   case llvm::Triple::aarch64:
 break;
   case llvm::Triple::mipsel:
@@ -157,9 +151,6 @@
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
 break;
-  case llvm::Triple::arm:
-reg_interface = new RegisterInfoPOSIX_arm(arch);
-break;
   case llvm::Triple::x86:
 reg_interface = new RegisterContextOpenBSD_i386(arch);
 break;
@@ -176,7 +167,8 @@
   break;
 }
 
-if (!reg_interface && arch.GetMachine() != llvm::Triple::aarch64) {
+if (!reg_interface && arch.GetMachine() != llvm::Triple::aarch64 &&
+arch.GetMachine() != llvm::Triple::arm) {
   LLDB_LOGF(log, "elf-core::%s:: Architecture(%d) or OS(%d) not supported",
 __FUNCTION__, arch.GetMachine(), arch.GetTriple().getOS());
   assert(false && "Architecture or OS not supported");
@@ -190,7 +182,8 @@
   break;
 case llvm::Triple::arm:
   m_thread_reg_ctx_sp = std::make_shared(
-  *this, reg_interface, m_gpregset_data, m_notes);
+  *this, std::make_unique(arch), m_gpregset_data,
+  m_notes);
   break;
 case llvm::Triple::mipsel:
 case llvm::Triple::mips:
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
@@ -18,7 +18,7 @@
 public:
   RegisterContextCorePOSIX_arm(
   lldb_private::Thread &thread,
-  lldb_private::RegisterInfoInterface *register_info,
+  std::unique_ptr register_info,
   const lldb_private::DataExtractor &gpregset,
   llvm::ArrayRef notes);
 
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
@@ -16,9 +16,9 @@
 using namespace lldb_private;
 
 RegisterContextCorePOSIX_arm::RegisterContextCorePOSIX_arm(
-Thread &thread, RegisterInfoInterface *register_info,
+Thread &thread, std::unique_ptr register_info,
 const DataExtractor &gpregset, llvm::ArrayRef notes)
-: RegisterContextPOSIX_arm(thread, 0, register_info) {
+: RegisterContextPOSIX_arm(thread, std::move(register_info)) {
   m_gpr_buffer = std::make_shared(gpregset.GetDataStart(),
   gpregset.GetByteSize());
   m_gpr.SetData(m_gpr_buffer);
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm.h
@@ -9,12 +9,14 @@
 #ifndef LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERINFOPOSIX_ARM_H
 #define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERINFOPOSIX_ARM_H
 
-#include "RegisterInfoInterface.h"
+#include "RegisterInfoAndSetInterface.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/lldb-private.h"
 
-class RegisterInfoPOSIX_arm : public lldb_private::RegisterInfoInterface {
+class RegisterInfoPOSIX_arm : public lldb_private::RegisterInfoAndSetInterface {
 public:
+  enum { GPRegSet = 0, FPRegSet

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

2020-09-01 Thread walter erquinigo via Phabricator via lldb-commits 
wallace updated this revision to Diff 289285.
wallace added a comment.

fix nit. Waiting for an additional approval


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/Target/TraceSettingsParser.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/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceSettingsParser.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
  lldb/test/API/commands/trace/intelpt-trace/trace_bad3.json
  llvm/include/llvm/Support/JSON.h
  llvm/lib/Support/JSON.cpp

Index: llvm/lib/Support/JSON.cpp
===
--- llvm/lib/Support/JSON.cpp
+++ llvm/lib/Support/JSON.cpp
@@ -77,6 +77,62 @@
 return V->getAsArray();
   return nullptr;
 }
+
+llvm::Error Object::createMissingKeyError(llvm::StringRef K) const {
+  std::string str;
+  llvm::raw_string_ostream OS(str);
+  json::Object obj = *this;
+  OS << llvm::formatv(
+  "JSON object is missing the \"{0}\" field.\nValue::\n{1:2}", K,
+  json::Value(std::move(obj)));
+  OS.flush();
+
+  return llvm::createStringError(std::errc::invalid_argument, OS.str().c_str());
+}
+
+llvm::Expected Object::getOrError(StringRef K) const {
+  if (const json::Value *value = get(K))
+return value;
+  else
+return createMissingKeyError(K);
+}
+
+llvm::Expected Object::getIntegerOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsIntegerOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected Object::getStringOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsStringOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected Object::getArrayOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsArrayOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected
+Object::getObjectOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsObjectOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected>
+Object::getOptionalStringOrError(StringRef K) const {
+  if (const json::Value *V = get(K))
+return V->getAsStringOrError();
+  return llvm::None;
+}
+
 bool operator==(const Object &LHS, const Object &RHS) {
   if (LHS.size() != RHS.size())
 return false;
@@ -150,6 +206,42 @@
   }
 }
 
+llvm::Error Value::createWrongTypeError(llvm::StringRef type) const {
+  std::string str;
+  llvm::raw_string_ostream OS(str);
+  OS << llvm::formatv("JSON value is expected to be \"{0}\".\nValue:\n{1:2}",
+  type, *this);
+  OS.flush();
+
+  return llvm::createStringError(std::errc::invalid_argument, OS.str().c_str());
+}
+
+llvm::Expected Value::getAsIntegerOrError() const {
+  llvm::Optional value = getAsInteger();
+  if (value.hasValue())
+return *value;
+  return createWrongTypeError("integer");
+}
+
+llvm::Expected Value::getAsStringOrError() const {
+  llvm::Optional value = getAsString();
+  if (value.hasValue())
+return *value;
+  return createWrongTypeError("string");
+}
+
+llvm::Expected Value::getAsArrayOrError() const {
+  if (const json::Array *value = getAsArray())
+return value;
+  return createWrongTypeError("array");
+}
+
+llvm::Expected Value::getAsObjectOrError() const {
+  if (const json::Object *value = getAsObject())
+return value;
+  return createWrongTypeError("object");
+}
+
 void Value::destroy() {
   switch (Type) {
   case T_Null:
@@ -715,4 +807,3 @@
 llvm_unreachable("json::Value format options should be an integer");
   json::OStream(OS, IndentAmount).value(E);
 }
-
Index: llvm/include/llvm/Support/JSON.h

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

2020-09-01 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: lldb/source/Commands/CommandObjectTrace.h:1
+//===-- CommandObjectTrace.h 
--===//
+//

Can you add `-*- C++ -*-` markers the the `.h` files you are adding? Otherwise 
tools (like phabricator) will think it's a C header.


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] D86670: [intel-pt] Add a basic implementation of the dump command

2020-09-01 Thread walter erquinigo via Phabricator via lldb-commits 
wallace updated this revision to Diff 289300.
wallace added a comment.

- Now using the TraceDumpOptions struct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

Files:
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/TestTraceDump.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/intelpt-trace/trace2.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace2.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace2.json
@@ -0,0 +1,53 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": [
+{
+  "pid": 1,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 11,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+},
+{
+  "pid": 2,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 21,
+  "traceFile": "3842849.trace"
+},
+{
+  "tid": 22,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -55,3 +55,5 @@
 self.expect("trace load -v " + trace_definition_file2, error=True,
 substrs=['error: JSON object is missing the "pid" field.'])
 self.assertEqual(self.dbg.GetNumTargets(), 0)
+
+self.expect("trace dump", substrs=["error: no trace data in this target"])
Index: lldb/test/API/commands/trace/TestTraceDump.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceDump.py
@@ -0,0 +1,48 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceDump(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 testDumpTrace(self):
+self.expect("trace dump", substrs=["error: no trace data in this target"])
+
+src_dir = self.getSourceDir()
+trace_definition_file = os.path.join(src_dir, "intelpt-trace", "trace2.json")
+self.expect("trace load " + trace_definition_file)
+
+# An invalid thread id should show an error message
+self.expect("trace dump -t 5678", substrs=['error: the thread id "5678" does not exist in this target'])
+
+# Only the specified thread should be printed
+self.expect("trace dump -t 22", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+# Verbose should output the entire JSON settings besides the thread specific information
+self.expect("trace dump -t 22 -v",
+  substrs=["Settings", "3842849.trace", "intel-pt", "Settings directory"],
+  patterns=["pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+# In this case all threads should be printed
+self.expect("trace dump", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '21' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+"pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+self.expect("trace dump -t 21 --thread-id 22", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '21' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+"pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+# We switch targets and check the threads of this target
+self.dbg.SetSelectedTarget(self.dbg.FindTargetWithProcessID(1))
+self.expect("

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

2020-09-01 Thread walter erquinigo via Phabricator via lldb-commits 
wallace updated this revision to Diff 289301.
wallace added a comment.

fix header c++ declaration


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/Target/TraceSettingsParser.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/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceSettingsParser.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
  lldb/test/API/commands/trace/intelpt-trace/trace_bad3.json
  llvm/include/llvm/Support/JSON.h
  llvm/lib/Support/JSON.cpp

Index: llvm/lib/Support/JSON.cpp
===
--- llvm/lib/Support/JSON.cpp
+++ llvm/lib/Support/JSON.cpp
@@ -77,6 +77,62 @@
 return V->getAsArray();
   return nullptr;
 }
+
+llvm::Error Object::createMissingKeyError(llvm::StringRef K) const {
+  std::string str;
+  llvm::raw_string_ostream OS(str);
+  json::Object obj = *this;
+  OS << llvm::formatv(
+  "JSON object is missing the \"{0}\" field.\nValue::\n{1:2}", K,
+  json::Value(std::move(obj)));
+  OS.flush();
+
+  return llvm::createStringError(std::errc::invalid_argument, OS.str().c_str());
+}
+
+llvm::Expected Object::getOrError(StringRef K) const {
+  if (const json::Value *value = get(K))
+return value;
+  else
+return createMissingKeyError(K);
+}
+
+llvm::Expected Object::getIntegerOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsIntegerOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected Object::getStringOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsStringOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected Object::getArrayOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsArrayOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected
+Object::getObjectOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsObjectOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected>
+Object::getOptionalStringOrError(StringRef K) const {
+  if (const json::Value *V = get(K))
+return V->getAsStringOrError();
+  return llvm::None;
+}
+
 bool operator==(const Object &LHS, const Object &RHS) {
   if (LHS.size() != RHS.size())
 return false;
@@ -150,6 +206,42 @@
   }
 }
 
+llvm::Error Value::createWrongTypeError(llvm::StringRef type) const {
+  std::string str;
+  llvm::raw_string_ostream OS(str);
+  OS << llvm::formatv("JSON value is expected to be \"{0}\".\nValue:\n{1:2}",
+  type, *this);
+  OS.flush();
+
+  return llvm::createStringError(std::errc::invalid_argument, OS.str().c_str());
+}
+
+llvm::Expected Value::getAsIntegerOrError() const {
+  llvm::Optional value = getAsInteger();
+  if (value.hasValue())
+return *value;
+  return createWrongTypeError("integer");
+}
+
+llvm::Expected Value::getAsStringOrError() const {
+  llvm::Optional value = getAsString();
+  if (value.hasValue())
+return *value;
+  return createWrongTypeError("string");
+}
+
+llvm::Expected Value::getAsArrayOrError() const {
+  if (const json::Array *value = getAsArray())
+return value;
+  return createWrongTypeError("array");
+}
+
+llvm::Expected Value::getAsObjectOrError() const {
+  if (const json::Object *value = getAsObject())
+return value;
+  return createWrongTypeError("object");
+}
+
 void Value::destroy() {
   switch (Type) {
   case T_Null:
@@ -715,4 +807,3 @@
 llvm_unreachable("json::Value format options should be an integer");
   json::OStream(OS, IndentAmount).value(E);
 }
-
Index: llvm/include/llvm/Support/JSON.h
=

[Lldb-commits] [PATCH] D86987: [lldb/interpreter] Improve REPL init file compatibility

2020-09-01 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
Herald added a subscriber: lldb-commits.
mib requested review of this revision.

This patch changes the command interpreter sourcing logic for the REPL
init file. Instead of looking for a arbitrary file name, it standardizes
the REPL init file name to match to following scheme:

  `.lldbinit--repl`

This will make the naming more homogenous and the sourcing logic future-proof.

rdar://65836048

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86987

Files:
  lldb/docs/man/lldb.rst
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -491,7 +491,7 @@
   SBCommandInterpreter sb_interpreter = m_debugger.GetCommandInterpreter();
 
   // Before we handle any options from the command line, we parse the
-  // .lldbinit file in the user's home directory.
+  // REPL init file or the default file in the user's home directory.
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInHomeDirectory(result, m_option_data.m_repl);
   if (m_option_data.m_debug_mode) {
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -68,6 +68,7 @@
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Utility/Args.h"
 
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/TargetList.h"
@@ -2093,17 +2094,14 @@
 
 static void GetHomeREPLInitFile(llvm::SmallVectorImpl &init_file,
 LanguageType language) {
-  std::string init_file_name;
-
-  switch (language) {
-  // TODO: Add support for a language used with a REPL.
-  default:
+  if (language == LanguageType::eLanguageTypeUnknown)
 return;
-  }
 
+  llvm::Twine init_file_name = llvm::Twine(".lldbinit-") +
+   Language::GetNameForLanguageType(language) +
+   "-repl";
   FileSystem::Instance().GetHomeDirectory(init_file);
-  llvm::sys::path::append(init_file, init_file_name);
-
+  llvm::sys::path::append(init_file, init_file_name.str());
   FileSystem::Instance().Resolve(init_file);
 }
 
Index: lldb/docs/man/lldb.rst
===
--- lldb/docs/man/lldb.rst
+++ lldb/docs/man/lldb.rst
@@ -311,9 +311,11 @@
 and ~/.lldbinit-Xcode for Xcode. If there is no application specific init
 file, :program:`lldb` will look for an init file in the home directory.
 If launched with a `REPL`_ option, it will first look for a REPL configuration
-file, specific to the REPL language. If this file doesn't exist, or 
:program:`lldb`
-wasn't launch with `REPL`_, meaning there is neither a REPL init file nor an
-application specific init file, `lldb` will fallback to the global ~/.lldbinit.
+file, specific to the REPL language. The init file should be named as follow:
+`.lldbinit--repl` (i.e. `.lldbinit-swift-repl`). If this file doesn't
+exist, or :program:`lldb` wasn't launch with `REPL`_, meaning there is neither
+a REPL init file nor an application specific init file, `lldb` will fallback to
+the global ~/.lldbinit.
 
 Secondly, it will look for an .lldbinit file in the current working directory.
 For security reasons, :program:`lldb` will print a warning and not source this


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -491,7 +491,7 @@
   SBCommandInterpreter sb_interpreter = m_debugger.GetCommandInterpreter();
 
   // Before we handle any options from the command line, we parse the
-  // .lldbinit file in the user's home directory.
+  // REPL init file or the default file in the user's home directory.
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInHomeDirectory(result, m_option_data.m_repl);
   if (m_option_data.m_debug_mode) {
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -68,6 +68,7 @@
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Utility/Args.h"
 
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/TargetList.h"
@@ -2093,17 +2094,14 @@
 
 static void GetHomeREPLInitFile(llvm::SmallVectorImpl &init_file,
 LanguageType language) {
-  std::string init_file_name;
-
-  switch (language) {
-  // TODO: Add support for a language used wi

[Lldb-commits] [PATCH] D86987: [lldb/interpreter] Improve REPL init file compatibility

2020-09-01 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib updated this revision to Diff 289307.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86987

Files:
  lldb/docs/man/lldb.rst
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -491,7 +491,7 @@
   SBCommandInterpreter sb_interpreter = m_debugger.GetCommandInterpreter();
 
   // Before we handle any options from the command line, we parse the
-  // .lldbinit file in the user's home directory.
+  // REPL init file or the default file in the user's home directory.
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInHomeDirectory(result, m_option_data.m_repl);
   if (m_option_data.m_debug_mode) {
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -68,6 +68,7 @@
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Utility/Args.h"
 
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/TargetList.h"
@@ -2093,17 +2094,16 @@
 
 static void GetHomeREPLInitFile(llvm::SmallVectorImpl &init_file,
 LanguageType language) {
-  std::string init_file_name;
-
-  switch (language) {
-  // TODO: Add support for a language used with a REPL.
-  default:
+  if (language == LanguageType::eLanguageTypeUnknown)
 return;
-  }
 
+  std::string init_file_name =
+  (llvm::Twine(".lldbinit-") +
+   llvm::Twine(Language::GetNameForLanguageType(language)) +
+   llvm::Twine("-repl"))
+  .str();
   FileSystem::Instance().GetHomeDirectory(init_file);
   llvm::sys::path::append(init_file, init_file_name);
-
   FileSystem::Instance().Resolve(init_file);
 }
 
Index: lldb/docs/man/lldb.rst
===
--- lldb/docs/man/lldb.rst
+++ lldb/docs/man/lldb.rst
@@ -311,9 +311,11 @@
 and ~/.lldbinit-Xcode for Xcode. If there is no application specific init
 file, :program:`lldb` will look for an init file in the home directory.
 If launched with a `REPL`_ option, it will first look for a REPL configuration
-file, specific to the REPL language. If this file doesn't exist, or 
:program:`lldb`
-wasn't launch with `REPL`_, meaning there is neither a REPL init file nor an
-application specific init file, `lldb` will fallback to the global ~/.lldbinit.
+file, specific to the REPL language. The init file should be named as follow:
+`.lldbinit--repl` (i.e. `.lldbinit-swift-repl`). If this file doesn't
+exist, or :program:`lldb` wasn't launch with `REPL`_, meaning there is neither
+a REPL init file nor an application specific init file, `lldb` will fallback to
+the global ~/.lldbinit.
 
 Secondly, it will look for an .lldbinit file in the current working directory.
 For security reasons, :program:`lldb` will print a warning and not source this


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -491,7 +491,7 @@
   SBCommandInterpreter sb_interpreter = m_debugger.GetCommandInterpreter();
 
   // Before we handle any options from the command line, we parse the
-  // .lldbinit file in the user's home directory.
+  // REPL init file or the default file in the user's home directory.
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInHomeDirectory(result, m_option_data.m_repl);
   if (m_option_data.m_debug_mode) {
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -68,6 +68,7 @@
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Utility/Args.h"
 
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/TargetList.h"
@@ -2093,17 +2094,16 @@
 
 static void GetHomeREPLInitFile(llvm::SmallVectorImpl &init_file,
 LanguageType language) {
-  std::string init_file_name;
-
-  switch (language) {
-  // TODO: Add support for a language used with a REPL.
-  default:
+  if (language == LanguageType::eLanguageTypeUnknown)
 return;
-  }
 
+  std::string init_file_name =
+  (llvm::Twine(".lldbinit-") +
+   llvm::Twine(Language::GetNameForLanguageType(language)) +
+   llvm::Twine("-repl"))
+  .str();
   FileSystem::Instance().GetHomeDirectory(init_file);
   llvm::sys::path::append(init_file, init_file_name);
-
   FileSystem::Instance().Resolve(init_file);
 }
 
Index: lldb/docs/man/lldb.rs

[Lldb-commits] [lldb] 0224738 - [lldb/interpreter] Improve REPL init file compatibility

2020-09-01 Thread Med Ismail Bennani via lldb-commits 

Author: Med Ismail Bennani
Date: 2020-09-02T01:21:22+02:00
New Revision: 0224738c1abdee59923a539204f12c4c99621506

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

LOG: [lldb/interpreter] Improve REPL init file compatibility

This patch changes the command interpreter sourcing logic for the REPL
init file. Instead of looking for a arbitrary file name, it standardizes
the REPL init file name to match to following scheme:

  `.lldbinit--repl`

This will make the naming more homogenous and the sourcing logic future-proof.

rdar://65836048

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/docs/man/lldb.rst
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/tools/driver/Driver.cpp

Removed: 




diff  --git a/lldb/docs/man/lldb.rst b/lldb/docs/man/lldb.rst
index 19a5eaab8e8c..65ac462fed94 100644
--- a/lldb/docs/man/lldb.rst
+++ b/lldb/docs/man/lldb.rst
@@ -311,9 +311,11 @@ program. This would be ~/.lldbinit-lldb for the command 
line :program:`lldb`
 and ~/.lldbinit-Xcode for Xcode. If there is no application specific init
 file, :program:`lldb` will look for an init file in the home directory.
 If launched with a `REPL`_ option, it will first look for a REPL configuration
-file, specific to the REPL language. If this file doesn't exist, or 
:program:`lldb`
-wasn't launch with `REPL`_, meaning there is neither a REPL init file nor an
-application specific init file, `lldb` will fallback to the global ~/.lldbinit.
+file, specific to the REPL language. The init file should be named as follow:
+`.lldbinit--repl` (i.e. `.lldbinit-swift-repl`). If this file doesn't
+exist, or :program:`lldb` wasn't launch with `REPL`_, meaning there is neither
+a REPL init file nor an application specific init file, `lldb` will fallback to
+the global ~/.lldbinit.
 
 Secondly, it will look for an .lldbinit file in the current working directory.
 For security reasons, :program:`lldb` will print a warning and not source this

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index e25c24ccd401..60e08346e655 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -68,6 +68,7 @@
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Utility/Args.h"
 
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/TargetList.h"
@@ -2093,17 +2094,16 @@ static void GetHomeInitFile(llvm::SmallVectorImpl 
&init_file,
 
 static void GetHomeREPLInitFile(llvm::SmallVectorImpl &init_file,
 LanguageType language) {
-  std::string init_file_name;
-
-  switch (language) {
-  // TODO: Add support for a language used with a REPL.
-  default:
+  if (language == LanguageType::eLanguageTypeUnknown)
 return;
-  }
 
+  std::string init_file_name =
+  (llvm::Twine(".lldbinit-") +
+   llvm::Twine(Language::GetNameForLanguageType(language)) +
+   llvm::Twine("-repl"))
+  .str();
   FileSystem::Instance().GetHomeDirectory(init_file);
   llvm::sys::path::append(init_file, init_file_name);
-
   FileSystem::Instance().Resolve(init_file);
 }
 

diff  --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 13cb9bee116d..3837d06ed8d8 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -491,7 +491,7 @@ int Driver::MainLoop() {
   SBCommandInterpreter sb_interpreter = m_debugger.GetCommandInterpreter();
 
   // Before we handle any options from the command line, we parse the
-  // .lldbinit file in the user's home directory.
+  // REPL init file or the default file in the user's home directory.
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInHomeDirectory(result, m_option_data.m_repl);
   if (m_option_data.m_debug_mode) {



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

[Lldb-commits] [PATCH] D86987: [lldb/interpreter] Improve REPL init file compatibility

2020-09-01 Thread Med Ismail Bennani 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 rG0224738c1abd: [lldb/interpreter] Improve REPL init file 
compatibility (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86987

Files:
  lldb/docs/man/lldb.rst
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -491,7 +491,7 @@
   SBCommandInterpreter sb_interpreter = m_debugger.GetCommandInterpreter();
 
   // Before we handle any options from the command line, we parse the
-  // .lldbinit file in the user's home directory.
+  // REPL init file or the default file in the user's home directory.
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInHomeDirectory(result, m_option_data.m_repl);
   if (m_option_data.m_debug_mode) {
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -68,6 +68,7 @@
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Utility/Args.h"
 
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/TargetList.h"
@@ -2093,17 +2094,16 @@
 
 static void GetHomeREPLInitFile(llvm::SmallVectorImpl &init_file,
 LanguageType language) {
-  std::string init_file_name;
-
-  switch (language) {
-  // TODO: Add support for a language used with a REPL.
-  default:
+  if (language == LanguageType::eLanguageTypeUnknown)
 return;
-  }
 
+  std::string init_file_name =
+  (llvm::Twine(".lldbinit-") +
+   llvm::Twine(Language::GetNameForLanguageType(language)) +
+   llvm::Twine("-repl"))
+  .str();
   FileSystem::Instance().GetHomeDirectory(init_file);
   llvm::sys::path::append(init_file, init_file_name);
-
   FileSystem::Instance().Resolve(init_file);
 }
 
Index: lldb/docs/man/lldb.rst
===
--- lldb/docs/man/lldb.rst
+++ lldb/docs/man/lldb.rst
@@ -311,9 +311,11 @@
 and ~/.lldbinit-Xcode for Xcode. If there is no application specific init
 file, :program:`lldb` will look for an init file in the home directory.
 If launched with a `REPL`_ option, it will first look for a REPL configuration
-file, specific to the REPL language. If this file doesn't exist, or 
:program:`lldb`
-wasn't launch with `REPL`_, meaning there is neither a REPL init file nor an
-application specific init file, `lldb` will fallback to the global ~/.lldbinit.
+file, specific to the REPL language. The init file should be named as follow:
+`.lldbinit--repl` (i.e. `.lldbinit-swift-repl`). If this file doesn't
+exist, or :program:`lldb` wasn't launch with `REPL`_, meaning there is neither
+a REPL init file nor an application specific init file, `lldb` will fallback to
+the global ~/.lldbinit.
 
 Secondly, it will look for an .lldbinit file in the current working directory.
 For security reasons, :program:`lldb` will print a warning and not source this


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -491,7 +491,7 @@
   SBCommandInterpreter sb_interpreter = m_debugger.GetCommandInterpreter();
 
   // Before we handle any options from the command line, we parse the
-  // .lldbinit file in the user's home directory.
+  // REPL init file or the default file in the user's home directory.
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInHomeDirectory(result, m_option_data.m_repl);
   if (m_option_data.m_debug_mode) {
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -68,6 +68,7 @@
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Utility/Args.h"
 
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/TargetList.h"
@@ -2093,17 +2094,16 @@
 
 static void GetHomeREPLInitFile(llvm::SmallVectorImpl &init_file,
 LanguageType language) {
-  std::string init_file_name;
-
-  switch (language) {
-  // TODO: Add support for a language used with a REPL.
-  default:
+  if (language == LanguageType::eLanguageTypeUnknown)
 return;
-  }
 
+  std::string init_file_name =
+  (llvm::Twine(".lldbinit-") +
+   llvm::Twine(Language::GetNameForLanguageType(language)) +
+   llvm::Twine("-repl"))
+  .s

[Lldb-commits] [lldb] 82139b8 - Simplify Symbol Status Message to Only Debug Info Size

2020-09-01 Thread Walter Erquinigo via lldb-commits 

Author: Yifan Shen
Date: 2020-09-01T16:25:20-07:00
New Revision: 82139b8770ee07f0b778be7af22c529098ef12ec

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

LOG: Simplify Symbol Status Message to Only Debug Info Size

The Symbol Status in modules view is simplified so that only when the module 
has debug info and its size is non-zero, will the status message be displayed. 
The symbol status message is renamed to debug info size and flag message like 
"Symbols not found" and "Symbols loaded" is deleted.

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

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
lldb/tools/lldb-vscode/JSONUtils.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py 
b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
index a16430fccae1..db70e4a8124b 100644
--- a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -32,26 +32,18 @@ def run_test(self, symbol_basename, expect_debug_info_size):
 self.assertIn('path', program_module, 'make sure path is in module')
 self.assertEqual(program, program_module['path'])
 self.assertTrue('symbolFilePath' not in program_module, 'Make sure 
a.out.stripped has no debug info')
-self.assertEqual('Symbols not found.', program_module['symbolStatus'])
 symbols_path = self.getBuildArtifact(symbol_basename)
 self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" 
"%s"' % (program, symbols_path)))
 
-def checkSymbolsLoaded():
-active_modules = self.vscode.get_active_modules()
-program_module = active_modules[program_basename]
-return 'Symbols loaded.' == program_module['symbolStatus']
-
 def checkSymbolsLoadedWithSize():
 active_modules = self.vscode.get_active_modules()
 program_module = active_modules[program_basename]
-symbolsStatus = program_module['symbolStatus']
-symbol_regex = re.compile(r"Symbols loaded. 
\([0-9]+(\.[0-9]*)?[KMG]?B\)")
+symbolsStatus = program_module['debugInfoSize']
+symbol_regex = re.compile(r"[0-9]+(\.[0-9]*)?[KMG]?B")
 return symbol_regex.match(program_module['symbolStatus'])
 
 if expect_debug_info_size:
 self.waitUntil(checkSymbolsLoadedWithSize)
-else:
-self.waitUntil(checkSymbolsLoaded)
 active_modules = self.vscode.get_active_modules()
 program_module = active_modules[program_basename]
 self.assertEqual(program_basename, program_module['name'])

diff  --git a/lldb/tools/lldb-vscode/JSONUtils.cpp 
b/lldb/tools/lldb-vscode/JSONUtils.cpp
index f6cdcf5a46cf..36156ca2c42f 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -354,9 +354,7 @@ static uint64_t GetDebugInfoSize(lldb::SBModule module) {
 
 static std::string ConvertDebugInfoSizeToString(uint64_t debug_info) {
   std::ostringstream oss;
-  oss << " (";
   oss << std::fixed << std::setprecision(1);
-
   if (debug_info < 1024) {
 oss << debug_info << "B";
   } else if (debug_info < 1024 * 1024) {
@@ -368,9 +366,7 @@ static std::string ConvertDebugInfoSizeToString(uint64_t 
debug_info) {
   } else {
 double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
 oss << gb << "GB";
-;
   }
-  oss << ")";
   return oss.str();
 }
 llvm::json::Value CreateModule(lldb::SBModule &module) {
@@ -386,11 +382,13 @@ llvm::json::Value CreateModule(lldb::SBModule &module) {
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
 std::string symbol_str = "Symbols loaded.";
+std::string debug_info_size;
 uint64_t debug_info = GetDebugInfoSize(module);
 if (debug_info > 0) {
-  symbol_str += ConvertDebugInfoSizeToString(debug_info);
+  debug_info_size = ConvertDebugInfoSizeToString(debug_info);
 }
 object.try_emplace("symbolStatus", symbol_str);
+object.try_emplace("debugInfoSize", debug_info_size);
 char symbol_path_arr[PATH_MAX];
 module.GetSymbolFileSpec().GetPath(symbol_path_arr,
sizeof(symbol_path_arr));



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

[Lldb-commits] [PATCH] D86662: Simplify Symbol Status Message to Only Debug Info Size

2020-09-01 Thread Phabricator via lldb-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG82139b8770ee: Simplify Symbol Status Message to Only Debug 
Info Size (authored by aelitashen, committed by Walter Erquinigo 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86662

Files:
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/tools/lldb-vscode/JSONUtils.cpp


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -354,9 +354,7 @@
 
 static std::string ConvertDebugInfoSizeToString(uint64_t debug_info) {
   std::ostringstream oss;
-  oss << " (";
   oss << std::fixed << std::setprecision(1);
-
   if (debug_info < 1024) {
 oss << debug_info << "B";
   } else if (debug_info < 1024 * 1024) {
@@ -368,9 +366,7 @@
   } else {
 double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
 oss << gb << "GB";
-;
   }
-  oss << ")";
   return oss.str();
 }
 llvm::json::Value CreateModule(lldb::SBModule &module) {
@@ -386,11 +382,13 @@
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
 std::string symbol_str = "Symbols loaded.";
+std::string debug_info_size;
 uint64_t debug_info = GetDebugInfoSize(module);
 if (debug_info > 0) {
-  symbol_str += ConvertDebugInfoSizeToString(debug_info);
+  debug_info_size = ConvertDebugInfoSizeToString(debug_info);
 }
 object.try_emplace("symbolStatus", symbol_str);
+object.try_emplace("debugInfoSize", debug_info_size);
 char symbol_path_arr[PATH_MAX];
 module.GetSymbolFileSpec().GetPath(symbol_path_arr,
sizeof(symbol_path_arr));
Index: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
===
--- lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -32,26 +32,18 @@
 self.assertIn('path', program_module, 'make sure path is in module')
 self.assertEqual(program, program_module['path'])
 self.assertTrue('symbolFilePath' not in program_module, 'Make sure 
a.out.stripped has no debug info')
-self.assertEqual('Symbols not found.', program_module['symbolStatus'])
 symbols_path = self.getBuildArtifact(symbol_basename)
 self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" 
"%s"' % (program, symbols_path)))
 
-def checkSymbolsLoaded():
-active_modules = self.vscode.get_active_modules()
-program_module = active_modules[program_basename]
-return 'Symbols loaded.' == program_module['symbolStatus']
-
 def checkSymbolsLoadedWithSize():
 active_modules = self.vscode.get_active_modules()
 program_module = active_modules[program_basename]
-symbolsStatus = program_module['symbolStatus']
-symbol_regex = re.compile(r"Symbols loaded. 
\([0-9]+(\.[0-9]*)?[KMG]?B\)")
+symbolsStatus = program_module['debugInfoSize']
+symbol_regex = re.compile(r"[0-9]+(\.[0-9]*)?[KMG]?B")
 return symbol_regex.match(program_module['symbolStatus'])
 
 if expect_debug_info_size:
 self.waitUntil(checkSymbolsLoadedWithSize)
-else:
-self.waitUntil(checkSymbolsLoaded)
 active_modules = self.vscode.get_active_modules()
 program_module = active_modules[program_basename]
 self.assertEqual(program_basename, program_module['name'])


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -354,9 +354,7 @@
 
 static std::string ConvertDebugInfoSizeToString(uint64_t debug_info) {
   std::ostringstream oss;
-  oss << " (";
   oss << std::fixed << std::setprecision(1);
-
   if (debug_info < 1024) {
 oss << debug_info << "B";
   } else if (debug_info < 1024 * 1024) {
@@ -368,9 +366,7 @@
   } else {
 double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
 oss << gb << "GB";
-;
   }
-  oss << ")";
   return oss.str();
 }
 llvm::json::Value CreateModule(lldb::SBModule &module) {
@@ -386,11 +382,13 @@
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
 std::string symbol_str = "Symbols loaded.";
+std::string debug_info_size;
 uint64_t debug_info = GetDebugInfoSize(module);
 if (debug_info > 0) {
-  symbol_str += ConvertDebugInfoSizeToString(debug_info);
+  debug_info_size = ConvertDebu

[Lldb-commits] [lldb] 9390b34 - [lldb] Move ScriptCommand and RegexCommand under Commands (NFC)

2020-09-01 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2020-09-01T17:33:39-07:00
New Revision: 9390b346fc207c3edabbca9665e77260b030cfe0

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

LOG: [lldb] Move ScriptCommand and RegexCommand under Commands (NFC)

Move the CommandObjectScript and CommandObjectRegexCommand under
Commands where all the other CommandObject implementations live.

Although neither implementations currently use the TableGen-generated
CommandOptions.inc, this move would have been necessary anyway if they
were to in the future.

Added: 
lldb/source/Commands/CommandObjectRegexCommand.cpp
lldb/source/Commands/CommandObjectRegexCommand.h
lldb/source/Commands/CommandObjectScript.cpp
lldb/source/Commands/CommandObjectScript.h

Modified: 
lldb/source/Commands/CMakeLists.txt
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Interpreter/CMakeLists.txt
lldb/source/Interpreter/CommandInterpreter.cpp

Removed: 
lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
lldb/source/Interpreter/CommandObjectRegexCommand.cpp
lldb/source/Interpreter/CommandObjectScript.cpp
lldb/source/Interpreter/CommandObjectScript.h



diff  --git a/lldb/source/Commands/CMakeLists.txt 
b/lldb/source/Commands/CMakeLists.txt
index 28bcfacdf3e8..3e57670fd040 100644
--- a/lldb/source/Commands/CMakeLists.txt
+++ b/lldb/source/Commands/CMakeLists.txt
@@ -21,8 +21,10 @@ add_lldb_library(lldbCommands
   CommandObjectPlugin.cpp
   CommandObjectProcess.cpp
   CommandObjectQuit.cpp
+  CommandObjectRegexCommand.cpp
   CommandObjectRegister.cpp
   CommandObjectReproducer.cpp
+  CommandObjectScript.cpp
   CommandObjectSession.cpp
   CommandObjectSettings.cpp
   CommandObjectSource.cpp

diff  --git a/lldb/source/Commands/CommandObjectCommands.cpp 
b/lldb/source/Commands/CommandObjectCommands.cpp
index 92730b6111bb..96ce82d84248 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -6,15 +6,13 @@
 //
 
//===--===//
 
-#include "llvm/ADT/StringRef.h"
-
 #include "CommandObjectCommands.h"
 #include "CommandObjectHelp.h"
+#include "CommandObjectRegexCommand.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/IOHandler.h"
 #include "lldb/Interpreter/CommandHistory.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
-#include "lldb/Interpreter/CommandObjectRegexCommand.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Interpreter/OptionValueBoolean.h"
@@ -24,6 +22,7 @@
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/StringList.h"
+#include "llvm/ADT/StringRef.h"
 
 using namespace lldb;
 using namespace lldb_private;

diff  --git a/lldb/source/Interpreter/CommandObjectRegexCommand.cpp 
b/lldb/source/Commands/CommandObjectRegexCommand.cpp
similarity index 96%
rename from lldb/source/Interpreter/CommandObjectRegexCommand.cpp
rename to lldb/source/Commands/CommandObjectRegexCommand.cpp
index 7485fd76cc25..1bf29d3c047b 100644
--- a/lldb/source/Interpreter/CommandObjectRegexCommand.cpp
+++ b/lldb/source/Commands/CommandObjectRegexCommand.cpp
@@ -6,8 +6,7 @@
 //
 
//===--===//
 
-#include "lldb/Interpreter/CommandObjectRegexCommand.h"
-
+#include "CommandObjectRegexCommand.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 
@@ -17,7 +16,7 @@ using namespace lldb_private;
 // CommandObjectRegexCommand constructor
 CommandObjectRegexCommand::CommandObjectRegexCommand(
 CommandInterpreter &interpreter, llvm::StringRef name, llvm::StringRef 
help,
-  llvm::StringRef syntax, uint32_t max_matches, uint32_t completion_type_mask,
+llvm::StringRef syntax, uint32_t max_matches, uint32_t 
completion_type_mask,
 bool is_removable)
 : CommandObjectRaw(interpreter, name, help, syntax),
   m_max_matches(max_matches), m_completion_type_mask(completion_type_mask),

diff  --git a/lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h 
b/lldb/source/Commands/CommandObjectRegexCommand.h
similarity index 85%
rename from lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
rename to lldb/source/Commands/CommandObjectRegexCommand.h
index cbd50511c483..2f65c2cd815d 100644
--- a/lldb/include/lldb/Interpreter/CommandObjectRegexCommand.h
+++ b/lldb/source/Commands/CommandObjectRegexCommand.h
@@ -21,10 +21,10 @@ namespace lldb_private {
 
 class CommandObjectRegexCommand : public CommandObjectRaw {
 public:
-  CommandObjectRegexCommand(CommandInterpreter &interpreter, llvm::StringRef 
name,
-llvm::StringRef help, llvm

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

2020-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, mib, l.frisken, jingham.
JDevlieghere requested review of this revision.

Make it possible to run the `script` command with a different language than 
currently selected.

  $ ./bin/lldb -l python
  (lldb) script -l lua
  >>> io.stdout:write("Hello, Wolrd!\n")
  Hello, Wolrd!

Because of the ability to pass anything after the `script` command, it is 
implemented as a `CommandObjectRaw`. In a first attempt I tried to change this 
to a `CommandObjectParsed` in order to have the existing infrastructure parse 
the language argument and have the script contents as the first argument. That 
didn't work out, first because it unconditionally removed quotes from within 
the script expression and second because it attempts to expand script commands 
between backticks. Given how easy it is to parse the language argument, I think 
it's warranted to keep the `CommandObjectRaw` and just do the parsing manually.


https://reviews.llvm.org/D86996

Files:
  lldb/source/Commands/CommandObjectScript.cpp
  lldb/test/Shell/ScriptInterpreter/Lua/lua-python.test
  lldb/test/Shell/ScriptInterpreter/Lua/lua.test
  lldb/test/Shell/ScriptInterpreter/None/none.test
  lldb/test/Shell/ScriptInterpreter/Python/python.test

Index: lldb/test/Shell/ScriptInterpreter/Python/python.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/python.test
@@ -0,0 +1,9 @@
+# REQUIRES: python
+# RUN: %lldb --script-language python -o 'script print("{}".format(1000+100+10+1))' 2>&1 | FileCheck %s
+# RUN: %lldb -o 'script -l python print("{}".format(1000+100+10+1))' 2>&1 | FileCheck %s
+# RUN: %lldb -o 'script --language python print("{}".format(1000+100+10+1))' 2>&1 | FileCheck %s
+# CHECK: 
+
+# RUN: %lldb -o 'script --language invalid print("{}".format(1000+100+10+1))' 2>&1 | FileCheck %s --check-prefix INVALID
+# INVALID: SyntaxError
+# INVALID-NOT: 
Index: lldb/test/Shell/ScriptInterpreter/None/none.test
===
--- lldb/test/Shell/ScriptInterpreter/None/none.test
+++ lldb/test/Shell/ScriptInterpreter/None/none.test
@@ -1,2 +1,5 @@
 # RUN: %lldb --script-language none -o 'script' 2>&1 | FileCheck %s
 # CHECK: error: the script-lang setting is set to none - scripting not available
+
+# RUN: %lldb -o 'script -l none foo' 2>&1 | FileCheck %s --check-prefix ERROR
+# ERROR: error: the script-lang setting is set to none - scripting not available
Index: lldb/test/Shell/ScriptInterpreter/Lua/lua.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/lua.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/lua.test
@@ -1,3 +1,5 @@
 # REQUIRES: lua
-# RUN: %lldb --script-language lua -o 'script print(1000+100+10+1)' 2>&1 | FileCheck %s
+# RUN: %lldb --script-language lua -o 'script io.stdout:write(1000+100+10+1, "\n")' 2>&1 | FileCheck %s
+# RUN: %lldb -o 'script -l lua io.stdout:write(1000+100+10+1, "\n")' 2>&1 | FileCheck %s
+# RUN: %lldb -o 'script --language lua io.stdout:write(1000+100+10+1, "\n")' 2>&1 | FileCheck %s
 # CHECK: 
Index: lldb/test/Shell/ScriptInterpreter/Lua/lua-python.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/lua-python.test
@@ -0,0 +1,17 @@
+# REQUIRES: lua
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: echo "int main() { return 0; }" | %clang_host -x c - -o a.out
+# RUN: cat %s | %lldb 2>&1 | FileCheck %s
+script -l lua
+target = lldb.debugger:CreateTarget("a.out")
+print("target is valid:", tostring(target:IsValid()))
+lldb.debugger:SetSelectedTarget(target)
+quit
+# CHECK: target is valid: true
+script -l python
+print("selected target: {}".format(lldb.debugger.GetSelectedTarget()))
+# CHECK: selected target: a.out
Index: lldb/source/Commands/CommandObjectScript.cpp
===
--- lldb/source/Commands/CommandObjectScript.cpp
+++ lldb/source/Commands/CommandObjectScript.cpp
@@ -25,21 +25,63 @@
   interpreter, "script",
   "Invoke the script interpreter with provided code and display any "
   "results.  Start the interactive interpreter if no code is supplied.",
-  "script []") {}
+  "script [-l ] []") {}
 
 CommandObjectScript::~CommandObjectScript() {}
 
+struct CommandParsed {
+  CommandParsed(llvm::StringRef command)
+  : language(llvm::None), command(command) {}
+  CommandParsed(llvm::Optional language,
+llvm::StringRef command)
+  : language(language), command(command) {}
+  llvm::Optional language;
+  llvm::StringRef command;
+};
+
+/// Manually parse the -l  or --language 
+/// argument from the command. This is necessary because CommandObjectScript is
+/// an instance of CommandObjectRaw. We cannot use Comma

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

2020-09-01 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectScript.cpp:65
+
+  return CommandParsed(command);
+}

this is missing error handling for unsupported languages, such as `script -l 
swift`



Comment at: lldb/test/Shell/ScriptInterpreter/Lua/lua-python.test:7
+# RUN: cd %t
+# RUN: echo "int main() { return 0; }" | %clang_host -x c - -o a.out
+# RUN: cat %s | %lldb 2>&1 | FileCheck %s

is `a.out` not a universal default?



Comment at: lldb/test/Shell/ScriptInterpreter/Python/python.test:8
+# RUN: %lldb -o 'script --language invalid print("{}".format(1000+100+10+1))' 
2>&1 | FileCheck %s --check-prefix INVALID
+# INVALID: SyntaxError
+# INVALID-NOT: 

So this `SyntaxError` is from python because it's attempting to evaluate 
`--language invalid print(...)`? I think it'd be good to handle unknown 
languages, but maybe you have a reason for this?


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

https://reviews.llvm.org/D86996

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

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

2020-09-01 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectScript.cpp:50-51
+
+  std::tie(head, tail) = tail.split(' ');
+  if (head != "-l" && head != "--language")
+return CommandParsed(command);

generally, lldb supports more than space delimited  flags. For example I often 
type `expr -lobjc -- ...` with no space. A quick check shows `--language=objc` 
is also supported. Should these cases be handled too?


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

https://reviews.llvm.org/D86996

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

[Lldb-commits] [PATCH] D86996: [lldb] Add -l/--language option to script command

2020-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere planned changes to this revision.
JDevlieghere marked 3 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Commands/CommandObjectScript.cpp:50-51
+
+  std::tie(head, tail) = tail.split(' ');
+  if (head != "-l" && head != "--language")
+return CommandParsed(command);

kastiglione wrote:
> generally, lldb supports more than space delimited  flags. For example I 
> often type `expr -lobjc -- ...` with no space. A quick check shows 
> `--language=objc` is also supported. Should these cases be handled too?
Yes, if that works for other command it should work for this command too. I was 
already on the fence about this, but supporting that would definitely tip the 
scales beyond the complexity I'm willing to accept to work around the 
limitations of `CommandObjectParsed`. I'll need to come up with a different 
solution. 



Comment at: lldb/source/Commands/CommandObjectScript.cpp:65
+
+  return CommandParsed(command);
+}

kastiglione wrote:
> this is missing error handling for unsupported languages, such as `script -l 
> swift`
I actually did that on purpose, I don't think either Python or Lua support 
statements that begin with `-l` or `--language` but I didn't want to intercept 
this unless it meant something to LLDB. But I agree that's not great UX. 



Comment at: lldb/test/Shell/ScriptInterpreter/Lua/lua-python.test:7
+# RUN: cd %t
+# RUN: echo "int main() { return 0; }" | %clang_host -x c - -o a.out
+# RUN: cat %s | %lldb 2>&1 | FileCheck %s

kastiglione wrote:
> is `a.out` not a universal default?
Not sure about Windows, but regardless I prefer to be explicit. 


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

https://reviews.llvm.org/D86996

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