[Lldb-commits] [PATCH] D89600: [lldb] Move copying of reproducer out of process

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Hm... Are you sure that this will help with anything? IIUC, the serialization 
of the VFS file map still happens in the context of the crashing process. This 
requires walking a bunch of data structures with liberally-asserting code, 
which will abort on any inconsistency it runs into. If *that* succeeds, I'd 
expect that the copying of the files will succeed too.

I think it would be better if we moved the vfs map construction into the other 
process too. Maybe via something like this:

- The first process opens a "log" file, which will contain the list of files to 
be copied. For added safety the file should be opened append-only.
- As it works, it writes file names to this file.
- Upon a crash, the re-execed process reads this file, constructs the vfs map, 
and copies all necessary files. It should be prepared to handle/ignore any 
garbled entries, and the format of the file should make this easy. (Since the 
log file must by definition contain enough information to recreate the vfs map, 
it might be simpler to not write out the map, but just always recreate it in 
memory when replaying.)




Comment at: lldb/tools/driver/Driver.cpp:857
+str->append(" --reproducer-finalize ");
+str->append(reproducer_path);
+llvm::sys::AddSignalHandler(reproducer_handler,

I guess this won't work if there are spaces anywhere in the path..



Comment at: lldb/tools/driver/Options.td:235
   HelpText<"Tells the debugger to replay a reproducer from .">;
+def reproducer_finalize: Separate<["--", "-"], "reproducer-finalize">,
+  MetaVarName<"">,

Should this be a hidden argument?


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

https://reviews.llvm.org/D89600

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

[Lldb-commits] [lldb] dfb2266 - [lldb] Make DW_AT_declaration-with-children.s test more realistic

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-10-19T10:34:13+02:00
New Revision: dfb22663287602a90ba249a96c9916227430a99e

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

LOG: [lldb] Make DW_AT_declaration-with-children.s test more realistic

(Re)add DW_AT_specification and DW_AT_object_pointer attributes. These
were removed in fa89f641c, as they were bogus due to bad test case
reduction.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s

Removed: 




diff  --git 
a/lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s 
b/lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
index 3fdee57b6687..0de018afd1b9 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
@@ -147,6 +147,10 @@ c1:
 .byte   6   # DW_FORM_data4
 .byte   64  # DW_AT_frame_base
 .byte   24  # DW_FORM_exprloc
+.byte   100 # DW_AT_object_pointer
+.byte   19  # DW_FORM_ref4
+.byte   71  # DW_AT_specification
+.byte   19  # DW_FORM_ref4
 .byte   0   # EOM(1)
 .byte   0   # EOM(2)
 .byte   11  # Abbreviation Code
@@ -202,6 +206,7 @@ c1:
 .byte   3   # Abbrev [3] 
DW_TAG_structure_type
 .asciz  "A" # DW_AT_name
 # DW_AT_declaration
+.LA_A:
 .byte   4   # Abbrev [4] DW_TAG_subprogram
 .asciz  "A" # DW_AT_name
 # DW_AT_declaration
@@ -218,6 +223,9 @@ c1:
 .long   .LZN1AC2Ev_end-_ZN1AC2Ev# DW_AT_high_pc
 .byte   1   # DW_AT_frame_base
 .byte   86
+.long   .Lthis-.Lcu_begin0  # DW_AT_object_pointer
+.long   .LA_A-.Lcu_begin0   # DW_AT_specification
+.Lthis:
 .byte   11  # Abbrev [11] 
DW_TAG_formal_parameter
 .byte   2   # DW_AT_location
 .byte   145



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

[Lldb-commits] [PATCH] D83302: [lldb/DWARF] Don't treat class declarations with children as definitions

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D83302#2337264 , @jankratochvil 
wrote:

> Fixed stale+unused `DW_AT_object_pointer`+`DW_AT_specification` by 
> rGfa89f641cf9f 
> .

Thanks for catching that. I've re-added the attributes with the correct values 
in dfb226632 
. I 
must've thought the test the test would be too unrealistic since I left them in 
(but then forgot to update them...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83302

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

[Lldb-commits] [PATCH] D89646: [nfc] [lldb] Move `LookupAddress` to `DWARFCompileUnit`

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

This looks fine. The dwo construction code 
(SymbolFileDWARFDwo::GetDWOCompileUnitForHash) already ensures that the 
returned object is really a compile unit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89646

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

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D87868#2337537 , @aadsm wrote:

> Should I still go ahead with this since @labath implemented the memory 
> allocation on lldb-server?

I think going for global symbols still makes, and could still benefit some 
platforms, as my implementation is only for x86 linux.

Which reminds me I wanted to implement that stuff for non-x86 arches too. 
Giving it a try on arm would be appreciated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87868

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

[Lldb-commits] [PATCH] D89383: [lldb] Correct vFile:pread/pwrite packet docs

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 298954.
DavidSpickett added a comment.

- listing what we use decimal for is redundant
- correct spelling of hexadecimal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89383

Files:
  lldb/docs/lldb-platform-packets.txt


Index: lldb/docs/lldb-platform-packets.txt
===
--- lldb/docs/lldb-platform-packets.txt
+++ lldb/docs/lldb-platform-packets.txt
@@ -414,9 +414,8 @@
 //
 //  COMPATIBILITY
 //The gdb-remote serial protocol documentation says that numbers
-//in "vFile:" packets should be hexidecimal. Instead lldb uses
-//decimal for the number of bytes and offset.
-//lldb-server can process either format.
+//in "vFile:" packets should be hexadecimal. Instead lldb uses
+//decimal.
 
 
 //--
@@ -439,8 +438,8 @@
 //
 //  COMPATIBILITY
 //The gdb-remote serial protocol documentation says that numbers
-//in "vFile:" packets should be hexidecimal. Instead lldb uses
-//decimal for the offset. lldb-server can process either format.
+//in "vFile:" packets should be hexadecimal. Instead lldb uses
+//decimal.
 
 
 


Index: lldb/docs/lldb-platform-packets.txt
===
--- lldb/docs/lldb-platform-packets.txt
+++ lldb/docs/lldb-platform-packets.txt
@@ -414,9 +414,8 @@
 //
 //  COMPATIBILITY
 //The gdb-remote serial protocol documentation says that numbers
-//in "vFile:" packets should be hexidecimal. Instead lldb uses
-//decimal for the number of bytes and offset.
-//lldb-server can process either format.
+//in "vFile:" packets should be hexadecimal. Instead lldb uses
+//decimal.
 
 
 //--
@@ -439,8 +438,8 @@
 //
 //  COMPATIBILITY
 //The gdb-remote serial protocol documentation says that numbers
-//in "vFile:" packets should be hexidecimal. Instead lldb uses
-//decimal for the offset. lldb-server can process either format.
+//in "vFile:" packets should be hexadecimal. Instead lldb uses
+//decimal.
 
 
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D87172: Check if debug line sequences are starting after the first code segment

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:506
+for (uint32_t i = 0; i < num_sections; ++i) {
+  SectionSP section_sp(section_list->GetSectionAtIndex(i));
+  const addr_t section_file_address = section_sp->GetFileAddress();

Shouldn't this be also checking the permissions?



Comment at: 
lldb/test/Shell/SymbolFile/DWARF/line-entries-invalid-addresses.yaml:3
+# RUN: %lldb %t -b -o "image lookup -f main.cpp -l 2 -v" | FileCheck %s
+# CHECK: LineEntry: {{.*}}main.cpp:2:{{.*}}
+

I think you also need to check for the "1 matches found", otherwise this will 
succeed even without the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87172

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

[Lldb-commits] [lldb] 0e5248b - [nfc] [lldb] Move LookupAddress to DWARFCompileUnit

[Jan Kratochvil via lldb-commits]

Author: Jan Kratochvil
Date: 2020-10-19T12:44:33+02:00
New Revision: 0e5248be8675db09b48680b9208b70f0e0908895

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

LOG: [nfc] [lldb] Move LookupAddress to DWARFCompileUnit

LookupAddress makes no sense for DWARFTypeUnit.
Also make GetNonSkeletonUnit to preserve the called type.

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
index f54fe0662aa2..9ca160b474fc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
@@ -99,3 +99,18 @@ void DWARFCompileUnit::BuildAddressRangeTable(
 }
   }
 }
+
+DWARFCompileUnit &DWARFCompileUnit::GetNonSkeletonUnit() {
+  return llvm::cast(DWARFUnit::GetNonSkeletonUnit());
+}
+
+DWARFDIE DWARFCompileUnit::LookupAddress(const dw_addr_t address) {
+  if (DIE()) {
+const DWARFDebugAranges &func_aranges = GetFunctionAranges();
+
+// Re-check the aranges auto pointer contents in case it was created above
+if (!func_aranges.IsEmpty())
+  return GetDIE(func_aranges.FindAddress(address));
+  }
+  return DWARFDIE();
+}

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
index 3ec161f7dd51..ab3017ba0ffc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
@@ -20,6 +20,10 @@ class DWARFCompileUnit : public DWARFUnit {
 
   static bool classof(const DWARFUnit *unit) { return !unit->IsTypeUnit(); }
 
+  DWARFCompileUnit &GetNonSkeletonUnit();
+
+  DWARFDIE LookupAddress(const dw_addr_t address);
+
 private:
   DWARFCompileUnit(SymbolFileDWARF &dwarf, lldb::user_id_t uid,
const DWARFUnitHeader &header,

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index b70beb501946..5da23e7c89f1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -392,17 +392,6 @@ void DWARFUnit::AddUnitDIE(const DWARFDebugInfoEntry 
&cu_die) {
   m_dwo = std::shared_ptr(std::move(dwo_symbol_file), dwo_cu);
 }
 
-DWARFDIE DWARFUnit::LookupAddress(const dw_addr_t address) {
-  if (DIE()) {
-const DWARFDebugAranges &func_aranges = GetFunctionAranges();
-
-// Re-check the aranges auto pointer contents in case it was created above
-if (!func_aranges.IsEmpty())
-  return GetDIE(func_aranges.FindAddress(address));
-  }
-  return DWARFDIE();
-}
-
 size_t DWARFUnit::GetDebugInfoSize() const {
   return GetLengthByteSize() + GetLength() - GetHeaderByteSize();
 }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index 1d8236c4ed42..abe16182ef62 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -105,7 +105,6 @@ class DWARFUnit : public lldb_private::UserID {
   };
   ScopedExtractDIEs ExtractDIEsScoped();
 
-  DWARFDIE LookupAddress(const dw_addr_t address);
   bool Verify(lldb_private::Stream *s) const;
   virtual void Dump(lldb_private::Stream *s) const = 0;
   /// Get the data that contains the DIE information for this unit.

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 9454ec51954c..be9832a14dfa 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1811,7 +1811,8 @@ void 
SymbolFileDWARF::ResolveFunctionAndBlock(lldb::addr_t file_vm_addr,
   bool lookup_block,
   SymbolContext &sc) {
   assert(sc.comp_unit);
-  DWARFUnit &cu = GetDWARFCompileUnit(sc.comp_unit)->GetNonSkeletonUnit();
+  DWARFCompileUnit &cu =
+  GetDWARFCompileUnit(sc.comp_unit)->GetNonSkeletonUnit();
   DWARFDIE function_die = cu.LookupAddress(file_vm_addr);
   DWARFDIE block_die;
   if (function_die) {



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

[Lldb-commits] [PATCH] D89646: [nfc] [lldb] Move `LookupAddress` to `DWARFCompileUnit`

[Jan Kratochvil 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 rG0e5248be8675: [nfc] [lldb] Move LookupAddress to 
DWARFCompileUnit (authored by jankratochvil).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89646

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1811,7 +1811,8 @@
   bool lookup_block,
   SymbolContext &sc) {
   assert(sc.comp_unit);
-  DWARFUnit &cu = GetDWARFCompileUnit(sc.comp_unit)->GetNonSkeletonUnit();
+  DWARFCompileUnit &cu =
+  GetDWARFCompileUnit(sc.comp_unit)->GetNonSkeletonUnit();
   DWARFDIE function_die = cu.LookupAddress(file_vm_addr);
   DWARFDIE block_die;
   if (function_die) {
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -105,7 +105,6 @@
   };
   ScopedExtractDIEs ExtractDIEsScoped();
 
-  DWARFDIE LookupAddress(const dw_addr_t address);
   bool Verify(lldb_private::Stream *s) const;
   virtual void Dump(lldb_private::Stream *s) const = 0;
   /// Get the data that contains the DIE information for this unit.
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -392,17 +392,6 @@
   m_dwo = std::shared_ptr(std::move(dwo_symbol_file), dwo_cu);
 }
 
-DWARFDIE DWARFUnit::LookupAddress(const dw_addr_t address) {
-  if (DIE()) {
-const DWARFDebugAranges &func_aranges = GetFunctionAranges();
-
-// Re-check the aranges auto pointer contents in case it was created above
-if (!func_aranges.IsEmpty())
-  return GetDIE(func_aranges.FindAddress(address));
-  }
-  return DWARFDIE();
-}
-
 size_t DWARFUnit::GetDebugInfoSize() const {
   return GetLengthByteSize() + GetLength() - GetHeaderByteSize();
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
@@ -20,6 +20,10 @@
 
   static bool classof(const DWARFUnit *unit) { return !unit->IsTypeUnit(); }
 
+  DWARFCompileUnit &GetNonSkeletonUnit();
+
+  DWARFDIE LookupAddress(const dw_addr_t address);
+
 private:
   DWARFCompileUnit(SymbolFileDWARF &dwarf, lldb::user_id_t uid,
const DWARFUnitHeader &header,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
@@ -99,3 +99,18 @@
 }
   }
 }
+
+DWARFCompileUnit &DWARFCompileUnit::GetNonSkeletonUnit() {
+  return llvm::cast(DWARFUnit::GetNonSkeletonUnit());
+}
+
+DWARFDIE DWARFCompileUnit::LookupAddress(const dw_addr_t address) {
+  if (DIE()) {
+const DWARFDebugAranges &func_aranges = GetFunctionAranges();
+
+// Re-check the aranges auto pointer contents in case it was created above
+if (!func_aranges.IsEmpty())
+  return GetDIE(func_aranges.FindAddress(address));
+  }
+  return DWARFDIE();
+}


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1811,7 +1811,8 @@
   bool lookup_block,
   SymbolContext &sc) {
   assert(sc.comp_unit);
-  DWARFUnit &cu = GetDWARFCompileUnit(sc.comp_unit)->GetNonSkeletonUnit();
+  DWARFCompileUnit &cu =
+  GetDWARFCompileUnit(sc.comp_unit)->GetNonSkeletonUnit();
   DWARFDIE function_die = cu.LookupAddress(file_vm_addr);
   DWARFDIE block_die;
   if (function_die) {
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -105,7 +105,6 @@

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

[Ilya Bukonkin via Phabricator via lldb-commits]

fallkrum updated this revision to Diff 299004.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88483

Files:
  lldb/bindings/interface/SBType.i
  lldb/include/lldb/API/SBModule.h
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/Symbol/Type.h
  lldb/source/API/SBType.cpp
  lldb/source/Symbol/Type.cpp
  lldb/test/API/functionalities/type_get_module/Makefile
  lldb/test/API/functionalities/type_get_module/TestTypeGetModule.py
  lldb/test/API/functionalities/type_get_module/compile_unit1.c
  lldb/test/API/functionalities/type_get_module/compile_unit2.c
  lldb/test/API/functionalities/type_get_module/main.c

Index: lldb/test/API/functionalities/type_get_module/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/type_get_module/main.c
@@ -0,0 +1,2 @@
+
+int main() { return 0; }
Index: lldb/test/API/functionalities/type_get_module/compile_unit2.c
===
--- /dev/null
+++ lldb/test/API/functionalities/type_get_module/compile_unit2.c
@@ -0,0 +1,7 @@
+
+struct compile_unit2_type {
+  int x;
+  int y;
+};
+
+struct compile_unit2_type compile_unit2_var = {2, 2};
Index: lldb/test/API/functionalities/type_get_module/compile_unit1.c
===
--- /dev/null
+++ lldb/test/API/functionalities/type_get_module/compile_unit1.c
@@ -0,0 +1,7 @@
+
+struct compile_unit1_type {
+  int x;
+  int y;
+};
+
+struct compile_unit1_type compile_unit1_var = {1, 1};
Index: lldb/test/API/functionalities/type_get_module/TestTypeGetModule.py
===
--- /dev/null
+++ lldb/test/API/functionalities/type_get_module/TestTypeGetModule.py
@@ -0,0 +1,42 @@
+"""
+Test that SBType returns SBModule of executable file but not
+of compile unit's object file.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestTypeGetModule(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+target  = lldbutil.run_to_breakpoint_make_target(self)
+exe_module = target.GetModuleAtIndex(0)
+
+type1_name = 'compile_unit1_type'
+type2_name = 'compile_unit2_type'
+
+num_comp_units = exe_module.GetNumCompileUnits()
+self.assertEqual(num_comp_units, 3)
+
+comp_unit = exe_module.GetCompileUnitAtIndex(1)
+type_name = comp_unit.GetTypes().GetTypeAtIndex(0).GetName()
+self.assertEqual(type_name, type1_name)
+
+comp_unit = exe_module.GetCompileUnitAtIndex(2)
+type_name = comp_unit.GetTypes().GetTypeAtIndex(0).GetName()
+self.assertEqual(type_name, type2_name)
+
+type1 = target.FindFirstType(type1_name)
+self.assertTrue(type1.IsValid())
+
+type2 = target.FindFirstType(type2_name)
+self.assertTrue(type2.IsValid())
+
+self.assertTrue(exe_module == type1.GetModule() and
+exe_module == type2.GetModule())
Index: lldb/test/API/functionalities/type_get_module/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/type_get_module/Makefile
@@ -0,0 +1,3 @@
+
+C_SOURCES := main.c compile_unit1.c compile_unit2.c
+include Makefile.rules
Index: lldb/source/Symbol/Type.cpp
===
--- lldb/source/Symbol/Type.cpp
+++ lldb/source/Symbol/Type.cpp
@@ -727,6 +727,14 @@
   return ModuleSP();
 }
 
+ModuleSP Type::GetExeModule() {
+  if (m_compiler_type) {
+SymbolFile *symbol_file = m_compiler_type.GetTypeSystem()->GetSymbolFile();
+return symbol_file->GetObjectFile()->GetModule();
+  }
+  return ModuleSP();
+}
+
 TypeAndOrName::TypeAndOrName(TypeSP &in_type_sp) {
   if (in_type_sp) {
 m_compiler_type = in_type_sp->GetForwardCompilerType();
@@ -821,6 +829,7 @@
 void TypeImpl::SetType(const lldb::TypeSP &type_sp) {
   if (type_sp) {
 m_static_type = type_sp->GetForwardCompilerType();
+m_exe_module_wp = type_sp->GetExeModule();
 m_module_wp = type_sp->GetModule();
   } else {
 m_static_type.Clear();
@@ -847,6 +856,15 @@
 }
 
 bool TypeImpl::CheckModule(lldb::ModuleSP &module_sp) const {
+  return CheckModuleCommon(m_module_wp, module_sp);
+}
+
+bool TypeImpl::CheckExeModule(lldb::ModuleSP &module_sp) const {
+  return CheckModuleCommon(m_exe_module_wp, module_sp);
+}
+
+bool TypeImpl::CheckModuleCommon(const lldb::ModuleWP &input_module_wp,
+ lldb::ModuleSP &module_sp) const {
   // Check if we have a module for this type. If we do and the shared pointer
   // is can be successfully initialized with m_module_wp, return true. 

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

[Ilya Bukonkin via Phabricator via lldb-commits]

fallkrum marked 2 inline comments as done.
fallkrum added a comment.

Did not noticed it must be 3 slashes instead of 2 :)

> Also, please make sure that your patch is well formatted (git clang-format 
> HEAD~) before submitting it.

git-clang-format HEAD~ says the patch is well formatted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88483

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

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

[Ilya Bukonkin via Phabricator via lldb-commits]

fallkrum added a comment.

Can you please give me a couple of words on what m_static_type and 
m_dynamic_type is in TypeImpl class, in what cases can it be initialized with 
both of those types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88483

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

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D89283#2336280 , @vsk wrote:

> In D89283#2336120 , @wallace wrote:
>
>> @vsk, I agree with you regarding the files. At the moment our implementation 
>> of intel-pt tracing doesn't support collecting a trace, but soon we'll do 
>> so. Then, we'll be able to generate these trace files on the fly as the 
>> tests run, so I imagine I'll be deleting these binary files. For the time 
>> being I doubt I'll include any new binary, as what is included is more than 
>> enough to test the basic decoding functionalities.
>
> That seems promising. Deleting those binary files after the fact doesn't 
> address the issue, though, as they'd be part of the history. I have a 
> question about that ld-2.17.so file in particular: is there no way to 
> decoder/traverse a trace of a process that loads a dylib, without copying all 
> of ld.so into the source tree? That seems very surprising -- I'd expect the 
> decoder API to allow you to skip right over PC ranges that have nothing to do 
> with the binary you want to debug.

I would very much like to keep some tests which use pre-baked traces (binaries 
are a different matter). I have a couple of reasons for that:

- tests exercising the tracing code path require appropriate hardware. Checked 
in traces can run everywhere (assuming one can build the pt library there)
- due to their systemic nature, it will be hard to test various edge conditions 
(missing binaries, corrupt traces, ...) with a end-to-end test
- we had some issues with end-to-end tests being flaky due to the fact that cpu 
writes the traces asynchronously -- I am not sure we ever fully figured that out

Now, we're definitely going to need tests which check the tracing 
functionality, but I think that having these kinds of traces is definitely 
good. As for ld.so, while it's not the end of the world (we have much larger 
binaries), it would definitely be nice to avoid it. What functionality is that 
test exercising? If you want traces that cross multiple modules, maybe you 
could capture a trace from the middle of an application, after ld.so is done, 
and show how the application is ping-ponging between some functions in 
different shared libraries.

In D89283#2336090 , @vsk wrote:

> One possible alternative:
>
> - Design a textual description for the raw trace contents, and possibly a way 
> to convert an existing trace file into this textual format

I think that converting the textual trace description would essentially mean 
reimplementing the intel-pt library. That might be nice (llvm is definitely 
fond of reimplementing things), but I'm not sure if that's what these guys have 
signed up for.

> - Check in assembly, and use llvm-mc/clang to generate executables during 
> testing

This is definitely doable, though I'd probably go for yaml2obj, as 
llvm-mc&clang cannot guarantee the exact placement of instructions in memory. 
It looks like obj2yaml has grown program header support since last time I 
checked this (previously you had to write them by hand), so it may be that a 
plain `obj2yaml | yaml2obj` would just work now. (It probably won't produce a 
fully functional binary, but it might be close enough.)




Comment at: lldb/include/lldb/Target/Trace.h:160-179
+  /// Determine whether the trace plug-in supports the \a GetInstructionCount
+  /// method.
+  ///
+  /// Some trace plug-ins might prefer to provide instructions lazily on the
+  /// fly, being unable to provide the total number of instructions in a single
+  /// call.
+  ///

How about `Optional GetInstructionCount`? That makes it less likely to 
develop an accidental dependancy on this interface (though I fear that might 
still happen without an subclass which actually returns None here).



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:31
+if (m_custom_error_message.empty()) {
+  std::ostringstream os;
+  os << pt_errstr(pt_errcode(GetErrorCode())) << ": 0x" << std::hex

`raw_string_ostream` would be more llvm-y (the std::hex part in particular is 
very non-idiomatic)



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:41
+
+std::vector &DecodedThread::GetInstructions() {
+  return m_instructions;

Do you want anyone to modify the vector? Return ArrayRef



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:32
+///   returned.
+int FindNextSynchronizationPoint(pt_insn_decoder &decoder) {
+  // Try to sync the decoder. If it fails, then get

static



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:80
+///   error code in case of errors.
+int ProcessPTEvents(pt_insn_decoder &decoder, int errcode) {
+  while (errcode & pts_event_pending) {

static

[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/tools/lldb-server/lldb-gdbserver.cpp:405
+  bool HasError = false;
+  opt::InputArgList Args = Opts.parseArgs(argc - 1, argv + 1, OPT_UNKNOWN,
+  Saver, [&](llvm::StringRef Msg) {

MaskRay wrote:
> Seems that the variable naming is inconsistent with the rest of LLDB and this 
> file.
The only consistent thing about naming in lldb is that it's inconsistent :P. 
Code that heavily interacts with llvm is particularly problematic, since it's 
going to look bad that no matter how you write it...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89477

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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D87442#2334786 , @DavidSpickett 
wrote:

>> Woa, back up. I though you were just going to add the one flag you need 
>> right now... :(
>
> I was going for the showing flags as a feature of the "memory region" command 
> then later adding the memory tagging flag to that.
> I see your point though and yeah I don't need all the flags to unblock mte.
>
> With the perspective I was coming from, adding a set of getter/setter for 
> 20ish flags wasn't an appealing idea:
>
>   bool m_memory_tagged;
>   OptionalBool GetMemoryTagged() const { return m_memory_tagged; }
>   void SetMemoryTagged(bool v) { m_memory_tagged = v; }
>   
>
> If it's just mt then I just add another set, can always merge it with a flags 
> (plural) interface later if we accumulate more.
>
> So if it makes sense to you, I will:
>
> - add only "mt" flag, with it's own getter/setter
> - keep the protocol changes (but only recognise the 1 flag)
> - keep the extra testing, use of expected etc. where it still applies
> - add tests to run on a memory tagging enabled kernel with qemu
>
> Then we can at least agree in principle, even if this doesn't land until the 
> new tagging commands have also been reviewed. (which is fine by me, but I 
> don't have them ready yet)
>
> Thanks for all your comments so far!

Yep, that sounds like a plan.




Comment at: lldb/source/Plugins/Process/Utility/LinuxProcMaps.h:18
 
-typedef std::function LinuxMapCallback;
+typedef llvm::Expected 
ExpectedMemoryRegionInfo;
+typedef std::function LinuxMapCallback;

We don't usually typedef expecteds like this, and the result is not much 
shorter than the original.



Comment at: lldb/unittests/Process/Utility/CMakeLists.txt:5
+  LINK_LIBS
+ lldbPluginProcessLinux
+  )

DavidSpickett wrote:
> labath wrote:
> > Why is this needed?
> It won't link without it. I followed the format of the other tests e.g. 
> unittests/Process/POSIX/CMakeLists.txt
> (you header suggestion does work fine though)
What will not link? This definitely can't be the right solution as 
lldbPluginProcessLinux is not even being built on non-linux hosts (but 
Plugins/Process/Utility is).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

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

[Lldb-commits] [PATCH] D87590: Backport D79219, D85820, D86134 to 10.0 branch

[Brian J. Cardiff via Phabricator via lldb-commits]

bcardiff abandoned this revision.
bcardiff added a comment.

Closing in favor of D84563 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87590

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

[Lldb-commits] [PATCH] D89600: [lldb] Move copying of reproducer out of process

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere planned changes to this revision.
JDevlieghere added a comment.

In D89600#2337868 , @labath wrote:

> Hm... Are you sure that this will help with anything? IIUC, the serialization 
> of the VFS file map still happens in the context of the crashing process. 
> This requires walking a bunch of data structures with liberally-asserting 
> code, which will abort on any inconsistency it runs into. If *that* succeeds, 
> I'd expect that the copying of the files will succeed too.

I only have a handful of crash logs to base myself off. Currently the walking 
and the copying are intertwined, so I can't say for sure. My idea was to give 
this a try and see where that got us and make more invasive changes (like what 
you proposed) if needed. But since you sound pretty convinced that we'll need 
it anyway I will extend the patch.

> I think it would be better if we moved the vfs map construction into the 
> other process too. Maybe via something like this:
>
> - The first process opens a "log" file, which will contain the list of files 
> to be copied. For added safety the file should be opened append-only.
> - As it works, it writes file names to this file.
> - Upon a crash, the re-execed process reads this file, constructs the vfs 
> map, and copies all necessary files. It should be prepared to handle/ignore 
> any garbled entries, and the format of the file should make this easy. (Since 
> the log file must by definition contain enough information to recreate the 
> vfs map, it might be simpler to not write out the map, but just always 
> recreate it in memory when replaying.)

Yes, that would be similar to how we immediately flush GDB remote packets to 
disk. It would require adding some "immediate" mode to the FileCollector and an 
API to convert between the formats. I think the new format could be just host 
paths separated by newlines.




Comment at: lldb/tools/driver/Options.td:235
   HelpText<"Tells the debugger to replay a reproducer from .">;
+def reproducer_finalize: Separate<["--", "-"], "reproducer-finalize">,
+  MetaVarName<"">,

labath wrote:
> Should this be a hidden argument?
Yep


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

https://reviews.llvm.org/D89600

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

[Lldb-commits] [PATCH] D89334: [lldb] Support Python imports relative the to the current file being sourced

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D89334#2334881 , @labath wrote:

> In D89334#2332452 , @JDevlieghere 
> wrote:
>
>> In D89334#2331903 , @labath wrote:
>>
>>> In D89334#2330287 , @JDevlieghere 
>>> wrote:
>>>
 I guess then the user should have called `command script import 
 awesome_backtrace_analyzer` to import the package rather than the 
 `main.py` inside it. But I get your point. FWIW just adding the `$ROOT` is 
 how I did the original implementation before adding the tests for the 
 nested directories and `.py` files that Dave suggested. It would solve 
 this issues but then doesn't support those scenarios. I don't know if it 
 would be less confusing that you can't pass a relative path to a `.py` 
 file or that you can't import another module as you described.
>>>
>>> I don't think the two options are mutually exclusive. I'm pretty sure this 
>>> is just a limitation of our current importing code, which could be fixed to 
>>> import `awesome_backtrace_analyzer/main.py` as 
>>> `awesome_backtrace_analyzer.main` like it would be from python.
>>
>> I don't think we can do that in the general case without breaking users (see 
>> below). I guess we could do it for imports not relative to the current 
>> working directory, as this has never worked before. It would then replace 
>> the logic described by the comment on line  2793.
>
> In general, we cannot impose any kind of natural structure on the path the 
> user gives us -- in an absolute path, the python root could be anywhere. For 
> cwd-relative imports, we could pretend that the cwd is the root, but that 
> seems somewhat unintuitive (and breaks users) (*). Even the usage of the 
> directory of the sourced file as root seems moderately unintuitive to me, 
> though I think it might be a good fit for the motivational use case for this 
> feature.

The issue is that we "resolve" cwd-relative paths to absolute paths and treat 
the result as an absolute path and the "." is only in the system path to make 
relative imports from inside the module work. We could do the same for the 
"source root" but that would mean adding yet another path to the system path.

The more I think about it the more I feel like the current approach in this 
patch is fits best with what we're already doing. If it doesn't work for the 
user they can always adjust the system path in the top level (imported) module.


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

https://reviews.llvm.org/D89334

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

[Lldb-commits] [PATCH] D89477: [lldb] Port lldb gdb-server to libOption

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lldb/source/Utility/Args.cpp:178
 
+Args::Args(llvm::ArrayRef args) : Args() {
+  for (llvm::StringRef arg : args)

labath wrote:
> JDevlieghere wrote:
> > Maybe StringList should have a ctor that takes an `ArrayRef`, 
> > but probably not worth the extra copies here. 
> I think it can have that constructor if it's needed (though I'd rather delete 
> that class altogether), but I think this constructor would be good to have 
> regardless.
Agreed on both accounts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89477

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

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

You can make up types in the expression parser.  So for instance, I can do:

In D88483#2338408 , @fallkrum wrote:

> Can you please give me a couple of words on what m_static_type and 
> m_dynamic_type is in TypeImpl class, in what cases can it be initialized with 
> both of those types?

ValueObjectDynamic value uses a TypeImpl to store its type, and it stores both 
the static type (the one returned by the parsed expression or was in the DWARF 
variable records for some local value) and the dynamic type it figured out by 
looking at the value in memory in this TypeImpl.  See for instance 
ValueObjectDynamicValue::UpdateValue.  ValueObjectDynamicValue could also get 
its static type from its parent.  I'm not sure why it doesn't do that and 
stores it in its TypeImpl instead; presumably this was more convenient 
somewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88483

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

[Lldb-commits] [PATCH] D89716: [LLDB][TestSuite] Improve skipIfWindowsAndNonEnglish in decorators.py

[Alexandre Ganea via Phabricator via lldb-commits]

aganea created this revision.
aganea added reviewers: amccarth, ted, labath.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
aganea requested review of this revision.
Herald added a subscriber: JDevlieghere.

Improve rG79809f58b02419a5d1bfb6c9a59dbd13cd038c77 
 as 
suggested by @ted  :

//"`if lldbplatformutil.getPlatform() != "windows":` will check if the lldb 
platform is windows; this won’t be true if running embedded lldb tests on 
windows. Since we want to see if the host is windows, better to say `if 
sys.platform != "win32":`"//


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89716

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -597,7 +597,7 @@
 def skipIfWindowsAndNonEnglish(func):
 """Decorate the item to skip tests that should be skipped on non-English 
locales on Windows."""
 def is_Windows_NonEnglish(self):
-if lldbplatformutil.getPlatform() != "windows":
+if sys.platform != "win32":
 return None
 kernel = ctypes.windll.kernel32
 if locale.windows_locale[ kernel.GetUserDefaultUILanguage() ] == 
"en_US":


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -597,7 +597,7 @@
 def skipIfWindowsAndNonEnglish(func):
 """Decorate the item to skip tests that should be skipped on non-English locales on Windows."""
 def is_Windows_NonEnglish(self):
-if lldbplatformutil.getPlatform() != "windows":
+if sys.platform != "win32":
 return None
 kernel = ctypes.windll.kernel32
 if locale.windows_locale[ kernel.GetUserDefaultUILanguage() ] == "en_US":
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D89716: [LLDB][TestSuite] Improve skipIfWindowsAndNonEnglish in decorators.py

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Yep


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89716

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

[Lldb-commits] [PATCH] D89716: [LLDB][TestSuite] Improve skipIfWindowsAndNonEnglish in decorators.py

[Alexandre Ganea via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG89b72209ad9b: [LLDB][TestSuite] Improve 
skipIfWindowsAndNonEnglish in decorators.py (authored by aganea).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89716

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -597,7 +597,7 @@
 def skipIfWindowsAndNonEnglish(func):
 """Decorate the item to skip tests that should be skipped on non-English 
locales on Windows."""
 def is_Windows_NonEnglish(self):
-if lldbplatformutil.getPlatform() != "windows":
+if sys.platform != "win32":
 return None
 kernel = ctypes.windll.kernel32
 if locale.windows_locale[ kernel.GetUserDefaultUILanguage() ] == 
"en_US":


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -597,7 +597,7 @@
 def skipIfWindowsAndNonEnglish(func):
 """Decorate the item to skip tests that should be skipped on non-English locales on Windows."""
 def is_Windows_NonEnglish(self):
-if lldbplatformutil.getPlatform() != "windows":
+if sys.platform != "win32":
 return None
 kernel = ctypes.windll.kernel32
 if locale.windows_locale[ kernel.GetUserDefaultUILanguage() ] == "en_US":
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 89b7220 - [LLDB][TestSuite] Improve skipIfWindowsAndNonEnglish in decorators.py

[Alexandre Ganea via lldb-commits]

Author: Alexandre Ganea
Date: 2020-10-19T14:28:08-04:00
New Revision: 89b72209ad9baf585f6765df7c668da112dea230

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

LOG: [LLDB][TestSuite] Improve skipIfWindowsAndNonEnglish in decorators.py

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

Added: 


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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index bb4a459abe68..1775c07c5b7a 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -597,7 +597,7 @@ def skipIfWindows(func):
 def skipIfWindowsAndNonEnglish(func):
 """Decorate the item to skip tests that should be skipped on non-English 
locales on Windows."""
 def is_Windows_NonEnglish(self):
-if lldbplatformutil.getPlatform() != "windows":
+if sys.platform != "win32":
 return None
 kernel = ctypes.windll.kernel32
 if locale.windows_locale[ kernel.GetUserDefaultUILanguage() ] == 
"en_US":



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

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Vitaly Buka via lldb-commits]

Thanks!
Done.

On Sat, 17 Oct 2020 at 12:45, Galina Kistanova  wrote:

> Thanks everyone for keeping your annotated builders in the staging area!
> Much appreciated.
>
> Please feel free to move all the green builders back to the production. It
> has a new AnnotatedCommand now.
>
> Thanks
>
> Galina
>
>
> On Thu, Oct 15, 2020 at 12:46 AM Vitaly Buka 
> wrote:
>
>> Ok, I can switch them back to staging. However today's staging was
>> frequently offline.
>>
>>
>> On Wed, 14 Oct 2020 at 21:44, Galina Kistanova 
>> wrote:
>>
>>> Thanks everyone!
>>>
>>> I would like to keep the bots in the staging till Friday. And if
>>> everything is good, then apply the changes to the production and let you
>>> move all the green bots back there.
>>>
>>> Thanks
>>>
>>> Galina
>>>
>>>
>>> On Tue, Oct 13, 2020 at 10:43 PM Vitaly Buka 
>>> wrote:
>>>
 They do on staging.
 http://lab.llvm.org:8014/#/builders/sanitizer-windows
 http://lab.llvm.org:8014/#/builders/clang-x64-windows-msvc

 Galina, when do you plan to push this to the primary server?
 If it's a few days, I'd rather keep bots on staging to have colors.



 On Tue, 13 Oct 2020 at 11:11, Reid Kleckner  wrote:

> FWIW, I don't see any issues with my two bots that use buildbot
> annotated commands:
> http://lab.llvm.org:8011/#/builders/sanitizer-windows
> http://lab.llvm.org:8011/#/builders/clang-x64-windows-msvc
> The individual steps don't highlight as green or red, but that's OK
> for now.
>
> On Mon, Oct 12, 2020 at 7:19 PM Galina Kistanova 
> wrote:
>
>> We have a better version of AnnotatedCommand on the staging. It
>> should be a functional equivalent of the old one.
>> We need to stress test it well before moving to the production build
>> bot.
>>
>> For that we need all sanitizer + other bots which use the
>> AnnotatedCommand directly or indirectly moved temporarily to the staging.
>>
>> Please let me know when that could be arranged.
>>
>> Thanks
>>
>> Galina
>>
>> On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner 
>> wrote:
>>
>>> On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
>>> lldb-commits@lists.llvm.org> wrote:
>>>
 They are online now -
 http://lab.llvm.org:8011/#/waterfall?tags=sanitizer

 AnnotatedCommand has severe design conflict with the new buildbot.
 We have changed it to be safe and still do something useful, but it
 will need more love and care.

 Please let me know if you have some spare time to work on porting
 AnnotatedCommand.

>>>
>>> That's unfortunate, it would've been good to know that earlier. I
>>> and another team member have spent a fair amount of time porting things 
>>> to
>>> use more AnnotatedCommand steps, because it gives us the flexibility to
>>> test steps locally and make changes to the steps without restarting the
>>> buildbot master. IMO that is the Right Way to define steps: a script 
>>> that
>>> you can run locally on a machine that satisfies the OS and dep 
>>> requirements
>>> of the script.
>>>
>>> I am restarting the two bots that I am responsible for, and may need
>>> some help debugging further issues soon. I'll let you know.
>>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Vitaly Buka via lldb-commits]

There is some issue, at least on staging.
http://lab.llvm.org:8014/#/builders/89/builds/184 has multiple failed
stages but marked as SUCCESS


On Mon, 19 Oct 2020 at 11:43, Vitaly Buka  wrote:

> Thanks!
> Done.
>
> On Sat, 17 Oct 2020 at 12:45, Galina Kistanova 
> wrote:
>
>> Thanks everyone for keeping your annotated builders in the staging area!
>> Much appreciated.
>>
>> Please feel free to move all the green builders back to the production.
>> It has a new AnnotatedCommand now.
>>
>> Thanks
>>
>> Galina
>>
>>
>> On Thu, Oct 15, 2020 at 12:46 AM Vitaly Buka 
>> wrote:
>>
>>> Ok, I can switch them back to staging. However today's staging was
>>> frequently offline.
>>>
>>>
>>> On Wed, 14 Oct 2020 at 21:44, Galina Kistanova 
>>> wrote:
>>>
 Thanks everyone!

 I would like to keep the bots in the staging till Friday. And if
 everything is good, then apply the changes to the production and let you
 move all the green bots back there.

 Thanks

 Galina


 On Tue, Oct 13, 2020 at 10:43 PM Vitaly Buka 
 wrote:

> They do on staging.
> http://lab.llvm.org:8014/#/builders/sanitizer-windows
> http://lab.llvm.org:8014/#/builders/clang-x64-windows-msvc
>
> Galina, when do you plan to push this to the primary server?
> If it's a few days, I'd rather keep bots on staging to have colors.
>
>
>
> On Tue, 13 Oct 2020 at 11:11, Reid Kleckner  wrote:
>
>> FWIW, I don't see any issues with my two bots that use buildbot
>> annotated commands:
>> http://lab.llvm.org:8011/#/builders/sanitizer-windows
>> http://lab.llvm.org:8011/#/builders/clang-x64-windows-msvc
>> The individual steps don't highlight as green or red, but that's OK
>> for now.
>>
>> On Mon, Oct 12, 2020 at 7:19 PM Galina Kistanova <
>> gkistan...@gmail.com> wrote:
>>
>>> We have a better version of AnnotatedCommand on the staging. It
>>> should be a functional equivalent of the old one.
>>> We need to stress test it well before moving to the production build
>>> bot.
>>>
>>> For that we need all sanitizer + other bots which use the
>>> AnnotatedCommand directly or indirectly moved temporarily to the 
>>> staging.
>>>
>>> Please let me know when that could be arranged.
>>>
>>> Thanks
>>>
>>> Galina
>>>
>>> On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner 
>>> wrote:
>>>
 On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
 lldb-commits@lists.llvm.org> wrote:

> They are online now -
> http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>
> AnnotatedCommand has severe design conflict with the new buildbot.
> We have changed it to be safe and still do something useful, but
> it will need more love and care.
>
> Please let me know if you have some spare time to work on porting
> AnnotatedCommand.
>

 That's unfortunate, it would've been good to know that earlier. I
 and another team member have spent a fair amount of time porting 
 things to
 use more AnnotatedCommand steps, because it gives us the flexibility to
 test steps locally and make changes to the steps without restarting the
 buildbot master. IMO that is the Right Way to define steps: a script 
 that
 you can run locally on a machine that satisfies the OS and dep 
 requirements
 of the script.

 I am restarting the two bots that I am responsible for, and may
 need some help debugging further issues soon. I'll let you know.

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

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

[Ilya Bukonkin via Phabricator via lldb-commits]

fallkrum added a comment.

In D88483#2339256 , @jingham wrote:

> In D88483#2338408 , @fallkrum wrote:
>
>> Can you please give me a couple of words on what m_static_type and 
>> m_dynamic_type is in TypeImpl class, in what cases can it be initialized 
>> with both of those types?
>
> ValueObjectDynamic value uses a TypeImpl to store its type, and it stores 
> both the static type (the one returned by the parsed expression or was in the 
> DWARF variable records for some local value) and the dynamic type it figured 
> out by looking at the value in memory in this TypeImpl.  See for instance 
> ValueObjectDynamicValue::UpdateValue.  ValueObjectDynamicValue could also get 
> its static type from its parent.  I'm not sure why it doesn't do that and 
> stores it in its TypeImpl instead; presumably this was more convenient 
> somewhere.

Thanks for explanation. 
Could please tell if I understood you correctly: from the start we load into 
some module’s type system some type for example from DWARF, but during runtime 
it may be altered and as a consequence ‘original’ static type becomes dynamic 
in-memory type? It still seems to me I do not completely understand relation 
between static and it’s dynamic part, if it is correctly to say so. Would like 
to understand the origin of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88483

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

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D88483#2339845 , @fallkrum wrote:

> In D88483#2339256 , @jingham wrote:
>
>> In D88483#2338408 , @fallkrum wrote:
>>
>>> Can you please give me a couple of words on what m_static_type and 
>>> m_dynamic_type is in TypeImpl class, in what cases can it be initialized 
>>> with both of those types?
>>
>> ValueObjectDynamic value uses a TypeImpl to store its type, and it stores 
>> both the static type (the one returned by the parsed expression or was in 
>> the DWARF variable records for some local value) and the dynamic type it 
>> figured out by looking at the value in memory in this TypeImpl.  See for 
>> instance ValueObjectDynamicValue::UpdateValue.  ValueObjectDynamicValue 
>> could also get its static type from its parent.  I'm not sure why it doesn't 
>> do that and stores it in its TypeImpl instead; presumably this was more 
>> convenient somewhere.
>
> Thanks for explanation. 
> Could please tell if I understood you correctly: from the start we load into 
> some module’s type system some type for example from DWARF, but during 
> runtime it may be altered and as a consequence ‘original’ static type becomes 
> dynamic in-memory type? It still seems to me I do not completely understand 
> relation between static and it’s dynamic part, if it is correctly to say so. 
> Would like to understand the origin of this.

This is confusing, in part because TypeImpl is mixing two concepts, Types and 
Values.

A Type can really only have one type, itself...  Types may have base-class and 
derived class Types, but it is itself a definite type.

But a Value though originally known by its declared type, but can also have a 
more specific type based on how it was created.

The most common example of this is w.r.t. base & derived classes.  For 
instance, if you stopped are in a method of a base class in some class 
hierarchy, the Value representing the "this" pointer has a static type which is 
that of the base class.  That's pretty obvious.  But if a derived class object 
is the one actually calling the base class method at the point where you are 
stopped in that method, then the full (which in lldb we call the "dynamic") 
type of the "this" Value is the derived class.  In C++, the dynamic type is 
determined by looking at the vtable pointer in the object, which always points 
at the most specific class of the object.  In ObjC we look at the "isa" 
pointer, which again is always the most specific class.  As these examples 
show, to have a dynamic type you have to have a Value whose vtable or isa 
pointer you can look up.

And dynamic values can change over time:

  BaseClass *a = new BaseClass();
  ...
  free(a);
  a = new DerivedClass();

In this case, the static type of `a` is always `BaseClass *`, and at first it's 
dynamic type is also `BaseClass *`.  But after we free & re-initialize it, the 
dynamic type becomes `DerivedClass *`

I'm assuming TypeImpl picked up a "dynamic" type field because that allowed us 
to represent the Type of a ValueObject with static and dynamic types using one 
entity.  Then, for instance, if we failed to find the dynamic value for some 
reason, this would automatically fall back to the static type.  But dynamic 
types are really a concept that pertains to values not to types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88483

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

[Lldb-commits] [PATCH] D87172: Check if debug line sequences are starting after the first code segment

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

See inlined comments for full details.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:506
+for (uint32_t i = 0; i < num_sections; ++i) {
+  SectionSP section_sp(section_list->GetSectionAtIndex(i));
+  const addr_t section_file_address = section_sp->GetFileAddress();

labath wrote:
> Shouldn't this be also checking the permissions?
Yes we need to check permissions. But we also have to watch out for sections 
that contain sections. For ELF, we have the PT_LOAD[N] sections that contain 
sections. On Mac, we have __TEXT that contains sections, but the first section 
can be __PAGEZERO (which has no read, no write, no execute).

So the main question becomes: if a section contains subsections, do we ignore 
the top level section? I would prefer to see us do this.

So this code should be factored into a lambda or function:

```
bool SymbolFileDWARF::SetFirstCodeAddress(const SectionList §ion_list) {
  const uint32_t num_sections = section_list.GetSize();
  for (uint32_t i = 0; i < num_sections; ++i) {
SectionSP section_sp(section_list.GetSectionAtIndex(i));
if (sections_sp->GetChildren().GetSize() > 0) {
SetFirstCodeAddress(sections_sp->GetChildren());
continue;
}
if (HasExecutePermissions(section_sp)) {
  const addr_t section_file_address = section_sp->GetFileAddress();
  if (m_first_code_address > section_file_address)
m_first_code_address = section_file_address;
}
  }
}
```

And then this code will just look like:
```
const SectionList *section_list = m_objfile_sp->GetModule()->GetSectionList();
if (section_list)
  SetFirstCodeAddress(*section_list);
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87172

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

[Lldb-commits] [lldb] 8a203bb - [trace] rename ThreadIntelPT into TraceTrace

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-10-19T15:15:02-07:00
New Revision: 8a203bb22d161d23c6b1195f85ae025e87f03bae

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

LOG: [trace] rename ThreadIntelPT into TraceTrace

Renamed ThreadIntelPT to TreaceThread, making it a top-level class. I noticed 
that this class can and shuld work for any trace plugin and there's nothing 
intel-pt specific in it.
With that TraceThread change, I was able to move most of the json file parsing 
logic to the base class TraceSessionFileParser, which makes adding new plug-ins 
easier.

This originally was part of https://reviews.llvm.org/D89283

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

Added: 
lldb/include/lldb/Target/ProcessTrace.h
lldb/include/lldb/Target/ThreadTrace.h
lldb/source/Target/ProcessTrace.cpp
lldb/source/Target/ThreadTrace.cpp

Modified: 
lldb/include/lldb/Target/TraceSessionFileParser.h
lldb/include/lldb/lldb-forward.h
lldb/source/API/SystemInitializerFull.cpp
lldb/source/Plugins/Process/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/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
lldb/source/Target/CMakeLists.txt
lldb/source/Target/TraceSessionFileParser.cpp

Removed: 
lldb/source/Plugins/Process/Trace/CMakeLists.txt
lldb/source/Plugins/Process/Trace/ProcessTrace.cpp
lldb/source/Plugins/Process/Trace/ProcessTrace.h
lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.h



diff  --git a/lldb/source/Plugins/Process/Trace/ProcessTrace.h 
b/lldb/include/lldb/Target/ProcessTrace.h
similarity index 90%
rename from lldb/source/Plugins/Process/Trace/ProcessTrace.h
rename to lldb/include/lldb/Target/ProcessTrace.h
index 450aa1e91d8f..d5a052c1d73b 100644
--- a/lldb/source/Plugins/Process/Trace/ProcessTrace.h
+++ b/lldb/include/lldb/Target/ProcessTrace.h
@@ -6,22 +6,17 @@
 //
 
//===--===//
 
-#ifndef LLDB_SOURCE_PLUGINS_PROCESS_TRACE_PROCESSTRACE_H
-#define LLDB_SOURCE_PLUGINS_PROCESS_TRACE_PROCESSTRACE_H
+#ifndef LLDB_TARGET_PROCESSTRACE_H
+#define LLDB_TARGET_PROCESSTRACE_H
 
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
 
 namespace lldb_private {
-namespace process_trace {
 
 class ProcessTrace : public Process {
 public:
-  static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
-lldb::ListenerSP listener_sp,
-const FileSpec *crash_file_path);
-
   static void Initialize();
 
   static void Terminate();
@@ -78,9 +73,13 @@ class ProcessTrace : public Process {
 
   bool UpdateThreadList(ThreadList &old_thread_list,
 ThreadList &new_thread_list) override;
+
+private:
+  static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
+lldb::ListenerSP listener_sp,
+const FileSpec *crash_file_path);
 };
 
-} // namespace process_trace
 } // namespace lldb_private
 
-#endif // LLDB_SOURCE_PLUGINS_PROCESS_TRACE_PROCESSTRACE_H
+#endif // LLDB_TARGET_PROCESSTRACE_H

diff  --git a/lldb/include/lldb/Target/ThreadTrace.h 
b/lldb/include/lldb/Target/ThreadTrace.h
new file mode 100644
index ..a32b33867c26
--- /dev/null
+++ b/lldb/include/lldb/Target/ThreadTrace.h
@@ -0,0 +1,61 @@
+//===-- ThreadTrace.h ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_THREADTRACE_H
+#define LLDB_TARGET_THREADTRACE_H
+
+#include "lldb/Target/Thread.h"
+
+namespace lldb_private {
+
+/// \class ThreadTrace ThreadTrace.h
+///
+/// Thread implementation used for representing threads gotten from trace
+/// session files, which are similar to threads from core files.
+///
+/// See \a TraceSessionFileParser for more information regarding trace session
+/// files.
+class ThreadTrace : public Thread {
+public:
+  /// \param[in] process
+  /// The process who owns this thread.
+  ///
+  /// \param[in] tid
+  /// The tid of this thread.
+  ///
+  /// \param[in] trace_file.
+  /// The file that contains the list of instructions that were traced

[Lldb-commits] [PATCH] D89408: [trace] rename ThreadIntelPT to ThreadTrace

[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 rG8a203bb22d16: [trace] rename ThreadIntelPT into TraceTrace 
(authored by Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89408

Files:
  lldb/include/lldb/Target/ProcessTrace.h
  lldb/include/lldb/Target/ThreadTrace.h
  lldb/include/lldb/Target/TraceSessionFileParser.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/Trace/CMakeLists.txt
  lldb/source/Plugins/Process/Trace/ProcessTrace.cpp
  lldb/source/Plugins/Process/Trace/ProcessTrace.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/ProcessTrace.cpp
  lldb/source/Target/ThreadTrace.cpp
  lldb/source/Target/TraceSessionFileParser.cpp

Index: lldb/source/Target/TraceSessionFileParser.cpp
===
--- lldb/source/Target/TraceSessionFileParser.cpp
+++ lldb/source/Target/TraceSessionFileParser.cpp
@@ -10,8 +10,11 @@
 
 #include 
 
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/ThreadTrace.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -87,6 +90,81 @@
   return schema_builder.str();
 }
 
+ThreadTraceSP TraceSessionFileParser::ParseThread(ProcessSP &process_sp,
+  const JSONThread &thread) {
+  lldb::tid_t tid = static_cast(thread.tid);
+
+  FileSpec trace_file(thread.trace_file);
+  NormalizePath(trace_file);
+
+  ThreadTraceSP thread_sp =
+  std::make_shared(*process_sp, tid, trace_file);
+  process_sp->GetThreadList().AddThread(thread_sp);
+  return thread_sp;
+}
+
+Expected
+TraceSessionFileParser::ParseProcess(const JSONProcess &process) {
+  TargetSP target_sp;
+  Status error = m_debugger.GetTargetList().CreateTarget(
+  m_debugger, /*user_exe_path*/ StringRef(), process.triple,
+  eLoadDependentsNo,
+  /*platform_options*/ nullptr, target_sp);
+
+  if (!target_sp)
+return error.ToError();
+
+  ParsedProcess parsed_process;
+  parsed_process.target_sp = target_sp;
+
+  m_debugger.GetTargetList().SetSelectedTarget(target_sp.get());
+
+  ProcessSP process_sp = target_sp->CreateProcess(
+  /*listener*/ nullptr, "trace",
+  /*crash_file*/ nullptr);
+
+  process_sp->SetID(static_cast(process.pid));
+
+  for (const JSONThread &thread : process.threads)
+parsed_process.threads.push_back(ParseThread(process_sp, thread));
+
+  for (const JSONModule &module : process.modules)
+if (Error err = ParseModule(target_sp, module))
+  return std::move(err);
+
+  if (!process.threads.empty())
+process_sp->GetThreadList().SetSelectedThreadByIndexID(0);
+
+  // We invoke DidAttach to create a correct stopped state for the process and
+  // its threads.
+  ArchSpec process_arch;
+  process_sp->DidAttach(process_arch);
+
+  return parsed_process;
+}
+
+Expected>
+TraceSessionFileParser::ParseCommonSessionFile(
+const JSONTraceSessionBase &session) {
+  std::vector parsed_processes;
+
+  auto onError = [&]() {
+// Delete all targets that were created so far in case of failures
+for (ParsedProcess &parsed_process : parsed_processes)
+  m_debugger.GetTargetList().DeleteTarget(parsed_process.target_sp);
+  };
+
+  for (const JSONProcess &process : session.processes) {
+if (Expected parsed_process = ParseProcess(process))
+  parsed_processes.push_back(std::move(*parsed_process));
+else {
+  onError();
+  return parsed_process.takeError();
+}
+  }
+  return parsed_processes;
+}
+
 namespace llvm {
 namespace json {
 
@@ -129,5 +207,12 @@
   return o && o.map("type", plugin_settings.type);
 }
 
+bool fromJSON(const Value &value,
+  TraceSessionFileParser::JSONTraceSessionBase &session,
+  Path path) {
+  ObjectMapper o(value, path);
+  return o && o.map("processes", session.processes);
+}
+
 } // namespace json
 } // namespace llvm
Index: lldb/source/Target/ThreadTrace.cpp
===
--- lldb/source/Target/ThreadTrace.cpp
+++ lldb/source/Target/ThreadTrace.cpp
@@ -1,4 +1,4 @@
-//===-- ThreadIntelPT.cpp -===//
+//===-- ThreadTrace.cpp -

[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D87868#2337537 , @aadsm wrote:

> Should I still go ahead with this since @labath implemented the memory 
> allocation on lldb-server?

yes! It might be best to move this lookup functionality into 
lldb_private::Target so other bits of code that try to call a function 
directory can benefit. But we don't need to do that for this patch if you don't 
have time as there are not too many locations that do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87868

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

[Lldb-commits] [PATCH] D87172: Check if debug line sequences are starting after the first code segment

[António Afonso via Phabricator via lldb-commits]

aadsm added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:506
+for (uint32_t i = 0; i < num_sections; ++i) {
+  SectionSP section_sp(section_list->GetSectionAtIndex(i));
+  const addr_t section_file_address = section_sp->GetFileAddress();

clayborg wrote:
> labath wrote:
> > Shouldn't this be also checking the permissions?
> Yes we need to check permissions. But we also have to watch out for sections 
> that contain sections. For ELF, we have the PT_LOAD[N] sections that contain 
> sections. On Mac, we have __TEXT that contains sections, but the first 
> section can be __PAGEZERO (which has no read, no write, no execute).
> 
> So the main question becomes: if a section contains subsections, do we ignore 
> the top level section? I would prefer to see us do this.
> 
> So this code should be factored into a lambda or function:
> 
> ```
> bool SymbolFileDWARF::SetFirstCodeAddress(const SectionList §ion_list) {
>   const uint32_t num_sections = section_list.GetSize();
>   for (uint32_t i = 0; i < num_sections; ++i) {
> SectionSP section_sp(section_list.GetSectionAtIndex(i));
> if (sections_sp->GetChildren().GetSize() > 0) {
> SetFirstCodeAddress(sections_sp->GetChildren());
> continue;
> }
> if (HasExecutePermissions(section_sp)) {
>   const addr_t section_file_address = section_sp->GetFileAddress();
>   if (m_first_code_address > section_file_address)
> m_first_code_address = section_file_address;
> }
>   }
> }
> ```
> 
> And then this code will just look like:
> ```
> const SectionList *section_list = m_objfile_sp->GetModule()->GetSectionList();
> if (section_list)
>   SetFirstCodeAddress(*section_list);
> ```
> 
Sorry about this. I completely changed the original logic with my last update. 
I was checking the section type before and also going down the section 
hierarchy but not anymore.

We should go through all sub sections just like I did in my original code, I 
was also checking if it was a code section, rather than permissions, it's a bit 
more abstract that way.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87172

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

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 3 inline comments as done.
wallace added a comment.

@labath is right regarding the need of pre-baked binaries to test specific 
conditions. I'll remove the ld binary, as it really tests nothing useful, and 
i'll try to use yaml to represent the binaries in a more concise format.




Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:43-48
+  [ 0] 0x40052d
+  [ 1] 0x400529
+  [ 2] 0x400525
+  [ 3] 0x400521
+  [ 4] 0x40052d
+  [ 5] 0x400529

labath wrote:
> Are you sure that printing this backwards is the best way to display this? 
> The resulting disassembly is going to look quite weird. I think that printing 
> this in the "normal" direction would make it easier to figure out what the 
> program was doing. For people who are only interested in the final PC value 
> it should not be a problem to skip to the last line of the output (the last 
> line is also more likely to remain visible if the dump produces lots of data).
First of all, I'm thinking about adding a flag to this command to choose the 
direction, as there are benefits of both.

Let's say, if you are interested in reading/understanding the last instructions 
up to a breakpoint, then reading the trace in reverse makes sense, as you don't 
know where to start reading from, but you know where to end. Imagine you have 
100K instructions, where do you start? It seems sometimes better to read the 
last instructions and then ask for a few of the earlier instructions, and keep 
doing that until you find what you are interested in. On the other hand, if you 
want to analyze forwards what happens from a certain point, this API is quite 
annoying and I imagine you'd prefer to read it forwards.

So I propose

  thread trace dump instructions --count <> --start-position <> [--forwards | 
-f] [--backwards | -b]

I'd keep -b as default, as it's useful when analyzing crashes or stops on 
breakpoints. The default --start-position when reading forwards could be the 
oldest chronological instruction, and the default when reading backwards could 
be the earliest chronologically.

With this, I'd change the indices. I'd make index [0] to be the oldest 
chronologically and [|trace| -1] to be the most recent.

@labath, @clayborg, what do you think? This might be flexible enough for the 
different kind of usages.



Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:159-160
+substrs=['''thread #1: tid = 3842849, total instructions = 2
+  [0] no memory mapped at this address: 0x400518
+  [1] no memory mapped at this address: 0x400511'''])
+

clayborg wrote:
> These lines should start with the address like all other lines. Then the 
> question is what the output should look like. Do we really need to tell the 
> user that there is no memory mapped here? Can we just print "" or 
> nothing if we have no information like:
> 
> ```
> [0] 0x400518: 
> [1] 0x400511: 
> ```
> 
I think it's highly important to tell the user that this is a very important 
error and not make it apparently inoffensive with the formatting. Let me 
elaborate why this is not an inoffensive error.

First of all, the encoded trace is composed of packets, composed of two main 
packets:

- PSB: synchronization packet that contains the current PC. These packets are 
sporadic (often one for each 4KB of data), as they are big in size.
- TNT: taken/not taken packet that contains one bit per branch executed by the 
processor. These packets are probably the most frequent and they don't include 
any PC.

When decoding, the decoder finds first a PSB packet, gaining the knowledge of 
the current PC, then it starts traversing the binary instruction by instruction 
until it finds a branch, in which case it finds the next TNT packet and learns 
if that branch was taken or not, then continuing the traversal in the correct 
direction. 

This means that when the decoder can't read a memory address, then it won't be 
able to decode any TNT packets until the next PSB synchronization point. In 
fact, in this diff, when there's an instruction decoding error, we skip to the 
next PSB and resume decoding from there. This problem implies that we are 
skipping potentially thousands of instructions. In other words, if you see

[0]: 0x400518
[1]: 0x400511
[2]: no memory mapped at this address: 0x400502
[3]: 0x400500

Then that means that between instructions [3] and [1] there were an unknown 
number of instructions that couldn't be decoded, the first one of them being at 
0x400502. We won't be able to do anything useful with those instructions, and 
the user would need to provide the missing module and redecode to reconstruct 
the full trace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/c

[Lldb-commits] [PATCH] D89600: [lldb] Move copying of reproducer out of process

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 299214.
JDevlieghere added a comment.

- Implement a FlushingFileCollector that appends paths to a file
- Create the mapping out-of-process
- Add an InProcessFinalizer that uses the current reproducer instance to copy 
over the files in-process


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

https://reviews.llvm.org/D89600

Files:
  lldb/include/lldb/API/SBReproducer.h
  lldb/include/lldb/Host/FileSystem.h
  lldb/include/lldb/Utility/Reproducer.h
  lldb/include/lldb/Utility/ReproducerProvider.h
  lldb/source/API/SBReproducer.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h
  lldb/source/Utility/Reproducer.cpp
  lldb/source/Utility/ReproducerProvider.cpp
  lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test
  lldb/tools/driver/Driver.cpp
  lldb/tools/driver/Options.td

Index: lldb/tools/driver/Options.td
===
--- lldb/tools/driver/Options.td
+++ lldb/tools/driver/Options.td
@@ -232,6 +232,8 @@
 def replay: Separate<["--", "-"], "replay">,
   MetaVarName<"">,
   HelpText<"Tells the debugger to replay a reproducer from .">;
+def reproducer_finalize: Separate<["--", "-"], "reproducer-finalize">,
+  MetaVarName<"">;
 def no_version_check: F<"reproducer-no-version-check">,
   HelpText<"Disable the reproducer version check.">;
 def no_verification: F<"reproducer-no-verify">,
Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -729,25 +729,10 @@
   signal(signo, sigcont_handler);
 }
 
-void reproducer_handler(void *argv0) {
+void reproducer_handler(void *cmd) {
   if (SBReproducer::Generate()) {
-auto exe = static_cast(argv0);
-llvm::outs() << "\n";
-llvm::outs() << "Crash reproducer for ";
-llvm::outs() << lldb::SBDebugger::GetVersionString() << '\n';
-llvm::outs() << '\n';
-llvm::outs() << "Reproducer written to '" << SBReproducer::GetPath()
- << "'\n";
-llvm::outs() << '\n';
-llvm::outs() << "Before attaching the reproducer to a bug report:\n";
-llvm::outs() << " - Look at the directory to ensure you're willing to "
-"share its content.\n";
-llvm::outs()
-<< " - Make sure the reproducer works by replaying the reproducer.\n";
-llvm::outs() << '\n';
-llvm::outs() << "Replay the reproducer with the following command:\n";
-llvm::outs() << exe << " -replay " << SBReproducer::GetPath() << "\n";
-llvm::outs() << "\n";
+std::system(static_cast(cmd));
+fflush(stdout);
   }
 }
 
@@ -799,6 +784,31 @@
 
 llvm::Optional InitializeReproducer(llvm::StringRef argv0,
  opt::InputArgList &input_args) {
+  if (auto *finalize_path = input_args.getLastArg(OPT_reproducer_finalize)) {
+if (const char *error = SBReproducer::Finalize(finalize_path->getValue())) {
+  WithColor::error() << "reproducer finalization failed: " << error << '\n';
+  return 1;
+}
+
+llvm::outs() << "\n";
+llvm::outs() << "Crash reproducer for ";
+llvm::outs() << lldb::SBDebugger::GetVersionString() << '\n';
+llvm::outs() << '\n';
+llvm::outs() << "Reproducer written to '" << SBReproducer::GetPath()
+ << "'\n";
+llvm::outs() << '\n';
+llvm::outs() << "Before attaching the reproducer to a bug report:\n";
+llvm::outs() << " - Look at the directory to ensure you're willing to "
+"share its content.\n";
+llvm::outs()
+<< " - Make sure the reproducer works by replaying the reproducer.\n";
+llvm::outs() << '\n';
+llvm::outs() << "Replay the reproducer with the following command:\n";
+llvm::outs() << argv0 << " -replay " << finalize_path->getValue() << "\n";
+llvm::outs() << "\n";
+return 0;
+  }
+
   if (auto *replay_path = input_args.getLastArg(OPT_replay)) {
 SBReplayOptions replay_options;
 replay_options.SetCheckVersion(!input_args.hasArg(OPT_no_version_check));
@@ -821,12 +831,6 @@
   }
 
   if (capture || capture_path) {
-// Register the reproducer signal handler.
-if (!input_args.hasArg(OPT_no_generate_on_signal)) {
-  llvm::sys::AddSignalHandler(reproducer_handler,
-  const_cast(argv0.data()));
-}
-
 if (capture_path) {
   if (!capture)
 WithColor::warning() << "-capture-path specified without -capture\n";
@@ -843,6 +847,19 @@
 }
 if (generate_on_exit)
   SBReproducer::SetAutoGenerate(true);
+
+// Register the reproducer signal handler.
+if (!input_args.hasArg(OPT_no_generate_on_signal)) {
+  if (const char *reproducer_path = SBReproducer::GetPath()) {
+// 

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:31
+if (m_custom_error_message.empty()) {
+  std::ostringstream os;
+  os << pt_errstr(pt_errcode(GetErrorCode())) << ": 0x" << std::hex

labath wrote:
> `raw_string_ostream` would be more llvm-y (the std::hex part in particular is 
> very non-idiomatic)
That or "lldb_private::StreamString". Both have similar functionality. I prefer 
StreamString because it is simpler. With raw_string_ostream, you have to make a 
std::string, put it into the raw_string_ostream and then flush it prior to 
getting the string result.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:41
+
+std::vector &DecodedThread::GetInstructions() {
+  return m_instructions;

labath wrote:
> Do you want anyone to modify the vector? Return ArrayRef
yeah llvm::ArrayRef to avoid making copies is good.



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:32
+///   returned.
+int FindNextSynchronizationPoint(pt_insn_decoder &decoder) {
+  // Try to sync the decoder. If it fails, then get

labath wrote:
> static
Make static or add an anonymous namespace around all of these functions so you 
don't have to mark them all as static.



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:80
+///   error code in case of errors.
+int ProcessPTEvents(pt_insn_decoder &decoder, int errcode) {
+  while (errcode & pts_event_pending) {

labath wrote:
> static
Make static or add an anonymous namespace around all of these functions so you 
don't have to mark them all as static.



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:94
+
+  std::vector instructions =
+  decoded_thread->GetInstructions();

labath wrote:
> this makes a copy, which you probably did not want.
returning a llvm::ArrayRef to avoid the copy



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:98
+  int delta = direction == TraceDirection::Forwards ? -1 : 1;
+  for (uint64_t i = position; i < instructions.size() && i >= 0; i += delta)
+if (!callback(i, instructions[i].GetLoadAddress()))

labath wrote:
> `i>=0` is always true. You'll have to do this trick with signed numbers 
> (ssize_t?)
Yes, switch to ssize_t, your delta is already signed. Also switch "delta" to 
ssize_t as well.



Comment at: lldb/source/Target/Trace.cpp:106
+
+  size_t last_index = (int64_t)start_position + count - 1;
+  TraverseInstructions(

labath wrote:
> The cast to int64_t won't change the actual value of the result (though it 
> may invoke UB due to signed wraparound). What exactly are you trying to 
> achieve here?
Lots os signed/unsigned match issues possible. Best to make this rock solid.



Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:48
+  [ 4] 0x40052d
+  [ 5] 0x400529
+  [ 6] 0x400525

If we reverse the direction, then hitting "enter" after doing one command won't 
flow as nicely as it does now. That being said I agree with Pavel that we 
should figure out what is expected. I generally think that earlier text is 
older.

I would not switch the indexes so that they change with any options that are 
specified. We currently have --start-position, but maybe this should be just 
--position? Or we specify:
```
--from-end  
```
 would be the index offset from the end (newest) of the data?
```
--from-start 
```
 would be the index offset from the start (oldest) of the data?

I would be fine with:
```
[--forwards | -f] [--backwards | -b]
```

but I think it would make sense to show the indexes in a consistent way 
regardless of what options are displayed. Maybe it makes sense to always show 
the true index where zero is the oldest and N is the newest?

We do need to make sure the auto repeat command looks good though which will be 
hard with oldest to newest ordering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

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

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[walter erquinigo via Phabricator via lldb-commits]

wallace marked an inline comment as done.
wallace added inline comments.



Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:48
+  [ 4] 0x40052d
+  [ 5] 0x400529
+  [ 6] 0x400525

clayborg wrote:
> If we reverse the direction, then hitting "enter" after doing one command 
> won't flow as nicely as it does now. That being said I agree with Pavel that 
> we should figure out what is expected. I generally think that earlier text is 
> older.
> 
> I would not switch the indexes so that they change with any options that are 
> specified. We currently have --start-position, but maybe this should be just 
> --position? Or we specify:
> ```
> --from-end  
> ```
>  would be the index offset from the end (newest) of the data?
> ```
> --from-start 
> ```
>  would be the index offset from the start (oldest) of the data?
> 
> I would be fine with:
> ```
> [--forwards | -f] [--backwards | -b]
> ```
> 
> but I think it would make sense to show the indexes in a consistent way 
> regardless of what options are displayed. Maybe it makes sense to always show 
> the true index where zero is the oldest and N is the newest?
> 
> We do need to make sure the auto repeat command looks good though which will 
> be hard with oldest to newest ordering.
What about this:

We expose the indices in a chronologically increasing way, where [0] is the 
oldest instruction and [N] is the newest. Then we have the two options 
suggested by Greg

  --from-end 

Where offset is an index or the string "end", meaning the last instruction of 
the trace, in case the user doesn't know the index of it. Then the instructions 
are printed

[offset]
[offset - 1]
...
[offset - K]

And if there's a repeat command, this would be printed

[offset - K - 1]
[offset - K - 2]
...

Which would look nicely as a contiguous list of instructions if concatenated.

The other option would be

  --from-start

Where offset is an index.  Then the instructions are printed

[offset]
[offset + 1]
[offset + 2]
...
[offset + K]

And after a repeat command, you'd get

[offset + K + 1]
[offset + K + 2]
...


I think this would serve all purposes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

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

[Lldb-commits] [lldb] f96e16b - lldb: Update for change in `clang::Lexer`'s constructor

[Duncan P . N . Exon Smith via lldb-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-10-19T20:09:27-04:00
New Revision: f96e16bc15a9a10857681b85b6c8824b9addd9b2

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

LOG: lldb: Update for change in `clang::Lexer`'s constructor

b3eff6b7bb31e7ef059a3d238de138849839fbbd updated `Lexer::Lexer` to take
`clang::MemoryBufferRef` instead of `clang::MemoryBuffer*`. Update LLDB
to fix the bots.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
index a429963277d1..937dbc00521e 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -231,7 +231,7 @@ TokenVerifier::TokenVerifier(std::string body) {
   Opts.CPlusPlus17 = true;
   Opts.LineComment = true;
 
-  Lexer lex(FID, buf.get(), SM, Opts);
+  Lexer lex(FID, buf->getMemBufferRef(), SM, Opts);
 
   Token token;
   bool exit = false;

diff  --git a/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp 
b/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
index aaf578c6f728..f3194beff1ec 100644
--- a/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
+++ b/lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
@@ -178,7 +178,7 @@ void ClangHighlighter::Highlight(const HighlightStyle 
&options,
   Opts.CPlusPlus17 = true;
   Opts.LineComment = true;
 
-  Lexer lex(FID, buf.get(), SM, Opts);
+  Lexer lex(FID, buf->getMemBufferRef(), SM, Opts);
   // The lexer should keep whitespace around.
   lex.SetKeepWhitespaceMode(true);
 



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

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 16 inline comments as done.
wallace added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp:188
+  if (int error = pt_image_set_callback(image, ReadProcessMemory, &process))
+return CreateLibiptError(error);
+

labath wrote:
> It looks like this can only fail if the image argument is null (which can 
> only happen if the decoder is null, which is checked). An assert would be 
> enough for that. (For a proper error handling you should have also freed the 
> decoder object on the error path, which is how i came to thing about this).
good catch!



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:98
+  int delta = direction == TraceDirection::Forwards ? -1 : 1;
+  for (uint64_t i = position; i < instructions.size() && i >= 0; i += delta)
+if (!callback(i, instructions[i].GetLoadAddress()))

clayborg wrote:
> labath wrote:
> > `i>=0` is always true. You'll have to do this trick with signed numbers 
> > (ssize_t?)
> Yes, switch to ssize_t, your delta is already signed. Also switch "delta" to 
> ssize_t as well.
TIL!



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:109
+  else {
+consumeError(decoded_thread.takeError());
+return 0;

labath wrote:
> I'm having doubts about this "I have an thread but was not able to decode 
> _anything_ about it" state is worth it. Having many different ways to report 
> errors just increases the chance of something going wrong, and in 
> `TraverseInstructions` you're already treating this state as a 
> pseudo-instruction.
> 
> Maybe that representation should be used all the way down? Or (and this may 
> be even better) we avoid creating such Threads in the first place (print an 
> error/warning when the target is created).
> Maybe that representation should be used all the way down? 

I'll follow that path. This will create consistency through the code


> Or (and this may be even better) we avoid creating such Threads in the first 
> place (print an error/warning when the target is created).
 
I wish I could do that, but decoding is very expensive and should be done 
lazily. According to Intel and my tests, if a thread was traced during T 
seconds, then decoding takes around 10T, which is a big amount of time if you 
were tracing 10 threads for 5 seconds, which would take 500 seconds to decode. 
At least for know we are not doing parallel decoding. I imagine at some point 
we'll have to work on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

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

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 299236.
wallace marked 3 inline comments as done.
wallace added a comment.

Addressed almost all changes.

What's left to discuss:

- I tried to use obj2yaml, but it creates files much larger than the binaries, 
so in terms of space, I'd rather keep the binaries. Besides, I removed ld.so, 
and the binaries that are left are tiny. I imagine that the binaries that will 
be committed in the future will also be tiny ones depicting specific edge cases.
- Showing the disassembly of the instructions, requested by Greg, is not yet 
done.
- I'm representing now all decoding errors as instructions, which in fact 
simplifies the API a good deal. However, I'm still not happy with the API, 
right now we have

class Trace {

  void TraverseInstructions(Thread, callback, ...);
  int GetInstructionsCount(Thread...);

}

Right now these methods are not doing anything (or returning 0) if the provided 
Thread is not traced by the Trace instance. Which is kind of what 
ThreadList::GetThreadAtIndex does, as well as many other APIs. To keep 
consistency with most of LLDB core APIs, we could leave the code as it is, 
which simplifies a lot the API.

class Trace {

  TracedThreadSP GetTraceForThread(Thread ...);

}

class TracedThread {

  void TraverseInstructions(callback, ...);
  int GetInstructionsCount()

}

This creates a level of indirection that forces the caller to check if the 
returned TracedTrace is a valid pointer or not. If the pointer is invalid, the 
caller can show an error message or fail silenty, otherwise, 
TraverseInstructions and GetInstructionsCount will perform valid operations.

This might become useful to avoid errors in complex scenarios as the API grows. 
But then, how often will someone invoke these methods with the wrong thread?

The first form of the API looks simpler without that level of indirection, 
which is cool.

What do you guys think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

Files:
  lldb/include/lldb/Target/ProcessTrace.h
  lldb/include/lldb/Target/Trace.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/ProcessTrace.cpp
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceSessionFileParser.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/intelpt-trace-multi-file/a.out
  lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.h
  lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.h
  lldb/test/API/commands/trace/intelpt-trace-multi-file/libbar.so
  lldb/test/API/commands/trace/intelpt-trace-multi-file/libfoo.so
  lldb/test/API/commands/trace/intelpt-trace-multi-file/main.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file-no-ld.json
  lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file.trace
  lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
  lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json
@@ -0,0 +1,31 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 2123123,
+  "model": 12123123,
+  "stepping": 1231231
+}
+  },
+  "processes": [
+{
+  "pid": 1234,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 3842849,
+  "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/intelpt-trace/trace_bad_image.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
@@ -0,0 +1,31 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": [
+{
+  "pid": 1234,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 3842849,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.o

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Vitaly Buka via lldb-commits]

it's not just staging, this build must be marked as failed
http://lab.llvm.org:8011/#/builders/74/builds/122


On Mon, 19 Oct 2020 at 11:50, Vitaly Buka  wrote:

> There is some issue, at least on staging.
> http://lab.llvm.org:8014/#/builders/89/builds/184 has multiple failed
> stages but marked as SUCCESS
>
>
> On Mon, 19 Oct 2020 at 11:43, Vitaly Buka  wrote:
>
>> Thanks!
>> Done.
>>
>> On Sat, 17 Oct 2020 at 12:45, Galina Kistanova 
>> wrote:
>>
>>> Thanks everyone for keeping your annotated builders in the staging area!
>>> Much appreciated.
>>>
>>> Please feel free to move all the green builders back to the production.
>>> It has a new AnnotatedCommand now.
>>>
>>> Thanks
>>>
>>> Galina
>>>
>>>
>>> On Thu, Oct 15, 2020 at 12:46 AM Vitaly Buka 
>>> wrote:
>>>
 Ok, I can switch them back to staging. However today's staging was
 frequently offline.


 On Wed, 14 Oct 2020 at 21:44, Galina Kistanova 
 wrote:

> Thanks everyone!
>
> I would like to keep the bots in the staging till Friday. And if
> everything is good, then apply the changes to the production and let you
> move all the green bots back there.
>
> Thanks
>
> Galina
>
>
> On Tue, Oct 13, 2020 at 10:43 PM Vitaly Buka 
> wrote:
>
>> They do on staging.
>> http://lab.llvm.org:8014/#/builders/sanitizer-windows
>> http://lab.llvm.org:8014/#/builders/clang-x64-windows-msvc
>>
>> Galina, when do you plan to push this to the primary server?
>> If it's a few days, I'd rather keep bots on staging to have colors.
>>
>>
>>
>> On Tue, 13 Oct 2020 at 11:11, Reid Kleckner  wrote:
>>
>>> FWIW, I don't see any issues with my two bots that use buildbot
>>> annotated commands:
>>> http://lab.llvm.org:8011/#/builders/sanitizer-windows
>>> http://lab.llvm.org:8011/#/builders/clang-x64-windows-msvc
>>> The individual steps don't highlight as green or red, but that's OK
>>> for now.
>>>
>>> On Mon, Oct 12, 2020 at 7:19 PM Galina Kistanova <
>>> gkistan...@gmail.com> wrote:
>>>
 We have a better version of AnnotatedCommand on the staging. It
 should be a functional equivalent of the old one.
 We need to stress test it well before moving to the production
 build bot.

 For that we need all sanitizer + other bots which use the
 AnnotatedCommand directly or indirectly moved temporarily to the 
 staging.

 Please let me know when that could be arranged.

 Thanks

 Galina

 On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner 
 wrote:

> On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>> They are online now -
>> http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>>
>> AnnotatedCommand has severe design conflict with the new buildbot.
>> We have changed it to be safe and still do something useful, but
>> it will need more love and care.
>>
>> Please let me know if you have some spare time to work on porting
>> AnnotatedCommand.
>>
>
> That's unfortunate, it would've been good to know that earlier. I
> and another team member have spent a fair amount of time porting 
> things to
> use more AnnotatedCommand steps, because it gives us the flexibility 
> to
> test steps locally and make changes to the steps without restarting 
> the
> buildbot master. IMO that is the Right Way to define steps: a script 
> that
> you can run locally on a machine that satisfies the OS and dep 
> requirements
> of the script.
>
> I am restarting the two bots that I am responsible for, and may
> need some help debugging further issues soon. I'll let you know.
>

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

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Galina Kistanova via lldb-commits]

Thanks, Vitaly.
Somebody is looking at this.


On Mon, Oct 19, 2020 at 9:56 PM Vitaly Buka  wrote:

> it's not just staging, this build must be marked as failed
> http://lab.llvm.org:8011/#/builders/74/builds/122
>
>
> On Mon, 19 Oct 2020 at 11:50, Vitaly Buka  wrote:
>
>> There is some issue, at least on staging.
>> http://lab.llvm.org:8014/#/builders/89/builds/184 has multiple failed
>> stages but marked as SUCCESS
>>
>>
>> On Mon, 19 Oct 2020 at 11:43, Vitaly Buka  wrote:
>>
>>> Thanks!
>>> Done.
>>>
>>> On Sat, 17 Oct 2020 at 12:45, Galina Kistanova 
>>> wrote:
>>>
 Thanks everyone for keeping your annotated builders in the staging
 area! Much appreciated.

 Please feel free to move all the green builders back to the production.
 It has a new AnnotatedCommand now.

 Thanks

 Galina


 On Thu, Oct 15, 2020 at 12:46 AM Vitaly Buka 
 wrote:

> Ok, I can switch them back to staging. However today's staging was
> frequently offline.
>
>
> On Wed, 14 Oct 2020 at 21:44, Galina Kistanova 
> wrote:
>
>> Thanks everyone!
>>
>> I would like to keep the bots in the staging till Friday. And if
>> everything is good, then apply the changes to the production and let you
>> move all the green bots back there.
>>
>> Thanks
>>
>> Galina
>>
>>
>> On Tue, Oct 13, 2020 at 10:43 PM Vitaly Buka 
>> wrote:
>>
>>> They do on staging.
>>> http://lab.llvm.org:8014/#/builders/sanitizer-windows
>>> http://lab.llvm.org:8014/#/builders/clang-x64-windows-msvc
>>>
>>> Galina, when do you plan to push this to the primary server?
>>> If it's a few days, I'd rather keep bots on staging to have colors.
>>>
>>>
>>>
>>> On Tue, 13 Oct 2020 at 11:11, Reid Kleckner  wrote:
>>>
 FWIW, I don't see any issues with my two bots that use buildbot
 annotated commands:
 http://lab.llvm.org:8011/#/builders/sanitizer-windows
 http://lab.llvm.org:8011/#/builders/clang-x64-windows-msvc
 The individual steps don't highlight as green or red, but that's OK
 for now.

 On Mon, Oct 12, 2020 at 7:19 PM Galina Kistanova <
 gkistan...@gmail.com> wrote:

> We have a better version of AnnotatedCommand on the staging. It
> should be a functional equivalent of the old one.
> We need to stress test it well before moving to the production
> build bot.
>
> For that we need all sanitizer + other bots which use the
> AnnotatedCommand directly or indirectly moved temporarily to the 
> staging.
>
> Please let me know when that could be arranged.
>
> Thanks
>
> Galina
>
> On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner 
> wrote:
>
>> On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
>> lldb-commits@lists.llvm.org> wrote:
>>
>>> They are online now -
>>> http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>>>
>>> AnnotatedCommand has severe design conflict with the new
>>> buildbot.
>>> We have changed it to be safe and still do something useful, but
>>> it will need more love and care.
>>>
>>> Please let me know if you have some spare time to work on
>>> porting AnnotatedCommand.
>>>
>>
>> That's unfortunate, it would've been good to know that earlier. I
>> and another team member have spent a fair amount of time porting 
>> things to
>> use more AnnotatedCommand steps, because it gives us the flexibility 
>> to
>> test steps locally and make changes to the steps without restarting 
>> the
>> buildbot master. IMO that is the Right Way to define steps: a script 
>> that
>> you can run locally on a machine that satisfies the OS and dep 
>> requirements
>> of the script.
>>
>> I am restarting the two bots that I am responsible for, and may
>> need some help debugging further issues soon. I'll let you know.
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D89283#2340586 , @wallace wrote:

> Addressed almost all changes.
>
> What's left to discuss:
>
> - I tried to use obj2yaml, but it creates files much larger than the 
> binaries, so in terms of space, I'd rather keep the binaries. Besides, I 
> removed ld.so, and the binaries that are left are tiny. I imagine that the 
> binaries that will be committed in the future will also be tiny ones 
> depicting specific edge cases.
> - Showing the disassembly of the instructions, requested by Greg, is not yet 
> done.
> - I'm representing now all decoding errors as instructions, which in fact 
> simplifies the API a good deal. However, I'm still not happy with the API, 
> right now we have
>
>   class Trace { void TraverseInstructions(Thread, callback, ...); int 
> GetInstructionsCount(Thread...); }

Do we need to know the instruction count? Again, won't this be really expensive 
to calculate?

> Right now these methods are not doing anything (or returning 0 in the case of 
> the instructions count) if the provided Thread is not traced by the Trace 
> instance.

If we get rid of

  int Trace::GetInstructionsCount(Thread...);

Then the first call to TraverseInstructions can return an appropriate error to 
the callback in the Expected right?

> Besides, eventually we'll add methods like
>
>   vector GetBacktrace(Thread, position)

We might want the thread to be able to produce real stack frames so we can 
re-use the current StackFrame classes. Each stack frame will probably only 
contain the PC. This call could be used to create the StackFrames in the trace 
thread class.

>   position ReverseNext(Thread, position)
>   position ReverseSingleStep(Thread, position)

We should be able to implement these in Trace.cpp and have it only use 
TraverseInstructions right? Do we also need ReverseContinue() to be able to 
backup until we hit a breakpoint or the start of the trace data?

> We could leave the code as it is, which results in a simple API with some 
> undefined behavior if an invalid Thread is passed.

I like the current API. Great if we can remove the 
Trace::GetInstructionsCount() API to keep things as simple as possible and 
allow us to lazily fetch instructions as needed as we eventually step and 
continue around.

If an invalid thread is passed, then TraverseInstructions can just return an 
error on the first callback. If we still need GetInstructionCount(), we can 
have it return 0 if the thread has no trace instructions.

> Another option is to have
>
>   class Trace {
> TracedThreadSP GetTraceForThread(Thread ...); 
>   }
>   
>   class TracedThread {
> void TraverseInstructions(callback, ...);
> int GetInstructionsCount()
>
> position ReverseNext(Thread, position)
> position ReverseSingleStep(Thread, position)
>   }
>
> This creates a level of indirection that forces the caller to check if the 
> returned TracedTrace is a valid pointer or not. If the pointer is invalid, 
> the caller can show an error message or fail silently, otherwise, 
> TraverseInstructions and GetInstructionsCount will perform valid operations.

I don't think we need this. I think we have enough API here to implement all of 
this stuff through the lldb_private::Thread class as it can call through to the 
Trace APIs with the thread class pointer.

> This might become useful to avoid errors in complex scenarios as the API 
> grows. But then, how often will someone invoke these methods with the wrong 
> thread? And this TracedThread object would have to be passed around a lot if 
> the developer doesn't want to if the thread is invalid in each callsite.
>
> The first form of the API looks simpler without that level of indirection, 
> which is cool. But the second one adds some boilerplate in each callsite, 
> with some safety benefits.
>
> What do you guys think?

Lets not do this extra indirection, I like the current API with or without 
GetInstructionsCount


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

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

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

> Do we need to know the instruction count? Again, won't this be really 
> expensive to calculate?

It is expensive, but today I realized that it might be necessary to decode the 
entire trace if you want things to work nicely.

Let's take, for example, a very common future use case: "you stop at a 
breakpoint, and then you do reverse-next and print the backtrace". Getting the 
backtrace requires decoding all the instructions, as potentially the first 
instruction in the trace corresponds to the oldest frame of the backtrace of 
your breakpoint position (which would be the case if you start tracing at 
main). You won't know all the frames of your backtrace unless you decode it 
all. In general, you can't have the backtrace of the i-th position unless you 
decode everything up to that position. And without the backtrace you really 
don't know where you are, it's hard for the user to make sense of the state of 
the program without it.

Also, if would be difficult to provide comprehensive position offsets to the 
user in the "dump instructions" command unless we also show the maximum index, 
which is equivalent to the instruction count.

> Lets not do this extra indirection, I like the current API with or without 
> GetInstructionsCount

That's reassuring. I'm also of that opinion.

> If an invalid thread is passed, then TraverseInstructions can just return an 
> error on the first callback. If we still need GetInstructionCount(), we can 
> have it return 0 if the thread has no trace instructions.

Sure

> We should be able to implement these in Trace.cpp and have it only use 
> TraverseInstructions right? Do we also need ReverseContinue() to be able to 
> backup until we hit a breakpoint or the start of the trace data?

Yes, definitely. We'll be adding more stuff to it like ReverseContinue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

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