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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

LGTM, modulo the defensive check. Incidentally, I was just looking at a 
DW_AT_const_value bug, but I think it's a different issue than this one.




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

I doubt this check is necessary -- surely we should be able to rely on 
`GetExpressionData` setting the data extractor to something reasonable if it 
returns success.

If anything, it would be nice to make a test with an empty DW_AT_const_value, 
which checks that lldb does something reasonable. It shouldn't that hard -- 
copy the DW_TAG_variable abbreviation, change DW_AT_const_value to 
DW_FORM_block, and make a new variable DIE with an empty block.



Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s:11
+# This is testing when how ValueObjectVariable handles the case where the
+# DWARFExpression holds the data that represents a constant value.
+# Compile at -O1 allows us to capture this case. Below is the code used

aprantl wrote:
> This sentence is complicated to parse.
It may be enough to say "Check that we're able to display variables whose value 
is given by DW_AT_const_value" -- the relationship between (or even existance) 
of ValueObjectVariable and DWARFExpression may change, but the test case is 
sufficiently generic to remain valid. Though, given the test name, even that 
sentence is pretty redundant.


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

https://reviews.llvm.org/D86311

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

[Lldb-commits] [PATCH] D79887: [lldb] Tab completion for process load/unload

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe1cd7cac8a36: [lldb] Tab completion for process load/unload 
(authored by MrHate, committed by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79887

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/test/API/functionalities/completion/Makefile
  lldb/test/API/functionalities/completion/TestCompletion.py
  lldb/test/API/functionalities/completion/shared.cpp

Index: lldb/test/API/functionalities/completion/shared.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/completion/shared.cpp
@@ -0,0 +1,3 @@
+int foo() {
+  return 123;
+}
Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -85,6 +85,31 @@
   ['mips',
'arm64'])
 
+def test_process_load(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, '// Break here', lldb.SBFileSpec("main.cpp"))
+self.complete_from_to('process load Makef', 'process load Makefile')
+
+@skipUnlessPlatform(["linux"])
+def test_process_unload(self):
+"""Test the completion for "process unload " """
+# This tab completion should not work without a running process.
+self.complete_from_to('process unload ',
+  'process unload ')
+
+self.build()
+lldbutil.run_to_source_breakpoint(self, '// Break here', lldb.SBFileSpec("main.cpp"))
+err = lldb.SBError()
+self.process().LoadImage(lldb.SBFileSpec(self.getBuildArtifact("libshared.so")), err)
+self.assertSuccess(err)
+
+self.complete_from_to('process unload ',
+  'process unload 0')
+
+self.process().UnloadImage(0)
+self.complete_from_to('process unload ',
+  'process unload ')
+
 def test_process_plugin_completion(self):
 subcommands = ['attach -P', 'connect -p', 'launch -p']
 
Index: lldb/test/API/functionalities/completion/Makefile
===
--- lldb/test/API/functionalities/completion/Makefile
+++ lldb/test/API/functionalities/completion/Makefile
@@ -1,3 +1,10 @@
 CXX_SOURCES := main.cpp
+USE_LIBDL := 1
+
+a.out: lib_shared
+
+lib_shared:
+	$(MAKE) -f $(MAKEFILE_RULES) \
+		DYLIB_ONLY=YES DYLIB_CXX_SOURCES=shared.cpp DYLIB_NAME=shared
 
 include Makefile.rules
Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -923,6 +923,17 @@
 
   ~CommandObjectProcessLoad() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+if (!m_exe_ctx.HasProcessScope())
+  return;
+
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
+request, nullptr);
+  }
+
   Options *GetOptions() override { return &m_options; }
 
 protected:
@@ -988,6 +999,24 @@
 
   ~CommandObjectProcessUnload() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+
+if (request.GetCursorIndex() || !m_exe_ctx.HasProcessScope())
+  return;
+
+Process *process = m_exe_ctx.GetProcessPtr();
+
+const std::vector &tokens = process->GetImageTokens();
+const size_t token_num = tokens.size();
+for (size_t i = 0; i < token_num; ++i) {
+  if (tokens[i] == LLDB_INVALID_IMAGE_TOKEN)
+continue;
+  request.TryCompleteCurrentArg(std::to_string(i));
+}
+  }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 Process *process = m_exe_ctx.GetProcessPtr();
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -739,6 +739,12 @@
 
   void SetShouldDetach(bool b) { m_should_detach = b; }
 
+  /// Get the image vector for the current process.
+  ///
+  /// \return
+  /// The constant reference to the member m_image_tokens.
+  const std::vector& GetImageTokens() { return m_image_tokens; }
+
   /// Get the image information address for the current process.
   ///
   /// Some runtimes have system functions that can help

[Lldb-commits] [lldb] e1cd7ca - [lldb] Tab completion for process load/unload

[Raphael Isemann via lldb-commits]

Author: Gongyu Deng
Date: 2020-08-21T10:36:39+02:00
New Revision: e1cd7cac8a36608616d515b64d12f2e86643970d

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

LOG: [lldb] Tab completion for process load/unload

1. Complete `process load` with the common disk file completion, so there is 
not test provided for it;
2. Complete `process unload` with the tokens of valid loaded images.

Thanks for Raphael's help on the test for `process unload`.

Reviewed By: teemperor

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

Added: 
lldb/test/API/functionalities/completion/shared.cpp

Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Commands/CommandObjectProcess.cpp
lldb/test/API/functionalities/completion/Makefile
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 42f10082b981..90172f39412d 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -739,6 +739,12 @@ class Process : public 
std::enable_shared_from_this,
 
   void SetShouldDetach(bool b) { m_should_detach = b; }
 
+  /// Get the image vector for the current process.
+  ///
+  /// \return
+  /// The constant reference to the member m_image_tokens.
+  const std::vector& GetImageTokens() { return m_image_tokens; }
+
   /// Get the image information address for the current process.
   ///
   /// Some runtimes have system functions that can help dynamic loaders locate

diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp 
b/lldb/source/Commands/CommandObjectProcess.cpp
index a5df62f23345..25fe2e4b8b1a 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -923,6 +923,17 @@ class CommandObjectProcessLoad : public 
CommandObjectParsed {
 
   ~CommandObjectProcessLoad() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+if (!m_exe_ctx.HasProcessScope())
+  return;
+
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
+request, nullptr);
+  }
+
   Options *GetOptions() override { return &m_options; }
 
 protected:
@@ -988,6 +999,24 @@ class CommandObjectProcessUnload : public 
CommandObjectParsed {
 
   ~CommandObjectProcessUnload() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+
+if (request.GetCursorIndex() || !m_exe_ctx.HasProcessScope())
+  return;
+
+Process *process = m_exe_ctx.GetProcessPtr();
+
+const std::vector &tokens = process->GetImageTokens();
+const size_t token_num = tokens.size();
+for (size_t i = 0; i < token_num; ++i) {
+  if (tokens[i] == LLDB_INVALID_IMAGE_TOKEN)
+continue;
+  request.TryCompleteCurrentArg(std::to_string(i));
+}
+  }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 Process *process = m_exe_ctx.GetProcessPtr();

diff  --git a/lldb/test/API/functionalities/completion/Makefile 
b/lldb/test/API/functionalities/completion/Makefile
index 8b20bcb0..f46742243bd9 100644
--- a/lldb/test/API/functionalities/completion/Makefile
+++ b/lldb/test/API/functionalities/completion/Makefile
@@ -1,3 +1,10 @@
 CXX_SOURCES := main.cpp
+USE_LIBDL := 1
+
+a.out: lib_shared
+
+lib_shared:
+   $(MAKE) -f $(MAKEFILE_RULES) \
+   DYLIB_ONLY=YES DYLIB_CXX_SOURCES=shared.cpp DYLIB_NAME=shared
 
 include Makefile.rules

diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py 
b/lldb/test/API/functionalities/completion/TestCompletion.py
index 4922cca7b722..4c81288b3836 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -85,6 +85,31 @@ def test_process_launch_arch(self):
   ['mips',
'arm64'])
 
+def test_process_load(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, '// Break here', 
lldb.SBFileSpec("main.cpp"))
+self.complete_from_to('process load Makef', 'process load Makefile')
+
+@skipUnlessPlatform(["linux"])
+def test_process_unload(self):
+"""Test the completion for "process unload " """
+# This tab completion should not work without a running process.
+self.complete_from_to('process unload ',
+  'process unload ')
+
+self.build()
+lldbutil.run_to_source_breakpoint(self, '/

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

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

In D86261#2229341 , @clayborg wrote:

> Hopefully this should be good to go, let me know if anyone has any issues.

Looks good, just please also add a test case which runs off the end of the text 
section. If you have problems generating this test with yaml2obj, I would also 
be fine with a checked-in binary for a special case like this.




Comment at: 
lldb/test/API/functionalities/postmortem/minidump-new/libbreakpad.yaml:15
+AddressAlign:0x0004
+Content: 040014000300474E5500

clayborg wrote:
> labath wrote:
> > I guess this should include a custom `Fill` pseudo-section so that we can 
> > guarantee the contents of whatever comes after it. Otherwise, yaml2obj 
> > might decide to place anything (or nothing) there. Something like this 
> > ought to do it:
> > ```
> > - Type: Fill
> >   Pattern: "DEADBEEF"
> >   Size: 0xsomething
> > ```
> We don't need to because I selected a multiple of 16 for the contents of the 
> text section! If I added one more byte, then we would need to.
I see. In that case, it would be good to have an test case which actually 
exercises the overflow path. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86261

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

[Lldb-commits] [PATCH] D83116: [DWARFYAML] Add support for referencing different abbrev tables.

[Xing GUO via Phabricator via lldb-commits]

Higuoxing updated this revision to Diff 286985.
Higuoxing added a comment.

Fix incorrect format string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83116

Files:
  llvm/include/llvm/ObjectYAML/DWARFYAML.h
  llvm/lib/ObjectYAML/DWARFEmitter.cpp
  llvm/lib/ObjectYAML/DWARFYAML.cpp
  llvm/test/ObjectYAML/MachO/DWARF-debug_abbrev.yaml
  llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
  llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
  llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
  llvm/tools/obj2yaml/dwarf2yaml.cpp
  llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

Index: llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
@@ -2487,6 +2487,7 @@
   - Value:   0x
   - Length:  16
 Version: 4
+AbbrevTableID:   0
 AbbrOffset:  0
 AddrSize:8
 Entries:
Index: llvm/tools/obj2yaml/dwarf2yaml.cpp
===
--- llvm/tools/obj2yaml/dwarf2yaml.cpp
+++ llvm/tools/obj2yaml/dwarf2yaml.cpp
@@ -23,6 +23,7 @@
 void dumpDebugAbbrev(DWARFContext &DCtx, DWARFYAML::Data &Y) {
   auto AbbrevSetPtr = DCtx.getDebugAbbrev();
   if (AbbrevSetPtr) {
+uint64_t AbbrevTableID = 0;
 for (auto AbbrvDeclSet : *AbbrevSetPtr) {
   Y.DebugAbbrev.emplace_back();
   for (auto AbbrvDecl : AbbrvDeclSet.second) {
@@ -39,6 +40,7 @@
 AttAbrv.Value = Attribute.getImplicitConstValue();
   Abbrv.Attributes.push_back(AttAbrv);
 }
+Y.DebugAbbrev.back().ID = AbbrevTableID++;
 Y.DebugAbbrev.back().Table.push_back(Abbrv);
   }
 }
@@ -172,6 +174,14 @@
 NewUnit.Version = CU->getVersion();
 if (NewUnit.Version >= 5)
   NewUnit.Type = (dwarf::UnitType)CU->getUnitType();
+const DWARFDebugAbbrev *DebugAbbrev = DCtx.getDebugAbbrev();
+NewUnit.AbbrevTableID = std::distance(
+DebugAbbrev->begin(),
+std::find_if(
+DebugAbbrev->begin(), DebugAbbrev->end(),
+[&](const std::pair &P) {
+  return P.first == CU->getAbbreviations()->getOffset();
+}));
 NewUnit.AbbrOffset = CU->getAbbreviations()->getOffset();
 NewUnit.AddrSize = CU->getAddressByteSize();
 for (auto DIE : CU->dies()) {
Index: llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
===
--- llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
+++ llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
@@ -279,6 +279,7 @@
 - Length:0x5678
   ## Test DWARFv4
   Version:   4
+  AbbrevTableID: 0
   AbbrOffset:0x5678
   AddrSize:  4
   Entries:
@@ -886,7 +887,7 @@
 
 ## RUN: not yaml2obj --docnum=16 %s 2>&1 | FileCheck %s --check-prefix=NO-ABBREV
 
-# NO-ABBREV: yaml2obj: error: non-empty compilation unit should have an associated abbrev table
+# NO-ABBREV: yaml2obj: error: cannot find abbrev table whose ID is 0 for compilation unit with index 0
 
 --- !ELF
 FileHeader:
@@ -902,3 +903,101 @@
 - AbbrCode: 1
   Values:
 - Value: 0x1234
+
+## o) Test that yaml2obj is able to generate compilation units according to the
+## associated abbrev table that is referenced by the 'AbbrevTableID'.
+
+# RUN: yaml2obj --docnum=17 %s -o %t17.o
+# RUN: llvm-readelf --hex-dump=.debug_info %t17.o | FileCheck %s --check-prefix=MULTI-TABLES
+
+#  MULTI-TABLES: Hex dump of section '.debug_info':
+# MULTI-TABLES-NEXT: 0x 0c00 04000800 0801 3412 4...
+##  ^---unit_length (4-byte)
+##   ^---   version (2-byte)
+##   ^  debug_abbrev_offset (4-byte)
+##^-address_size (1-byte)
+##  ^-  abbrev_code (ULEB128) 0x01
+## ^--- Form: DW_FORM_data4 (4-byte) 0x1234
+# MULTI-TABLES-NEXT: 0x0010 0c00 04000800 0801 2143 !C..
+##  ^---unit_length (4-byte)
+##   ^---   version (2-byte)
+##   ^  debug_abbrev_offset (4-byte)
+##^-address_size (1-byte)
+##  ^-  abbrev_code (ULEB128) 0x01
+##

[Lldb-commits] [PATCH] D83116: [DWARFYAML] Add support for referencing different abbrev tables.

[Xing GUO via Phabricator via lldb-commits]

Higuoxing updated this revision to Diff 286987.
Higuoxing added a comment.

Add missing test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83116

Files:
  llvm/include/llvm/ObjectYAML/DWARFYAML.h
  llvm/lib/ObjectYAML/DWARFEmitter.cpp
  llvm/lib/ObjectYAML/DWARFYAML.cpp
  llvm/test/ObjectYAML/MachO/DWARF-debug_abbrev.yaml
  llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
  llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
  llvm/test/tools/yaml2obj/ELF/DWARF/debug-abbrev.yaml
  llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
  llvm/tools/obj2yaml/dwarf2yaml.cpp
  llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

Index: llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
@@ -2487,6 +2487,7 @@
   - Value:   0x
   - Length:  16
 Version: 4
+AbbrevTableID:   0
 AbbrOffset:  0
 AddrSize:8
 Entries:
Index: llvm/tools/obj2yaml/dwarf2yaml.cpp
===
--- llvm/tools/obj2yaml/dwarf2yaml.cpp
+++ llvm/tools/obj2yaml/dwarf2yaml.cpp
@@ -23,6 +23,7 @@
 void dumpDebugAbbrev(DWARFContext &DCtx, DWARFYAML::Data &Y) {
   auto AbbrevSetPtr = DCtx.getDebugAbbrev();
   if (AbbrevSetPtr) {
+uint64_t AbbrevTableID = 0;
 for (auto AbbrvDeclSet : *AbbrevSetPtr) {
   Y.DebugAbbrev.emplace_back();
   for (auto AbbrvDecl : AbbrvDeclSet.second) {
@@ -39,6 +40,7 @@
 AttAbrv.Value = Attribute.getImplicitConstValue();
   Abbrv.Attributes.push_back(AttAbrv);
 }
+Y.DebugAbbrev.back().ID = AbbrevTableID++;
 Y.DebugAbbrev.back().Table.push_back(Abbrv);
   }
 }
@@ -172,6 +174,14 @@
 NewUnit.Version = CU->getVersion();
 if (NewUnit.Version >= 5)
   NewUnit.Type = (dwarf::UnitType)CU->getUnitType();
+const DWARFDebugAbbrev *DebugAbbrev = DCtx.getDebugAbbrev();
+NewUnit.AbbrevTableID = std::distance(
+DebugAbbrev->begin(),
+std::find_if(
+DebugAbbrev->begin(), DebugAbbrev->end(),
+[&](const std::pair &P) {
+  return P.first == CU->getAbbreviations()->getOffset();
+}));
 NewUnit.AbbrOffset = CU->getAbbreviations()->getOffset();
 NewUnit.AddrSize = CU->getAddressByteSize();
 for (auto DIE : CU->dies()) {
Index: llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
===
--- llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
+++ llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
@@ -279,6 +279,7 @@
 - Length:0x5678
   ## Test DWARFv4
   Version:   4
+  AbbrevTableID: 0
   AbbrOffset:0x5678
   AddrSize:  4
   Entries:
@@ -886,7 +887,7 @@
 
 ## RUN: not yaml2obj --docnum=16 %s 2>&1 | FileCheck %s --check-prefix=NO-ABBREV
 
-# NO-ABBREV: yaml2obj: error: non-empty compilation unit should have an associated abbrev table
+# NO-ABBREV: yaml2obj: error: cannot find abbrev table whose ID is 0 for compilation unit with index 0
 
 --- !ELF
 FileHeader:
@@ -902,3 +903,101 @@
 - AbbrCode: 1
   Values:
 - Value: 0x1234
+
+## o) Test that yaml2obj is able to generate compilation units according to the
+## associated abbrev table that is referenced by the 'AbbrevTableID'.
+
+# RUN: yaml2obj --docnum=17 %s -o %t17.o
+# RUN: llvm-readelf --hex-dump=.debug_info %t17.o | FileCheck %s --check-prefix=MULTI-TABLES
+
+#  MULTI-TABLES: Hex dump of section '.debug_info':
+# MULTI-TABLES-NEXT: 0x 0c00 04000800 0801 3412 4...
+##  ^---unit_length (4-byte)
+##   ^---   version (2-byte)
+##   ^  debug_abbrev_offset (4-byte)
+##^-address_size (1-byte)
+##  ^-  abbrev_code (ULEB128) 0x01
+## ^--- Form: DW_FORM_data4 (4-byte) 0x1234
+# MULTI-TABLES-NEXT: 0x0010 0c00 04000800 0801 2143 !C..
+##  ^---unit_length (4-byte)
+##   ^---   version (2-byte)
+##   ^  debug_abbrev_offset (4-byte)
+##^-address_size (1-byte)
+##  ^-  abbrev_code (U

[Lldb-commits] [PATCH] D83116: [DWARFYAML] Add support for referencing different abbrev tables.

[Xing GUO via Phabricator via lldb-commits]

Higuoxing requested review of this revision.
Higuoxing added a comment.

Hi @jhenderson

This change is causing build failure on armv7 platform and I've fixed it.

See: 
http://lab.llvm.org:8011/builders/clang-cmake-armv7-global-isel/builds/10400 
and http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/20261

Could you please review this change again? Thanks a lot!




Comment at: llvm/lib/ObjectYAML/DWARFYAML.cpp:69
+errc::invalid_argument,
+"the ID (%" PRIu64 ") of abbrev table with index %zu has been used 
"
+"by abbrev table with index %" PRIu64,

The build failure is caused by the incorrect format string. 
`AbbrevTable.index()` returns `size_t` and it was formatted as `%u`. We should 
use `%zu` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83116

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

[Lldb-commits] [PATCH] D83116: [DWARFYAML] Add support for referencing different abbrev tables.

[James Henderson via Phabricator via lldb-commits]

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

Looks good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83116

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

[Lldb-commits] [PATCH] D83116: [DWARFYAML] Add support for referencing different abbrev tables.

[Xing GUO via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5643dc3dce2: Recommit: [DWARFYAML] Add support for 
referencing different abbrev tables. (authored by Higuoxing).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83116

Files:
  llvm/include/llvm/ObjectYAML/DWARFYAML.h
  llvm/lib/ObjectYAML/DWARFEmitter.cpp
  llvm/lib/ObjectYAML/DWARFYAML.cpp
  llvm/test/ObjectYAML/MachO/DWARF-debug_abbrev.yaml
  llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
  llvm/test/ObjectYAML/MachO/DWARF5-debug_info.yaml
  llvm/test/tools/yaml2obj/ELF/DWARF/debug-abbrev.yaml
  llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
  llvm/tools/obj2yaml/dwarf2yaml.cpp
  llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

Index: llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp
@@ -2487,6 +2487,7 @@
   - Value:   0x
   - Length:  16
 Version: 4
+AbbrevTableID:   0
 AbbrOffset:  0
 AddrSize:8
 Entries:
Index: llvm/tools/obj2yaml/dwarf2yaml.cpp
===
--- llvm/tools/obj2yaml/dwarf2yaml.cpp
+++ llvm/tools/obj2yaml/dwarf2yaml.cpp
@@ -23,6 +23,7 @@
 void dumpDebugAbbrev(DWARFContext &DCtx, DWARFYAML::Data &Y) {
   auto AbbrevSetPtr = DCtx.getDebugAbbrev();
   if (AbbrevSetPtr) {
+uint64_t AbbrevTableID = 0;
 for (auto AbbrvDeclSet : *AbbrevSetPtr) {
   Y.DebugAbbrev.emplace_back();
   for (auto AbbrvDecl : AbbrvDeclSet.second) {
@@ -39,6 +40,7 @@
 AttAbrv.Value = Attribute.getImplicitConstValue();
   Abbrv.Attributes.push_back(AttAbrv);
 }
+Y.DebugAbbrev.back().ID = AbbrevTableID++;
 Y.DebugAbbrev.back().Table.push_back(Abbrv);
   }
 }
@@ -172,6 +174,14 @@
 NewUnit.Version = CU->getVersion();
 if (NewUnit.Version >= 5)
   NewUnit.Type = (dwarf::UnitType)CU->getUnitType();
+const DWARFDebugAbbrev *DebugAbbrev = DCtx.getDebugAbbrev();
+NewUnit.AbbrevTableID = std::distance(
+DebugAbbrev->begin(),
+std::find_if(
+DebugAbbrev->begin(), DebugAbbrev->end(),
+[&](const std::pair &P) {
+  return P.first == CU->getAbbreviations()->getOffset();
+}));
 NewUnit.AbbrOffset = CU->getAbbreviations()->getOffset();
 NewUnit.AddrSize = CU->getAddressByteSize();
 for (auto DIE : CU->dies()) {
Index: llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
===
--- llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
+++ llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
@@ -279,6 +279,7 @@
 - Length:0x5678
   ## Test DWARFv4
   Version:   4
+  AbbrevTableID: 0
   AbbrOffset:0x5678
   AddrSize:  4
   Entries:
@@ -886,7 +887,7 @@
 
 ## RUN: not yaml2obj --docnum=16 %s 2>&1 | FileCheck %s --check-prefix=NO-ABBREV
 
-# NO-ABBREV: yaml2obj: error: non-empty compilation unit should have an associated abbrev table
+# NO-ABBREV: yaml2obj: error: cannot find abbrev table whose ID is 0 for compilation unit with index 0
 
 --- !ELF
 FileHeader:
@@ -902,3 +903,101 @@
 - AbbrCode: 1
   Values:
 - Value: 0x1234
+
+## o) Test that yaml2obj is able to generate compilation units according to the
+## associated abbrev table that is referenced by the 'AbbrevTableID'.
+
+# RUN: yaml2obj --docnum=17 %s -o %t17.o
+# RUN: llvm-readelf --hex-dump=.debug_info %t17.o | FileCheck %s --check-prefix=MULTI-TABLES
+
+#  MULTI-TABLES: Hex dump of section '.debug_info':
+# MULTI-TABLES-NEXT: 0x 0c00 04000800 0801 3412 4...
+##  ^---unit_length (4-byte)
+##   ^---   version (2-byte)
+##   ^  debug_abbrev_offset (4-byte)
+##^-address_size (1-byte)
+##  ^-  abbrev_code (ULEB128) 0x01
+## ^--- Form: DW_FORM_data4 (4-byte) 0x1234
+# MULTI-TABLES-NEXT: 0x0010 0c00 04000800 0801 2143 !C..
+##  ^---unit_length (4-byte)
+##   ^---   version (2-byte)
+##   ^  debug_abbrev_offset (4-byte)
+##^-

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

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: shafik, aprantl.
Herald added a project: LLDB.
labath requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: sstefan1, JDevlieghere.

This fixes several issues in handling of DW_AT_const_value attributes:

- the first is that the size of the data given by data forms does not need to 
match the size of the underlying variable. We already had the case to handle 
this for DW_FORM_(us)data -- this extends the handling to other data forms. The 
main reason this was not picked up is because clang uses leb forms in these 
cases while gcc prefers the fixed-size ones.
- The handling of DW_AT_strp form was completely broken -- we would end up 
using the pointer value as the result. I've reorganized this code so that it 
handles all string forms uniformly.
- In case of a completely bogus form we would crash due to strlen(nullptr).

Depends on D86311 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86348

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
@@ -1,408 +1,158 @@
-# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
-# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+# Test handling of (optimized-out/location-less) variables whose value is
+# specified by DW_AT_const_value
 
-# CHECK: (lldb) target variable constant
-# CHECK: (U) constant = {
-# CHECK:   raw = 1688469761
-# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
-# CHECK: }
+# REQUIRES: x86
 
-# This is testing when how ValueObjectVariable handles the case where the
-# DWARFExpression holds the data that represents a constant value.
-# Compile at -O1 allows us to capture this case. Below is the code used
-# to generate the assembly:
-#
-# typedef union
-# {
-#   unsigned raw;
-#   struct
-#   {
-# unsigned a : 8;
-# unsigned b : 8;
-# unsigned c : 6;
-# unsigned d : 2;
-# unsigned e : 6;
-# unsigned f : 2;
-#   } ;
-# } U;
-#
-# static U __attribute__((used)) _type_anchor;
-# static const int constant = 0x64A40101;
-#
-# int g() { return constant; }
-#
-# int main() {
-#   U u;
-#   u.raw = 0x64A40101;
-# }
-#
-# Compiled as follows:
-#
-#   clang -gdwarf-4 -O1 dw_at_const_value_bug.c -S -o dw_at_const_value_bug.s
-#
-# I was able to obtain a global of type U with DW_AT_const_value but was able
-# to using int. This required modifying the DW_AT_type of constant to be type
-# U. After that stripping as much of the assembly as possible to give us a
-# smaller reproducer.
+# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
+# RUN: %lldb %t \
+# RUN:   -o "target variable udata data1 data2 data4 data8 string strp ref4" \
+# RUN:   -o exit | FileCheck %s
 
+# CHECK-LABEL: target variable udata data1 data2 data4 data8 string strp ref4
+## Variable specified via DW_FORM_udata. This is typical for clang (10).
+# CHECK: (unsigned long) udata = 4742474247424742
+## Variables specified via fixed-size forms. This is typical for gcc (9).
+# CHECK: (unsigned long) data1 = 47
+# CHECK: (unsigned long) data2 = 4742
+# CHECK: (unsigned long) data4 = 47424742
+# CHECK: (unsigned long) data8 = 4742474247424742
+## Variables specified using string forms. This behavior purely speculative -- I
+## don't know of any compiler that would represent character strings this way.
+# CHECK: (char [7]) string = "string"
+# CHECK: (char [7]) strp = "strp"
+## Bogus attribute form. Let's make sure we don't crash at least.
+# CHECK: (char [7]) ref4 = 
 
-.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
-	.no_dead_strip	__type_anchor
-	.section	__DWARF,__debug_str,regular,debug
-Linfo_string:
-  .zero 90
-	.asciz	"constant"  ## string offset=90
-	.asciz	"int"   ## string offset=99
-	.asciz	"_type_anchor"  ## string offset=103
-	.asciz	"U" ## string offset=116
-	.asciz	"raw"   ## string offset=118
-	.asciz	"unsigned int"  ## string offset=122
-	.asciz	"a" ## string offset=135
-	.asciz	"b" ## string offset=137
-	.asciz	"c" ## string offset=139
-	.asciz	"d" ## string offset=141
-	.asciz	"e" ## string offset=143
-	.asciz	"f" ## string offset=145
-	.asciz	"g" ## string offset=147
-	.asciz	"main"  ## string offset=149
-	.asciz	"u" ## string offset=154
-	.section	__DWARF,__debug_abbrev,regular,debug
-Lsection_abbrev:
-	.byte	1   ## Abbreviation Code
-	.byte	17 

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

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 287023.
labath added a comment.

Actually, keep the old const_value test, but give it a bitfield-specific name. 
Testing bitfields+const_values is still interesting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value-bitfields.s
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
@@ -1,408 +1,158 @@
-# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
-# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+# Test handling of (optimized-out/location-less) variables whose value is
+# specified by DW_AT_const_value
 
-# CHECK: (lldb) target variable constant
-# CHECK: (U) constant = {
-# CHECK:   raw = 1688469761
-# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
-# CHECK: }
+# REQUIRES: x86
 
-# This is testing when how ValueObjectVariable handles the case where the
-# DWARFExpression holds the data that represents a constant value.
-# Compile at -O1 allows us to capture this case. Below is the code used
-# to generate the assembly:
-#
-# typedef union
-# {
-#   unsigned raw;
-#   struct
-#   {
-# unsigned a : 8;
-# unsigned b : 8;
-# unsigned c : 6;
-# unsigned d : 2;
-# unsigned e : 6;
-# unsigned f : 2;
-#   } ;
-# } U;
-#
-# static U __attribute__((used)) _type_anchor;
-# static const int constant = 0x64A40101;
-#
-# int g() { return constant; }
-#
-# int main() {
-#   U u;
-#   u.raw = 0x64A40101;
-# }
-#
-# Compiled as follows:
-#
-#   clang -gdwarf-4 -O1 dw_at_const_value_bug.c -S -o dw_at_const_value_bug.s
-#
-# I was able to obtain a global of type U with DW_AT_const_value but was able
-# to using int. This required modifying the DW_AT_type of constant to be type
-# U. After that stripping as much of the assembly as possible to give us a
-# smaller reproducer.
+# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
+# RUN: %lldb %t \
+# RUN:   -o "target variable udata data1 data2 data4 data8 string strp ref4" \
+# RUN:   -o exit | FileCheck %s
 
+# CHECK-LABEL: target variable udata data1 data2 data4 data8 string strp ref4
+## Variable specified via DW_FORM_udata. This is typical for clang (10).
+# CHECK: (unsigned long) udata = 4742474247424742
+## Variables specified via fixed-size forms. This is typical for gcc (9).
+# CHECK: (unsigned long) data1 = 47
+# CHECK: (unsigned long) data2 = 4742
+# CHECK: (unsigned long) data4 = 47424742
+# CHECK: (unsigned long) data8 = 4742474247424742
+## Variables specified using string forms. This behavior purely speculative -- I
+## don't know of any compiler that would represent character strings this way.
+# CHECK: (char [7]) string = "string"
+# CHECK: (char [7]) strp = "strp"
+## Bogus attribute form. Let's make sure we don't crash at least.
+# CHECK: (char [7]) ref4 = 
 
-.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
-	.no_dead_strip	__type_anchor
-	.section	__DWARF,__debug_str,regular,debug
-Linfo_string:
-  .zero 90
-	.asciz	"constant"  ## string offset=90
-	.asciz	"int"   ## string offset=99
-	.asciz	"_type_anchor"  ## string offset=103
-	.asciz	"U" ## string offset=116
-	.asciz	"raw"   ## string offset=118
-	.asciz	"unsigned int"  ## string offset=122
-	.asciz	"a" ## string offset=135
-	.asciz	"b" ## string offset=137
-	.asciz	"c" ## string offset=139
-	.asciz	"d" ## string offset=141
-	.asciz	"e" ## string offset=143
-	.asciz	"f" ## string offset=145
-	.asciz	"g" ## string offset=147
-	.asciz	"main"  ## string offset=149
-	.asciz	"u" ## string offset=154
-	.section	__DWARF,__debug_abbrev,regular,debug
-Lsection_abbrev:
-	.byte	1   ## Abbreviation Code
-	.byte	17  ## DW_TAG_compile_unit
-	.byte	1   ## DW_CHILDREN_yes
-	.byte	37  ## DW_AT_producer
-	.byte	14  ## DW_FORM_strp
-	.byte	19  ## DW_AT_language
-	.byte	5   ## DW_FORM_data2
-	.byte	3   ## DW_AT_name
-	.byte	14  ## DW_FORM_strp
-	.byte	66  ## DW_AT_stmt_list
-	.byte	23  ## DW_FORM_sec_offset
-	.byte	27  ## DW_AT_comp_dir
-	.byte	14  ## DW_FORM_strp
-	.ascii	"\264B" ## DW_AT_GNU_pubnames
-	.byte	25  

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Thank you finding these additional fixed, it looks good but I don't know some 
of the details as well as Adrian so I would prefer he gives the LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

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

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

[Shafik Yaghmour via Phabricator via lldb-commits]

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



Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s:2
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+

aprantl wrote:
> I think this is missing a REQUIRES: line that checks for an x86 target?
I did not use that in D85376, is there something different in this case?


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

https://reviews.llvm.org/D86311

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

[Lldb-commits] [PATCH] D86355: Consume error for valid return from Target::GetEntryPointAddress()

[Dimitry Andric via Phabricator via lldb-commits]

dim created this revision.
dim added reviewers: aprantl, emaste, JDevlieghere, jingham.
Herald added subscribers: krytarowski, arichardson.
Herald added a project: LLDB.
dim requested review of this revision.

When `Target::GetEntryPointAddress()` calls
`exe_module->GetObjectFile()->GetEntryPointAddress()`, and the returned
`entry_addr` is valid, it can immediately be returned.

However, just before that, an `llvm::Error` value has been setup, but in
this case it is not consumed before returning, like is done further
below in the function.

In https://bugs.freebsd.org/248745 we got a bug report for this, where a
very simple test case aborts and dumps core:

  * thread #1, name = 'testcase', stop reason = breakpoint 1.1
  frame #0: 0x002018d4 testcase`main(argc=1, 
argv=0x7fffea18) at testcase.c:3:5
 1  int main(int argc, char *argv[])
 2  {
  -> 3  return 0;
 4  }
  (lldb) p argc
  Program aborted due to an unhandled Error:
  Error value was Success. (Note: Success values must still be checked prior to 
being destroyed).
  
  Thread 1 received signal SIGABRT, Aborted.
  thr_kill () at thr_kill.S:3
  3 thr_kill.S: No such file or directory.
  (gdb) bt
  #0  thr_kill () at thr_kill.S:3
  #1  0x0008049a0004 in __raise (s=6) at /usr/src/lib/libc/gen/raise.c:52
  #2  0x000804916229 in abort () at /usr/src/lib/libc/stdlib/abort.c:67
  #3  0x0451b5f5 in fatalUncheckedError () at 
/usr/src/contrib/llvm-project/llvm/lib/Support/Error.cpp:112
  #4  0x019cf008 in GetEntryPointAddress () at 
/usr/src/contrib/llvm-project/llvm/include/llvm/Support/Error.h:267
  #5  0x01bccbd8 in ConstructorSetup () at 
/usr/src/contrib/llvm-project/lldb/source/Target/ThreadPlanCallFunction.cpp:67
  #6  0x01bcd2c0 in ThreadPlanCallFunction () at 
/usr/src/contrib/llvm-project/lldb/source/Target/ThreadPlanCallFunction.cpp:114
  #7  0x020076d4 in InferiorCallMmap () at 
/usr/src/contrib/llvm-project/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:97
  #8  0x01f4be33 in DoAllocateMemory () at 
/usr/src/contrib/llvm-project/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:604
  #9  0x01fe51b9 in AllocatePage () at 
/usr/src/contrib/llvm-project/lldb/source/Target/Memory.cpp:347
  #10 0x01fe5385 in AllocateMemory () at 
/usr/src/contrib/llvm-project/lldb/source/Target/Memory.cpp:383
  #11 0x01974da2 in AllocateMemory () at 
/usr/src/contrib/llvm-project/lldb/source/Target/Process.cpp:2301
  #12 CanJIT () at 
/usr/src/contrib/llvm-project/lldb/source/Target/Process.cpp:2331
  #13 0x01a1bf3d in Evaluate () at 
/usr/src/contrib/llvm-project/lldb/source/Expression/UserExpression.cpp:190
  #14 0x019ce7a2 in EvaluateExpression () at 
/usr/src/contrib/llvm-project/lldb/source/Target/Target.cpp:2372
  #15 0x01ad784c in EvaluateExpression () at 
/usr/src/contrib/llvm-project/lldb/source/Commands/CommandObjectExpression.cpp:414
  #16 0x01ad86ae in DoExecute () at 
/usr/src/contrib/llvm-project/lldb/source/Commands/CommandObjectExpression.cpp:646
  #17 0x01a5e3ed in Execute () at 
/usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandObject.cpp:1003
  #18 0x01a6c4a3 in HandleCommand () at 
/usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:1762
  #19 0x01a6f98c in IOHandlerInputComplete () at 
/usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2760
  #20 0x01a90b08 in Run () at 
/usr/src/contrib/llvm-project/lldb/source/Core/IOHandler.cpp:548
  #21 0x019a6c6a in ExecuteIOHandlers () at 
/usr/src/contrib/llvm-project/lldb/source/Core/Debugger.cpp:903
  #22 0x01a70337 in RunCommandInterpreter () at 
/usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2946
  #23 0x01d9d812 in RunCommandInterpreter () at 
/usr/src/contrib/llvm-project/lldb/source/API/SBDebugger.cpp:1169
  #24 0x01918be8 in MainLoop () at 
/usr/src/contrib/llvm-project/lldb/tools/driver/Driver.cpp:675
  #25 0x0191a114 in main () at 
/usr/src/contrib/llvm-project/lldb/tools/driver/Driver.cpp:890

Fix the incorrect error catch by explicitly consuming the error, similar
to the `module_sp->GetObjectFile()->GetEntryPointAddress()` return path.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86355

Files:
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2415,8 +2415,11 @@
 
llvm::inconvertibleErrorCode());
   } else {
 Address entry_addr = exe_module->GetObjectFile()->GetEntryPointAddress();
-if (entry_addr.IsValid())
+if (entry_addr.IsValid()) {
+  // Discard the error.
+  llvm::consumeError(std::move(error));
   return entry_add

[Lldb-commits] [lldb] 2799031 - [lldb] Skip PDB and NativePDB tests with reproducers

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-21T09:09:45-07:00
New Revision: 2799031a14323eee48694a3aa4f56a171a687d00

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

LOG: [lldb] Skip PDB and NativePDB tests with reproducers

Added: 
lldb/test/Shell/SymbolFile/NativePDB/lit.local.cfg
lldb/test/Shell/SymbolFile/PDB/lit.local.cfg

Modified: 


Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/lit.local.cfg 
b/lldb/test/Shell/SymbolFile/NativePDB/lit.local.cfg
new file mode 100644
index ..c9b378b7a8a5
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/lit.local.cfg
@@ -0,0 +1,2 @@
+if 'lldb-repro' in config.available_features:
+  config.unsupported = True

diff  --git a/lldb/test/Shell/SymbolFile/PDB/lit.local.cfg 
b/lldb/test/Shell/SymbolFile/PDB/lit.local.cfg
new file mode 100644
index ..c9b378b7a8a5
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/PDB/lit.local.cfg
@@ -0,0 +1,2 @@
+if 'lldb-repro' in config.available_features:
+  config.unsupported = True



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

[Lldb-commits] [PATCH] D86355: Consume error for valid return from Target::GetEntryPointAddress()

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Target/Target.cpp:2408-2446
 llvm::Expected Target::GetEntryPointAddress() {
   Module *exe_module = GetExecutableModulePointer();
   llvm::Error error = llvm::Error::success();
   assert(!error); // Check the success value when assertions are enabled.
 
   if (!exe_module || !exe_module->GetObjectFile()) {
 error = llvm::make_error("No primary executable found",

I'm not particularly happy with the error handling in this function. The patch 
seems correct, but as I've demonstrated it's easy to get this wrong. I think we 
can rewrite the method as suggested to eliminate the issue. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86355

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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s:2
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+

shafik wrote:
> aprantl wrote:
> > I think this is missing a REQUIRES: line that checks for an x86 target?
> I did not use that in D85376, is there something different in this case?
All tests using x86 assembly technically require that, but I don't believe we 
have a bot which is configured to build without that target, so nothing 
enforces that. I'm pretty sure there are other tests that need this stanza, but 
don't have it.


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

https://reviews.llvm.org/D86311

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

[Lldb-commits] [lldb] 08249d7 - [lldb] Fix TestAPILog.py for reproducer replay

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-21T10:35:35-07:00
New Revision: 08249d7f72ff8f8648c5264e090813c9c2cad466

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

LOG: [lldb] Fix TestAPILog.py for reproducer replay

With the log file being a build artifact we don't need to clean it up.
If this happens before the reproducer is captured, the file will be
missing from the reproducer root but being part of the mapping.

Added: 


Modified: 
lldb/test/API/api/log/TestAPILog.py

Removed: 




diff  --git a/lldb/test/API/api/log/TestAPILog.py 
b/lldb/test/API/api/log/TestAPILog.py
index 72df276b06c3b..d5a2e4857e536 100644
--- a/lldb/test/API/api/log/TestAPILog.py
+++ b/lldb/test/API/api/log/TestAPILog.py
@@ -19,20 +19,15 @@ def test_api_log(self):
 """Test API logging"""
 logfile = self.getBuildArtifact("api-log.txt")
 
-def cleanup():
-if os.path.exists(logfile):
-os.unlink(logfile)
-
-if configuration.is_reproducer_replay():
-logfile = self.getReproducerRemappedPath(logfile)
-
-self.addTearDownHook(cleanup)
 self.expect("log enable lldb api -f {}".format(logfile))
 
 self.dbg.SetDefaultArchitecture(None)
 self.dbg.GetScriptingLanguage(None)
 target = self.dbg.CreateTarget(None)
 
+if configuration.is_reproducer_replay():
+logfile = self.getReproducerRemappedPath(logfile)
+
 self.assertTrue(os.path.isfile(logfile))
 with open(logfile, 'r') as f:
 log = f.read()



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

[Lldb-commits] [PATCH] D77480: Fix illegal early call to PyBuffer_Release in swig typemaps

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

aadsm added inline comments.



Comment at: lldb/bindings/python/python-typemaps.swig:500
+  }
+};
+

labath wrote:
> Could you also `= delete` the copy operations to make sure nothing funny 
> happens with those.
The `= delete` is unsupported in SWIG 2, only in 3: 
http://www.swig.org/Doc3.0/CPlusPlus11.html#CPlusPlus11_defaulted_deleted
Do we really need it, or is there a workaround it, or should we just bump the 
minimum requirements to SWIG 3?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77480

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

[Lldb-commits] [lldb] 57e0ef1 - [lldb] Make it a fatal error when %lldb cannot be substituted

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-21T11:18:21-07:00
New Revision: 57e0ef131b621dc88ad2db53a52bf86b498e49c3

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

LOG: [lldb] Make it a fatal error when %lldb cannot be substituted

Refuse to run the shell tests when %lldb cannot be substituted. This
prevents the test from silently running again the `lldb` in your PATH.

I noticed because when this happens, %lldb-init gets substituted with
lldb-init, which does not exists.

Added: 


Modified: 
lldb/test/Shell/helper/toolchain.py

Removed: 




diff  --git a/lldb/test/Shell/helper/toolchain.py 
b/lldb/test/Shell/helper/toolchain.py
index 64a0c6671516..2b075d5523d4 100644
--- a/lldb/test/Shell/helper/toolchain.py
+++ b/lldb/test/Shell/helper/toolchain.py
@@ -48,10 +48,12 @@ def use_lldb_substitutions(config):
 primary_tools = [
 ToolSubst('%lldb',
   command=FindTool('lldb'),
-  extra_args=['--no-lldbinit', '-S', lldb_init]),
+  extra_args=['--no-lldbinit', '-S', lldb_init],
+  unresolved='fatal'),
 ToolSubst('%lldb-init',
   command=FindTool('lldb'),
-  extra_args=['-S', lldb_init]),
+  extra_args=['-S', lldb_init],
+  unresolved='fatal'),
 ToolSubst('%debugserver',
   command=FindTool(dsname),
   extra_args=dsargs,



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

[Lldb-commits] [PATCH] D86355: Consume error for valid return from Target::GetEntryPointAddress()

[Dimitry Andric via Phabricator via lldb-commits]

dim added inline comments.



Comment at: lldb/source/Target/Target.cpp:2408-2446
 llvm::Expected Target::GetEntryPointAddress() {
   Module *exe_module = GetExecutableModulePointer();
   llvm::Error error = llvm::Error::success();
   assert(!error); // Check the success value when assertions are enabled.
 
   if (!exe_module || !exe_module->GetObjectFile()) {
 error = llvm::make_error("No primary executable found",

JDevlieghere wrote:
> I'm not particularly happy with the error handling in this function. The 
> patch seems correct, but as I've demonstrated it's easy to get this wrong. I 
> think we can rewrite the method as suggested to eliminate the issue. What do 
> you think?
Yeah, I think that is much less error-prone. The `consumeError` method has a 
relevent comment:

```
/// Uses of this method are potentially indicative of problems: perhaps the
/// error should be propagated further, or the error-producer should just
/// return an Optional in the first place.
```

In this case it was just problematic to instantiate an `Error` right at the 
start, instead of when an actual error condition occurred.

So I agree that your proposed approach is better. There is still one 
`consumeError()` in there, though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86355

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

[Lldb-commits] [PATCH] D86355: Consume error for valid return from Target::GetEntryPointAddress()

[Dimitry Andric via Phabricator via lldb-commits]

dim updated this revision to Diff 287077.
dim added a comment.

As @JDevlieghere suggests, only instantiate `Error` objects when necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86355

Files:
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2407,21 +2407,13 @@
 
 llvm::Expected Target::GetEntryPointAddress() {
   Module *exe_module = GetExecutableModulePointer();
-  llvm::Error error = llvm::Error::success();
-  assert(!error); // Check the success value when assertions are enabled.
 
-  if (!exe_module || !exe_module->GetObjectFile()) {
-error = llvm::make_error("No primary executable found",
-
llvm::inconvertibleErrorCode());
-  } else {
+  // Try to find the entry point address in the primary executable.
+  const bool has_primary_executable = exe_module && 
exe_module->GetObjectFile();
+  if (has_primary_executable) {
 Address entry_addr = exe_module->GetObjectFile()->GetEntryPointAddress();
 if (entry_addr.IsValid())
   return entry_addr;
-
-error = llvm::make_error(
-"Could not find entry point address for executable module \"" +
-exe_module->GetFileSpec().GetFilename().GetStringRef() + "\"",
-llvm::inconvertibleErrorCode());
   }
 
   const ModuleList &modules = GetImages();
@@ -2432,14 +2424,21 @@
   continue;
 
 Address entry_addr = module_sp->GetObjectFile()->GetEntryPointAddress();
-if (entry_addr.IsValid()) {
-  // Discard the error.
-  llvm::consumeError(std::move(error));
+if (entry_addr.IsValid())
   return entry_addr;
-}
   }
 
-  return std::move(error);
+  // We haven't found the entry point address. Return an appropriate error.
+  if (!has_primary_executable)
+return llvm::make_error(
+"No primary executable found and could not find entry point address in 
"
+"any executable module",
+llvm::inconvertibleErrorCode());
+
+  return llvm::make_error(
+  "Could not find entry point address for primary executable module \"" +
+  exe_module->GetFileSpec().GetFilename().GetStringRef() + "\"",
+  llvm::inconvertibleErrorCode());
 }
 
 lldb::addr_t Target::GetCallableLoadAddress(lldb::addr_t load_addr,


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2407,21 +2407,13 @@
 
 llvm::Expected Target::GetEntryPointAddress() {
   Module *exe_module = GetExecutableModulePointer();
-  llvm::Error error = llvm::Error::success();
-  assert(!error); // Check the success value when assertions are enabled.
 
-  if (!exe_module || !exe_module->GetObjectFile()) {
-error = llvm::make_error("No primary executable found",
-llvm::inconvertibleErrorCode());
-  } else {
+  // Try to find the entry point address in the primary executable.
+  const bool has_primary_executable = exe_module && exe_module->GetObjectFile();
+  if (has_primary_executable) {
 Address entry_addr = exe_module->GetObjectFile()->GetEntryPointAddress();
 if (entry_addr.IsValid())
   return entry_addr;
-
-error = llvm::make_error(
-"Could not find entry point address for executable module \"" +
-exe_module->GetFileSpec().GetFilename().GetStringRef() + "\"",
-llvm::inconvertibleErrorCode());
   }
 
   const ModuleList &modules = GetImages();
@@ -2432,14 +2424,21 @@
   continue;
 
 Address entry_addr = module_sp->GetObjectFile()->GetEntryPointAddress();
-if (entry_addr.IsValid()) {
-  // Discard the error.
-  llvm::consumeError(std::move(error));
+if (entry_addr.IsValid())
   return entry_addr;
-}
   }
 
-  return std::move(error);
+  // We haven't found the entry point address. Return an appropriate error.
+  if (!has_primary_executable)
+return llvm::make_error(
+"No primary executable found and could not find entry point address in "
+"any executable module",
+llvm::inconvertibleErrorCode());
+
+  return llvm::make_error(
+  "Could not find entry point address for primary executable module \"" +
+  exe_module->GetFileSpec().GetFilename().GetStringRef() + "\"",
+  llvm::inconvertibleErrorCode());
 }
 
 lldb::addr_t Target::GetCallableLoadAddress(lldb::addr_t load_addr,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 287078.
clayborg added a comment.

Added a test case with a 1 byte .text section that will overflow into the 15 
bytes of the .data section that follows. This will test the overflow case that 
was requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86261

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

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

[Lldb-commits] [PATCH] D77480: Fix illegal early call to PyBuffer_Release in swig typemaps

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added inline comments.



Comment at: lldb/bindings/python/python-typemaps.swig:500
+  }
+};
+

aadsm wrote:
> labath wrote:
> > Could you also `= delete` the copy operations to make sure nothing funny 
> > happens with those.
> The `= delete` is unsupported in SWIG 2, only in 3: 
> http://www.swig.org/Doc3.0/CPlusPlus11.html#CPlusPlus11_defaulted_deleted
> Do we really need it, or is there a workaround it, or should we just bump the 
> minimum requirements to SWIG 3?
It shouldn't be strictly necessary.  I put it in so if for some reason one of 
these values gets copied, it would result in a compiler error instead of a 
crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77480

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

[Lldb-commits] [PATCH] D77480: Fix illegal early call to PyBuffer_Release in swig typemaps

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added inline comments.



Comment at: lldb/bindings/python/python-typemaps.swig:500
+  }
+};
+

lawrence_danna wrote:
> aadsm wrote:
> > labath wrote:
> > > Could you also `= delete` the copy operations to make sure nothing funny 
> > > happens with those.
> > The `= delete` is unsupported in SWIG 2, only in 3: 
> > http://www.swig.org/Doc3.0/CPlusPlus11.html#CPlusPlus11_defaulted_deleted
> > Do we really need it, or is there a workaround it, or should we just bump 
> > the minimum requirements to SWIG 3?
> It shouldn't be strictly necessary.  I put it in so if for some reason one of 
> these values gets copied, it would result in a compiler error instead of a 
> crash.
This type could also just be moved into a header so swig doesn't need to parse 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77480

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

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

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 287079.
wallace added a comment.

- Added libipt as a dependency to build the new intel pt plugin. It's merely a

copy paste of what the old intel pt plugin does.

- Added a test with a sample trace.json definition file. It includes a simple

a.out object file, the source and an intel pt trace that goes from main to the
final return 0. The test checks that the plugin does the parsing of the
structured data correctly and creates the target, process and modules correctly.

- Added the necessary parsing functionalities in Trace.cpp to make sense of the

structured data.

- Added a simple parsing of the intel pt-specific information in the structured

data.

I didn't implement Dump nor Decoding, as in this case the test is quite
comprehensive regarding Load and I want to keep the diff small. I'll implement
the other features later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

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

Index: lldb/test/API/commands/trace/intelpt-trace/trace.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace.json
@@ -0,0 +1,26 @@
+{
+  "type": "intel-pt",
+  "processes": [
+{
+  "pid": 1234,
+  "threads": [
+{
+  "tid": 5678,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "filePath": "a.out",
+  "loadAddress": "0x0040"
+}
+  ]
+}
+  ],
+  "pt_cpu": {
+"vendor": "intel",
+"family": 6,
+"model": 79,
+"stepping": 1
+  }
+}
Index: lldb/test/API/commands/trace/intelpt-trace/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/main.cpp
@@ -0,0 +1,8 @@
+int main() {
+  int ret = 0;
+
+  for (int i = 0; i < 4; i++)
+ret ^= 1;
+
+  return ret;
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -0,0 +1,34 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceLoad(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+TestBase.setUp(self)
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The intel-pt test plugin is not enabled")
+
+
+def testLoadTrace(self):
+src_dir = self.getSourceDir()
+trace_definition_file = os.path.join(src_dir, "intelpt-trace", "trace.json")
+self.expect("trace load -v " + trace_definition_file, substrs=["intel-pt"])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertEqual(process.GetProcessID(), 1234)
+
+self.assertEqual(process.GetNumThreads(), 1)
+self.assertEqual(process.GetThreadAtIndex(0).GetThreadID(), 5678)
+
+self.assertEqual(target.GetNumModules(), 1)
+module = target.GetModuleAtIndex(0)
+path = module.GetFileSpec()
+self.assertEqual(path.fullpath, os.path.join(src_dir, "intelpt-trace", "a.out"))
+self.assertGreater(module.GetNumSections(), 0)
+self.assertEqual(module.GetSectionAtIndex(0).GetFileAddress(), 0x40)
Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -42,7 +42,12 @@
 buffer_or_error.getError().message());
 return return_sp;
   }
-  return ParseJSON(buffer_or_error.get()->getBuffer().str());
+  llvm::Expected value =
+  json::parse(buffer_or_error.get()->getBuffer().str());
+

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

[Greg Clayton via Phabricator via lldb-commits]

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

Lots of little things regarding the encoding and format of the JSON.




Comment at: lldb/source/Target/Trace.cpp:37
+  std::errc::invalid_argument,
+  "no trace plug-in matches the specified type: \"%s\"",
+  type.str().c_str());

Dump schema here as part of the error?



Comment at: lldb/source/Target/Trace.cpp:53
+  error.SetErrorStringWithFormat(
+  "structured data is missing the \"%s\" \"%s\" field: %s", field, type,
+  object_stream.GetData());

dump schema when we have an error?



Comment at: lldb/source/Target/Trace.cpp:65
+  error.SetErrorStringWithFormat("structured data is expected to be \"%s\": 
%s",
+ type, object_stream.GetData());
+  return false;

dump schema?




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

rename "filePath" to just "path"?



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

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



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

does JSON typically use camel case? Should this be "load-address"?



Comment at: lldb/source/Target/Trace.cpp:116-119
+  std::string full_path = m_info_dir;
+  full_path += "/";
+  full_path += file_path;
+  FileSpec file_spec(full_path);

We shouldn't assume that "file_path" is relative. This code should be something 
like:

```
FileSpec file_spec(file_path);
if (file_spec.IsRelative())
  file_spec.PrependPathComponent(m_info_dir);
```



Comment at: lldb/source/Target/Trace.cpp:127
+
+  ModuleSP module_sp(new Module(file_spec, ArchSpec()));
+  bool changed = true;

We should store a target triple as a mandatory key/value pair in the top level 
trace JSON file and access via a getter. Then we should also fill out a 
ModuleSpec with a path, UUID (optional) and architecture to make sure we don't 
end up getting the wrong file:
 
```
ModuleSpec module_spec;
module_spec.GetFileSpec() = file_spec;
module_spec.GetArchitecture() = Trace::GetArchitecture(); // This would be a 
new accessor that will hand out a architecture for the "triple" setting in the 
JSON trace settings
StringRef uuid_str;
if (module->GetValueForKeyAsString("uuid", uuid_str))
  module_spec.GetUUID().SetFromStringRef(uuid_str);
lldb::ModuleSP module_sp = target_sp->GetOrCreateModule(module_spec, false /* 
notify */, &error);
```

We wouldn't want to accidentally load "/usr/lib/libc.so" on a different machine 
with the wrong architecture since "libc.so" can exist on many systems.





Comment at: lldb/source/Target/Trace.cpp:131
+changed);
+  target_sp->GetImages().Append(module_sp);
+  return true;

Remove this since we create the module above already in the target in my inline 
comment code changes.



Comment at: lldb/source/Target/Trace.cpp:181
+
+bool Trace::ParseProcesses(Debugger &debugger, StructuredData::Dictionary 
&dict,
+   Status &error) {

This should be more generic like:

```
Trace::ParseSettings(...)
```

We will add more top level key/value pairs that follow a structured data schema 
which we should make available through a command eventually, so lets not limit 
this to just processes. We might add architecture or target triple, and much 
more in the future.



Comment at: lldb/source/Target/Trace.cpp:198
+return false;
+  return InitializeInstance(debugger, error);
+}

Maybe we should confine all plug-in specific settings to a key/value pair where 
the key is "arguments" or "plug-in-arguments" and the value is a dictionary. 
This will help ensure that no plug-in specific settings can ever conflict.
```
{ 
  "type": "intel-pt",
  "processes": [...],
    (other settin

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

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

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

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

[Adrian Prantl via Phabricator via lldb-commits]

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

LGTM with that caveat


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

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

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

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

@shafik would you mind preparing a separate commit that add the REQUIRES 
everywhere it is needed, or if there are many, create an x86 subdir with an 
implicit requirement (like we do in the llvm/test/debuginfo tests)?
If it weren't for Rosetta, these would break on Apple Silicon machines, for 
example.


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

https://reviews.llvm.org/D86311

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

[Lldb-commits] [lldb] 52e758f - [lldb] Fix build error in TestSimulatorPlatform.py

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-21T13:35:26-07:00
New Revision: 52e758f352e6fc7c2458e92cc0b6351bf9469628

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

LOG: [lldb] Fix build error in TestSimulatorPlatform.py

Before e5d08fcbac72 the Makefile would always compute the min-version,
even if it wasn't set in the triple. This nuance got lost when passing
the ARCH_CFLAGS directly from TestSimulatorPlatform.

Added: 


Modified: 
lldb/test/API/macosx/simulator/TestSimulatorPlatform.py

Removed: 




diff  --git a/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py 
b/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
index 7be192491251..0262f5af5004 100644
--- a/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
+++ b/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
@@ -46,15 +46,16 @@ def check_debugserver(self, log, expected_platform, 
expected_version):
 def run_with(self, arch, os, vers, env, expected_load_command):
 env_list = [env] if env else []
 triple = '-'.join([arch, 'apple', os + vers] + env_list)
+sdk = lldbutil.get_xcode_sdk(os, env)
 
 version_min = ''
-if vers:
-if env == 'simulator':
-version_min = '-m{}-simulator-version-min={}'.format(os, vers)
-elif os == 'macosx':
-version_min = '-m{}-version-min={}'.format(os, vers)
+if not vers:
+vers = lldbutil.get_xcode_sdk_version(sdk)
+if env == 'simulator':
+version_min = '-m{}-simulator-version-min={}'.format(os, vers)
+elif os == 'macosx':
+version_min = '-m{}-version-min={}'.format(os, vers)
 
-sdk = lldbutil.get_xcode_sdk(os, env)
 sdk_root = lldbutil.get_xcode_sdk_root(sdk)
 
 self.build(



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

[Lldb-commits] [PATCH] D77480: Fix illegal early call to PyBuffer_Release in swig typemaps

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

aadsm added inline comments.



Comment at: lldb/bindings/python/python-typemaps.swig:500
+  }
+};
+

lawrence_danna wrote:
> lawrence_danna wrote:
> > aadsm wrote:
> > > labath wrote:
> > > > Could you also `= delete` the copy operations to make sure nothing 
> > > > funny happens with those.
> > > The `= delete` is unsupported in SWIG 2, only in 3: 
> > > http://www.swig.org/Doc3.0/CPlusPlus11.html#CPlusPlus11_defaulted_deleted
> > > Do we really need it, or is there a workaround it, or should we just bump 
> > > the minimum requirements to SWIG 3?
> > It shouldn't be strictly necessary.  I put it in so if for some reason one 
> > of these values gets copied, it would result in a compiler error instead of 
> > a crash.
> This type could also just be moved into a header so swig doesn't need to 
> parse it.
ah nice, of course, I can do this. thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77480

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

[Lldb-commits] [PATCH] D86235: Fix swig scripts install target name

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

aadsm updated this revision to Diff 287106.
aadsm edited the summary of this revision.
aadsm added a comment.

Updated to use more friendly component name `lldb-python-scripts` name instead 
of `finish_swig_python_scripts`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86235

Files:
  lldb/bindings/python/CMakeLists.txt


Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -163,17 +163,17 @@
 string(REPLACE ${CMAKE_CFG_INTDIR} "\$\{CMAKE_INSTALL_CONFIG_NAME\}" 
LLDB_PYTHON_INSTALL_PATH ${LLDB_PYTHON_INSTALL_PATH})
 string(REPLACE ${CMAKE_CFG_INTDIR} "\$\{CMAKE_INSTALL_CONFIG_NAME\}" 
lldb_python_target_dir ${lldb_python_target_dir})
   endif()
-  set(swig_scripts_target "${swig_target}_scripts")
-  set(swig_scripts_install_target "${swig_target}_scripts_install")
-  add_custom_target(${swig_scripts_target})
-  add_dependencies(${swig_scripts_target} ${swig_target})
+  set(python_scripts_target "lldb-python-scripts")
+  set(python_scripts_install_target "install-${python_scripts_target}")
+  add_custom_target(${python_scripts_target})
+  add_dependencies(${python_scripts_target} ${swig_target})
   install(DIRECTORY ${lldb_python_target_dir}/../
   DESTINATION ${LLDB_PYTHON_INSTALL_PATH}
-  COMPONENT ${swig_scripts_target})
+  COMPONENT ${python_scripts_target})
   if (NOT LLVM_ENABLE_IDE)
-add_llvm_install_targets(${swig_scripts_install_target}
- COMPONENT ${swig_scripts_target}
- DEPENDS ${swig_scripts_target})
+add_llvm_install_targets(${python_scripts_install_target}
+ COMPONENT ${python_scripts_target}
+ DEPENDS ${python_scripts_target})
   endif()
 
   # Add a Post-Build Event to copy the custom Python DLL to the lldb binaries 
dir so that Windows can find it when launching


Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -163,17 +163,17 @@
 string(REPLACE ${CMAKE_CFG_INTDIR} "\$\{CMAKE_INSTALL_CONFIG_NAME\}" LLDB_PYTHON_INSTALL_PATH ${LLDB_PYTHON_INSTALL_PATH})
 string(REPLACE ${CMAKE_CFG_INTDIR} "\$\{CMAKE_INSTALL_CONFIG_NAME\}" lldb_python_target_dir ${lldb_python_target_dir})
   endif()
-  set(swig_scripts_target "${swig_target}_scripts")
-  set(swig_scripts_install_target "${swig_target}_scripts_install")
-  add_custom_target(${swig_scripts_target})
-  add_dependencies(${swig_scripts_target} ${swig_target})
+  set(python_scripts_target "lldb-python-scripts")
+  set(python_scripts_install_target "install-${python_scripts_target}")
+  add_custom_target(${python_scripts_target})
+  add_dependencies(${python_scripts_target} ${swig_target})
   install(DIRECTORY ${lldb_python_target_dir}/../
   DESTINATION ${LLDB_PYTHON_INSTALL_PATH}
-  COMPONENT ${swig_scripts_target})
+  COMPONENT ${python_scripts_target})
   if (NOT LLVM_ENABLE_IDE)
-add_llvm_install_targets(${swig_scripts_install_target}
- COMPONENT ${swig_scripts_target}
- DEPENDS ${swig_scripts_target})
+add_llvm_install_targets(${python_scripts_install_target}
+ COMPONENT ${python_scripts_target}
+ DEPENDS ${python_scripts_target})
   endif()
 
   # Add a Post-Build Event to copy the custom Python DLL to the lldb binaries dir so that Windows can find it when launching
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] d3a49b0 - [lldb] Remove --rerun-all-issues as its functionality no longer exists

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-21T14:28:08-07:00
New Revision: d3a49b03a57bb7448620c31f493932018e752c0d

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

LOG: [lldb] Remove --rerun-all-issues as its functionality no longer exists

The logic behind --rerun-all-issues was removed when we switched to LIT
as the test driver. This patch just removes the dotest option and
corresponding entry in configuration.py.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/dotest_args.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 6f4b89cb1793..7939a27badf0 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -135,9 +135,6 @@
 capture_path = None
 replay_path = None
 
-# Test rerun configuration vars
-rerun_all_issues = False
-
 # The names of all tests. Used to assert we don't have two tests with the
 # same base name.
 all_tests = set()

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 9b1c7fd39da4..870b85ef4c4b 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -416,10 +416,6 @@ def parseOptionsAndInitTestdirs():
 
 if args.replay_path:
 configuration.replay_path = args.replay_path
-
-# rerun-related arguments
-configuration.rerun_all_issues = args.rerun_all_issues
-
 if args.lldb_platform_name:
 configuration.lldb_platform_name = args.lldb_platform_name
 if args.lldb_platform_url:

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py 
b/lldb/packages/Python/lldbsuite/test/dotest_args.py
index 05dd523e744a..48d754e875f6 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -244,16 +244,6 @@ def create_parser():
 help='(Windows only) When LLDB crashes, display the Windows crash 
dialog.')
 group.set_defaults(disable_crash_dialog=True)
 
-# Re-run related arguments
-group = parser.add_argument_group('Test Re-run Options')
-group.add_argument(
-'--rerun-all-issues',
-action='store_true',
-help=('Re-run all issues that occurred during the test run '
-  'irrespective of the test method\'s marking as flakey. '
-  'Default behavior is to apply re-runs only to flakey '
-  'tests that generate issues.'))
-
 # Remove the reference to our helper function
 del X
 



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

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, aadsm, wallace.
Herald added a subscriber: mgrang.
Herald added a project: LLDB.
clayborg requested review of this revision.
Herald added a subscriber: JDevlieghere.

Breakpad creates minidump files that can a module loaded multiple times. We 
found that when a process mmap's the object file for a library, this can 
confuse breakpad into creating multiple modules in the module list. This patch 
fixes the GetFilteredModules() to check the linux maps for permissions and use 
the one that has execute permissions. Typically when people mmap a file into 
memory they don't map it as executable. This helps people to correctly load 
minidump files for post mortem analysis.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86375

Files:
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/source/Plugins/Process/minidump/MinidumpParser.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -689,6 +689,42 @@
   EXPECT_EQ(0x1000u, filtered_modules[0]->BaseOfImage);
 }
 
+TEST_F(MinidumpParserTest, MinidumpDuplicateModuleExecutableAddress) {
+  ASSERT_THAT_ERROR(SetUpFromYaml(R"(
+--- !minidump
+Streams:
+  - Type:ModuleList
+Modules:
+  - Base of Image:   0x400d9000
+Size of Image:   0x2000
+Module Name: '/usr/lib/libc.so'
+CodeView Record: ''
+  - Base of Image:   0x400ee000
+Size of Image:   0x1000
+Module Name: '/usr/lib/libc.so'
+CodeView Record: ''
+  - Type:LinuxMaps
+Text: |
+  400d9000-400db000 r--p  b3:04 227/usr/lib/libc.so
+  400dc000-400dd000 rw-p  00:00 0
+  400ee000-400ef000 r-xp 0001 b3:04 227/usr/lib/libc.so
+  400fc000-400fd000 rwxp 1000 b3:04 227/usr/lib/libc.so
+...
+)"),
+llvm::Succeeded());
+  // If we have a module mentioned twice in the module list, and we have full
+  // linux maps for all of the memory regions, make sure we pick the one that
+  // is marked as executable. If clients open an object file with mmap,
+  // breakpad can create multiple mappings for a library errnoneously and the
+  // lowest address isn't always the right address. In this case we check the
+  // memory region of the base of image address and make sure it is executable
+  // and prefer that one.
+  std::vector filtered_modules =
+  parser->GetFilteredModuleList();
+  ASSERT_EQ(1u, filtered_modules.size());
+  EXPECT_EQ(0x400ee000u, filtered_modules[0]->BaseOfImage);
+}
+
 TEST_F(MinidumpParserTest, MinidumpModuleOrder) {
   ASSERT_THAT_ERROR(SetUpFromYaml(R"(
 --- !minidump
@@ -721,4 +757,3 @@
   parser->GetMinidumpFile().getString(filtered_modules[1]->ModuleNameRVA),
   llvm::HasValue("/tmp/b"));
 }
-
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -338,32 +338,6 @@
   return ArchSpec(triple);
 }
 
-static MemoryRegionInfo GetMemoryRegionInfo(const MemoryRegionInfos ®ions,
-lldb::addr_t load_addr) {
-  MemoryRegionInfo region;
-  auto pos = llvm::upper_bound(regions, load_addr);
-  if (pos != regions.begin() &&
-  std::prev(pos)->GetRange().Contains(load_addr)) {
-return *std::prev(pos);
-  }
-
-  if (pos == regions.begin())
-region.GetRange().SetRangeBase(0);
-  else
-region.GetRange().SetRangeBase(std::prev(pos)->GetRange().GetRangeEnd());
-
-  if (pos == regions.end())
-region.GetRange().SetRangeEnd(UINT64_MAX);
-  else
-region.GetRange().SetRangeEnd(pos->GetRange().GetRangeBase());
-
-  region.SetReadable(MemoryRegionInfo::eNo);
-  region.SetWritable(MemoryRegionInfo::eNo);
-  region.SetExecutable(MemoryRegionInfo::eNo);
-  region.SetMapped(MemoryRegionInfo::eNo);
-  return region;
-}
-
 void ProcessMinidump::BuildMemoryRegions() {
   if (m_memory_regions)
 return;
@@ -388,7 +362,7 @@
   MemoryRegionInfo::RangeType section_range(load_addr,
 section_sp->GetByteSize());
   MemoryRegionInfo region =
-  ::GetMemoryRegionInfo(*m_memory_regions, load_addr);
+  MinidumpParser::GetMemoryRegionInfo(*m_memory_regions, load_addr);
   if (region.GetMapped() != MemoryRegionInfo::eYes &&
   region.GetRange().GetRangeBase() <= section_range.GetRangeBase() &&
   section_range.GetRangeEnd() <= region.GetRange().GetRangeEnd()) {
@@ -409,7 +3

[Lldb-commits] [PATCH] D86235: Fix swig scripts install target name

[António Afonso 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 rG02bf5632a94d: Fix swig scripts install target name (authored 
by aadsm).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86235

Files:
  lldb/bindings/python/CMakeLists.txt


Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -163,17 +163,17 @@
 string(REPLACE ${CMAKE_CFG_INTDIR} "\$\{CMAKE_INSTALL_CONFIG_NAME\}" 
LLDB_PYTHON_INSTALL_PATH ${LLDB_PYTHON_INSTALL_PATH})
 string(REPLACE ${CMAKE_CFG_INTDIR} "\$\{CMAKE_INSTALL_CONFIG_NAME\}" 
lldb_python_target_dir ${lldb_python_target_dir})
   endif()
-  set(swig_scripts_target "${swig_target}_scripts")
-  set(swig_scripts_install_target "${swig_target}_scripts_install")
-  add_custom_target(${swig_scripts_target})
-  add_dependencies(${swig_scripts_target} ${swig_target})
+  set(python_scripts_target "lldb-python-scripts")
+  set(python_scripts_install_target "install-${python_scripts_target}")
+  add_custom_target(${python_scripts_target})
+  add_dependencies(${python_scripts_target} ${swig_target})
   install(DIRECTORY ${lldb_python_target_dir}/../
   DESTINATION ${LLDB_PYTHON_INSTALL_PATH}
-  COMPONENT ${swig_scripts_target})
+  COMPONENT ${python_scripts_target})
   if (NOT LLVM_ENABLE_IDE)
-add_llvm_install_targets(${swig_scripts_install_target}
- COMPONENT ${swig_scripts_target}
- DEPENDS ${swig_scripts_target})
+add_llvm_install_targets(${python_scripts_install_target}
+ COMPONENT ${python_scripts_target}
+ DEPENDS ${python_scripts_target})
   endif()
 
   # Add a Post-Build Event to copy the custom Python DLL to the lldb binaries 
dir so that Windows can find it when launching


Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -163,17 +163,17 @@
 string(REPLACE ${CMAKE_CFG_INTDIR} "\$\{CMAKE_INSTALL_CONFIG_NAME\}" LLDB_PYTHON_INSTALL_PATH ${LLDB_PYTHON_INSTALL_PATH})
 string(REPLACE ${CMAKE_CFG_INTDIR} "\$\{CMAKE_INSTALL_CONFIG_NAME\}" lldb_python_target_dir ${lldb_python_target_dir})
   endif()
-  set(swig_scripts_target "${swig_target}_scripts")
-  set(swig_scripts_install_target "${swig_target}_scripts_install")
-  add_custom_target(${swig_scripts_target})
-  add_dependencies(${swig_scripts_target} ${swig_target})
+  set(python_scripts_target "lldb-python-scripts")
+  set(python_scripts_install_target "install-${python_scripts_target}")
+  add_custom_target(${python_scripts_target})
+  add_dependencies(${python_scripts_target} ${swig_target})
   install(DIRECTORY ${lldb_python_target_dir}/../
   DESTINATION ${LLDB_PYTHON_INSTALL_PATH}
-  COMPONENT ${swig_scripts_target})
+  COMPONENT ${python_scripts_target})
   if (NOT LLVM_ENABLE_IDE)
-add_llvm_install_targets(${swig_scripts_install_target}
- COMPONENT ${swig_scripts_target}
- DEPENDS ${swig_scripts_target})
+add_llvm_install_targets(${python_scripts_install_target}
+ COMPONENT ${python_scripts_target}
+ DEPENDS ${python_scripts_target})
   endif()
 
   # Add a Post-Build Event to copy the custom Python DLL to the lldb binaries dir so that Windows can find it when launching
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 02bf563 - Fix swig scripts install target name

[António Afonso via lldb-commits]

Author: António Afonso
Date: 2020-08-21T14:41:52-07:00
New Revision: 02bf5632a94da6c3570df002804f8d3f79c11bfc

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

LOG: Fix swig scripts install target name

LLVM install component targets needs to be in the form of: 
install-{target}[-stripped]

I tested with:
```
cmake ... -DLLVM_ENABLE_PROJECTS="clang;lldb" 
-DLLVM_DISTRIBUTION_COMPONENTS="lldb;liblldb;lldb-python-scripts;" ...
DESTDIR=... ninja install-distribution
```

@JDevlieghere `finish_swig_python_scripts` is a really weird name for a 
distribution component, any reason that it has to be this way?

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

Added: 


Modified: 
lldb/bindings/python/CMakeLists.txt

Removed: 




diff  --git a/lldb/bindings/python/CMakeLists.txt 
b/lldb/bindings/python/CMakeLists.txt
index d51730d18be0..b6584e389c83 100644
--- a/lldb/bindings/python/CMakeLists.txt
+++ b/lldb/bindings/python/CMakeLists.txt
@@ -163,17 +163,17 @@ function(finish_swig_python swig_target 
lldb_python_bindings_dir lldb_python_tar
 string(REPLACE ${CMAKE_CFG_INTDIR} "\$\{CMAKE_INSTALL_CONFIG_NAME\}" 
LLDB_PYTHON_INSTALL_PATH ${LLDB_PYTHON_INSTALL_PATH})
 string(REPLACE ${CMAKE_CFG_INTDIR} "\$\{CMAKE_INSTALL_CONFIG_NAME\}" 
lldb_python_target_dir ${lldb_python_target_dir})
   endif()
-  set(swig_scripts_target "${swig_target}_scripts")
-  set(swig_scripts_install_target "${swig_target}_scripts_install")
-  add_custom_target(${swig_scripts_target})
-  add_dependencies(${swig_scripts_target} ${swig_target})
+  set(python_scripts_target "lldb-python-scripts")
+  set(python_scripts_install_target "install-${python_scripts_target}")
+  add_custom_target(${python_scripts_target})
+  add_dependencies(${python_scripts_target} ${swig_target})
   install(DIRECTORY ${lldb_python_target_dir}/../
   DESTINATION ${LLDB_PYTHON_INSTALL_PATH}
-  COMPONENT ${swig_scripts_target})
+  COMPONENT ${python_scripts_target})
   if (NOT LLVM_ENABLE_IDE)
-add_llvm_install_targets(${swig_scripts_install_target}
- COMPONENT ${swig_scripts_target}
- DEPENDS ${swig_scripts_target})
+add_llvm_install_targets(${python_scripts_install_target}
+ COMPONENT ${python_scripts_target}
+ DEPENDS ${python_scripts_target})
   endif()
 
   # Add a Post-Build Event to copy the custom Python DLL to the lldb binaries 
dir so that Windows can find it when launching



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

[Lldb-commits] [PATCH] D86381: Move Py_buffer_RAII to .h file so SWIG 2 doesnt have to parse it

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

aadsm created this revision.
aadsm added reviewers: clayborg, wallace, lawrence_danna.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
aadsm requested review of this revision.
Herald added a subscriber: JDevlieghere.

`struct Py_buffer_RAII` definition uses explicit deleted functions which are 
not supported by SWIG 2 (only 3).
To get around this I moved this struct to an .h file that is included to avoid 
being parsed by swig.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86381

Files:
  lldb/bindings/python/python-typemaps.h
  lldb/bindings/python/python-typemaps.swig


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -1,5 +1,11 @@
 /* Typemap definitions, to allow SWIG to properly handle 'char**' data types. 
*/
 
+%inline %{
+
+#include "../bindings/python/python-typemaps.h"
+
+%}
+
 %typemap(in) char ** {
   /* Check if is a list  */
   if (PythonList::Check($input)) {
@@ -61,7 +67,7 @@
 
 %typemap(in) lldb::tid_t {
   PythonObject obj = Retain($input);
-  lldb::tid_t value = unwrapOrSetPythonException(As(obj)); 
+  lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
 return nullptr;
   $1 = value;
@@ -476,21 +482,6 @@
 }
 }
 
-%inline %{
-
-struct Py_buffer_RAII {
-  Py_buffer buffer = {};
-  Py_buffer_RAII() {};
-  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
-  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
-  ~Py_buffer_RAII() {
-if (buffer.obj)
-  PyBuffer_Release(&buffer);
-  }
-};
-
-%}
-
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
Index: lldb/bindings/python/python-typemaps.h
===
--- /dev/null
+++ lldb/bindings/python/python-typemaps.h
@@ -0,0 +1,12 @@
+// Defined here instead of a .swig file because SWIG 2 doesn't support
+// explicit deleted functions.
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII() {};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -1,5 +1,11 @@
 /* Typemap definitions, to allow SWIG to properly handle 'char**' data types. */
 
+%inline %{
+
+#include "../bindings/python/python-typemaps.h"
+
+%}
+
 %typemap(in) char ** {
   /* Check if is a list  */
   if (PythonList::Check($input)) {
@@ -61,7 +67,7 @@
 
 %typemap(in) lldb::tid_t {
   PythonObject obj = Retain($input);
-  lldb::tid_t value = unwrapOrSetPythonException(As(obj)); 
+  lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
 return nullptr;
   $1 = value;
@@ -476,21 +482,6 @@
 }
 }
 
-%inline %{
-
-struct Py_buffer_RAII {
-  Py_buffer buffer = {};
-  Py_buffer_RAII() {};
-  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
-  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
-  ~Py_buffer_RAII() {
-if (buffer.obj)
-  PyBuffer_Release(&buffer);
-  }
-};
-
-%}
-
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
Index: lldb/bindings/python/python-typemaps.h
===
--- /dev/null
+++ lldb/bindings/python/python-typemaps.h
@@ -0,0 +1,12 @@
+// Defined here instead of a .swig file because SWIG 2 doesn't support
+// explicit deleted functions.
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII() {};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D86355: Instantiate Error in Target::GetEntryPointAddress() only when necessary

[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/Target/Target.cpp:2408-2446
 llvm::Expected Target::GetEntryPointAddress() {
   Module *exe_module = GetExecutableModulePointer();
   llvm::Error error = llvm::Error::success();
   assert(!error); // Check the success value when assertions are enabled.
 
   if (!exe_module || !exe_module->GetObjectFile()) {
 error = llvm::make_error("No primary executable found",

dim wrote:
> JDevlieghere wrote:
> > I'm not particularly happy with the error handling in this function. The 
> > patch seems correct, but as I've demonstrated it's easy to get this wrong. 
> > I think we can rewrite the method as suggested to eliminate the issue. What 
> > do you think?
> Yeah, I think that is much less error-prone. The `consumeError` method has a 
> relevent comment:
> 
> ```
> /// Uses of this method are potentially indicative of problems: perhaps the
> /// error should be propagated further, or the error-producer should just
> /// return an Optional in the first place.
> ```
> 
> In this case it was just problematic to instantiate an `Error` right at the 
> start, instead of when an actual error condition occurred.
> 
> So I agree that your proposed approach is better. There is still one 
> `consumeError()` in there, though?
> There is still one consumeError() in there, though?

I don't think that's true anymore? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86355

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

[Lldb-commits] [PATCH] D86381: Move Py_buffer_RAII to .h file so SWIG 2 doesnt have to parse it

[Lawrence D'Anna via Phabricator via lldb-commits]

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

Looks fine to me, assuming that the tests pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86381

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

[Lldb-commits] [PATCH] D86388: Fix use-after-free in ThreadPlan, and add test.Background:ThreadPlan objects store a cached pointer to the associated Thread. To quotethe code:// We don't cache the thr

[Nicholas Allegra via Phabricator via lldb-commits]

comex created this revision.
comex added reviewers: jingham, clayborg.
Herald added a subscriber: aaron.ballman.
Herald added a project: LLDB.
comex requested review of this revision.
Herald added a subscriber: JDevlieghere.

...Thread represent
// the same underlying object on a later stop.

This can happen only when using an operating system plugin with
os-plugin-reports-all-threads = false (a new feature); otherwise, the
ThreadPlan will be wiped away when the Thread is.

Previously, this cached pointer was unowned, and ThreadPlan attempted to
prevent it from becoming stale by invalidating it in WillResume, reasoning that
the list of threads would only change whwen the target is running.  However, it
turns out that the pointer can be re-cached after it's invalidated but before
the target actually starts running.  At least one path where this happens is
ThreadPlan::ShouldReportRun -> GetPreviousPlan -> GetThread.

It might be possible to fix this by invalidating the pointer from other places,
but that seems unnecessarily risky and complicated.  Instead, just keep around
a ThreadSP and check IsValid(), which becomes false when Thread::DestroyThread()
is called.

Note: This does not create a retain cycle because Thread does not own
ThreadPlans.  (Even if it did, Thread::DestroyThread resets all of the thread's
owned pointers.)

As for testing, I made a small change to the existing reports-all-threads test
which causes it to trigger the use-after-free without the rest of the commit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86388

Files:
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/source/Target/ThreadPlan.cpp
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py


Index: 
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
===
--- 
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
+++ 
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
@@ -51,6 +51,13 @@
 (target, self.process, thread, thread_bkpt) = 
lldbutil.run_to_source_breakpoint(
 self, "first stop in thread - do a step out", self.main_file)
 
+# Disabling the thread breakpoint here ensures that we don't have an
+# unreported stop (to step over that breakpoint), which ensures that
+# ThreadPlan::ShouldReportRun is called in between 
ThreadPlan::WillResume
+# and when the thread actually disappears.  This previously triggered
+# a use-after-free.
+thread_bkpt.SetEnabled(False)
+
 main_bkpt = target.BreakpointCreateBySourceRegex('Stop here and do not 
make a memory thread for thread_1',
  self.main_file)
 self.assertEqual(main_bkpt.GetNumLocations(), 1, "Main breakpoint has 
one location")
Index: lldb/source/Target/ThreadPlan.cpp
===
--- lldb/source/Target/ThreadPlan.cpp
+++ lldb/source/Target/ThreadPlan.cpp
@@ -24,10 +24,10 @@
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
   m_stop_vote(stop_vote), m_run_vote(run_vote),
   m_takes_iteration_count(false), m_could_not_resolve_hw_bp(false),
-  m_thread(&thread), m_kind(kind), m_name(name), m_plan_complete_mutex(),
-  m_cached_plan_explains_stop(eLazyBoolCalculate), m_plan_complete(false),
-  m_plan_private(false), m_okay_to_discard(true), m_is_master_plan(false),
-  m_plan_succeeded(true) {
+  m_thread_sp(thread.shared_from_this()), m_kind(kind), m_name(name),
+  m_plan_complete_mutex(), m_cached_plan_explains_stop(eLazyBoolCalculate),
+  m_plan_complete(false), m_plan_private(false), m_okay_to_discard(true),
+  m_is_master_plan(false), m_plan_succeeded(true) {
   SetID(GetNextID());
 }
 
@@ -39,12 +39,11 @@
 const Target &ThreadPlan::GetTarget() const { return m_process.GetTarget(); }
 
 Thread &ThreadPlan::GetThread() {
-  if (m_thread)
-return *m_thread;
+  if (m_thread_sp && m_thread_sp->IsValid())
+return *m_thread_sp;
 
-  ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
-  m_thread = thread_sp.get();
-  return *m_thread;
+  m_thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
+  return *m_thread_sp;
 }
 
 bool ThreadPlan::PlanExplainsStop(Event *event_ptr) {
@@ -133,11 +132,7 @@
   StateAsCString(resume_state), StopOthers());
 }
   }
-  bool success = DoWillResume(resume_state, current_plan);
-  m_thread = nullptr; // We don't cache the thread pointer over resumes.  This
-  // Thread might go away, and another Thread represent
-  // the same underlying object on a later stop.
-  return success;
+  return DoWillResume(resume_state, current_plan);
 }
 
 ll

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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: jasonmolenda.
Herald added a subscriber: mgrang.
JDevlieghere requested review of this revision.

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


https://reviews.llvm.org/D86389

Files:
  lldb/include/lldb/Utility/Reproducer.h
  lldb/source/Symbol/LocateSymbolFile.cpp
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
  lldb/source/Utility/Reproducer.cpp

Index: lldb/source/Utility/Reproducer.cpp
===
--- lldb/source/Utility/Reproducer.cpp
+++ lldb/source/Utility/Reproducer.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/LLDBAssert.h"
+#include "lldb/Utility/UUID.h"
 
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
@@ -301,6 +302,77 @@
 m_collector->addDirectory(dir);
 }
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::repro::SymbolFileProvider::Entry)
+
+namespace llvm {
+namespace yaml {
+template <>
+struct MappingTraits {
+  static void mapping(IO &io,
+  lldb_private::repro::SymbolFileProvider::Entry &entry) {
+io.mapRequired("uuid", entry.uuid);
+io.mapRequired("module-path", entry.module_path);
+io.mapRequired("symbol-path", entry.symbol_path);
+  }
+};
+} // namespace yaml
+} // namespace llvm
+
+void SymbolFileProvider::AddSymbolFile(const UUID *uuid,
+   const FileSpec &module_file,
+   const FileSpec &symbol_file) {
+  if (!uuid || (!module_file && !symbol_file))
+return;
+  m_symbol_files.emplace_back(uuid->GetAsString(), module_file.GetPath(),
+  symbol_file.GetPath());
+}
+
+void SymbolFileProvider::Keep() {
+  FileSpec file = this->GetRoot().CopyByAppendingPathComponent(Info::file);
+  std::error_code ec;
+  llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text);
+  if (ec)
+return;
+
+  // Remove duplicates.
+  llvm::sort(m_symbol_files.begin(), m_symbol_files.end());
+  m_symbol_files.erase(
+  std::unique(m_symbol_files.begin(), m_symbol_files.end()),
+  m_symbol_files.end());
+
+  llvm::yaml::Output yout(os);
+  yout << m_symbol_files;
+}
+
+SymbolFileLoader::SymbolFileLoader(Loader *loader) {
+  if (!loader)
+return;
+
+  FileSpec file = loader->GetFile();
+  if (!file)
+return;
+
+  auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
+  if (auto err = error_or_file.getError())
+return;
+
+  llvm::yaml::Input yin((*error_or_file)->getBuffer());
+  yin >> m_symbol_files;
+}
+
+std::pair
+SymbolFileLoader::GetPaths(const UUID *uuid) const {
+  if (!uuid)
+return {};
+
+  auto it = std::lower_bound(m_symbol_files.begin(), m_symbol_files.end(),
+ SymbolFileProvider::Entry(uuid->GetAsString()));
+  if (it == m_symbol_files.end())
+return {};
+  return std::make_pair(FileSpec(it->module_path),
+FileSpec(it->symbol_path));
+}
+
 void ProviderBase::anchor() {}
 char CommandProvider::ID = 0;
 char FileProvider::ID = 0;
@@ -308,6 +380,7 @@
 char VersionProvider::ID = 0;
 char WorkingDirectoryProvider::ID = 0;
 char HomeDirectoryProvider::ID = 0;
+char SymbolFileProvider::ID = 0;
 const char *CommandProvider::Info::file = "command-interpreter.yaml";
 const char *CommandProvider::Info::name = "command-interpreter";
 const char *FileProvider::Info::file = "files.yaml";
@@ -318,3 +391,5 @@
 const char *WorkingDirectoryProvider::Info::name = "cwd";
 const char *HomeDirectoryProvider::Info::file = "home.txt";
 const char *HomeDirectoryProvider::Info::name = "home";
+const char *SymbolFileProvider::Info::file = "symbol-files.yaml";
+const char *SymbolFileProvider::Info::name = "symbol-files";
Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -27,6 +27,7 @@
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/UUID.h"
@@ -53,6 +54,17 @@
   return_module_spec.GetFileSpec().Clear();
   return_module_spec.GetSymbolFileSpec().Clear();
 
+  const UUID *uuid = module_spec.GetUUIDPtr();
+  const ArchSpec *arch = module_spec.GetArchitecturePtr();
+
+  if (repro::Loader *l = repr

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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I wasn't sure how to test this but Jason told me I can set 
`LLDB_APPLE_DSYMFORUUID_EXECUTABLE`.


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

https://reviews.llvm.org/D86389

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

[Lldb-commits] [PATCH] D86381: Move Py_buffer_RAII to .h file so SWIG 2 doesnt have to parse it

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

aadsm updated this revision to Diff 287144.
aadsm added a comment.

Added include guards, clang-format and python include


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86381

Files:
  lldb/bindings/python/python-typemaps.h
  lldb/bindings/python/python-typemaps.swig


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -1,5 +1,11 @@
 /* Typemap definitions, to allow SWIG to properly handle 'char**' data types. 
*/
 
+%inline %{
+
+#include "../bindings/python/python-typemaps.h"
+
+%}
+
 %typemap(in) char ** {
   /* Check if is a list  */
   if (PythonList::Check($input)) {
@@ -61,7 +67,7 @@
 
 %typemap(in) lldb::tid_t {
   PythonObject obj = Retain($input);
-  lldb::tid_t value = unwrapOrSetPythonException(As(obj)); 
+  lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
 return nullptr;
   $1 = value;
@@ -476,21 +482,6 @@
 }
 }
 
-%inline %{
-
-struct Py_buffer_RAII {
-  Py_buffer buffer = {};
-  Py_buffer_RAII() {};
-  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
-  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
-  ~Py_buffer_RAII() {
-if (buffer.obj)
-  PyBuffer_Release(&buffer);
-  }
-};
-
-%}
-
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
Index: lldb/bindings/python/python-typemaps.h
===
--- /dev/null
+++ lldb/bindings/python/python-typemaps.h
@@ -0,0 +1,19 @@
+#ifndef LLDB_BINDINGS_PYTHON_PYTHONTYPEMAPS_H
+#define LLDB_BINDINGS_PYTHON_PYTHONTYPEMAPS_H
+
+#include "../source/Plugins/ScriptInterpreter/Python/lldb-python.h"
+
+// Defined here instead of a .swig file because SWIG 2 doesn't support
+// explicit deleted functions.
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII(){};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+#endif // LLDB_BINDINGS_PYTHON_PYTHONTYPEMAPS_H


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -1,5 +1,11 @@
 /* Typemap definitions, to allow SWIG to properly handle 'char**' data types. */
 
+%inline %{
+
+#include "../bindings/python/python-typemaps.h"
+
+%}
+
 %typemap(in) char ** {
   /* Check if is a list  */
   if (PythonList::Check($input)) {
@@ -61,7 +67,7 @@
 
 %typemap(in) lldb::tid_t {
   PythonObject obj = Retain($input);
-  lldb::tid_t value = unwrapOrSetPythonException(As(obj)); 
+  lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
 return nullptr;
   $1 = value;
@@ -476,21 +482,6 @@
 }
 }
 
-%inline %{
-
-struct Py_buffer_RAII {
-  Py_buffer buffer = {};
-  Py_buffer_RAII() {};
-  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
-  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
-  ~Py_buffer_RAII() {
-if (buffer.obj)
-  PyBuffer_Release(&buffer);
-  }
-};
-
-%}
-
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
Index: lldb/bindings/python/python-typemaps.h
===
--- /dev/null
+++ lldb/bindings/python/python-typemaps.h
@@ -0,0 +1,19 @@
+#ifndef LLDB_BINDINGS_PYTHON_PYTHONTYPEMAPS_H
+#define LLDB_BINDINGS_PYTHON_PYTHONTYPEMAPS_H
+
+#include "../source/Plugins/ScriptInterpreter/Python/lldb-python.h"
+
+// Defined here instead of a .swig file because SWIG 2 doesn't support
+// explicit deleted functions.
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII(){};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+#endif // LLDB_BINDINGS_PYTHON_PYTHONTYPEMAPS_H
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D86381: Move Py_buffer_RAII to .h file so SWIG 2 doesnt have to parse it

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

aadsm updated this revision to Diff 287149.
aadsm added a comment.

Moved the header file to be in Plugins/ScriptInterpreter/Python so clang-tidy 
doesn't get confused solving include paths.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86381

Files:
  lldb/bindings/python/python-typemaps.swig
  lldb/source/Plugins/ScriptInterpreter/Python/lldb-python-typemaps.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/lldb-python-typemaps.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/lldb-python-typemaps.h
@@ -0,0 +1,19 @@
+#ifndef LLDB_SOURCE_PLUGINS_SCRIPTINTERPRETER_PYTHON_LLDB_PYTHON_TYPEMAPS_H
+#define LLDB_SOURCE_PLUGINS_SCRIPTINTERPRETER_PYTHON_LLDB_PYTHON_TYPEMAPS_H
+
+#include "lldb-python.h"
+
+// Defined here instead of a .swig file because SWIG 2 doesn't support
+// explicit deleted functions.
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII(){};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+#endif // LLDB_SOURCE_PLUGINS_SCRIPTINTERPRETER_PYTHON_LLDB_PYTHON_TYPEMAPS_H
Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -1,5 +1,11 @@
 /* Typemap definitions, to allow SWIG to properly handle 'char**' data types. 
*/
 
+%inline %{
+
+#include "../source/Plugins/ScriptInterpreter/Python/lldb-python-typemaps.h"
+
+%}
+
 %typemap(in) char ** {
   /* Check if is a list  */
   if (PythonList::Check($input)) {
@@ -61,7 +67,7 @@
 
 %typemap(in) lldb::tid_t {
   PythonObject obj = Retain($input);
-  lldb::tid_t value = unwrapOrSetPythonException(As(obj)); 
+  lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
 return nullptr;
   $1 = value;
@@ -476,21 +482,6 @@
 }
 }
 
-%inline %{
-
-struct Py_buffer_RAII {
-  Py_buffer buffer = {};
-  Py_buffer_RAII() {};
-  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
-  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
-  ~Py_buffer_RAII() {
-if (buffer.obj)
-  PyBuffer_Release(&buffer);
-  }
-};
-
-%}
-
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640


Index: lldb/source/Plugins/ScriptInterpreter/Python/lldb-python-typemaps.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/lldb-python-typemaps.h
@@ -0,0 +1,19 @@
+#ifndef LLDB_SOURCE_PLUGINS_SCRIPTINTERPRETER_PYTHON_LLDB_PYTHON_TYPEMAPS_H
+#define LLDB_SOURCE_PLUGINS_SCRIPTINTERPRETER_PYTHON_LLDB_PYTHON_TYPEMAPS_H
+
+#include "lldb-python.h"
+
+// Defined here instead of a .swig file because SWIG 2 doesn't support
+// explicit deleted functions.
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII(){};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+#endif // LLDB_SOURCE_PLUGINS_SCRIPTINTERPRETER_PYTHON_LLDB_PYTHON_TYPEMAPS_H
Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -1,5 +1,11 @@
 /* Typemap definitions, to allow SWIG to properly handle 'char**' data types. */
 
+%inline %{
+
+#include "../source/Plugins/ScriptInterpreter/Python/lldb-python-typemaps.h"
+
+%}
+
 %typemap(in) char ** {
   /* Check if is a list  */
   if (PythonList::Check($input)) {
@@ -61,7 +67,7 @@
 
 %typemap(in) lldb::tid_t {
   PythonObject obj = Retain($input);
-  lldb::tid_t value = unwrapOrSetPythonException(As(obj)); 
+  lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
 return nullptr;
   $1 = value;
@@ -476,21 +482,6 @@
 }
 }
 
-%inline %{
-
-struct Py_buffer_RAII {
-  Py_buffer buffer = {};
-  Py_buffer_RAII() {};
-  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
-  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
-  ~Py_buffer_RAII() {
-if (buffer.obj)
-  PyBuffer_Release(&buffer);
-  }
-};
-
-%}
-
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 86fc193 - [lldb] Don't pass --rerun-all-issues on Windows.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-21T19:58:24-07:00
New Revision: 86fc1933099d8818c7d7559ae41e5903a1daf9bd

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

LOG: [lldb] Don't pass --rerun-all-issues on Windows.

The functionality has been removed for a while and now the dotest
argument has been removed asll.

Added: 


Modified: 
lldb/test/API/CMakeLists.txt

Removed: 




diff  --git a/lldb/test/API/CMakeLists.txt b/lldb/test/API/CMakeLists.txt
index 192c0adc66a2..1cd705c56540 100644
--- a/lldb/test/API/CMakeLists.txt
+++ b/lldb/test/API/CMakeLists.txt
@@ -68,9 +68,6 @@ if ("${LLDB_TEST_COMPILER}" STREQUAL "")
 endif()
 
 if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
-  # All tests are currently flaky on Windows, so rerun them all once when they 
fail.
-  set(LLDB_TEST_COMMON_ARGS ${LLDB_TEST_COMMON_ARGS} --rerun-all-issues)
-
   set(LLDB_TEST_DEBUG_TEST_CRASHES
 0
 CACHE BOOL "(Windows only) Enables debugging of tests in the test suite by 
showing the crash dialog when lldb crashes")



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

[Lldb-commits] [PATCH] D86381: Move Py_buffer_RAII to .h file so SWIG 2 doesnt have to parse it

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

aadsm updated this revision to Diff 287160.
aadsm added a comment.

After reading a bit more how clang-tidy works this isn't fixable because it 
actually needs to compile it. I also didn't find a way to exclude a file from 
it.
My plan is to just land this and then make a PR to add this file to 
https://github.com/google/llvm-premerge-checks/blob/master/scripts/clang-tidy.ignore


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86381

Files:
  lldb/bindings/python/python-typemaps.h
  lldb/bindings/python/python-typemaps.swig


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -1,5 +1,11 @@
 /* Typemap definitions, to allow SWIG to properly handle 'char**' data types. 
*/
 
+%inline %{
+
+#include "../bindings/python/python-typemaps.h"
+
+%}
+
 %typemap(in) char ** {
   /* Check if is a list  */
   if (PythonList::Check($input)) {
@@ -61,7 +67,7 @@
 
 %typemap(in) lldb::tid_t {
   PythonObject obj = Retain($input);
-  lldb::tid_t value = unwrapOrSetPythonException(As(obj)); 
+  lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
 return nullptr;
   $1 = value;
@@ -476,21 +482,6 @@
 }
 }
 
-%inline %{
-
-struct Py_buffer_RAII {
-  Py_buffer buffer = {};
-  Py_buffer_RAII() {};
-  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
-  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
-  ~Py_buffer_RAII() {
-if (buffer.obj)
-  PyBuffer_Release(&buffer);
-  }
-};
-
-%}
-
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
Index: lldb/bindings/python/python-typemaps.h
===
--- /dev/null
+++ lldb/bindings/python/python-typemaps.h
@@ -0,0 +1,17 @@
+#ifndef LLDB_BINDINGS_PYTHON_PYTHON_TYPEMAPS_H
+#define LLDB_BINDINGS_PYTHON_PYTHON_TYPEMAPS_H
+
+// Defined here instead of a .swig file because SWIG 2 doesn't support
+// explicit deleted functions.
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII(){};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+#endif // LLDB_BINDINGS_PYTHON_PYTHON_TYPEMAPS_H


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -1,5 +1,11 @@
 /* Typemap definitions, to allow SWIG to properly handle 'char**' data types. */
 
+%inline %{
+
+#include "../bindings/python/python-typemaps.h"
+
+%}
+
 %typemap(in) char ** {
   /* Check if is a list  */
   if (PythonList::Check($input)) {
@@ -61,7 +67,7 @@
 
 %typemap(in) lldb::tid_t {
   PythonObject obj = Retain($input);
-  lldb::tid_t value = unwrapOrSetPythonException(As(obj)); 
+  lldb::tid_t value = unwrapOrSetPythonException(As(obj));
   if (PyErr_Occurred())
 return nullptr;
   $1 = value;
@@ -476,21 +482,6 @@
 }
 }
 
-%inline %{
-
-struct Py_buffer_RAII {
-  Py_buffer buffer = {};
-  Py_buffer_RAII() {};
-  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
-  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
-  ~Py_buffer_RAII() {
-if (buffer.obj)
-  PyBuffer_Release(&buffer);
-  }
-};
-
-%}
-
 // These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,
 // and fixed so they will not crash if PyObject_GetBuffer fails.
 // https://github.com/swig/swig/issues/1640
Index: lldb/bindings/python/python-typemaps.h
===
--- /dev/null
+++ lldb/bindings/python/python-typemaps.h
@@ -0,0 +1,17 @@
+#ifndef LLDB_BINDINGS_PYTHON_PYTHON_TYPEMAPS_H
+#define LLDB_BINDINGS_PYTHON_PYTHON_TYPEMAPS_H
+
+// Defined here instead of a .swig file because SWIG 2 doesn't support
+// explicit deleted functions.
+struct Py_buffer_RAII {
+  Py_buffer buffer = {};
+  Py_buffer_RAII(){};
+  Py_buffer &operator=(const Py_buffer_RAII &) = delete;
+  Py_buffer_RAII(const Py_buffer_RAII &) = delete;
+  ~Py_buffer_RAII() {
+if (buffer.obj)
+  PyBuffer_Release(&buffer);
+  }
+};
+
+#endif // LLDB_BINDINGS_PYTHON_PYTHON_TYPEMAPS_H
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Target/ThreadPlan.h:603
 
-  Thread *m_thread; // Stores a cached value of the thread, which is set to
-// nullptr when the thread resumes.  Don't use this 
anywhere
-// but ThreadPlan::GetThread().
+  lldb::ThreadSP m_thread_sp; // Stores a cached value of the thread.  Don't 
use
+  // use this anywhere but ThreadPlan::GetThread().

drive-by nit: since you're touching this, can you make it a Doxygen comment 
(`///`) above the variable? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86388

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

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

[Jonas Devlieghere via Phabricator via lldb-commits]

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

- Add test.
- Implement dump method.

Unfortunately, the yaml2obj roundtrip does not preserve the UUID, so the test 
uses the binary.


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

https://reviews.llvm.org/D86389

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

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