[Lldb-commits] [lldb] a0249fe - [DebugInfo] Rename section identifiers which are deprecated in DWARFv5. NFC.

2020-04-06 Thread Igor Kudrin via lldb-commits

Author: Igor Kudrin
Date: 2020-04-06T13:28:06+07:00
New Revision: a0249fe91c7ba0dabf0e8789171fb4aea5fca1cb

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

LOG: [DebugInfo] Rename section identifiers which are deprecated in DWARFv5. 
NFC.

This is a preparation for an upcoming patch which adds support for
DWARFv5 unit index sections. The patch adds tag "_EXT_" to identifiers
which reference sections that are deprecated in the DWARFv5 standard.
See D75929 for the discussion.

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
llvm/tools/llvm-dwp/llvm-dwp.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index 941a547ddbf9..874978bf1398 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -76,7 +76,7 @@ void DWARFDebugInfo::ParseUnitsFor(DIERef::Section section) {
   if (m_context.isDwo())
 index = &llvm::getDWARFUnitIndex(m_context.GetAsLLVM(),
  section == DIERef::Section::DebugTypes
- ? llvm::DW_SECT_TYPES
+ ? llvm::DW_SECT_EXT_TYPES
  : llvm::DW_SECT_INFO);
   lldb::offset_t offset = 0;
   while (data.ValidOffset(offset)) {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 8447d4fcea4c..b8d16758246f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -480,7 +480,7 @@ DWARFDataExtractor DWARFUnit::GetLocationData() const {
   const DWARFDataExtractor &data =
   GetVersion() >= 5 ? Ctx.getOrLoadLocListsData() : Ctx.getOrLoadLocData();
   if (const llvm::DWARFUnitIndex::Entry *entry = m_header.GetIndexEntry()) {
-if (const auto *contribution = entry->getContribution(llvm::DW_SECT_LOC))
+if (const auto *contribution = 
entry->getContribution(llvm::DW_SECT_EXT_LOC))
   return DWARFDataExtractor(data, contribution->Offset,
 contribution->Length);
 return DWARFDataExtractor();

diff  --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h 
b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
index d6b5e72cf16c..018962c0bd50 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
@@ -21,12 +21,12 @@ class raw_ostream;
 
 enum DWARFSectionKind {
   DW_SECT_INFO = 1,
-  DW_SECT_TYPES,
+  DW_SECT_EXT_TYPES,
   DW_SECT_ABBREV,
   DW_SECT_LINE,
-  DW_SECT_LOC,
+  DW_SECT_EXT_LOC,
   DW_SECT_STR_OFFSETS,
-  DW_SECT_MACINFO,
+  DW_SECT_EXT_MACINFO,
   DW_SECT_MACRO,
 };
 

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp 
b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index 2150df411f42..bdd2a7240df6 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -763,7 +763,7 @@ const DWARFUnitIndex &DWARFContext::getTUIndex() {
 
   DataExtractor TUIndexData(DObj->getTUIndexSection(), isLittleEndian(), 0);
 
-  TUIndex = std::make_unique(DW_SECT_TYPES);
+  TUIndex = std::make_unique(DW_SECT_EXT_TYPES);
   TUIndex->parse(TUIndexData);
   return *TUIndex;
 }
@@ -959,7 +959,7 @@ void DWARFContext::parseNormalUnits() {
   });
   NormalUnits.finishedInfoUnits();
   DObj->forEachTypesSections([&](const DWARFSection &S) {
-NormalUnits.addUnitsForSection(*this, S, DW_SECT_TYPES);
+NormalUnits.addUnitsForSection(*this, S, DW_SECT_EXT_TYPES);
   });
 }
 
@@ -971,7 +971,7 @@ void DWARFContext::parseDWOUnits(bool Lazy) {
   });
   DWOUnits.finishedInfoUnits();
   DObj->forEachTypesDWOSections([&](const DWARFSection &S) {
-DWOUnits.addUnitsForDWOSection(*this, S, DW_SECT_TYPES, Lazy);
+DWOUnits.addUnitsForDWOSection(*this, S, DW_SECT_EXT_TYPES, Lazy);
   });
 }
 

diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp 
b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
index 80f09dbd4c18..a42185b8015c 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -183,7 +183,7 @@ DWARFUnit::DWARFUnit(DWARFContext &DC, const DWARFSection 
&Section,
 // data based on the index entries.
 StringRef Data = LocSection->Data;
 if (auto *Index

[Lldb-commits] [PATCH] D77141: [DebugInfo] Rename section identifiers which are deprecated in DWARFv5. NFC.

2020-04-06 Thread Igor Kudrin via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0249fe91c7b: [DebugInfo] Rename section identifiers which 
are deprecated in DWARFv5. NFC. (authored by ikudrin).

Changed prior to commit:
  https://reviews.llvm.org/D77141?vs=254802&id=255232#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77141

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
  llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp

Index: llvm/tools/llvm-dwp/llvm-dwp.cpp
===
--- llvm/tools/llvm-dwp/llvm-dwp.cpp
+++ llvm/tools/llvm-dwp/llvm-dwp.cpp
@@ -259,7 +259,7 @@
   C.Length = I->Length;
   ++I;
 }
-unsigned TypesIndex = getContributionIndex(DW_SECT_TYPES);
+unsigned TypesIndex = getContributionIndex(DW_SECT_EXT_TYPES);
 auto &C = Entry.Contributions[TypesIndex];
 Out.emitBytes(Types.substr(
 C.Offset - TUEntry.Contributions[TypesIndex].Offset, C.Length));
@@ -281,7 +281,7 @@
   UnitIndexEntry Entry = CUEntry;
   // Zero out the debug_info contribution
   Entry.Contributions[0] = {};
-  auto &C = Entry.Contributions[getContributionIndex(DW_SECT_TYPES)];
+  auto &C = Entry.Contributions[getContributionIndex(DW_SECT_EXT_TYPES)];
   C.Offset = TypesOffset;
   auto PrevOffset = Offset;
   // Length of the unit, including the 4 byte length field.
@@ -452,7 +452,7 @@
 
   if (DWARFSectionKind Kind = SectionPair->second.second) {
 auto Index = getContributionIndex(Kind);
-if (Kind != DW_SECT_TYPES) {
+if (Kind != DW_SECT_EXT_TYPES) {
   CurEntry.Contributions[Index].Offset = ContributionOffsets[Index];
   ContributionOffsets[Index] +=
   (CurEntry.Contributions[Index].Length = Contents.size());
@@ -536,10 +536,10 @@
   MCSection *const TUIndexSection = MCOFI.getDwarfTUIndexSection();
   const StringMap> KnownSections = {
   {"debug_info.dwo", {MCOFI.getDwarfInfoDWOSection(), DW_SECT_INFO}},
-  {"debug_types.dwo", {MCOFI.getDwarfTypesDWOSection(), DW_SECT_TYPES}},
+  {"debug_types.dwo", {MCOFI.getDwarfTypesDWOSection(), DW_SECT_EXT_TYPES}},
   {"debug_str_offsets.dwo", {StrOffsetSection, DW_SECT_STR_OFFSETS}},
   {"debug_str.dwo", {StrSection, static_cast(0)}},
-  {"debug_loc.dwo", {MCOFI.getDwarfLocDWOSection(), DW_SECT_LOC}},
+  {"debug_loc.dwo", {MCOFI.getDwarfLocDWOSection(), DW_SECT_EXT_LOC}},
   {"debug_line.dwo", {MCOFI.getDwarfLineDWOSection(), DW_SECT_LINE}},
   {"debug_abbrev.dwo", {MCOFI.getDwarfAbbrevDWOSection(), DW_SECT_ABBREV}},
   {"debug_cu_index", {CUIndexSection, static_cast(0)}},
@@ -603,7 +603,7 @@
   P.first->second.DWOName = ID.DWOName;
   addAllTypes(Out, TypeIndexEntries, TypesSection, CurTypesSection,
   CurEntry,
-  ContributionOffsets[getContributionIndex(DW_SECT_TYPES)]);
+  ContributionOffsets[getContributionIndex(DW_SECT_EXT_TYPES)]);
   continue;
 }
 
@@ -642,13 +642,14 @@
 if (!CurTypesSection.empty()) {
   if (CurTypesSection.size() != 1)
 return make_error("multiple type unit sections in .dwp file");
-  DWARFUnitIndex TUIndex(DW_SECT_TYPES);
+  DWARFUnitIndex TUIndex(DW_SECT_EXT_TYPES);
   DataExtractor TUIndexData(CurTUIndexSection, Obj.isLittleEndian(), 0);
   if (!TUIndex.parse(TUIndexData))
 return make_error("failed to parse tu_index");
   addAllTypesFromDWP(
   Out, TypeIndexEntries, TUIndex, TypesSection, CurTypesSection.front(),
-  CurEntry, ContributionOffsets[getContributionIndex(DW_SECT_TYPES)]);
+  CurEntry,
+  ContributionOffsets[getContributionIndex(DW_SECT_EXT_TYPES)]);
 }
   }
 
@@ -659,7 +660,7 @@
  TypeIndexEntries);
 
   // Lie about the type contribution
-  ContributionOffsets[getContributionIndex(DW_SECT_TYPES)] = 0;
+  ContributionOffsets[getContributionIndex(DW_SECT_EXT_TYPES)] = 0;
   // Unlie about the info contribution
   ContributionOffsets[0] = 1;
 
Index: llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
===
--- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -352,7 +352,7 @@
 
   OS << "Verifying .debug_types Unit Header Chain...\n";
   DObj.forEachTypesSections([&](const DWARFSection &S) {
-NumErrors += verifyUnitSection(S, DW_SECT_TYPES);
+NumErrors += verifyUnitSection(S, DW_SECT_EXT_TYPES);
   });
   return NumErrors == 0;
 }
Index: llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
===

[Lldb-commits] [PATCH] D75929: [DebugInfo] Support DWARFv5 index sections.

2020-04-06 Thread Igor Kudrin via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG714324b79ae2: [DebugInfo] Support DWARFv5 index sections. 
(authored by ikudrin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75929

Files:
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
  llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
  llvm/test/DebugInfo/X86/dwp-v2-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v2-tu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
  llvm/tools/llvm-dwp/llvm-dwp.cpp

Index: llvm/tools/llvm-dwp/llvm-dwp.cpp
===
--- llvm/tools/llvm-dwp/llvm-dwp.cpp
+++ llvm/tools/llvm-dwp/llvm-dwp.cpp
@@ -216,11 +216,12 @@
   StringRef DWPName;
 };
 
-// Convert a section identifier into the index to use with
+// Convert an internal section identifier into the index to use with
 // UnitIndexEntry::Contributions.
 static unsigned getContributionIndex(DWARFSectionKind Kind) {
-  assert(Kind >= DW_SECT_INFO);
-  return Kind - DW_SECT_INFO;
+  // Assuming the pre-standard DWP format.
+  assert(serializeSectionKind(Kind, 2) >= DW_SECT_INFO);
+  return serializeSectionKind(Kind, 2) - DW_SECT_INFO;
 }
 
 // Convert a UnitIndexEntry::Contributions index to the corresponding on-disk
Index: llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
@@ -0,0 +1,43 @@
+## The test checks that we can parse and dump a TU index section that is
+## compliant to the DWARFv5 standard.
+
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o - | \
+# RUN:   llvm-dwarfdump -debug-tu-index - | \
+# RUN:   FileCheck %s
+
+# CHECK:  .debug_tu_index contents:
+# CHECK-NEXT: version = 5 slots = 2
+# CHECK-EMPTY:
+# CHECK-NEXT: Index Signature  INFO ABBREV   LINE STR_OFFSETS
+# CHECK-NEXT: - --    
+# CHECK-NEXT: 1 0x1111 [0x1000, 0x1010) [0x2000, 0x2020) [0x3000, 0x3030) [0x4000, 0x4040)
+
+.section .debug_tu_index, "", @progbits
+## Header:
+.short 5# Version
+.space 2# Padding
+.long 4 # Section count
+.long 1 # Unit count
+.long 2 # Slot count
+## Hash Table of Signatures:
+.quad 0x1111
+.quad 0
+## Parallel Table of Indexes:
+.long 1
+.long 0
+## Table of Section Offsets:
+## Row 0:
+.long 1 # DW_SECT_INFO
+.long 3 # DW_SECT_ABBREV
+.long 4 # DW_SECT_LINE
+.long 6 # DW_SECT_STR_OFFSETS
+## Row 1:
+.long 0x1000# Offset in .debug_info.dwo
+.long 0x2000# Offset in .debug_abbrev.dwo
+.long 0x3000# Offset in .debug_line.dwo
+.long 0x4000# Offset in .debug_str_offsets.dwo
+## Table of Section Sizes:
+.long 0x10  # Size in .debug_info.dwo
+.long 0x20  # Size in .debug_abbrev.dwo
+.long 0x30  # Size in .debug_line.dwo
+.long 0x40  # Size in .debug_str_offsets.dwo
Index: llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
@@ -0,0 +1,52 @@
+## The test checks that we can parse and dump a CU index section that is
+## compliant to the DWARFv5 standard.
+
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o - | \
+# RUN:   llvm-dwarfdump -debug-cu-index - | \
+# RUN:   FileCheck %s
+
+# CHECK:  .debug_cu_index contents:
+# CHECK-NEXT: version = 5 slots = 2
+# CHECK-EMPTY:
+# CHECK-NEXT: Index Signature  INFO ABBREV   LINE LOCLISTS STR_OFFSETS  MACRORNGLISTS
+# CHECK-NEXT: - --       
+# CHECK-NEXT: 1 0x1111 [0x1000, 0x1010) [0x2000, 0x2020) [0x3000, 0x3030) [0x4000, 0x4040) [0x5000, 0x5050) [0x6000, 0x6060) [0x7000, 0x7070)
+
+.section .debug_cu_index, "", @progbits
+## Header:
+.short 5# Version
+.space 2# Padding
+.long 7 # Section count
+.long 1 # Unit count
+.long 2 # Slot count
+## Hash Table of Signatures:
+.quad 0x1111
+.quad 0
+## Parallel Table of Indexes:
+.long 1
+.lo

[Lldb-commits] [lldb] acb0b99 - [lldb][NFC] Modernize lang/cpp/scope test

2020-04-06 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-04-06T09:36:54+02:00
New Revision: acb0b99c8e4f1dc65a7f1e26da9db77239a67da7

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

LOG: [lldb][NFC] Modernize lang/cpp/scope test

Added: 


Modified: 
lldb/test/API/lang/cpp/scope/TestCppScope.py
lldb/test/API/lang/cpp/scope/main.cpp

Removed: 




diff  --git a/lldb/test/API/lang/cpp/scope/TestCppScope.py 
b/lldb/test/API/lang/cpp/scope/TestCppScope.py
index 213e7fbe9022..1320bd2185f8 100644
--- a/lldb/test/API/lang/cpp/scope/TestCppScope.py
+++ b/lldb/test/API/lang/cpp/scope/TestCppScope.py
@@ -1,91 +1,27 @@
-"""
-Test scopes in C++.
-"""
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-
-class TestCppScopes(TestBase):
+class TestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
-def test_all_but_c(self):
-self.do_test(False)
-
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
-def test_c(self):
-self.do_test(True)
-
-def do_test(self, test_c):
+def test(self):
 self.build()
-
-# Get main source file
-src_file = os.path.join(self.getSourceDir(), "main.cpp")
-src_file_spec = lldb.SBFileSpec(src_file)
-self.assertTrue(src_file_spec.IsValid(), "Main source file")
-
-# Get the path of the executable
-exe_path = self.getBuildArtifact("a.out")
-
-# Load the executable
-target = self.dbg.CreateTarget(exe_path)
-self.assertTrue(target.IsValid(), VALID_TARGET)
-
-# Break on main function
-main_breakpoint = target.BreakpointCreateBySourceRegex(
-"// break here", src_file_spec)
-self.assertTrue(
-main_breakpoint.IsValid() and main_breakpoint.GetNumLocations() >= 
1,
-VALID_BREAKPOINT)
-
-# Launch the process
-args = None
-env = None
-process = target.LaunchSimple(
-args, env, self.get_process_working_directory())
-self.assertTrue(process.IsValid(), PROCESS_IS_VALID)
-
-# Get the thread of the process
-self.assertTrue(
-process.GetState() == lldb.eStateStopped,
-PROCESS_STOPPED)
-thread = lldbutil.get_stopped_thread(
-process, lldb.eStopReasonBreakpoint)
-
-# Get current fream of the thread at the breakpoint
-frame = thread.GetSelectedFrame()
-
-# Test result for scopes of variables
-
-global_variables = frame.GetVariables(True, True, True, False)
-global_variables_assert = {
-'A::a': ,
-'B::a': ,
-'C::a': ,
-'::a': ,
-'a': 
-}
-
-self.assertTrue(
-global_variables.GetSize() == 4,
-"target variable returns all variables")
-for variable in global_variables:
-name = variable.GetName()
-self.assertTrue(
-name in global_variables_assert,
-"target variable returns wrong variable " + name)
-
-for name in global_variables_assert:
-if name is "C::a" and not test_c:
-continue
-if name is not "C::a" and test_c:
-continue
-
-value = frame.EvaluateExpression(name)
-assert_value = global_variables_assert[name]
-self.assertTrue(
-value.IsValid() and value.GetValueAsSigned() == assert_value,
-name + " = " + str(assert_value))
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
+
+# Test that global variables contain the right scope operators.
+global_vars = self.frame().GetVariables(False, False, True, False)
+global_var_names = [v.GetName() for v in global_vars]
+expected_var_names = ["A::a", "B::a", "C::a", "::a"]
+self.assertEqual(global_var_names, expected_var_names)
+
+# Test lookup in scopes.
+self.expect_expr("A::a", result_value="")
+self.expect_expr("B::a", result_value="")
+self.expect_expr("C::a", result_value="")
+self.expect_expr("::a", result_value="")
+# Check that lookup without scope returns the same result.
+self.expect_expr("a", result_value="")

diff  --git a/lldb/test/API/lang/cpp/scope/main.cpp 
b/lldb/test/API/lang/cpp/scope/main.cpp
index da5d7ed529d1..20997a8dd1a7 100644
--- a/lldb/test/API/lang/cpp/scope/main.cpp
+++ b/lldb/test/API/lang/cpp/scope/main.cpp
@@ -1,17 +1,17 @@
 class A {
 public:
-static int a;
- 

[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

2020-04-06 Thread Yuri Per via Phabricator via lldb-commits
yuri created this revision.
yuri added reviewers: labath, clayborg, EugeneBi.
yuri added a project: LLDB.
Herald added a subscriber: lldb-commits.

In D49685  sysroot behaviour was partially 
fixed. But files from local filesystem with same path still has priority over 
files from sysroot.

This patch fixes it by removing fallback to local filesystem from 
RemoteAwarePlatform::GetModuleSpec(). It is not actually required because 
higher level code do such fallback itself. See, for example, resolver in 
Platform::GetSharedModule().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77529

Files:
  lldb/source/Target/RemoteAwarePlatform.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-bin_sh.core


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -209,6 +209,33 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# linux-x86_64-bin_sh.core is a copy of linux-x86_64.core with 
executable file path patched to /bin/sh:
+# sed 
's/\/home\/labath\/test\/a\.out/\/bin\/sh\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@/g;s/a\.out/sh\c@\c@\c@/g'
 < linux-x86_64.core > linux-x86_64-bin_sh.core
+
+# Copy linux-x86_64.out to tmp_sysroot/bin/sh
+tmp_sysroot = os.path.join(self.getBuildDir(), 
"lldb_x86_64_mock_sysroot")
+executable = os.path.join(tmp_sysroot, "bin", "sh")
+lldbutil.mkdir_p(os.path.dirname(executable))
+shutil.copyfile("linux-x86_64.out", executable)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % 
tmp_sysroot)
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-x86_64-bin_sh.core")
+
+# Check that we found executable from the sysroot
+mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
+self.assertEqual(mod_path, executable)
+self.check_all(process, self._x86_64_pid, self._x86_64_regions, "sh")
+
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("ARM")
 def test_arm_core(self):
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -21,7 +21,7 @@
 return m_remote_platform_sp->GetModuleSpec(module_file_spec, arch,
module_spec);
 
-  return Platform::GetModuleSpec(module_file_spec, arch, module_spec);
+  return false;
 }
 
 Status RemoteAwarePlatform::RunShellCommand(


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -209,6 +209,33 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# linux-x86_64-bin_sh.core is a copy of linux-x86_64.core with executable file path patched to /bin/sh:
+# sed 's/\/home\/labath\/test\/a\.out/\/bin\/sh\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@\c@/g;s/a\.out/sh\c@\c@\c@/g' < linux-x86_64.core > linux-x86_64-bin_sh.core
+
+# Copy linux-x86_64.out to tmp_sysroot/bin/sh
+tmp_sysroot = os.path.join(self.getBuildDir(), "lldb_x86_64_mock_sysroot")
+executable = os.path.join(tmp_sysroot, "bin", "sh")
+lldbutil.mkdir_p(os.path.dirname(executable))
+shutil.copyfile("linux-x86_64.out", executable)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % tmp_sysroot)
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-x86_64-bin_sh.core")
+
+# Check that we found executable from the sysroot
+mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
+self.assertEqual(mod_path, executable)
+self.check_all(process, self._x86_64_pid, self._x86_64_regions, "sh")
+
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("ARM")
 def test_arm_core(self):
Index: lldb/source/Target/RemoteAwarePla

[Lldb-commits] [PATCH] D61548: Fix use of 'is' operator for comparison

2020-04-06 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

I rewrote this test in acb0b99c8e4f1dc65a7f1e26da9db77239a67da7 
 so this 
isn't necessary anymore. Thanks for the patch though!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61548



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


[Lldb-commits] [lldb] 4f644ff - [lldb] XFAIL TestThreadPlanCommands _only_ on aarch64

2020-04-06 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-04-06T10:36:02+02:00
New Revision: 4f644ff9e8738b5fde4825b929ec69d0b78ed2ea

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

LOG: [lldb] XFAIL TestThreadPlanCommands _only_ on aarch64

The test works fine on x86-linux.

Added: 


Modified: 
lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py 
b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
index 5214a3f6b0fe..ddc88cce75f1 100644
--- a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
+++ b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
@@ -17,7 +17,7 @@ class TestThreadPlanCommands(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIfWindows
-@expectedFailureAll(oslist=["linux"])
+@expectedFailureAll(oslist=["linux"], archs=["aarch64"])
 def test_thread_plan_actions(self):
 self.build()
 self.main_source_file = lldb.SBFileSpec("main.c")



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


[Lldb-commits] [lldb] 3c2dc28 - [lldb] Also apply Fix-Its in "note:" diagnostics that belong to an error diagnostic

2020-04-06 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-04-06T10:37:33+02:00
New Revision: 3c2dc28d812c917e01f46b3bcf5b5e0a2a803276

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

LOG: [lldb] Also apply Fix-Its in "note:" diagnostics that belong to an error 
diagnostic

Summary:
LLDB currently applies Fix-Its if they are attached to a Clang diagnostic that 
has the
severity "error". Fix-Its connected to warnings and other severities are 
supposed to
be ignored as LLDB doesn't seem to trust Clang Fix-Its in these situations.

However, LLDB also ignores all Fix-Its coming from "note:" diagnostics. These 
diagnostics
are usually emitted alongside other diagnostics (both warnings and errors), 
either to keep
a single diagnostic message shorter or because the Fix-It is in a different 
source line. As they
are technically their own (non-error) diagnostics, we currently are ignoring 
all Fix-Its associated with them.

For example, this is a possible Clang diagnostic with a Fix-It that is 
currently ignored:
```
error: :2:10: too many arguments provided to function-like 
macro invocation
ToStr(0, {,})
 ^
:1:9: macro 'ToStr' defined here
#define ToStr(x) #x
^
:2:1: cannot use initializer list at the beginning of a 
macro argument
ToStr(0, {,})
^
```

We also don't store "note:" diagnostics at all, as LLDB's abstraction around 
the whole diagnostic
concept doesn't have such a concept. The text of "note:" diagnostics is instead
appended to the last non-note diagnostic (which is causing that there is no 
"note:" text in the
diagnostic above, as all the "note:" diagnostics have been appended to the 
first "error: ..." text).

This patch fixes the ignored Fix-Its in note-diagnostics by appending them to 
the last non-note
diagnostic, similar to the way we handle the text in these diagnostics.

Reviewers: JDevlieghere, jingham

Reviewed By: JDevlieghere

Subscribers: abidh

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

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/test/API/commands/expression/fixits/TestFixIts.py

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index e5de4b4651df..9996f2608e31 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -147,6 +147,14 @@ class ClangExpressionParser::LLDBPreprocessorCallbacks : 
public PPCallbacks {
   llvm::StringRef getErrorString() { return m_error_stream.GetString(); }
 };
 
+static void AddAllFixIts(ClangDiagnostic *diag, const clang::Diagnostic &Info) 
{
+  for (auto &fix_it : Info.getFixItHints()) {
+if (fix_it.isNull())
+  continue;
+diag->AddFixitHint(fix_it);
+  }
+}
+
 class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
 public:
   ClangDiagnosticManagerAdapter(DiagnosticOptions &opts) {
@@ -162,6 +170,17 @@ class ClangDiagnosticManagerAdapter : public 
clang::DiagnosticConsumer {
 m_manager = manager;
   }
 
+  /// Returns the last ClangDiagnostic message that the DiagnosticManager
+  /// received or a nullptr if the DiagnosticMangager hasn't seen any
+  /// Clang diagnostics yet.
+  ClangDiagnostic *MaybeGetLastClangDiag() const {
+if (m_manager->Diagnostics().empty())
+  return nullptr;
+lldb_private::Diagnostic *diag = m_manager->Diagnostics().back().get();
+ClangDiagnostic *clang_diag = dyn_cast(diag);
+return clang_diag;
+  }
+
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
 const clang::Diagnostic &Info) override {
 if (!m_manager) {
@@ -204,6 +223,23 @@ class ClangDiagnosticManagerAdapter : public 
clang::DiagnosticConsumer {
 case DiagnosticsEngine::Level::Note:
   m_manager->AppendMessageToDiagnostic(m_output);
   make_new_diagnostic = false;
+
+  // 'note:' diagnostics for errors and warnings can also contain Fix-Its.
+  // We add these Fix-Its to the last error diagnostic to make sure
+  // that we later have all Fix-Its related to an 'error' diagnostic when
+  // we apply them to the user expression.
+  auto *clang_diag = MaybeGetLastClangDiag();
+  // If we don't have a previous diagnostic there is nothing to do.
+  // If the previous diagnostic already has its own Fix-Its, assume that
+  // the 'note:' Fix-It is just an alternative way to solve the issue and
+  // ignore these Fix-Its.
+  if (!clang_diag || clang_diag->HasFixIts())
+break;
+  // Ignore all Fix-Its that are not associated with an error.
+  if (clang_diag->GetSeverity() != eDiagnosticSeverityError)
+  

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D77287#1960390 , @compnerd wrote:

> I think that the basic test is sufficient for this.


That test does not seem to be exercising the "unload" part of this patch. It 
would also be nice to run some basic command like "image list" to verify that 
the loaded binary is indeed listed.

(I don't really have a hard objection to this being a lit test, but it does 
sound to me like at that point, this will be reimplementing TestLoadUnload.py)

> I think that the path sanitizing and conversion should be a subsequent change.

Why is that? The need for sanitation is a direct consequence of how you've 
chosen to implement this patch -- the posix version of this function does not 
do sanitation, but it does not need to, as it does not embed the library name 
into the compiled expression. I can see how the support for wide strings might 
be considered a separate feature, but given that supporting that is a matter of 
adding a single `L` to the compiled expression, I don't see a problem with 
including that here.


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

https://reviews.llvm.org/D77287



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


[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Checking for presence this way seems ok-ish (definitely better than the 
environment variable). A possible alternative would be to pass some argument to 
dotest from cmake which would identify the status of the pt support.

Anyway, if you're going to add a new public API for this purpose, please create 
a separate patch for that with a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77452



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


[Lldb-commits] [PATCH] D77055: [lldb] Also apply Fix-Its in "note:" diagnostics that belong to an error diagnostic

2020-04-06 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c2dc28d812c: [lldb] Also apply Fix-Its in "note:" 
diagnostics that belong to an error… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77055

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/test/API/commands/expression/fixits/TestFixIts.py


Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -61,6 +61,14 @@
 self.assertTrue(value.GetError().Success())
 self.assertEquals(value.GetValueAsUnsigned(), 20)
 
+# Try a Fix-It that is stored in the 'note:' diagnostic of an error.
+# The Fix-It here is adding parantheses around the ToStr parameters.
+fixit_in_note_expr ="#define ToStr(x) #x\nToStr(0 {, })"
+value = frame.EvaluateExpression(fixit_in_note_expr, options)
+self.assertTrue(value.IsValid())
+self.assertTrue(value.GetError().Success(), value.GetError())
+self.assertEquals(value.GetSummary(), '"(0 {, })"')
+
 # Now turn off the fixits, and the expression should fail:
 options.SetAutoApplyFixIts(False)
 value = frame.EvaluateExpression(two_error_expression, options)
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -147,6 +147,14 @@
   llvm::StringRef getErrorString() { return m_error_stream.GetString(); }
 };
 
+static void AddAllFixIts(ClangDiagnostic *diag, const clang::Diagnostic &Info) 
{
+  for (auto &fix_it : Info.getFixItHints()) {
+if (fix_it.isNull())
+  continue;
+diag->AddFixitHint(fix_it);
+  }
+}
+
 class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
 public:
   ClangDiagnosticManagerAdapter(DiagnosticOptions &opts) {
@@ -162,6 +170,17 @@
 m_manager = manager;
   }
 
+  /// Returns the last ClangDiagnostic message that the DiagnosticManager
+  /// received or a nullptr if the DiagnosticMangager hasn't seen any
+  /// Clang diagnostics yet.
+  ClangDiagnostic *MaybeGetLastClangDiag() const {
+if (m_manager->Diagnostics().empty())
+  return nullptr;
+lldb_private::Diagnostic *diag = m_manager->Diagnostics().back().get();
+ClangDiagnostic *clang_diag = dyn_cast(diag);
+return clang_diag;
+  }
+
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
 const clang::Diagnostic &Info) override {
 if (!m_manager) {
@@ -204,6 +223,23 @@
 case DiagnosticsEngine::Level::Note:
   m_manager->AppendMessageToDiagnostic(m_output);
   make_new_diagnostic = false;
+
+  // 'note:' diagnostics for errors and warnings can also contain Fix-Its.
+  // We add these Fix-Its to the last error diagnostic to make sure
+  // that we later have all Fix-Its related to an 'error' diagnostic when
+  // we apply them to the user expression.
+  auto *clang_diag = MaybeGetLastClangDiag();
+  // If we don't have a previous diagnostic there is nothing to do.
+  // If the previous diagnostic already has its own Fix-Its, assume that
+  // the 'note:' Fix-It is just an alternative way to solve the issue and
+  // ignore these Fix-Its.
+  if (!clang_diag || clang_diag->HasFixIts())
+break;
+  // Ignore all Fix-Its that are not associated with an error.
+  if (clang_diag->GetSeverity() != eDiagnosticSeverityError)
+break;
+  AddAllFixIts(clang_diag, Info);
+  break;
 }
 if (make_new_diagnostic) {
   // ClangDiagnostic messages are expected to have no whitespace/newlines
@@ -218,13 +254,8 @@
   // enough context in an expression for the warning to be useful.
   // FIXME: Should we try to filter out FixIts that apply to our generated
   // code, and not the user's expression?
-  if (severity == eDiagnosticSeverityError) {
-for (const clang::FixItHint &fixit : Info.getFixItHints()) {
-  if (fixit.isNull())
-continue;
-  new_diagnostic->AddFixitHint(fixit);
-}
-  }
+  if (severity == eDiagnosticSeverityError)
+AddAllFixIts(new_diagnostic.get(), Info);
 
   m_manager->AddDiagnostic(std::move(new_diagnostic));
 }


Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixi

[Lldb-commits] [lldb] 203a8ad - [lldb] Add option to retry Fix-Its multiple times to failed expressions

2020-04-06 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-04-06T11:25:36+02:00
New Revision: 203a8adb65429ec6b8989afdf5956564972b9703

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

LOG: [lldb] Add option to retry Fix-Its multiple times to failed expressions

Summary:
Usually when Clang emits an error Fix-It it does two things. It emits the 
diagnostic and then it fixes the
currently generated AST to reflect the applied Fix-It. While emitting the 
diagnostic is easy to implement,
fixing the currently generated AST is often tricky. That causes that some 
Fix-Its just keep the AST as-is or
abort the parsing process entirely. Once the parser stopped, any Fix-Its for 
the rest of the expression are
not detected and when the user manually applies the Fix-It, the next expression 
will just produce a new
Fix-It.

This is often occurring with quickly made Fix-Its that are just used to bridge 
temporary API changes
and that often are not worth implementing a proper API fixup in addition to the 
diagnostic. To still
give some kind of reasonable user-experience for users that have these Fix-Its 
and rely on them to
fix their expressions, this patch adds the ability to retry parsing with 
applied Fix-Its multiple time to
give the normal Fix-It experience where things Clang knows how to fix are not 
causing actual expression
error (at least when automatically applying Fix-Its is activated).

The way this is implemented is just by having another setting in the expression 
options that specify how
often we should try applying Fix-Its and then reparse the expression. The 
default setting is still 1 for everyone
so this should not affect the speed in which we fail to parse expressions.

Reviewers: jingham, JDevlieghere, friss, shafik

Reviewed By: shafik

Subscribers: shafik, abidh

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

Added: 


Modified: 
lldb/bindings/interface/SBExpressionOptions.i
lldb/include/lldb/API/SBExpressionOptions.h
lldb/include/lldb/Target/Target.h
lldb/source/API/SBExpressionOptions.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Expression/UserExpression.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
lldb/test/API/commands/expression/fixits/TestFixIts.py

Removed: 




diff  --git a/lldb/bindings/interface/SBExpressionOptions.i 
b/lldb/bindings/interface/SBExpressionOptions.i
index 5dbd7007c01c..33a6ed745a8d 100644
--- a/lldb/bindings/interface/SBExpressionOptions.i
+++ b/lldb/bindings/interface/SBExpressionOptions.i
@@ -126,6 +126,14 @@ public:
 bool
 GetAutoApplyFixIts();
 
+%feature("docstring", "Sets how often LLDB should retry applying fix-its 
to an expression.") SetRetriesWithFixIts;
+void
+SetRetriesWithFixIts(uint64_t retries);
+
+%feature("docstring", "Gets how often LLDB will retry applying fix-its to 
an expression.") GetRetriesWithFixIts;
+uint64_t
+GetRetriesWithFixIts();
+
 bool
 GetTopLevel();
 

diff  --git a/lldb/include/lldb/API/SBExpressionOptions.h 
b/lldb/include/lldb/API/SBExpressionOptions.h
index 14b52684ddce..9fc6e9ea957e 100644
--- a/lldb/include/lldb/API/SBExpressionOptions.h
+++ b/lldb/include/lldb/API/SBExpressionOptions.h
@@ -86,6 +86,10 @@ class LLDB_API SBExpressionOptions {
 
   bool GetAutoApplyFixIts();
 
+  void SetRetriesWithFixIts(uint64_t retries);
+
+  uint64_t GetRetriesWithFixIts();
+
   bool GetTopLevel();
 
   void SetTopLevel(bool b = true);

diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index f0a57b8f3827..27997e5ea76d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -135,6 +135,8 @@ class TargetProperties : public Properties {
 
   bool GetEnableAutoApplyFixIts() const;
 
+  uint64_t GetNumberOfRetriesWithFixits() const;
+
   bool GetEnableNotifyAboutFixIts() const;
 
   bool GetEnableSaveObjects() const;
@@ -385,6 +387,12 @@ class EvaluateExpressionOptions {
 
   bool GetAutoApplyFixIts() const { return m_auto_apply_fixits; }
 
+  void SetRetriesWithFixIts(uint64_t number_of_retries) {
+m_retries_with_fixits = number_of_retries;
+  }
+
+  uint64_t GetRetriesWithFixIts() const { return m_retries_with_fixits; }
+
   bool IsForUtilityExpr() const { return m_running_utility_expression; }
 
   void SetIsForUtilityExpr(bool b) { m_running_utility_expression = b; }
@@ -406,6 +414,7 @@ class EvaluateExpressionOptions {
   bool m_ansi_color_errors = false;
   bool m_result_is_internal = false;
   bool m_auto_apply_fixits = true;
+  uint64_t m_retries_with_fixits = 1;
   /// True if the executed code should be treated as utility code that is only
   /// used by LLDB internally.
   bool m_running_utility_expression = false;

diff  --git a/lldb/sourc

[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

2020-04-06 Thread Yuri Per via Phabricator via lldb-commits
yuri updated this revision to Diff 255260.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77529

Files:
  lldb/source/Target/RemoteAwarePlatform.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -209,6 +209,39 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# Prepare patched core file
+core_file = os.path.join(self.getBuildDir(), 
"lldb_x86_64_patched.core")
+with open('linux-x86_64.core', 'rb') as f:
+core = f.read()
+core = core.replace(b'/home/labath/test/a.out', 
b'/bin/sh\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0')
+core = core.replace(b'a.out', b'sh\0\0\0')
+with open(core_file, 'wb') as f:
+f.write(core)
+
+# Copy linux-x86_64.out to tmp_sysroot/bin/sh
+tmp_sysroot = os.path.join(self.getBuildDir(), 
"lldb_x86_64_mock_sysroot")
+executable = os.path.join(tmp_sysroot, "bin", "sh")
+lldbutil.mkdir_p(os.path.dirname(executable))
+shutil.copyfile("linux-x86_64.out", executable)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % 
tmp_sysroot)
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore(core_file)
+
+# Check that we found executable from the sysroot
+mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
+self.assertEqual(mod_path, executable)
+self.check_all(process, self._x86_64_pid, self._x86_64_regions, "sh")
+
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("ARM")
 def test_arm_core(self):
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -21,7 +21,7 @@
 return m_remote_platform_sp->GetModuleSpec(module_file_spec, arch,
module_spec);
 
-  return Platform::GetModuleSpec(module_file_spec, arch, module_spec);
+  return false;
 }
 
 Status RemoteAwarePlatform::RunShellCommand(


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -209,6 +209,39 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# Prepare patched core file
+core_file = os.path.join(self.getBuildDir(), "lldb_x86_64_patched.core")
+with open('linux-x86_64.core', 'rb') as f:
+core = f.read()
+core = core.replace(b'/home/labath/test/a.out', b'/bin/sh\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0')
+core = core.replace(b'a.out', b'sh\0\0\0')
+with open(core_file, 'wb') as f:
+f.write(core)
+
+# Copy linux-x86_64.out to tmp_sysroot/bin/sh
+tmp_sysroot = os.path.join(self.getBuildDir(), "lldb_x86_64_mock_sysroot")
+executable = os.path.join(tmp_sysroot, "bin", "sh")
+lldbutil.mkdir_p(os.path.dirname(executable))
+shutil.copyfile("linux-x86_64.out", executable)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % tmp_sysroot)
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore(core_file)
+
+# Check that we found executable from the sysroot
+mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
+self.assertEqual(mod_path, executable)
+self.check_all(process, self._x86_64_pid, self._x86_64_regions, "sh")
+
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("ARM")
 def test_arm_core(self):
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -21,7 +21,7 @@
 return m_remote_platform_sp->GetModuleSpec(module_file_spec, arch,
mo

[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks for doing this. This looks more-or-less like what I was expecting. The 
need for the "bool" return value from the index functions is unfortunate (I did 
not anticipate that), but it does seem unavoidable.

Since none of these functions (I guess) are storing the callback function, 
could you replace std::function with llvm::function_ref ?




Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:130-163
+  bool GetValues(ConstString unique_cstr,
+ std::function callback) const {
+for (const Entry &entry : llvm::make_range(std::equal_range(
+ m_map.begin(), m_map.end(), unique_cstr, Compare(
+  if (callback(entry.value))
+return true;
+

I'm not sure these functions are really needed. They have just one caller, and 
they'd be trivial if this class provided appropriate abstractions. Maybe just 
add begin/end/equal_range methods, so that one can write:
```
for (value: map / map.equal_range(str))
  stuff
```
?

(ideally, I'd like to have iterators instead of callbacks for the index classes 
too, but iterators and class hierarchies don't mix very well)



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h:79
   llvm::Optional ToDIERef(const DebugNames::Entry &entry);
-  void Append(const DebugNames::Entry &entry, DIEArray &offsets);
+  bool Append(const DebugNames::Entry &entry,
+  std::function callback);

Maybe rename this to `ProcessEntry` or something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327



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


[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.



Comment at: lldb/include/lldb/Core/SearchFilter.h:102
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const FileSpec &spec);

kwk wrote:
> jankratochvil wrote:
> > I am not against it but FYI in a paragraph above there is already written: 
> > `The default implementation is "Everything Passes."`
> > English: IMO missing "returns".
> Thank you for the missing "returns". I'd like to keep the comment.
You can keep _a_ comment if you want, but this comment is even internally 
redundant. I don't see a need to mention overriding, saying "The default 
(base?) implementation returns true" should be clear enough.


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

https://reviews.llvm.org/D77376



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


[Lldb-commits] [PATCH] D77460: [lldb] NFC: Fix trivial typo in comments, documents, and messages

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Thanks for doing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77460



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


[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Looks good to me, but let's wait for @shafik and @aprantl to take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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


[Lldb-commits] [PATCH] D77214: [lldb] Add option to retry Fix-Its multiple times to failed expressions

2020-04-06 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG203a8adb6542: [lldb] Add option to retry Fix-Its multiple 
times to failed expressions (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77214

Files:
  lldb/bindings/interface/SBExpressionOptions.i
  lldb/include/lldb/API/SBExpressionOptions.h
  lldb/include/lldb/Target/Target.h
  lldb/source/API/SBExpressionOptions.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  lldb/test/API/commands/expression/fixits/TestFixIts.py

Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -81,3 +81,71 @@
 self.assertTrue(
 error_string.find("my_pointer->second.a") != -1,
 "Fix was right")
+
+
+# Test repeatedly applying Fix-Its to expressions and reparsing them.
+multiple_runs_options = lldb.SBExpressionOptions()
+multiple_runs_options.SetAutoApplyFixIts(True)
+multiple_runs_options.SetTopLevel(True)
+
+# An expression that needs two parse attempts with one Fix-It each
+# to be successfully parsed.
+two_runs_expr = """
+struct Data { int m; };
+
+template
+struct S1 : public T {
+  using T::TypeDef;
+  int f() {
+Data d;
+d.m = 123;
+// The first error as the using above requires a 'typename '.
+// Will trigger a Fix-It that puts 'typename' in the right place.
+typename S1::TypeDef i = &d;
+// i has the type "Data *", so this should be i.m.
+// The second run will change the . to -> via the Fix-It.
+return i.m;
+  }
+};
+
+struct ClassWithTypeDef {
+  typedef Data *TypeDef;
+};
+
+int test_X(int i) {
+  S1 s1;
+  return s1.f();
+}
+"""
+
+# Disable retries which will fail.
+multiple_runs_options.SetRetriesWithFixIts(0)
+value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
+self.assertIn("expression failed to parse, fixed expression suggested:",
+  value.GetError().GetCString())
+self.assertIn("using typename T::TypeDef",
+  value.GetError().GetCString())
+# The second Fix-It shouldn't be suggested here as Clang should have
+# aborted the parsing process.
+self.assertNotIn("i->m",
+  value.GetError().GetCString())
+
+# Retry once, but the expression needs two retries.
+multiple_runs_options.SetRetriesWithFixIts(1)
+value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
+self.assertIn("expression failed to parse, fixed expression suggested:",
+  value.GetError().GetCString())
+# Both our fixed expressions should be in the suggested expression.
+self.assertIn("using typename T::TypeDef",
+  value.GetError().GetCString())
+self.assertIn("i->m",
+  value.GetError().GetCString())
+
+# Retry twice, which will get the expression working.
+multiple_runs_options.SetRetriesWithFixIts(2)
+value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
+# This error signals success for top level expressions.
+self.assertEquals(value.GetError().GetCString(), "error: No value")
+
+# Test that the code above compiles to the right thing.
+self.expect_expr("test_X(1)", result_type="int", result_value="123")
Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -55,6 +55,9 @@
   def AutoApplyFixIts: Property<"auto-apply-fixits", "Boolean">,
 DefaultTrue,
 Desc<"Automatically apply fix-it hints to expressions.">;
+  def RetriesWithFixIts: Property<"retries-with-fixits", "UInt64">,
+DefaultUnsignedValue<1>,
+Desc<"Maximum number of attempts to fix an expression with Fix-Its">;
   def NotifyAboutFixIts: Property<"notify-about-fixits", "Boolean">,
 DefaultTrue,
 Desc<"Print the fixed expression text.">;
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3740,6 +3740,12 @@
   nullptr, idx, g_target_properties[idx].default_uint_value != 0)

Re: [Lldb-commits] [lldb] b6cd964 - Fix typo in xfail decorator for lldb thread plan list tests

2020-04-06 Thread Jan Kratochvil via lldb-commits
Hi,

I get XPASSes now on Fedora 31 x86_64:

http://lab.llvm.org:8014/builders/lldb-x86_64-fedora/builds/7169
http://lab.llvm.org:8014/builders/lldb-x86_64-fedora?numbuilds=1000

So maybe to remove the expected failure?


Jan



On Sun, 05 Apr 2020 17:18:54 +0200, Muhammad Omair Javaid via lldb-commits 
wrote:
> 
> Author: Muhammad Omair Javaid
> Date: 2020-04-05T20:16:46+05:00
> New Revision: b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc
> DIFF: 
> https://github.com/llvm/llvm-project/commit/b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc.diff
> 
> LOG: Fix typo in xfail decorator for lldb thread plan list tests
> 
> Added: 
> 
> 
> Modified: 
> lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
> 
> Removed: 
> 
> 
> 
> 
> diff  --git 
> a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py 
> b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
> index b4ae9107aceb..5214a3f6b0fe 100644
> --- a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
> +++ b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
> @@ -17,7 +17,7 @@ class TestThreadPlanCommands(TestBase):
>  NO_DEBUG_INFO_TESTCASE = True
>  
>  @skipIfWindows
> -@expectedFailureAll(oslist=["Linux"])
> +@expectedFailureAll(oslist=["linux"])
>  def test_thread_plan_actions(self):
>  self.build()
>  self.main_source_file = lldb.SBFileSpec("main.c")
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
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

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks for getting back to this. Is there any reasonable way of testing this 
(e.g. an existing python class with temporary buffer storage), or would we have 
to implement our own PyBuffer object?




Comment at: lldb/bindings/python/python-typemaps.swig:493-495
+class Py_buffer_RAII {
+public:
+  Py_buffer m_buffer = {};

please delete `public`, s/class/struct and delete `m_` from `m_buffer`. ([[ 
http://llvm.org/docs/CodingStandards.html#use-of-class-and-struct-keywords || 
llvm guidelines ]] say we should use struct for classes with public members, 
and lldb does not (typically) use the `m_` prefix in such classes.)



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

Could you also `= delete` the copy operations to make sure nothing funny 
happens with those.


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] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D77444#1962952 , @clayborg wrote:

> The biggest issue is maintaining the API. We can't add anything to 
> SBCommandPluginInterface


If we're going to be adding a new inheritable class for this feature, I'd 
recommend adding about half a dozen or so dummy virtual methods to it so we 
reserve vtable space for future expansion.




Comment at: lldb/test/API/api/auto-repeat-command/TestSBCommandAutoRepeat.py:1
+"""Test the lldb public C++ api for returning SBCommandReturnObject."""
+

Could you try making this a unit test (like the regular c++ unit tests)?

I know we have already have tests following this pattern, but that was because 
they were all written before we had googletest, and they all have a lot of 
problems I'm not keen on repeating.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444



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


Re: [Lldb-commits] [lldb] b6cd964 - Fix typo in xfail decorator for lldb thread plan list tests

2020-04-06 Thread Pavel Labath via lldb-commits
I guess these should go away after 4f644ff9e (which restrict the
decorator to aarch64). Judging by
,
Jim is going to look into a better solution for that this week.

pl


On 06/04/2020 11:50, Jan Kratochvil wrote:
> Hi,
> 
> I get XPASSes now on Fedora 31 x86_64:
> 
> http://lab.llvm.org:8014/builders/lldb-x86_64-fedora/builds/7169
> http://lab.llvm.org:8014/builders/lldb-x86_64-fedora?numbuilds=1000
> 
> So maybe to remove the expected failure?
> 
> 
> Jan
> 
> 
> 
> On Sun, 05 Apr 2020 17:18:54 +0200, Muhammad Omair Javaid via lldb-commits 
> wrote:
>>
>> Author: Muhammad Omair Javaid
>> Date: 2020-04-05T20:16:46+05:00
>> New Revision: b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc
>>
>> URL: 
>> https://github.com/llvm/llvm-project/commit/b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc.diff
>>
>> LOG: Fix typo in xfail decorator for lldb thread plan list tests
>>
>> Added: 
>> 
>>
>> Modified: 
>> lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
>>
>> Removed: 
>> 
>>
>>
>> 
>> diff  --git 
>> a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py 
>> b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
>> index b4ae9107aceb..5214a3f6b0fe 100644
>> --- a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
>> +++ b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
>> @@ -17,7 +17,7 @@ class TestThreadPlanCommands(TestBase):
>>  NO_DEBUG_INFO_TESTCASE = True
>>  
>>  @skipIfWindows
>> -@expectedFailureAll(oslist=["Linux"])
>> +@expectedFailureAll(oslist=["linux"])
>>  def test_thread_plan_actions(self):
>>  self.build()
>>  self.main_source_file = lldb.SBFileSpec("main.c")
>>
>>
>> 
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 

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


[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks for doing this. There are so many levels at which the search for modules 
is retried and I am glad that we can solve this problem by getting rid of one 
of them. Unfortunately, all of that complexity also means it's hard to 
guarantee that this won't break anything, but this patch is so simple, I think 
it's worth giving it a shot...

I just have a question about the test case inline...




Comment at: 
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py:221
+core = f.read()
+core = core.replace(b'/home/labath/test/a.out', 
b'/bin/sh\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0')
+core = core.replace(b'a.out', b'sh\0\0\0')

Would it be possible to make this test not depend on the existence of 
`/bin/sh`? Maybe if you place the "wrong" file in `$BUILD/a.out`, and the 
"right" file at `$BUILD/sysroot/$BUILD/a.out` and then check that the "right" 
file was selected?

I guess that will require creating a new core file with a sufficiently long 
path so that it can be replaced by the right runtime value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77529



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


[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

2020-04-06 Thread Yuri Per via Phabricator via lldb-commits
yuri added a comment.

Build failure is a bug of //tidy// script and is not caused by the patch itself.

  
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/lldb/source/Target/RemoteAwarePlatform.cpp:9:10:
 error: 'lldb/Target/RemoteAwarePlatform.h' file not found 
[clang-diagnostic-error]
  #include "lldb/Target/RemoteAwarePlatform.h"
   ^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77529



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


Re: [Lldb-commits] [lldb] b6cd964 - Fix typo in xfail decorator for lldb thread plan list tests

2020-04-06 Thread Omair Javaid via lldb-commits
Thanks Pavel and Jan, Sorry for the inconvenience

On Mon, 6 Apr 2020 at 15:42, Pavel Labath  wrote:

> I guess these should go away after 4f644ff9e (which restrict the
> decorator to aarch64). Judging by
> <
> http://lists.llvm.org/pipermail/lldb-commits/Week-of-Mon-20200330/063428.html
> >,
> Jim is going to look into a better solution for that this week.
>
> pl
>
>
> On 06/04/2020 11:50, Jan Kratochvil wrote:
> > Hi,
> >
> > I get XPASSes now on Fedora 31 x86_64:
> >
> > http://lab.llvm.org:8014/builders/lldb-x86_64-fedora/builds/7169
> > http://lab.llvm.org:8014/builders/lldb-x86_64-fedora?numbuilds=1000
> >
> > So maybe to remove the expected failure?
> >
> >
> > Jan
> >
> >
> >
> > On Sun, 05 Apr 2020 17:18:54 +0200, Muhammad Omair Javaid via
> lldb-commits wrote:
> >>
> >> Author: Muhammad Omair Javaid
> >> Date: 2020-04-05T20:16:46+05:00
> >> New Revision: b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc
> >>
> >> URL:
> https://github.com/llvm/llvm-project/commit/b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc
> >> DIFF:
> https://github.com/llvm/llvm-project/commit/b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc.diff
> >>
> >> LOG: Fix typo in xfail decorator for lldb thread plan list tests
> >>
> >> Added:
> >>
> >>
> >> Modified:
> >> lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
> >>
> >> Removed:
> >>
> >>
> >>
> >>
> 
> >> diff  --git
> a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
> b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
> >> index b4ae9107aceb..5214a3f6b0fe 100644
> >> ---
> a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
> >> +++
> b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
> >> @@ -17,7 +17,7 @@ class TestThreadPlanCommands(TestBase):
> >>  NO_DEBUG_INFO_TESTCASE = True
> >>
> >>  @skipIfWindows
> >> -@expectedFailureAll(oslist=["Linux"])
> >> +@expectedFailureAll(oslist=["linux"])
> >>  def test_thread_plan_actions(self):
> >>  self.build()
> >>  self.main_source_file = lldb.SBFileSpec("main.c")
> >>
> >>
> >>
> >> ___
> >> lldb-commits mailing list
> >> lldb-commits@lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >
>
>

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


[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

2020-04-06 Thread Yuri Per via Phabricator via lldb-commits
yuri marked an inline comment as done.
yuri added inline comments.



Comment at: 
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py:221
+core = f.read()
+core = core.replace(b'/home/labath/test/a.out', 
b'/bin/sh\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0')
+core = core.replace(b'a.out', b'sh\0\0\0')

labath wrote:
> Would it be possible to make this test not depend on the existence of 
> `/bin/sh`? Maybe if you place the "wrong" file in `$BUILD/a.out`, and the 
> "right" file at `$BUILD/sysroot/$BUILD/a.out` and then check that the "right" 
> file was selected?
> 
> I guess that will require creating a new core file with a sufficiently long 
> path so that it can be replaced by the right runtime value.
Yes, it's possible.
Can you suggest how to upload binary file into review? Looks like git diff with 
--binary flag is not supported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77529



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


[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

2020-04-06 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
kwk updated this revision to Diff 255300.
kwk added a comment.

- Simplify comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -417,12 +417,6 @@
   return true;
 }
 
-bool SearchFilterByModule::CompUnitPasses(FileSpec &fileSpec) { return true; }
-
-bool SearchFilterByModule::CompUnitPasses(CompileUnit &compUnit) {
-  return true;
-}
-
 void SearchFilterByModule::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;
@@ -543,14 +537,6 @@
   return true;
 }
 
-bool SearchFilterByModuleList::CompUnitPasses(FileSpec &fileSpec) {
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(CompileUnit &compUnit) {
-  return true;
-}
-
 void SearchFilterByModuleList::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;
Index: lldb/include/lldb/Core/SearchFilter.h
===
--- lldb/include/lldb/Core/SearchFilter.h
+++ lldb/include/lldb/Core/SearchFilter.h
@@ -98,6 +98,8 @@
   ///The file spec to check against the filter.
   /// \return
   ///\b true if \a spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool ModulePasses(const FileSpec &spec);
 
   /// Call this method with a Module to see if that module passes the filter.
@@ -107,6 +109,8 @@
   ///
   /// \return
   ///\b true if \a module passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool ModulePasses(const lldb::ModuleSP &module_sp);
 
   /// Call this method with a Address to see if \a address passes the filter.
@@ -116,6 +120,8 @@
   ///
   /// \return
   ///\b true if \a address passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool AddressPasses(Address &addr);
 
   /// Call this method with a FileSpec to see if \a file spec passes the
@@ -126,6 +132,8 @@
   ///
   /// \return
   ///\b true if \a file spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(FileSpec &fileSpec);
 
   /// Call this method with a CompileUnit to see if \a comp unit passes the
@@ -136,6 +144,8 @@
   ///
   /// \return
   ///\b true if \a Comp Unit passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit &compUnit);
 
   /// Call this method with a Function to see if \a function passes the
@@ -321,10 +331,6 @@
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -372,10 +378,6 @@
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

2020-04-06 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
kwk marked 2 inline comments as done.
kwk added a comment.

@jankratochvil thanks for the tip.
@labath, addressed your comments and will now push it.




Comment at: lldb/include/lldb/Core/SearchFilter.h:102
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const FileSpec &spec);

labath wrote:
> kwk wrote:
> > jankratochvil wrote:
> > > I am not against it but FYI in a paragraph above there is already 
> > > written: `The default implementation is "Everything Passes."`
> > > English: IMO missing "returns".
> > Thank you for the missing "returns". I'd like to keep the comment.
> You can keep _a_ comment if you want, but this comment is even internally 
> redundant. I don't see a need to mention overriding, saying "The default 
> (base?) implementation returns true" should be clear enough.
@labath. That is fine. I've changed the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77376



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


[Lldb-commits] [PATCH] D77295: [lldb/Core] Fix a race in the Communication class

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 255310.
labath marked an inline comment as done.
labath added a comment.

refactor EOF handling


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77295

Files:
  lldb/source/Core/Communication.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/CommunicationTest.cpp

Index: lldb/unittests/Core/CommunicationTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/CommunicationTest.cpp
@@ -0,0 +1,35 @@
+//===-- CommunicationTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/Communication.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/Pipe.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(CommunicationTest, SynchronizeWhileClosing) {
+  // Set up a communication object reading from a pipe.
+  Pipe pipe;
+  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
+llvm::Succeeded());
+
+  Communication comm("test");
+  comm.SetConnection(std::make_unique(
+  pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true));
+  comm.SetCloseOnEOF(true);
+  ASSERT_TRUE(comm.StartReadThread());
+
+  // Ensure that we can safely synchronize with the read thread while it is
+  // closing the read end (in response to us closing the write end).
+  pipe.CloseWriteFileDescriptor();
+  comm.SynchronizeWithReadThread();
+
+  ASSERT_TRUE(comm.StopReadThread());
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(LLDBCoreTests
+  CommunicationTest.cpp
   MangledTest.cpp
   RichManglingContextTest.cpp
   StreamCallbackTest.cpp
Index: lldb/source/Core/Communication.cpp
===
--- lldb/source/Core/Communication.cpp
+++ lldb/source/Core/Communication.cpp
@@ -315,16 +315,12 @@
   Status error;
   ConnectionStatus status = eConnectionStatusSuccess;
   bool done = false;
+  bool disconnect = false;
   while (!done && comm->m_read_thread_enabled) {
 size_t bytes_read = comm->ReadFromConnection(
 buf, sizeof(buf), std::chrono::seconds(5), status, &error);
-if (bytes_read > 0)
+if (bytes_read > 0 || status == eConnectionStatusEndOfFile)
   comm->AppendBytesToCache(buf, bytes_read, true, status);
-else if ((bytes_read == 0) && status == eConnectionStatusEndOfFile) {
-  if (comm->GetCloseOnEOF())
-comm->Disconnect();
-  comm->AppendBytesToCache(buf, bytes_read, true, status);
-}
 
 switch (status) {
 case eConnectionStatusSuccess:
@@ -332,11 +328,12 @@
 
 case eConnectionStatusEndOfFile:
   done = true;
+  disconnect = comm->GetCloseOnEOF();
   break;
 case eConnectionStatusError: // Check GetError() for details
   if (error.GetType() == eErrorTypePOSIX && error.GetError() == EIO) {
 // EIO on a pipe is usually caused by remote shutdown
-comm->Disconnect();
+disconnect = comm->GetCloseOnEOF();
 done = true;
   }
   if (error.Fail())
@@ -365,9 +362,15 @@
   if (log)
 LLDB_LOGF(log, "%p Communication::ReadThread () thread exiting...", p);
 
-  comm->m_read_thread_did_exit = true;
-  // Let clients know that this thread is exiting
   comm->BroadcastEvent(eBroadcastBitNoMorePendingInput);
+  {
+std::lock_guard guard(comm->m_synchronize_mutex);
+comm->m_read_thread_did_exit = true;
+if (disconnect)
+  comm->Disconnect();
+  }
+
+  // Let clients know that this thread is exiting
   comm->BroadcastEvent(eBroadcastBitReadThreadDidExit);
   return {};
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D77295: [lldb/Core] Fix a race in the Communication class

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath marked 3 inline comments as done.
labath added inline comments.



Comment at: lldb/source/Core/Communication.cpp:318
   bool done = false;
+  bool disconnect = false;
   while (!done && comm->m_read_thread_enabled) {

clayborg wrote:
> Can we just init "disconnect" right here?:
> 
> ```
> const bool disconnect = comm->GetCloseOnEOF();
> ```
> 
That won't work because we there are paths out of this function which should 
not disconnect (even in the close-on-eof case). For example, the read thread 
can be disabled by setting `comm->m_read_thread_enabled = false` in which case 
we should exit even though we have not reached EOF.



Comment at: lldb/source/Core/Communication.cpp:322-327
 if (bytes_read > 0)
   comm->AppendBytesToCache(buf, bytes_read, true, status);
 else if ((bytes_read == 0) && status == eConnectionStatusEndOfFile) {
-  if (comm->GetCloseOnEOF())
-comm->Disconnect();
+  disconnect = comm->GetCloseOnEOF();
   comm->AppendBytesToCache(buf, bytes_read, true, status);
 }

clayborg wrote:
> Is there a reason we call AppendBytesToCache with zero bytes in the else? If 
> we do need to call it (it might listen to the "status"?) then this:
> 
> ```
> if (bytes_read > 0)
>   comm->AppendBytesToCache(buf, bytes_read, true, status);
> else if ((bytes_read == 0) && status == eConnectionStatusEndOfFile) {
>   disconnect = comm->GetCloseOnEOF();
>   comm->AppendBytesToCache(buf, bytes_read, true, status);
> }
> ```
> can just be:
> 
> ```
> comm->AppendBytesToCache(buf, bytes_read, true, status);
> ```
> 
> bytes_read is a size_t so it can't be less than zero. Then we can move the 
> "disconnect = comm->GetCloseOnEOF();" into the eConnectionStatusEndOfFile 
> case in the switch below.
`AppendBytesToCache` calls a callback and I believe the idea is to call it with 
a 0-byte buffer on EOF. But the refactor looks good. I've made the 
`AppendBytesToCache` call conditional on `bytes_read > 0 || status == EOF` to 
preserve the existing behavior of not calling the callback in case of errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77295



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


[Lldb-commits] [lldb] a53bf9b - [lldb-server] jThreadsInfo returns stack memory

2020-04-06 Thread Pavel Labath via lldb-commits

Author: Jaroslav Sevcik
Date: 2020-04-06T15:43:19+02:00
New Revision: a53bf9b7c8f1ca950226a55c0e99fd706a7b6ad2

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

LOG: [lldb-server] jThreadsInfo returns stack memory

This patch adds parts of the stack that should be useful for unwinding
to the jThreadsInfo reply from lldb-server. We return the top of the
stack (12 words), and we also try to walk the frame pointer linked list
and return the memory containing frame pointer and return address pairs.
The idea is to cover the cases with and without frame pointer omission.

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

Added: 
lldb/test/API/tools/lldb-server/threads-info/Makefile

lldb/test/API/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py
lldb/test/API/tools/lldb-server/threads-info/main.cpp

Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 7d6cb2a3484b..ce889c94d512 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -31,6 +31,7 @@
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
@@ -562,6 +563,119 @@ GetRegistersAsJSON(NativeThreadProtocol &thread) {
   return register_object;
 }
 
+static llvm::Optional
+GetRegisterValue(NativeRegisterContext ®_ctx, uint32_t generic_regnum) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
+  uint32_t reg_num = reg_ctx.ConvertRegisterKindToRegisterNumber(
+  eRegisterKindGeneric, generic_regnum);
+  const RegisterInfo *const reg_info_p =
+  reg_ctx.GetRegisterInfoAtIndex(reg_num);
+
+  if (reg_info_p == nullptr || reg_info_p->value_regs != nullptr) {
+LLDB_LOGF(log, "%s failed to get register info for register index %" 
PRIu32,
+  __FUNCTION__, reg_num);
+return {};
+  }
+
+  RegisterValue reg_value;
+  Status error = reg_ctx.ReadRegister(reg_info_p, reg_value);
+  if (error.Fail()) {
+LLDB_LOGF(log, "%s failed to read register '%s' index %" PRIu32 ": %s",
+  __FUNCTION__,
+  reg_info_p->name ? reg_info_p->name : "",
+  reg_num, error.AsCString());
+return {};
+  }
+  return reg_value;
+}
+
+static json::Object CreateMemoryChunk(json::Array &stack_memory_chunks,
+  addr_t address,
+  std::vector &bytes) {
+  json::Object chunk;
+  chunk.try_emplace("address", static_cast(address));
+  StreamString stream;
+  for (uint8_t b : bytes)
+stream.PutHex8(b);
+  chunk.try_emplace("bytes", stream.GetString().str());
+  return chunk;
+}
+
+static json::Array GetStackMemoryAsJSON(NativeProcessProtocol &process,
+NativeThreadProtocol &thread) {
+  uint32_t address_size = process.GetArchitecture().GetAddressByteSize();
+  const size_t kStackTopMemoryInfoWordSize = 12;
+  size_t stack_top_memory_info_byte_size =
+  kStackTopMemoryInfoWordSize * address_size;
+  const size_t kMaxStackSize = 128 * 1024;
+  const size_t kMaxFrameSize = 4 * 1024;
+  size_t fp_and_ra_size = 2 * address_size;
+  const size_t kMaxFrameCount = 128;
+
+  NativeRegisterContext ®_ctx = thread.GetRegisterContext();
+
+  json::Array stack_memory_chunks;
+
+  lldb::addr_t sp_value;
+  if (llvm::Optional optional_sp_value =
+  GetRegisterValue(reg_ctx, LLDB_REGNUM_GENERIC_SP)) {
+sp_value = optional_sp_value->GetAsUInt64();
+  } else {
+return stack_memory_chunks;
+  }
+  lldb::addr_t fp_value;
+  if (llvm::Optional optional_fp_value =
+  GetRegisterValue(reg_ctx, LLDB_REGNUM_GENERIC_FP)) {
+fp_value = optional_fp_value->GetAsUInt64();
+  } else {
+return stack_memory_chunks;
+  }
+
+  // First, make sure we copy the top stack_top_memory_info_byte_size bytes
+  // from the stack.
+  size_t byte_count = std::min(stack_top_memory_info_byte_size,
+   static_cast(fp_value - sp_value));
+  std::vector buf(byte_count, 0);
+
+  size_t bytes_read = 0;
+  Status error = process.ReadMemoryWithoutTrap(sp_value, buf.data(), 
byte_count,
+   bytes_read);
+  if (error.Success() && bytes_read > 0) {
+buf.resize(bytes_read);
+stack_memory_chunk

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa53bf9b7c8f1: [lldb-server] jThreadsInfo returns stack 
memory (authored by jarin, committed by labath).

Changed prior to commit:
  https://reviews.llvm.org/D74398?vs=251714&id=255316#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74398

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
  lldb/test/API/tools/lldb-server/threads-info/Makefile
  lldb/test/API/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py
  lldb/test/API/tools/lldb-server/threads-info/main.cpp

Index: lldb/test/API/tools/lldb-server/threads-info/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/threads-info/main.cpp
@@ -0,0 +1,27 @@
+int main() {
+#if defined(__x86_64__)
+  // We setup two fake frames with frame pointer linking. The test will then
+  // check that lldb-server's jThreadsInfo reply includes the top frame's
+  // contents and the linked list of (frame-pointer, return-address) pairs. We
+  // pretend the next frame is too large to stop the frame walk.
+  asm volatile("movabsq $0xf00d, %rax\n\t"
+   "pushq   %rax\n\t"   // fake return address
+   "leaq0x1000(%rsp), %rbp\n\t" // larger than kMaxFrameSize
+   "pushq   %rbp\n\t"
+   "movq%rsp, %rbp\n\t"
+   "pushq   $1\n\t" // fake frame contents
+   "pushq   $2\n\t"
+   "pushq   $3\n\t"
+   "\n\t"
+   "movabsq $0xfeed, %rax\n\t"
+   "push%rax\n\t" // second fake return address
+   "pushq   %rbp\n\t"
+   "movq%rsp, %rbp\n\t"
+   "pushq   $4\n\t" // fake frame contents
+   "pushq   $5\n\t"
+   "pushq   $6\n\t"
+   "\n\t"
+   "int3\n\t");
+#endif
+  return 0;
+}
Index: lldb/test/API/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py
@@ -0,0 +1,99 @@
+
+import json
+
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+def invert_byte_order(a):
+return "".join(reversed([a[i:i+2] for i in range(0, len(a),2)]))
+
+def decode_hex(a):
+return int(invert_byte_order(a), 16)
+
+def encode_hex(a):
+return invert_byte_order("%016x" % a)
+
+class TestGdbRemoteThreadsInfoMemory(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(archs=no_match(["x86_64"]))
+def threadsInfoStackCorrect(self):
+procs = self.prep_debug_monitor_and_inferior()
+
+self.add_register_info_collection_packets()
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+
+# Gather register info.
+reg_infos = self.parse_register_info_packets(context)
+self.assertIsNotNone(reg_infos)
+self.add_lldb_register_index(reg_infos)
+# Index register info entries by name.
+reg_infos = {info['name']: info for info in reg_infos}
+
+# Send vCont packet to resume the inferior.
+self.test_sequence.add_log_lines(["read packet: $vCont;c#a8",
+  {"direction": "send",
+   "regex": r"^\$T([0-9a-fA-F]{2}).*#[0-9a-fA-F]{2}$",
+   "capture": {1: "hex_exit_code"}},
+  ],
+ True)
+
+# Send g packet to retrieve the register bank
+self.test_sequence.add_log_lines(
+[
+"read packet: $jThreadsInfo#c1",
+{
+"direction": "send",
+"regex": r"^\$(.*)#[0-9a-fA-F]{2}$",
+"capture": {
+1: "threads_info"}},
+],
+True)
+
+context = self.expect_gdbremote_sequence()
+threads_info = context["threads_info"]
+threads_info = json.loads(self.decode_gdbremote_binary(threads_info))
+self.assertEqual(1, len(threads_info))
+thread = threads_info[0]
+
+# Read the stack pointer and the frame pointer from the jThreadsInfo
+# reply.
+rsp_id = reg_infos["rsp"]["lldb_register_index"]
+sp = decode_hex(thread["registers"][str(rsp_id)])
+rbp_id = reg_infos["rbp"]["lldb_register_index"]
+fp = decode_hex(thread["registers"][str(rbp_id)])
+
+# The top frame size is 3 words.
+  

[Lldb-commits] [PATCH] D77045: Add invalidate list to primary regs in arm64 register infos

2020-04-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D77045#1956879 , @omjavaid wrote:

> Adding a testcase would be tricky as these register overlap in memory and we 
> store them with overlapping offsets with their children we should not need to 
> invalidate the children when we write the parent but for some strange 
> unexplainable reason QEMU was behaving strangely and not updating the first 
> half in certain random cases. I just thought invalidation of children will 
> force a read after write for that case.


Thanks for the explanation, but I'm afraid I still don't get what is going on 
here. Can you walk me through the individual steps here? Something like:

1. user does "register write x0 xx"
2. lldb translates that to the appropriate `p` packet
3. ???
4. user does "register read w0"
5. bad value comes out because...




Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:593-594
 
+#define STRINGIZE2(x) #x
+#define STRINGIZE(x) STRINGIZE2(x)
+

omjavaid wrote:
> labath wrote:
> > What's up with the indirection?
> We want to pass nullptr or register name as is then do stringize on regname 
> only. According to C macro expansion rules if we want our macros expanded 
> first and then do the # then we ll need double indirection.
I've figured it's something like that, though it's still not clear to me why 
that is needed in this particular case.

However, now I have a different question. As the only thing you're using the 
alt_name argument for is to turn it into a string, why not just pass a string 
in the first place:
```
DEFINE_GPR64(x0, nullptr, ...)
...
DEFINE_GPR64(fp, "x29", ...)
...
```



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:597
 static lldb_private::RegisterInfo g_register_infos_arm64_le[] = {
 // DEFINE_GPR64(name, GENERIC KIND)
+DEFINE_GPR64(x0, nullptr, LLDB_REGNUM_GENERIC_ARG1, g_x0_invalidates),

This comment is out of date now.


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

https://reviews.llvm.org/D77045



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


[Lldb-commits] [lldb] 9072df8 - [lldb][nfc] remove overriden funcs with default impl

2020-04-06 Thread Konrad Kleine via lldb-commits

Author: Konrad Kleine
Date: 2020-04-06T10:05:59-04:00
New Revision: 9072df8ac1431dba51394b77ead766aada82867f

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

LOG: [lldb][nfc] remove overriden funcs with default impl

Summary:
These `SearchFilter` methods all return `true` by their default
implementation:

```lang=c++
virtual bool ModulePasses(const FileSpec &spec);
virtual bool ModulePasses(const lldb::ModuleSP &module_sp);
virtual bool AddressPasses(Address &addr);
virtual bool CompUnitPasses(FileSpec &fileSpec);
virtual bool CompUnitPasses(CompileUnit &compUnit);
```

That's why I've documented the default behavior and remove the overrides
(except for `AddressPasses`) in these `SearchFilter`-subclasses which all just
repeated the default implementation: `SearchFilterByModule`,
`SearchFilterByModuleList`.

Reviewers: jankratochvil, labath

Reviewed By: jankratochvil, labath

Subscribers: labath, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Core/SearchFilter.h
lldb/source/Core/SearchFilter.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/SearchFilter.h 
b/lldb/include/lldb/Core/SearchFilter.h
index 178489a88f3d..9f56bbe40324 100644
--- a/lldb/include/lldb/Core/SearchFilter.h
+++ b/lldb/include/lldb/Core/SearchFilter.h
@@ -98,6 +98,8 @@ class SearchFilter {
   ///The file spec to check against the filter.
   /// \return
   ///\b true if \a spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool ModulePasses(const FileSpec &spec);
 
   /// Call this method with a Module to see if that module passes the filter.
@@ -107,6 +109,8 @@ class SearchFilter {
   ///
   /// \return
   ///\b true if \a module passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool ModulePasses(const lldb::ModuleSP &module_sp);
 
   /// Call this method with a Address to see if \a address passes the filter.
@@ -116,6 +120,8 @@ class SearchFilter {
   ///
   /// \return
   ///\b true if \a address passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool AddressPasses(Address &addr);
 
   /// Call this method with a FileSpec to see if \a file spec passes the
@@ -126,6 +132,8 @@ class SearchFilter {
   ///
   /// \return
   ///\b true if \a file spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(FileSpec &fileSpec);
 
   /// Call this method with a CompileUnit to see if \a comp unit passes the
@@ -136,6 +144,8 @@ class SearchFilter {
   ///
   /// \return
   ///\b true if \a Comp Unit passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit &compUnit);
 
   /// Call this method with a Function to see if \a function passes the
@@ -321,10 +331,6 @@ class SearchFilterByModule : public SearchFilter {
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -372,10 +378,6 @@ class SearchFilterByModuleList : public SearchFilter {
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;

diff  --git a/lldb/source/Core/SearchFilter.cpp 
b/lldb/source/Core/SearchFilter.cpp
index 494e88dde267..ea51fb379181 100644
--- a/lldb/source/Core/SearchFilter.cpp
+++ b/lldb/source/Core/SearchFilter.cpp
@@ -417,12 +417,6 @@ bool SearchFilterByModule::AddressPasses(Address &address) 
{
   return true;
 }
 
-bool SearchFilterByModule::CompUnitPasses(FileSpec &fileSpec) { return true; }
-
-bool SearchFilterByModule::CompUnitPasses(CompileUnit &compUnit) {
-  return true;
-}
-
 void SearchFilterByModule::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;
@@ -543,14 +537,6 @@ bool SearchFilterByModuleList::AddressPasses(Address 
&address) {
   return true;
 }
 
-bool SearchFilterByModuleList::CompUnitPasses(FileSpec &fileSpec) {
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(CompileUnit &compUnit) {
-  return true;
-}
-
 void SearchFilterByModuleList::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;



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

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

2020-04-06 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
kwk marked an inline comment as done.
Closed by commit rG9072df8ac143: [lldb][nfc] remove overriden funcs with 
default impl (authored by kwk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -417,12 +417,6 @@
   return true;
 }
 
-bool SearchFilterByModule::CompUnitPasses(FileSpec &fileSpec) { return true; }
-
-bool SearchFilterByModule::CompUnitPasses(CompileUnit &compUnit) {
-  return true;
-}
-
 void SearchFilterByModule::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;
@@ -543,14 +537,6 @@
   return true;
 }
 
-bool SearchFilterByModuleList::CompUnitPasses(FileSpec &fileSpec) {
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(CompileUnit &compUnit) {
-  return true;
-}
-
 void SearchFilterByModuleList::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;
Index: lldb/include/lldb/Core/SearchFilter.h
===
--- lldb/include/lldb/Core/SearchFilter.h
+++ lldb/include/lldb/Core/SearchFilter.h
@@ -98,6 +98,8 @@
   ///The file spec to check against the filter.
   /// \return
   ///\b true if \a spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool ModulePasses(const FileSpec &spec);
 
   /// Call this method with a Module to see if that module passes the filter.
@@ -107,6 +109,8 @@
   ///
   /// \return
   ///\b true if \a module passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool ModulePasses(const lldb::ModuleSP &module_sp);
 
   /// Call this method with a Address to see if \a address passes the filter.
@@ -116,6 +120,8 @@
   ///
   /// \return
   ///\b true if \a address passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool AddressPasses(Address &addr);
 
   /// Call this method with a FileSpec to see if \a file spec passes the
@@ -126,6 +132,8 @@
   ///
   /// \return
   ///\b true if \a file spec passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(FileSpec &fileSpec);
 
   /// Call this method with a CompileUnit to see if \a comp unit passes the
@@ -136,6 +144,8 @@
   ///
   /// \return
   ///\b true if \a Comp Unit passes, and \b false otherwise.
+  ///
+  /// \note the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit &compUnit);
 
   /// Call this method with a Function to see if \a function passes the
@@ -321,10 +331,6 @@
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -372,10 +378,6 @@
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Follow DW_AT_decl_file when setting breakpoint

2020-04-06 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
kwk updated this revision to Diff 255339.
kwk added a comment.

- rebased onto master to get rid of NFC change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.cpp
  lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
  lldb/test/Shell/Breakpoint/Inputs/search-support-files.h
  lldb/test/Shell/Breakpoint/Inputs/search-support-files2.h
  lldb/test/Shell/Breakpoint/search-support-files.test

Index: lldb/test/Shell/Breakpoint/search-support-files.test
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/search-support-files.test
@@ -0,0 +1,51 @@
+# In these tests we will set breakpoints on a function by name. That function
+# is defined in a header file (search-support-files.h) and will therefore be
+# inlined into the file that includes it (search-support-files.cpp).
+#
+# TODO(kwk): Check that we can also do the same with C++ methods in files?
+#(See https://lldb.llvm.org/use/tutorial.html and look for --method.)
+
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: %build %p/Inputs/search-support-files.cpp -o dummy.out
+# RUN: %lldb -b -s %s dummy.out | FileCheck --color --dump-input=fail %s 
+
+#---
+# Set breakpoint by function name.
+
+breakpoint set -n inlined_42
+# CHECK: (lldb) breakpoint set -n inlined_42
+# CHECK-NEXT: Breakpoint 1: where = dummy.out`inlined_42() + 4 at search-support-files.h:1:20, address = 0x0{{.*}}
+
+breakpoint set -n main
+# CHECK: (lldb) breakpoint set -n main
+# CHECK-NEXT: Breakpoint 2: where = dummy.out`main + 22 at search-support-files.cpp:5:11, address = 0x0{{.*}}
+
+#---
+# Set breakpoint by function name and filename (the one in which the function is
+# inlined, aka the compilation unit).
+
+breakpoint set -n inlined_42 -f search-support-files.cpp
+# CHECK: (lldb) breakpoint set -n inlined_42 -f search-support-files.cpp
+# CHECK-NEXT: Breakpoint 3: no locations (pending).
+
+#---
+# Set breakpoint by function name and source filename (the file in which the
+# function is defined).
+#
+# NOTE: This test is the really interesting one as it shows that we can now
+#   search by source files that are themselves no compulation units.
+
+breakpoint set -n inlined_42 -f search-support-files.h
+# CHECK: (lldb) breakpoint set -n inlined_42 -f search-support-files.h
+# CHECK-NEXT: Breakpoint 4: where = dummy.out`inlined_42() + 4 at search-support-files.h:1:20, address = 0x0{{.*}}
+
+#---
+# Set breakpoint by function name and source filename. This time the file
+# doesn't exist to prove that we haven't widen the search space too much. When
+# we search for a function in file that doesn't exist, we should get no results.
+
+breakpoint set -n inlined_42 -f file-not-existing.h
+# CHECK: (lldb) breakpoint set -n inlined_42 -f file-not-existing.h
+# CHECK-NEXT: Breakpoint 5: no locations (pending).
+# CHECK-NEXT: WARNING: Unable to resolve breakpoint to any actual locations.
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files2.h
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files2.h
@@ -0,0 +1 @@
+int return_zero() { return 0; }
\ No newline at end of file
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.h
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.h
@@ -0,0 +1 @@
+int inlined_42() { return 42; }
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
@@ -0,0 +1,7 @@
+#include "search-support-files.h"
+#include "search-support-files2.h"
+
+int main(int argc, char *argv[]) {
+  int a = inlined_42();
+  return return_zero() + a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -715,9 +715,13 @@
   FileSpec cu_spec;
   if (sym_ctx.comp_unit)
 cu_spec = sym_ctx.comp_unit->GetPrimaryFile();
-  if (m_cu_spec_list.FindFileIndex(0, cu_spec, false) == UINT32_MAX)
-return false; // Fails the file check
-  return SearchFilterByModuleList::ModulePasses(sym_ctx.module_sp); 
+  if (m_cu_spec_list.FindFileIndex(0, cu_spec, false) != UINT32_MAX)
+return true;
+  // ^ If the primary source file associated with the symbol's compile unit is in
+ 

[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-06 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil marked 2 inline comments as done.
jankratochvil added inline comments.



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:130-163
+  bool GetValues(ConstString unique_cstr,
+ std::function callback) const {
+for (const Entry &entry : llvm::make_range(std::equal_range(
+ m_map.begin(), m_map.end(), unique_cstr, Compare(
+  if (callback(entry.value))
+return true;
+

labath wrote:
> I'm not sure these functions are really needed. They have just one caller, 
> and they'd be trivial if this class provided appropriate abstractions. Maybe 
> just add begin/end/equal_range methods, so that one can write:
> ```
> for (value: map / map.equal_range(str))
>   stuff
> ```
> ?
> 
> (ideally, I'd like to have iterators instead of callbacks for the index 
> classes too, but iterators and class hierarchies don't mix very well)
Do you mean the standard iteration:
```
for (std::pair pair : map)
  stuff;
```
or really
```
for (DIERef ref : map)
  stuff;
```
?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327



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


[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-06 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil updated this revision to Diff 255340.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327

Files:
  lldb/include/lldb/Core/UniqueCStringMap.h
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -31,8 +31,8 @@
 
   DWARFCompileUnit *GetDWOCompileUnitForHash(uint64_t hash);
 
-  size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_die_offsets) override;
+  bool GetObjCMethods(lldb_private::ConstString class_name,
+  llvm::function_ref callback) override;
 
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -95,10 +95,10 @@
   return GetBaseSymbolFile().GetForwardDeclClangTypeToDie();
 }
 
-size_t SymbolFileDWARFDwo::GetObjCMethodDIEOffsets(
-lldb_private::ConstString class_name, DIEArray &method_die_offsets) {
-  return GetBaseSymbolFile().GetObjCMethodDIEOffsets(class_name,
- method_die_offsets);
+bool SymbolFileDWARFDwo::GetObjCMethods(
+lldb_private::ConstString class_name,
+llvm::function_ref callback) {
+  return GetBaseSymbolFile().GetObjCMethods(class_name, callback);
 }
 
 UniqueDWARFASTTypeMap &SymbolFileDWARFDwo::GetUniqueDWARFASTTypeMap() {
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -237,8 +237,8 @@
   lldb_private::CompileUnit *
   GetCompUnitForDWARFCompUnit(DWARFCompileUnit &dwarf_cu);
 
-  virtual size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_die_offsets);
+  virtual bool GetObjCMethods(lldb_private::ConstString class_name,
+  llvm::function_ref callback);
 
   bool Supports_DW_AT_APPLE_objc_complete_type(DWARFUnit *cu);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1457,11 +1457,9 @@
   return static_cast(non_dwo_cu->GetUserData());
 }
 
-size_t SymbolFileDWARF::GetObjCMethodDIEOffsets(ConstString class_name,
-DIEArray &method_die_offsets) {
-  method_die_offsets.clear();
-  m_index->GetObjCMethods(class_name, method_die_offsets);
-  return method_die_offsets.size();
+bool SymbolFileDWARF::GetObjCMethods(
+ConstString class_name, llvm::function_ref callback) {
+  return m_index->GetObjCMethods(class_name, callback);
 }
 
 bool SymbolFileDWARF::GetFunction(const DWARFDIE &die, SymbolContext &sc) {
@@ -2042,24 +2040,20 @@
   context, basename))
 basename = name.GetStringRef();
 
-  DIEArray die_offsets;
-  m_index->GetGlobalVariables(ConstString(basename), die_offsets);
-  const size_t num_die_matches = die_offsets.size();
-
-  SymbolContext sc;
-  sc.module_sp = m_objfile_sp->GetModule();
-  assert(sc.module_sp);
-
   // Loop invariant: Variables up to this index have been checked for context
   // matches.
   uint32_t pruned_idx = original_size;
 
-  for (size_t i = 0; i < num_die_matches; ++i) {
-const DIERef &die_ref = die_offsets[i];
+  SymbolCo

[Lldb-commits] [lldb] e9264b7 - [lldb] NFC: Fix trivial typo in comments, documents, and messages

2020-04-06 Thread Kazuaki Ishizaki via lldb-commits

Author: Kazuaki Ishizaki
Date: 2020-04-07T01:06:16+09:00
New Revision: e9264b746b81a63323d884ea07b2ebfbb660d004

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

LOG: [lldb] NFC: Fix trivial typo in comments, documents, and messages

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

Added: 


Modified: 
lldb/bindings/interface/SBBlock.i
lldb/bindings/interface/SBExpressionOptions.i
lldb/bindings/interface/SBFile.i
lldb/docs/lldb-platform-packets.txt
lldb/examples/interposing/darwin/fd_interposing/FDInterposing.cpp
lldb/examples/python/mach_o.py
lldb/include/lldb/Symbol/LineEntry.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/Target.h
lldb/include/lldb/Target/ThreadPlan.h
lldb/include/lldb/Utility/Connection.h
lldb/include/lldb/Utility/Reproducer.h
lldb/include/lldb/Utility/Status.h
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/packages/Python/lldbsuite/test/test_runner/README.txt
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/scripts/verify_api.py
lldb/source/Commands/CommandObjectWatchpoint.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Host/common/Editline.cpp
lldb/source/Host/common/NativeProcessProtocol.cpp
lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/source/Plugins/Process/Darwin/CFUtils.h
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.h
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Symbol/ArmUnwindInfo.cpp
lldb/source/Target/RegisterContextUnwind.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/StopInfo.cpp
lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py

lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py

lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
lldb/test/API/functionalities/load_unload/TestLoadUnload.py
lldb/test/API/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py
lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py

lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
lldb/test/API/lang/cpp/namespace/ns3.cpp
lldb/test/API/macosx/lc-note/kern-ver-str/TestKernVerStrLCNOTE.py
lldb/test/API/macosx/lc-note/kern-ver-str/create-empty-corefile.cpp

lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
lldb/test/API/python_api/frame/TestFrames.py
lldb/test/API/python_api/function_symbol/main.c
lldb/test/API/python_api/target/main.c
lldb/test/API/python_api/thread/TestThreadAPI.py
lldb/test/API/tools/lldb-server/main.cpp
lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py

lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointEvents.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py

lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setExceptionBreakpoin

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

2020-04-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
compnerd added a comment.

In D77287#1963242 , @labath wrote:

> In D77287#1960390 , @compnerd wrote:
>
> > I think that the basic test is sufficient for this.
>
>
> That test does not seem to be exercising the "unload" part of this patch. It 
> would also be nice to run some basic command like "image list" to verify that 
> the loaded binary is indeed listed.
>
> (I don't really have a hard objection to this being a lit test, but it does 
> sound to me like at that point, this will be reimplementing TestLoadUnload.py)
>
> > I think that the path sanitizing and conversion should be a subsequent 
> > change.
>
> Why is that? The need for sanitation is a direct consequence of how you've 
> chosen to implement this patch -- the posix version of this function does not 
> do sanitation, but it does not need to, as it does not embed the library name 
> into the compiled expression. I can see how the support for wide strings 
> might be considered a separate feature, but given that supporting that is a 
> matter of adding a single `L` to the compiled expression, I don't see a 
> problem with including that here.


Actually, it changes the APIs used and the path that this goes down on the 
Windows side, so it has a much broader impact than it appears.


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

https://reviews.llvm.org/D77287



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


[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

2020-04-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
compnerd added a comment.

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

https://reviews.llvm.org/D77287



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


[Lldb-commits] [PATCH] D77460: [lldb] NFC: Fix trivial typo in comments, documents, and messages

2020-04-06 Thread Kazuaki Ishizaki via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe9264b746b81: [lldb] NFC: Fix trivial typo in comments, 
documents, and messages (authored by kiszk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77460

Files:
  lldb/bindings/interface/SBBlock.i
  lldb/bindings/interface/SBExpressionOptions.i
  lldb/bindings/interface/SBFile.i
  lldb/docs/lldb-platform-packets.txt
  lldb/examples/interposing/darwin/fd_interposing/FDInterposing.cpp
  lldb/examples/python/mach_o.py
  lldb/include/lldb/Symbol/LineEntry.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Utility/Connection.h
  lldb/include/lldb/Utility/Reproducer.h
  lldb/include/lldb/Utility/Status.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/packages/Python/lldbsuite/test/test_runner/README.txt
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/scripts/verify_api.py
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Host/common/Editline.cpp
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Process/Darwin/CFUtils.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Symbol/ArmUnwindInfo.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py
  
lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
  
lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
  lldb/test/API/functionalities/load_unload/TestLoadUnload.py
  lldb/test/API/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py
  lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
  
lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
  lldb/test/API/lang/cpp/namespace/ns3.cpp
  lldb/test/API/macosx/lc-note/kern-ver-str/TestKernVerStrLCNOTE.py
  lldb/test/API/macosx/lc-note/kern-ver-str/create-empty-corefile.cpp
  
lldb/test/API/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
  lldb/test/API/python_api/frame/TestFrames.py
  lldb/test/API/python_api/function_symbol/main.c
  lldb/test/API/python_api/target/main.c
  lldb/test/API/python_api/thread/TestThreadAPI.py
  lldb/test/API/tools/lldb-server/main.cpp
  lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py
  
lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointEvents.py
  lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
  
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setExceptionBreakpoints.py
  
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setFunctionBreakpoints.py
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/test/API/tools/lldb-vscode/stackTrace/TestVSCode_stackTrace.py
  lldb/test/API/tools/lldb-vscode/step/TestVSC

[Lldb-commits] [PATCH] D77568: Return correct entry in RangeDataVector::FindEntryThatContains

2020-04-06 Thread Shivam Mittal via Phabricator via lldb-commits
shivammittal99 created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The FindEntryThatContains function does not return the correct entry in
the case when two non-consecutive entries contain the range.
Eg. Vector = [(0, 40, 0), (10, 10, 1)] and query = (25, 1). The function
currently returns nullptr, but it should return entry 0.

Signed-off-by: Shivam Mittal 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77568

Files:
  lldb/include/lldb/Utility/RangeMap.h
  lldb/unittests/Utility/RangeMapTest.cpp


Index: lldb/unittests/Utility/RangeMapTest.cpp
===
--- lldb/unittests/Utility/RangeMapTest.cpp
+++ lldb/unittests/Utility/RangeMapTest.cpp
@@ -53,10 +53,7 @@
   // With overlapping intervals, the intention seems to be to return the first
   // interval which contains the address.
   EXPECT_THAT(Map.FindEntryThatContains(25), EntryIs(0));
-
-  // However, this does not always succeed.
-  // TODO: This should probably return the range (0, 40) as well.
-  EXPECT_THAT(Map.FindEntryThatContains(35), nullptr);
+  EXPECT_THAT(Map.FindEntryThatContains(35), EntryIs(0));
 }
 
 TEST(RangeDataVector, CustomSort) {
Index: lldb/include/lldb/Utility/RangeMap.h
===
--- lldb/include/lldb/Utility/RangeMap.h
+++ lldb/include/lldb/Utility/RangeMap.h
@@ -540,14 +540,13 @@
 if (!m_entries.empty()) {
   typename Collection::const_iterator begin = m_entries.begin();
   typename Collection::const_iterator end = m_entries.end();
-  typename Collection::const_iterator pos =
-  std::lower_bound(begin, end, range, BaseLessThan);
-
-  while (pos != begin && pos[-1].Contains(range))
---pos;
+  typename Collection::const_iterator limit =
+  std::upper_bound(begin, end, range, BaseLessThan);
 
-  if (pos != end && pos->Contains(range))
-return &(*pos);
+  for (auto pos = begin; pos < limit; ++pos) {
+if (pos->Contains(range))
+  return &(*pos);
+  }
 }
 return nullptr;
   }


Index: lldb/unittests/Utility/RangeMapTest.cpp
===
--- lldb/unittests/Utility/RangeMapTest.cpp
+++ lldb/unittests/Utility/RangeMapTest.cpp
@@ -53,10 +53,7 @@
   // With overlapping intervals, the intention seems to be to return the first
   // interval which contains the address.
   EXPECT_THAT(Map.FindEntryThatContains(25), EntryIs(0));
-
-  // However, this does not always succeed.
-  // TODO: This should probably return the range (0, 40) as well.
-  EXPECT_THAT(Map.FindEntryThatContains(35), nullptr);
+  EXPECT_THAT(Map.FindEntryThatContains(35), EntryIs(0));
 }
 
 TEST(RangeDataVector, CustomSort) {
Index: lldb/include/lldb/Utility/RangeMap.h
===
--- lldb/include/lldb/Utility/RangeMap.h
+++ lldb/include/lldb/Utility/RangeMap.h
@@ -540,14 +540,13 @@
 if (!m_entries.empty()) {
   typename Collection::const_iterator begin = m_entries.begin();
   typename Collection::const_iterator end = m_entries.end();
-  typename Collection::const_iterator pos =
-  std::lower_bound(begin, end, range, BaseLessThan);
-
-  while (pos != begin && pos[-1].Contains(range))
---pos;
+  typename Collection::const_iterator limit =
+  std::upper_bound(begin, end, range, BaseLessThan);
 
-  if (pos != end && pos->Contains(range))
-return &(*pos);
+  for (auto pos = begin; pos < limit; ++pos) {
+if (pos->Contains(range))
+  return &(*pos);
+  }
 }
 return nullptr;
   }
___
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

2020-04-06 Thread Lawrence D'Anna via Phabricator via lldb-commits
lawrence_danna added a comment.

In D77480#1963315 , @labath wrote:

> Thanks for getting back to this. Is there any reasonable way of testing this 
> (e.g. an existing python class with temporary buffer storage), or would we 
> have to implement our own PyBuffer object?


I don't how of any existing class that does that, I think we'd need to 
implement our own.


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


Re: [Lldb-commits] [lldb] b6cd964 - Fix typo in xfail decorator for lldb thread plan list tests

2020-04-06 Thread Jim Ingham via lldb-commits
Yes.  Now that I think about it a little more, I’m a bit surprised that on most 
platforms we do have to step in & back out again to step over this function.  
That’s what we always used to do, but the work Greg & Co. did a little while 
back added the assumption that call’s return to the PC directly following the 
call, so we can step directly over.  Is that optimization only effective on 
aarch64?

Anyway, doesn’t matter, this was supposed to be testing the listing of internal 
plans, not our strategies for stepping.  Though it sound like if we want to 
ensure we are stepping directly over we might write a similar test to make sure 
we actually are doing that…

Jim


> On Apr 6, 2020, at 3:42 AM, Pavel Labath  wrote:
> 
> I guess these should go away after 4f644ff9e (which restrict the
> decorator to aarch64). Judging by
> ,
> Jim is going to look into a better solution for that this week.
> 
> pl
> 
> 
> On 06/04/2020 11:50, Jan Kratochvil wrote:
>> Hi,
>> 
>> I get XPASSes now on Fedora 31 x86_64:
>> 
>> http://lab.llvm.org:8014/builders/lldb-x86_64-fedora/builds/7169
>> http://lab.llvm.org:8014/builders/lldb-x86_64-fedora?numbuilds=1000
>> 
>> So maybe to remove the expected failure?
>> 
>> 
>> Jan
>> 
>> 
>> 
>> On Sun, 05 Apr 2020 17:18:54 +0200, Muhammad Omair Javaid via lldb-commits 
>> wrote:
>>> 
>>> Author: Muhammad Omair Javaid
>>> Date: 2020-04-05T20:16:46+05:00
>>> New Revision: b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc
>>> 
>>> URL: 
>>> https://github.com/llvm/llvm-project/commit/b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc
>>> DIFF: 
>>> https://github.com/llvm/llvm-project/commit/b6cd964ac7cb9b55dfcdbe43c5502c2c0f6cbebc.diff
>>> 
>>> LOG: Fix typo in xfail decorator for lldb thread plan list tests
>>> 
>>> Added: 
>>> 
>>> 
>>> Modified: 
>>>lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
>>> 
>>> Removed: 
>>> 
>>> 
>>> 
>>> 
>>> diff  --git 
>>> a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py 
>>> b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
>>> index b4ae9107aceb..5214a3f6b0fe 100644
>>> --- a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
>>> +++ b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
>>> @@ -17,7 +17,7 @@ class TestThreadPlanCommands(TestBase):
>>> NO_DEBUG_INFO_TESTCASE = True
>>> 
>>> @skipIfWindows
>>> -@expectedFailureAll(oslist=["Linux"])
>>> +@expectedFailureAll(oslist=["linux"])
>>> def test_thread_plan_actions(self):
>>> self.build()
>>> self.main_source_file = lldb.SBFileSpec("main.c")
>>> 
>>> 
>>> 
>>> ___
>>> lldb-commits mailing list
>>> lldb-commits@lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>> 
> 

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


[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In D77444#1963354 , @labath wrote:

> In D77444#1962952 , @clayborg wrote:
>
> > The biggest issue is maintaining the API. We can't add anything to 
> > SBCommandPluginInterface
>
>
> If we're going to be adding a new inheritable class for this feature, I'd 
> recommend adding about half a dozen or so dummy virtual methods to it so we 
> reserve vtable space for future expansion.


I second that!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444



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


[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 255426.
wallace added a comment.
Herald added a subscriber: mgorny.

- Moved the test to gtest. It's much better this way and I learned gtest
- Changed the API. Some notes:

Within the scope of a subcommand, the subcommand doesn't know the parent's
command name. It only knows its own name and it doesn't have any referrence to
its parent. That makes it very difficult to implement an enum option for
eCommandNameRepeat, as @jingham suggested. The GetRepeatCommand signature also
doesn't provide useful info about the parsed arguments.

I took a look at the existing implementations for GetRepeatCommand, and it seems
that most of them just return nullptr, "", or the same command without flags.

I don't want to change any existing core command interpreter function, so I
think that a simple API that covers most of the cases is just providing nullptr
for repeating the same command, "" for not repeating, or a custom string for
any other case. If in the future anyone needs something very customized, a new
override could be created, but I don't think this will happen anytime soon.

Another option is to provide a callback function instead of a string, but I
don't know if it's too much.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444

Files:
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/Interpreter/CMakeLists.txt
  lldb/unittests/Interpreter/TestAutoRepeat.cpp

Index: lldb/unittests/Interpreter/TestAutoRepeat.cpp
===
--- /dev/null
+++ lldb/unittests/Interpreter/TestAutoRepeat.cpp
@@ -0,0 +1,141 @@
+//===-- TestAutoRepeat.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===/
+
+#include "gtest/gtest.h"
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+#include 
+
+using namespace lldb;
+
+class TestAutoRepeat : public testing::Test {
+protected:
+  void SetUp() override {
+SBDebugger::Initialize();
+m_dbg = SBDebugger::Create(false /*source_init_files*/);
+m_interp = m_dbg.GetCommandInterpreter();
+  }
+
+  SBDebugger m_dbg;
+  SBCommandInterpreter m_interp;
+};
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  DummyCommand(const char *message) : m_message(message) {}
+
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject &result) {
+result.PutCString(m_message.c_str());
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+
+private:
+  std::string m_message;
+};
+
+TEST_F(TestAutoRepeat, SingleWordCommand) {
+  // We first test a command without autorepeat
+  DummyCommand dummy("It worked");
+  m_interp.AddCommand("dummy", &dummy, nullptr /*help*/);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("dummy", result, true /*add_to_history*/);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+assert(!result.Succeeded() &&
+   "The command should fail as a repeated command");
+EXPECT_STREQ(result.GetError(), "error: No auto repeat.\n");
+  }
+
+  // Now we test a command with autorepeat
+  m_interp.AddCommand("dummy_with_autorepeat", &dummy, nullptr /*help*/,
+  nullptr /*syntax*/, nullptr /*auto_repeat_command*/);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("dummy_with_autorepeat", result,
+   true /*add_to_history*/);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+}
+
+TEST_F(TestAutoRepeat, MultiWordCommand) {
+  auto command = m_interp.AddMultiwordCommand("multicommand", nullptr /*help*/);
+  // We first test a subcommand without autorepeat
+  DummyCommand subcommand("It worked again");
+  command.AddCommand("subcommand", &subcommand, nullptr /*help*/);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("multicommand subcommand", result,
+   true /*add_to_history*/);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked again\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+assert(!result.Succeeded() 

[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 255429.
wallace added a comment.

nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444

Files:
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/Interpreter/CMakeLists.txt
  lldb/unittests/Interpreter/TestAutoRepeat.cpp

Index: lldb/unittests/Interpreter/TestAutoRepeat.cpp
===
--- /dev/null
+++ lldb/unittests/Interpreter/TestAutoRepeat.cpp
@@ -0,0 +1,141 @@
+//===-- TestAutoRepeat.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===/
+
+#include "gtest/gtest.h"
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+#include 
+
+using namespace lldb;
+
+class TestAutoRepeat : public testing::Test {
+protected:
+  void SetUp() override {
+SBDebugger::Initialize();
+m_dbg = SBDebugger::Create(false /*source_init_files*/);
+m_interp = m_dbg.GetCommandInterpreter();
+  }
+
+  SBDebugger m_dbg;
+  SBCommandInterpreter m_interp;
+};
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  DummyCommand(const char *message) : m_message(message) {}
+
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject &result) {
+result.PutCString(m_message.c_str());
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+
+private:
+  std::string m_message;
+};
+
+TEST_F(TestAutoRepeat, SingleWordCommand) {
+  // We first test a command without autorepeat
+  DummyCommand dummy("It worked");
+  m_interp.AddCommand("dummy", &dummy, nullptr /*help*/);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("dummy", result, true /*add_to_history*/);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+assert(!result.Succeeded() &&
+   "The command should fail as a repeated command");
+EXPECT_STREQ(result.GetError(), "error: No auto repeat.\n");
+  }
+
+  // Now we test a command with autorepeat
+  m_interp.AddCommand("dummy_with_autorepeat", &dummy, nullptr /*help*/,
+  nullptr /*syntax*/, nullptr /*auto_repeat_command*/);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("dummy_with_autorepeat", result,
+   true /*add_to_history*/);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked\n");
+  }
+}
+
+TEST_F(TestAutoRepeat, MultiWordCommand) {
+  auto command = m_interp.AddMultiwordCommand("multicommand", nullptr /*help*/);
+  // We first test a subcommand without autorepeat
+  DummyCommand subcommand("It worked again");
+  command.AddCommand("subcommand", &subcommand, nullptr /*help*/);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("multicommand subcommand", result,
+   true /*add_to_history*/);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked again\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+assert(!result.Succeeded() &&
+   "The command should fail as a repeated command");
+EXPECT_STREQ(result.GetError(), "error: No auto repeat.\n");
+  }
+
+  // We first test a subcommand with autorepeat
+  command.AddCommand("subcommand_with_autorepeat", &subcommand,
+ nullptr /*help*/, nullptr /*syntax*/,
+ nullptr /*auto_repeat_command*/);
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("multicommand subcommand_with_autorepeat", result,
+   true /*add_to_history*/);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked again\n");
+  }
+  {
+SBCommandReturnObject result;
+m_interp.HandleCommand("", result);
+EXPECT_TRUE(result.Succeeded());
+EXPECT_STREQ(result.GetOutput(), "It worked again\n");
+  }
+
+  DummyCommand subcommand2("It worked again 2");
+  // We now test a subcommand with autorepeat of the command name
+  command.AddCommand(
+  "subcommand_with_custom_autorepeat", &subcommand2, nullptr /*help*/,
+  nullptr /*syntax*/,
+  "multicommand subcommand_with_autorepeat" /*auto_repeat_command*/);
+  {

Re: [Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread Jim Ingham via lldb-commits
Then only time that I can find where I customized the string based on the 
incoming full command was for “source list” because I needed to know whether 
the listing was going forward or backward, so I needed to write that into the 
“keep doing what you were doing” command.  That could have also been 
implemented by remembering the direction as well as the “current source 
location” internally, and then have “source list” mean “keep going the 
direction you were going.”  So I don’t think dynamically constructing the next 
command string is necessary to cover the needed cases.

BTW, I thought when a command returned the command string for the next command, 
the command interpreter prepended it with the chain of parent commands 
containing the command that was presenting the “next command string”.  If it 
doesn’t do that, it probably should.  A command shouldn’t really know where it 
is sitting in the command hierarchy, so somebody has to do that for it.  

I don’t know of a case where a command would want its repeat command to be a 
wholly different command.  That seems odd, but if we do find we need that 
internally, we could add some way of saying “this command is not a variant of 
me…”  But in any case, for these purposes, it seems like the three useful cases 
are really “no repeat”, “repeat the command I was given” and “invoke my 
continue-from-where-I-left-off” command - which by convention is the command 
with no arguments.  If we make sure that when the repeat command is actually 
used the command interpreter adds the command’s parents, then I think we could 
do this with an enum.

Jim


> On Apr 6, 2020, at 11:57 AM, walter erquinigo via Phabricator 
>  wrote:
> 
> wallace updated this revision to Diff 255426.
> wallace added a comment.
> Herald added a subscriber: mgorny.
> 
> - Moved the test to gtest. It's much better this way and I learned gtest
> - Changed the API. Some notes:
> 
> Within the scope of a subcommand, the subcommand doesn't know the parent's
> command name. It only knows its own name and it doesn't have any referrence to
> its parent. That makes it very difficult to implement an enum option for
> eCommandNameRepeat, as @jingham suggested. The GetRepeatCommand signature also
> doesn't provide useful info about the parsed arguments.
> 
> I took a look at the existing implementations for GetRepeatCommand, and it 
> seems
> that most of them just return nullptr, "", or the same command without flags.
> 
> I don't want to change any existing core command interpreter function, so I
> think that a simple API that covers most of the cases is just providing 
> nullptr
> for repeating the same command, "" for not repeating, or a custom string for
> any other case. If in the future anyone needs something very customized, a new
> override could be created, but I don't think this will happen anytime soon.
> 
> Another option is to provide a callback function instead of a string, but I
> don't know if it's too much.
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D77444/new/
> 
> https://reviews.llvm.org/D77444
> 
> Files:
>  lldb/include/lldb/API/SBCommandInterpreter.h
>  lldb/source/API/SBCommandInterpreter.cpp
>  lldb/unittests/Interpreter/CMakeLists.txt
>  lldb/unittests/Interpreter/TestAutoRepeat.cpp
> 
> 

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


Re: [Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

2020-04-06 Thread Walter via lldb-commits
> BTW, I thought when a command returned the command string for the next
command, the command interpreter prepended it with the chain of parent
commands containing the command that was presenting the “next command
string”.

That is correct. The GetRepeatCommand function gets the full command with
the type Args. I could look for the current subcommand name within the Args
and use the tokens up to that position as the repeat command, however, it
would fail if there's token with the same value, like "memory read read".
It might be a very degenerate case and I'm okay with implementing this and
support enums if you agree with it. If you have a better idea, it'd be
great.

On Mon, Apr 6, 2020 at 12:23 PM Jim Ingham  wrote:

> Then only time that I can find where I customized the string based on the
> incoming full command was for “source list” because I needed to know
> whether the listing was going forward or backward, so I needed to write
> that into the “keep doing what you were doing” command.  That could have
> also been implemented by remembering the direction as well as the “current
> source location” internally, and then have “source list” mean “keep going
> the direction you were going.”  So I don’t think dynamically constructing
> the next command string is necessary to cover the needed cases.
>
> BTW, I thought when a command returned the command string for the next
> command, the command interpreter prepended it with the chain of parent
> commands containing the command that was presenting the “next command
> string”.  If it doesn’t do that, it probably should.  A command shouldn’t
> really know where it is sitting in the command hierarchy, so somebody has
> to do that for it.
>
> I don’t know of a case where a command would want its repeat command to be
> a wholly different command.  That seems odd, but if we do find we need that
> internally, we could add some way of saying “this command is not a variant
> of me…”  But in any case, for these purposes, it seems like the three
> useful cases are really “no repeat”, “repeat the command I was given” and
> “invoke my continue-from-where-I-left-off” command - which by convention is
> the command with no arguments.  If we make sure that when the repeat
> command is actually used the command interpreter adds the command’s
> parents, then I think we could do this with an enum.
>
> Jim
>
>
> > On Apr 6, 2020, at 11:57 AM, walter erquinigo via Phabricator <
> revi...@reviews.llvm.org> wrote:
> >
> > wallace updated this revision to Diff 255426.
> > wallace added a comment.
> > Herald added a subscriber: mgorny.
> >
> > - Moved the test to gtest. It's much better this way and I learned gtest
> > - Changed the API. Some notes:
> >
> > Within the scope of a subcommand, the subcommand doesn't know the
> parent's
> > command name. It only knows its own name and it doesn't have any
> referrence to
> > its parent. That makes it very difficult to implement an enum option for
> > eCommandNameRepeat, as @jingham suggested. The GetRepeatCommand
> signature also
> > doesn't provide useful info about the parsed arguments.
> >
> > I took a look at the existing implementations for GetRepeatCommand, and
> it seems
> > that most of them just return nullptr, "", or the same command without
> flags.
> >
> > I don't want to change any existing core command interpreter function,
> so I
> > think that a simple API that covers most of the cases is just providing
> nullptr
> > for repeating the same command, "" for not repeating, or a custom string
> for
> > any other case. If in the future anyone needs something very customized,
> a new
> > override could be created, but I don't think this will happen anytime
> soon.
> >
> > Another option is to provide a callback function instead of a string,
> but I
> > don't know if it's too much.
> >
> >
> > Repository:
> >  rG LLVM Github Monorepo
> >
> > CHANGES SINCE LAST ACTION
> >  https://reviews.llvm.org/D77444/new/
> >
> > https://reviews.llvm.org/D77444
> >
> > Files:
> >  lldb/include/lldb/API/SBCommandInterpreter.h
> >  lldb/source/API/SBCommandInterpreter.cpp
> >  lldb/unittests/Interpreter/CMakeLists.txt
> >  lldb/unittests/Interpreter/TestAutoRepeat.cpp
> >
> > 
>
>

-- 
Walter Erquínigo Pezo

Every problem has a simple, fast and wrong solution.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D77452: [intel-pt] Improve the way the test determines whether to run

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 255445.
wallace edited the summary of this revision.
wallace added a comment.

See the new description of the diff for the updates


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77452

Files:
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/utils/lldb-dotest/lldb-dotest.in


Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -1,6 +1,7 @@
 #!@PYTHON_EXECUTABLE@
 import subprocess
 import sys
+import os
 
 dotest_path = '@LLDB_SOURCE_DIR_CONFIGURED@/test/API/dotest.py'
 build_dir = '@LLDB_TEST_BUILD_DIRECTORY_CONFIGURED@'
@@ -11,6 +12,7 @@
 dsymutil = '@LLDB_TEST_DSYMUTIL_CONFIGURED@'
 filecheck = '@LLDB_TEST_FILECHECK_CONFIGURED@'
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
+lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@".lower()
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -26,6 +28,11 @@
 cmd.extend(['--filecheck', filecheck])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
 cmd.extend(wrapper_args)
+
+if lldb_build_intel_pt not in ["", "false", "off", "n", "no", "ignore"] \
+and not lldb_build_intel_pt.endswith("-NOTFOUND"):
+os.environ['TEST_INTEL_PT'] = "true"
+
 # Invoke dotest.py and return exit code.
 print(' '.join(cmd))
 sys.exit(subprocess.call(cmd))
Index: 
lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -14,22 +14,21 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+TestBase.setUp(self)
+if os.environ["TEST_INTEL_PT"] != "true":
+self.skipTest("Intel PT not supported")
+
+plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], 
"liblldbIntelFeatures.so")
+self.runCmd("plugin load " + plugin_path)
+
 @skipIf(oslist=no_match(['linux']))
 @skipIf(archs=no_match(['i386', 'x86_64']))
 @skipIfRemote
 def test_basic_flow(self):
 """Test collection, decoding, and dumping instructions"""
-if os.environ.get('TEST_INTEL_PT') != '1':
-self.skipTest("The environment variable TEST_INTEL_PT=1 is needed 
to run this test.")
-
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
 
 self.build()
-
-self.runCmd("plugin load " + plugin_file)
-
 exe = self.getBuildArtifact("a.out")
 lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
 # We start tracing from main
@@ -52,9 +51,9 @@
 self.expect("processor-trace show-instr-log -c 100",
 patterns=[
 # We expect to have seen the first instruction of 'fun'
-hex(fun_start_adddress),  
+hex(fun_start_adddress),
 # We expect to see the exit condition of the for loop
-"at main.cpp:" + str(line_number('main.cpp', '// Break for 
loop')) 
+"at main.cpp:" + str(line_number('main.cpp', '// Break for 
loop'))
 ])
 
 self.runCmd("processor-trace stop")


Index: lldb/utils/lldb-dotest/lldb-dotest.in
===
--- lldb/utils/lldb-dotest/lldb-dotest.in
+++ lldb/utils/lldb-dotest/lldb-dotest.in
@@ -1,6 +1,7 @@
 #!@PYTHON_EXECUTABLE@
 import subprocess
 import sys
+import os
 
 dotest_path = '@LLDB_SOURCE_DIR_CONFIGURED@/test/API/dotest.py'
 build_dir = '@LLDB_TEST_BUILD_DIRECTORY_CONFIGURED@'
@@ -11,6 +12,7 @@
 dsymutil = '@LLDB_TEST_DSYMUTIL_CONFIGURED@'
 filecheck = '@LLDB_TEST_FILECHECK_CONFIGURED@'
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
+lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@".lower()
 
 if __name__ == '__main__':
 wrapper_args = sys.argv[1:]
@@ -26,6 +28,11 @@
 cmd.extend(['--filecheck', filecheck])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
 cmd.extend(wrapper_args)
+
+if lldb_build_intel_pt not in ["", "false", "off", "n", "no", "ignore"] \
+and not lldb_build_intel_pt.endswith("-NOTFOUND"):
+os.environ['TEST_INTEL_PT'] = "true"
+
 # Invoke dotest.py and return exit code.
 print(' '.join(cmd))
 sys.exit(subprocess.call(cmd))
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/te

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

2020-04-06 Thread Lawrence D'Anna via Phabricator via lldb-commits
lawrence_danna updated this revision to Diff 255447.
lawrence_danna added a comment.

review fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77480

Files:
  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
@@ -488,39 +488,52 @@
 }
 }
 
+%inline %{
+
+struct Py_buffer_RAII {
+  Py_buffer buffer;
+  Py_buffer_RAII() { buffer = {}; };
+  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
+//
+// I've also moved the call to PyBuffer_Release to the end of the SWIG wrapper,
+// doing it right away is not legal according to the python buffer protocol.
 
 %define %pybuffer_mutable_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_WRITABLE);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_WRITABLE);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size/sizeof($*1_type));
 }
 %enddef
 
 %define %pybuffer_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; const void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_CONTIG_RO);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_CONTIG_RO);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size / sizeof($*1_type));
 }


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -488,39 +488,52 @@
 }
 }
 
+%inline %{
+
+struct Py_buffer_RAII {
+  Py_buffer buffer;
+  Py_buffer_RAII() { buffer = {}; };
+  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
+//
+// I've also moved the call to PyBuffer_Release to the end of the SWIG wrapper,
+// doing it right away is not legal according to the python buffer protocol.
 
 %define %pybuffer_mutable_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_WRITABLE);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_WRITABLE);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size/sizeof($*1_type));
 }
 %enddef
 
 %define %pybuffer_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; const void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_CONTIG_RO);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_CONTIG_RO);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size / sizeof($*1_type));
 }
___
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

2020-04-06 Thread Lawrence D'Anna via Phabricator via lldb-commits
lawrence_danna marked 2 inline comments as done.
lawrence_danna added a comment.

fixed


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] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision.
wallace added reviewers: labath, clayborg.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.
wallace updated this revision to Diff 255454.
wallace added a comment.
wallace updated this revision to Diff 255455.

.


wallace added a comment.

Improve some names


I don't have an immediate use for this, but I had already implemented
it, so I better make a diff for this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77582

Files:
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/Interpreter/CMakeLists.txt
  lldb/unittests/Interpreter/TestUserCommand.cpp

Index: lldb/unittests/Interpreter/TestUserCommand.cpp
===
--- /dev/null
+++ lldb/unittests/Interpreter/TestUserCommand.cpp
@@ -0,0 +1,45 @@
+//===-- TestUserCommand.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===/
+
+#include "gtest/gtest.h"
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+#include 
+
+using namespace lldb;
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject &result) {
+result.PutCString("It works");
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+};
+
+TEST(TestUserCommand, TestSimpleCommand) {
+  SBDebugger::Initialize();
+  SBDebugger dbg = SBDebugger::Create(false /*source_init_files*/);
+  SBCommandInterpreter interp = dbg.GetCommandInterpreter();
+
+  DummyCommand command;
+  interp.AddCommand("dummy", &command, nullptr /*help*/);
+
+  EXPECT_TRUE(interp.UserCommandExists("dummy"));
+
+  SBCommandReturnObject result;
+  interp.HandleCommand("dummy", result, true /*add_to_history*/);
+  EXPECT_TRUE(result.Succeeded());
+  EXPECT_STREQ(result.GetOutput(), "It works\n");
+}
Index: lldb/unittests/Interpreter/CMakeLists.txt
===
--- lldb/unittests/Interpreter/CMakeLists.txt
+++ lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,8 +1,10 @@
 add_lldb_unittest(InterpreterTests
   TestCompletion.cpp
   TestOptionArgParser.cpp
+  TestUserCommand.cpp
 
   LINK_LIBS
+liblldb
 lldbInterpreter
 lldbUtilityHelpers
   )
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -216,6 +216,14 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_RECORD_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+ (const char *), cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_RECORD_METHOD(bool, SBCommandInterpreter, AliasExists, (const char *),
  cmd);
@@ -874,6 +882,8 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreter, operator bool, ());
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, CommandExists,
(const char *));
+  LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+   (const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, AliasExists,
(const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, IsActive, ());
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -91,8 +91,28 @@
 
   bool IsValid() const;
 
+  /// Determine whether a command exists in this CommandInterpreter.
+  ///
+  /// For commands registered using the LLDB API, see UserCommandExists
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Determine whether a user command exists in this CommandInterpreter.
+  /// Users commands are the ones registered using the LLDB API and are not
+  /// provided by default in LLDB.
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false oth

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 255455.
wallace added a comment.

Improve some names


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582

Files:
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/Interpreter/CMakeLists.txt
  lldb/unittests/Interpreter/TestUserCommand.cpp

Index: lldb/unittests/Interpreter/TestUserCommand.cpp
===
--- /dev/null
+++ lldb/unittests/Interpreter/TestUserCommand.cpp
@@ -0,0 +1,45 @@
+//===-- TestUserCommand.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===/
+
+#include "gtest/gtest.h"
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+#include 
+
+using namespace lldb;
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject &result) {
+result.PutCString("It works");
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+};
+
+TEST(TestUserCommand, TestSimpleCommand) {
+  SBDebugger::Initialize();
+  SBDebugger dbg = SBDebugger::Create(false /*source_init_files*/);
+  SBCommandInterpreter interp = dbg.GetCommandInterpreter();
+
+  DummyCommand command;
+  interp.AddCommand("dummy", &command, nullptr /*help*/);
+
+  EXPECT_TRUE(interp.UserCommandExists("dummy"));
+
+  SBCommandReturnObject result;
+  interp.HandleCommand("dummy", result, true /*add_to_history*/);
+  EXPECT_TRUE(result.Succeeded());
+  EXPECT_STREQ(result.GetOutput(), "It works\n");
+}
Index: lldb/unittests/Interpreter/CMakeLists.txt
===
--- lldb/unittests/Interpreter/CMakeLists.txt
+++ lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,8 +1,10 @@
 add_lldb_unittest(InterpreterTests
   TestCompletion.cpp
   TestOptionArgParser.cpp
+  TestUserCommand.cpp
 
   LINK_LIBS
+liblldb
 lldbInterpreter
 lldbUtilityHelpers
   )
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -216,6 +216,14 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_RECORD_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+ (const char *), cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_RECORD_METHOD(bool, SBCommandInterpreter, AliasExists, (const char *),
  cmd);
@@ -874,6 +882,8 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreter, operator bool, ());
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, CommandExists,
(const char *));
+  LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+   (const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, AliasExists,
(const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, IsActive, ());
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -91,8 +91,28 @@
 
   bool IsValid() const;
 
+  /// Determine whether a command exists in this CommandInterpreter.
+  ///
+  /// For commands registered using the LLDB API, see UserCommandExists
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Determine whether a user command exists in this CommandInterpreter.
+  /// Users commands are the ones registered using the LLDB API and are not
+  /// provided by default in LLDB.
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
+  bool UserCommandExists(const char *cmd);
+
   bool AliasExists(const char *cmd);
 
   lldb::SBBroadcaster GetBroadcaster();
Index: lldb/bindings/interface/SBCommandInterpreter.i
===
--- lldb/bindin

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 255454.
wallace added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582

Files:
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/Interpreter/CMakeLists.txt
  lldb/unittests/Interpreter/TestUserCommand.cpp

Index: lldb/unittests/Interpreter/TestUserCommand.cpp
===
--- /dev/null
+++ lldb/unittests/Interpreter/TestUserCommand.cpp
@@ -0,0 +1,45 @@
+//===-- TestUserCommand.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===/
+
+#include "gtest/gtest.h"
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+#include 
+
+using namespace lldb;
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject &result) {
+result.PutCString("It works");
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+};
+
+TEST(TestUserCommand, TestCreation) {
+  SBDebugger::Initialize();
+  SBDebugger dbg = SBDebugger::Create(false /*source_init_files*/);
+  SBCommandInterpreter interp = dbg.GetCommandInterpreter();
+
+  DummyCommand command;
+  interp.AddCommand("dummy", &command, nullptr /*help*/);
+
+  EXPECT_TRUE(interp.UserCommandExists("dummy"));
+
+  SBCommandReturnObject result;
+  interp.HandleCommand("dummy", result, true /*add_to_history*/);
+  EXPECT_TRUE(result.Succeeded());
+  EXPECT_STREQ(result.GetOutput(), "It works\n");
+}
Index: lldb/unittests/Interpreter/CMakeLists.txt
===
--- lldb/unittests/Interpreter/CMakeLists.txt
+++ lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,8 +1,10 @@
 add_lldb_unittest(InterpreterTests
   TestCompletion.cpp
   TestOptionArgParser.cpp
+  TestUserCommand.cpp
 
   LINK_LIBS
+liblldb
 lldbInterpreter
 lldbUtilityHelpers
   )
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -216,6 +216,14 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_RECORD_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+ (const char *), cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_RECORD_METHOD(bool, SBCommandInterpreter, AliasExists, (const char *),
  cmd);
@@ -874,6 +882,8 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreter, operator bool, ());
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, CommandExists,
(const char *));
+  LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+   (const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, AliasExists,
(const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, IsActive, ());
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -91,8 +91,28 @@
 
   bool IsValid() const;
 
+  /// Determine whether a command exists in this CommandInterpreter.
+  ///
+  /// For commands registered using the LLDB API, see UserCommandExists
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Determine whether a user command exists in this CommandInterpreter.
+  /// Users commands are the ones registered using the LLDB API and are not
+  /// provided by default in LLDB.
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
+  bool UserCommandExists(const char *cmd);
+
   bool AliasExists(const char *cmd);
 
   lldb::SBBroadcaster GetBroadcaster();
Index: lldb/bindings/interface/SBCommandInterpreter.i
===
--- lldb/bindings/interface/SBCommand

[Lldb-commits] [lldb] 6f9ea26 - [debugserver] Get rid of `else` after `return`. NFC.

2020-04-06 Thread Davide Italiano via lldb-commits

Author: Davide Italiano
Date: 2020-04-06T13:35:48-07:00
New Revision: 6f9ea26002914cd3bcc27f09e65a151c81682352

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

LOG: [debugserver] Get rid of `else` after `return`. NFC.

Added: 


Modified: 
lldb/tools/debugserver/source/DNB.cpp

Removed: 




diff  --git a/lldb/tools/debugserver/source/DNB.cpp 
b/lldb/tools/debugserver/source/DNB.cpp
index d0dd8fd31841..b87ef5768a96 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -421,7 +421,8 @@ nub_process_t DNBProcessAttachByName(const char *name, 
struct timespec *timeout,
   if (num_matching_proc_infos == 0) {
 DNBLogError("error: no processes match '%s'\n", name);
 return INVALID_NUB_PROCESS;
-  } else if (num_matching_proc_infos > 1) {
+  }
+  if (num_matching_proc_infos > 1) {
 DNBLogError("error: %llu processes match '%s':\n",
 (uint64_t)num_matching_proc_infos, name);
 size_t i;



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


[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-06 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil updated this revision to Diff 255463.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327

Files:
  lldb/include/lldb/Core/UniqueCStringMap.h
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -31,8 +31,8 @@
 
   DWARFCompileUnit *GetDWOCompileUnitForHash(uint64_t hash);
 
-  size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_die_offsets) override;
+  bool GetObjCMethods(lldb_private::ConstString class_name,
+  llvm::function_ref callback) override;
 
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -95,10 +95,10 @@
   return GetBaseSymbolFile().GetForwardDeclClangTypeToDie();
 }
 
-size_t SymbolFileDWARFDwo::GetObjCMethodDIEOffsets(
-lldb_private::ConstString class_name, DIEArray &method_die_offsets) {
-  return GetBaseSymbolFile().GetObjCMethodDIEOffsets(class_name,
- method_die_offsets);
+bool SymbolFileDWARFDwo::GetObjCMethods(
+lldb_private::ConstString class_name,
+llvm::function_ref callback) {
+  return GetBaseSymbolFile().GetObjCMethods(class_name, callback);
 }
 
 UniqueDWARFASTTypeMap &SymbolFileDWARFDwo::GetUniqueDWARFASTTypeMap() {
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -237,8 +237,8 @@
   lldb_private::CompileUnit *
   GetCompUnitForDWARFCompUnit(DWARFCompileUnit &dwarf_cu);
 
-  virtual size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
- DIEArray &method_die_offsets);
+  virtual bool GetObjCMethods(lldb_private::ConstString class_name,
+  llvm::function_ref callback);
 
   bool Supports_DW_AT_APPLE_objc_complete_type(DWARFUnit *cu);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1457,11 +1457,9 @@
   return static_cast(non_dwo_cu->GetUserData());
 }
 
-size_t SymbolFileDWARF::GetObjCMethodDIEOffsets(ConstString class_name,
-DIEArray &method_die_offsets) {
-  method_die_offsets.clear();
-  m_index->GetObjCMethods(class_name, method_die_offsets);
-  return method_die_offsets.size();
+bool SymbolFileDWARF::GetObjCMethods(
+ConstString class_name, llvm::function_ref callback) {
+  return m_index->GetObjCMethods(class_name, callback);
 }
 
 bool SymbolFileDWARF::GetFunction(const DWARFDIE &die, SymbolContext &sc) {
@@ -2042,24 +2040,20 @@
   context, basename))
 basename = name.GetStringRef();
 
-  DIEArray die_offsets;
-  m_index->GetGlobalVariables(ConstString(basename), die_offsets);
-  const size_t num_die_matches = die_offsets.size();
-
-  SymbolContext sc;
-  sc.module_sp = m_objfile_sp->GetModule();
-  assert(sc.module_sp);
-
   // Loop invariant: Variables up to this index have been checked for context
   // matches.
   uint32_t pruned_idx = original_size;
 
-  for (size_t i = 0; i < num_die_matches; ++i) {
-const DIERef &die_ref = die_offsets[i];
+  SymbolCo

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

2020-04-06 Thread Lawrence D'Anna via Phabricator via lldb-commits
lawrence_danna updated this revision to Diff 255464.
lawrence_danna added a comment.

delete operator= too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77480

Files:
  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
@@ -488,39 +488,53 @@
 }
 }
 
+%inline %{
+
+struct Py_buffer_RAII {
+  Py_buffer buffer;
+  Py_buffer_RAII() { buffer = {}; };
+  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
+//
+// I've also moved the call to PyBuffer_Release to the end of the SWIG wrapper,
+// doing it right away is not legal according to the python buffer protocol.
 
 %define %pybuffer_mutable_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_WRITABLE);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_WRITABLE);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size/sizeof($*1_type));
 }
 %enddef
 
 %define %pybuffer_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; const void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_CONTIG_RO);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_CONTIG_RO);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size / sizeof($*1_type));
 }


Index: lldb/bindings/python/python-typemaps.swig
===
--- lldb/bindings/python/python-typemaps.swig
+++ lldb/bindings/python/python-typemaps.swig
@@ -488,39 +488,53 @@
 }
 }
 
+%inline %{
+
+struct Py_buffer_RAII {
+  Py_buffer buffer;
+  Py_buffer_RAII() { buffer = {}; };
+  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
+//
+// I've also moved the call to PyBuffer_Release to the end of the SWIG wrapper,
+// doing it right away is not legal according to the python buffer protocol.
 
 %define %pybuffer_mutable_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_WRITABLE);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_WRITABLE);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size/sizeof($*1_type));
 }
 %enddef
 
 %define %pybuffer_binary(TYPEMAP, SIZE)
-%typemap(in) (TYPEMAP, SIZE) {
+%typemap(in) (TYPEMAP, SIZE) (Py_buffer_RAII view) {
   int res; Py_ssize_t size = 0; const void *buf = 0;
-  Py_buffer view;
-  res = PyObject_GetBuffer($input, &view, PyBUF_CONTIG_RO);
+  res = PyObject_GetBuffer($input, &view.buffer, PyBUF_CONTIG_RO);
   if (res < 0) {
 PyErr_Clear();
 %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
   }
-  size = view.len;
-  buf = view.buf;
-  PyBuffer_Release(&view);
+  size = view.buffer.len;
+  buf = view.buffer.buf;
   $1 = ($1_ltype) buf;
   $2 = ($2_ltype) (size / sizeof($*1_type));
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-06 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil marked 2 inline comments as done.
jankratochvil added inline comments.



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:130-163
+  bool GetValues(ConstString unique_cstr,
+ std::function callback) const {
+for (const Entry &entry : llvm::make_range(std::equal_range(
+ m_map.begin(), m_map.end(), unique_cstr, Compare(
+  if (callback(entry.value))
+return true;
+

jankratochvil wrote:
> labath wrote:
> > I'm not sure these functions are really needed. They have just one caller, 
> > and they'd be trivial if this class provided appropriate abstractions. 
> > Maybe just add begin/end/equal_range methods, so that one can write:
> > ```
> > for (value: map / map.equal_range(str))
> >   stuff
> > ```
> > ?
> > 
> > (ideally, I'd like to have iterators instead of callbacks for the index 
> > classes too, but iterators and class hierarchies don't mix very well)
> Do you mean the standard iteration:
> ```
> for (std::pair pair : map)
>   stuff;
> ```
> or really
> ```
> for (DIERef ref : map)
>   stuff;
> ```
> ?
> 
Implemented. IIUC for regexes it should be matched in a caller - or to make 
also an iterator for regexes?
```
  for (const auto &entry : m_map.equal_range(name))
if (callback(entry.value))
```
```
  for (const auto &entry : m_map)
if (regex.Execute(entry.cstring.GetCString())) {
  if (callback(entry.value))
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327



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


[Lldb-commits] [lldb] 836534f - Add more detailed symbol type categorization, based on a swift patch by

2020-04-06 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2020-04-06T14:05:33-07:00
New Revision: 836534f9970295ff559ef49d6c49958aac6171f9

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

LOG: Add more detailed symbol type categorization, based on a swift patch by
Greg Clayton a few years ago.

My patch to augment the symbol table in Mach-O files with the
dyld trie exports data structure only categorized symbols as code
or data, but Greg Clayton had a patch to do something similar to
swift a few years ago that had a more extensive categorization of
symbols, as well as extracting some objc class/ivar names from the
entries. This patch is basically just Greg's, updated a bit and
with a test case added to it.



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

Added: 
lldb/test/API/macosx/dyld-trie-symbols/main.mm

Modified: 
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/test/API/macosx/dyld-trie-symbols/Makefile
lldb/test/API/macosx/dyld-trie-symbols/TestDyldTrieSymbols.py

Removed: 
lldb/test/API/macosx/dyld-trie-symbols/main.cpp



diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 5044bed309dc..006ba468d6f2 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2036,6 +2036,66 @@ static bool ParseTrieEntries(DataExtractor &data, 
lldb::offset_t offset,
   return true;
 }
 
+static SymbolType GetSymbolType(const char *&symbol_name,
+bool &demangled_is_synthesized,
+const SectionSP &text_section_sp,
+const SectionSP &data_section_sp,
+const SectionSP &data_dirty_section_sp,
+const SectionSP &data_const_section_sp,
+const SectionSP &symbol_section) {
+  SymbolType type = eSymbolTypeInvalid;
+
+  const char *symbol_sect_name = symbol_section->GetName().AsCString();
+  if (symbol_section->IsDescendant(text_section_sp.get())) {
+if (symbol_section->IsClear(S_ATTR_PURE_INSTRUCTIONS |
+S_ATTR_SELF_MODIFYING_CODE |
+S_ATTR_SOME_INSTRUCTIONS))
+  type = eSymbolTypeData;
+else
+  type = eSymbolTypeCode;
+  } else if (symbol_section->IsDescendant(data_section_sp.get()) ||
+ symbol_section->IsDescendant(data_dirty_section_sp.get()) ||
+ symbol_section->IsDescendant(data_const_section_sp.get())) {
+if (symbol_sect_name &&
+::strstr(symbol_sect_name, "__objc") == symbol_sect_name) {
+  type = eSymbolTypeRuntime;
+
+  if (symbol_name) {
+llvm::StringRef symbol_name_ref(symbol_name);
+if (symbol_name_ref.startswith("OBJC_")) {
+  static const llvm::StringRef g_objc_v2_prefix_class("OBJC_CLASS_$_");
+  static const llvm::StringRef g_objc_v2_prefix_metaclass(
+  "OBJC_METACLASS_$_");
+  static const llvm::StringRef g_objc_v2_prefix_ivar("OBJC_IVAR_$_");
+  if (symbol_name_ref.startswith(g_objc_v2_prefix_class)) {
+symbol_name = symbol_name + g_objc_v2_prefix_class.size();
+type = eSymbolTypeObjCClass;
+demangled_is_synthesized = true;
+  } else if (symbol_name_ref.startswith(g_objc_v2_prefix_metaclass)) {
+symbol_name = symbol_name + g_objc_v2_prefix_metaclass.size();
+type = eSymbolTypeObjCMetaClass;
+demangled_is_synthesized = true;
+  } else if (symbol_name_ref.startswith(g_objc_v2_prefix_ivar)) {
+symbol_name = symbol_name + g_objc_v2_prefix_ivar.size();
+type = eSymbolTypeObjCIVar;
+demangled_is_synthesized = true;
+  }
+}
+  }
+} else if (symbol_sect_name &&
+   ::strstr(symbol_sect_name, "__gcc_except_tab") ==
+   symbol_sect_name) {
+  type = eSymbolTypeException;
+} else {
+  type = eSymbolTypeData;
+}
+  } else if (symbol_sect_name &&
+ ::strstr(symbol_sect_name, "__IMPORT") == symbol_sect_name) {
+type = eSymbolTypeTrampoline;
+  }
+  return type;
+}
+
 // Read the UUID out of a dyld_shared_cache file on-disk.
 UUID ObjectFileMachO::GetSharedCacheUUID(FileSpec dyld_shared_cache,
  const ByteOrder byte_order,
@@ -4536,22 +4596,20 @@ size_t ObjectFileMachO::ParseSymtab() {
 Address symbol_addr;
 if (module_sp->ResolveFileAddress(e.entry.address, symbol_addr)) {
   SectionSP symbol_section(symbol_addr.GetSection());
+  const char *symbol_name = e.entry.name.GetCString();
+  bool deman

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

2020-04-06 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision.
aprantl added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2016
+for (size_t i = 0; i < num_matches; ++i) {
+  const DIERef &die_ref = method_die_offsets[i];
   DWARFDebugInfo &debug_info = dwarf->DebugInfo();

Can this be made into a range-based for loop in a separate commit?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2057
 
-  if (die) {
-switch (die.Tag()) {
-default:
-case DW_TAG_subprogram:
-case DW_TAG_inlined_subroutine:
-case DW_TAG_try_block:
-case DW_TAG_catch_block:
-  break;
+  for (size_t i = 0; i < num_die_matches; ++i) {
+const DIERef &die_ref = die_offsets[i];

same here (later)



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2915
+  if (!log)
+continue;
+  std::string qualified_name;

These two continues IMO are a bit confusing to read this way. Perhaps in this 
case an if (log) block with just one continue at the end is easier to read. Up 
to you.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2951
+type_sp = resolved_type->shared_from_this();
+break;
   }

I think I'd prefer a return over a break, (iff they are equivalent!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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


[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-06 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:102
 
+  // Helper iterator for the 'equal_range' method.
+  class CString_iterator {

///


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327



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


[Lldb-commits] [PATCH] D77369: Augment the symbol type classification for dyld trie exports with more detail

2020-04-06 Thread Jason Molenda via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG836534f99702: Add more detailed symbol type categorization, 
based on a swift patch by Greg… (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77369

Files:
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/test/API/macosx/dyld-trie-symbols/Makefile
  lldb/test/API/macosx/dyld-trie-symbols/TestDyldTrieSymbols.py
  lldb/test/API/macosx/dyld-trie-symbols/main.cpp
  lldb/test/API/macosx/dyld-trie-symbols/main.mm

Index: lldb/test/API/macosx/dyld-trie-symbols/main.mm
===
--- /dev/null
+++ lldb/test/API/macosx/dyld-trie-symbols/main.mm
@@ -0,0 +1,149 @@
+#import 
+
+// SourceBase will be the base class of Source.  We'll pass a Source object into a
+// function as a SourceBase, and then see if the dynamic typing can get us through the KVO
+// goo and all the way back to Source.
+
+@interface SourceBase: NSObject
+{
+uint32_t _value;
+}
+- (SourceBase *) init;
+- (uint32_t) getValue;
+@end
+
+@implementation SourceBase
+- (SourceBase *) init
+{
+[super init];
+_value = 10;
+return self;
+}
+- (uint32_t) getValue
+{
+return _value;
+}
+@end
+
+// Source is a class that will be observed by the Observer class below.
+// When Observer sets itself up to observe this property (in initWithASource)
+// the KVO system will overwrite the "isa" pointer of the object with the "kvo'ed" 
+// one.
+
+@interface Source : SourceBase
+{
+int _property;
+}
+- (Source *) init;
+- (void) setProperty: (int) newValue;
+@end
+
+@implementation Source
+- (Source *) init
+{
+[super init];
+_property = 20;
+return self;
+}
+- (void) setProperty: (int) newValue
+{
+_property = newValue;  // This is the line in setProperty, make sure we step to here.
+}
+@end
+
+@interface SourceDerived : Source
+{
+int _derivedValue;
+}
+- (SourceDerived *) init;
+- (uint32_t) getValue;
+@end
+
+@implementation SourceDerived
+- (SourceDerived *) init
+{
+[super init];
+_derivedValue = 30;
+return self;
+}
+- (uint32_t) getValue
+{
+return _derivedValue;
+}
+@end
+
+// Observer is the object that will watch Source and cause KVO to swizzle it...
+
+@interface Observer : NSObject
+{
+Source *_source;
+}
++ (Observer *) observerWithSource: (Source *) source;
+- (Observer *) initWithASource: (Source *) source;
+- (void) observeValueForKeyPath: (NSString *) path 
+		   ofObject: (id) object
+			 change: (NSDictionary *) change
+			context: (void *) context;
+@end
+
+@implementation Observer
+
++ (Observer *) observerWithSource: (Source *) inSource;
+{
+Observer *retval;
+
+retval = [[Observer alloc] initWithASource: inSource];
+return retval;
+}
+
+- (Observer *) initWithASource: (Source *) source
+{
+[super init];
+_source = source;
+[_source addObserver: self 
+	forKeyPath: @"property" 
+	options: (NSKeyValueObservingOptionNew | NSKeyValueObservingOptionOld)
+	context: NULL];
+return self;
+}
+
+- (void) observeValueForKeyPath: (NSString *) path 
+		   ofObject: (id) object
+			 change: (NSDictionary *) change
+			context: (void *) context
+{
+printf ("Observer function called.\n");
+return;
+}
+@end
+
+
+int patval;  // external symbol, will not be completely stripped
+int pat(int in) {// external symbol, will not be completely stripped
+  if (patval == 0)
+patval = in;
+  return patval;
+}
+
+static int fooval;  // static symbol, stripped
+int foo() { // external symbol, will not be completely stripped
+  if (fooval == 0)
+fooval = 5;
+  return fooval;
+}
+
+int bazval = 10;   // external symbol, will not be completely stripped
+int baz () {   // external symbol, will not be completely stripped
+  return foo() + bazval;
+}
+
+static int barval = 15; // static symbol, stripped
+static int bar () { // static symbol, stripped; __lldb_unnamed_symbol from func starts
+  return baz() + barval;
+}
+
+int calculate ()   // external symbol, will not be completely stripped
+{
+  return bar();
+}
+
Index: lldb/test/API/macosx/dyld-trie-symbols/main.cpp
===
--- lldb/test/API/macosx/dyld-trie-symbols/main.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-int patval;  // external symbol, will not be completely stripped
-int pat(int in) {// external symbol, will not be completely stripped
-  if (patval == 0)
-patval = in;
-  return patval;
-}
-
-static int fooval;  // static symbol, stripped
-int foo() { // external symbol, will not be completely stripped
-  if (fooval == 0)
-fooval = 5;
-  return fooval;
-}
-
-int bazval = 10;   // external symbol, will not be completely stripped
-in

[Lldb-commits] [lldb] 1b7560b - [lldb/Test] Enable TestGdbRemoteThreadsInfoMemory.py on Windows.

2020-04-06 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-04-06T14:29:09-07:00
New Revision: 1b7560b877217adc828ed5c8d7818e14dccc7378

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

LOG: [lldb/Test] Enable TestGdbRemoteThreadsInfoMemory.py on Windows.

This test is currently XFAILed but is passing on the Windows bot.

Added: 


Modified: 

lldb/test/API/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py

Removed: 




diff  --git 
a/lldb/test/API/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py
 
b/lldb/test/API/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py
index 32bb765e390d..405221e9dc59 100644
--- 
a/lldb/test/API/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py
+++ 
b/lldb/test/API/tools/lldb-server/threads-info/TestGdbRemoteThreadsInfoMemory.py
@@ -89,7 +89,6 @@ def threadsInfoStackCorrect(self):
 self.assertEqual(encode_hex(next_fp) + encode_hex(0xf00d),
  chunks[2]["bytes"])
 
-@expectedFailureAll(oslist=["windows"])
 @skipIfNetBSD
 @llgs_test
 def test_g_returns_correct_data_with_suffix_llgs(self):



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


[Lldb-commits] [PATCH] D77588: [lldb/Reproducers] Make it possible to capture reproducers for the Python test suite.

2020-04-06 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.

This adds the necessary boiler plate to capture and replay API tests.

http://lists.llvm.org/pipermail/lldb-dev/2020-April/016100.html


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D77588

Files:
  lldb/bindings/headers.swig
  lldb/bindings/interface/SBReproducer.i
  lldb/bindings/interfaces.swig
  lldb/bindings/python.swig
  lldb/include/lldb/API/SBReproducer.h
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/source/API/SBReproducer.cpp
  lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/lldbtest.py

Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -1,6 +1,6 @@
 from __future__ import absolute_import
 import os
-
+import tempfile
 import subprocess
 import sys
 
@@ -86,6 +86,14 @@
 shutil.copy(python, copied_python)
 cmd[0] = copied_python
 
+reproducer_path = os.path.join(tempfile.gettempdir(), testFile)
+if 'lldb-repro-capture' in test.config.available_features:
+test.config.environment[
+'LLDB_REPRODUCER_CAPTURE_PATH'] = reproducer_path
+elif 'lldb-repro-replay' in test.config.available_features:
+test.config.environment[
+'LLDB_REPRODUCER_REPLAY_PATH'] = reproducer_path
+
 timeoutInfo = None
 try:
 out, err, exitCode = lit.util.executeCommand(
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -60,6 +60,17 @@
   config.environment['LLDB_CAPTURE_REPRODUCER'] = os.environ[
   'LLDB_CAPTURE_REPRODUCER']
 
+# Support running the test suite under the lldb-repro wrapper. This makes it
+# possible to capture a test suite run and then rerun all the test from the
+# just captured reproducer.
+lldb_repro_mode = lit_config.params.get('lldb-run-with-repro', None)
+if lldb_repro_mode:
+  lit_config.note("Running Shell test with lldb-repo in {} mode.".format(lldb_repro_mode))
+  if lldb_repro_mode == 'capture':
+config.available_features.add('lldb-repro-capture')
+  elif lldb_repro_mode == 'replay':
+config.available_features.add('lldb-repro-replay')
+
 # Clean the module caches in the test build directory. This is necessary in an
 # incremental build whenever clang changes underneath, so doing it once per
 # lit.py invocation is close enough.
Index: lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
===
--- lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
+++ lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -20,6 +20,7 @@
 @skipIfWindows
 @skipIfRemote
 @skipIfiOSSimulator
+@skipIfReproducer
 def test_reproducer_attach(self):
 """Test thread creation after process attach."""
 exe = '%s_%d' % (self.testMethodName, os.getpid())
Index: lldb/source/API/SBReproducer.cpp
===
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -124,6 +124,15 @@
   return nullptr;
 }
 
+const char *SBReproducer::APIReplay(const char *path) {
+  static std::string error;
+  if (auto e = Reproducer::Initialize(ReproducerMode::Replay, FileSpec(path))) {
+error = llvm::toString(std::move(e));
+return error.c_str();
+  }
+  return nullptr;
+}
+
 const char *SBReproducer::Replay(const char *path) {
   return SBReproducer::Replay(path, false);
 }
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -854,3 +854,11 @@
 return "ASAN unsupported"
 return None
 return skipTestIfFn(is_asan)(func)
+
+def skipIfReproducer(func):
+"""Skip this test if the environment is set up to run LLDB with reproducers."""
+def is_reproducer():
+if ('LLDB_REPRODUCER_CAPTURE_PATH' in os.environ or 'LLDB_REPRODUCER_REPLAY_PATH' in os.environ):
+return "reproducers unsupported"
+return None
+return skipTestIfFn(is_reproducer)(func)
Index: lldb/include/lldb/API/SBReproducer.h
===
--- lldb/include/lldb/API/SBReproducer.h
+++ lldb/include/lldb/API/SBReproducer.h
@@ -22,6 +22,7 @@
   static const char *Capture(const char *path);
   static const char *Replay(const char *path);
   static const char *Replay(const char *path, bool skip_version_check);
+  static const char *APIReplay(const char *path);
   static const char *GetPath();
   static bool SetAutoGenerate(bool b);
   static bool

[Lldb-commits] [lldb] 41610d6 - [gdb-remote] Moving prevents copy elision. Found by clang.

2020-04-06 Thread Davide Italiano via lldb-commits

Author: Davide Italiano
Date: 2020-04-06T14:59:27-07:00
New Revision: 41610d665013d716da245175ada1d9c5a8b79558

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

LOG: [gdb-remote] Moving prevents copy elision. Found by clang.

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 3e7ab27802a8..f8b09d5207ab 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -643,7 +643,7 @@ static json::Array 
GetStackMemoryAsJSON(NativeProcessProtocol &process,
   if (error.Success() && bytes_read > 0) {
 buf.resize(bytes_read);
 stack_memory_chunks.push_back(
-std::move(CreateMemoryChunk(stack_memory_chunks, sp_value, buf)));
+CreateMemoryChunk(stack_memory_chunks, sp_value, buf));
   }
 
   // Additionally, try to walk the frame pointer link chain. If the frame
@@ -662,7 +662,7 @@ static json::Array 
GetStackMemoryAsJSON(NativeProcessProtocol &process,
   break;
 
 stack_memory_chunks.push_back(
-std::move(CreateMemoryChunk(stack_memory_chunks, fp_value, 
fp_ra_buf)));
+CreateMemoryChunk(stack_memory_chunks, fp_value, fp_ra_buf));
 
 // Advance the stack pointer and the frame pointer.
 sp_value = fp_value;



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


[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support inline replay

2020-04-06 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.
Herald added a subscriber: abidh.
JDevlieghere added a parent revision: D77588: [lldb/Reproducers] Make it 
possible to capture reproducers for the Python test suite..

Support inline replay as described in the RFC [1] on lldb-dev. This extends the 
LLDB_RECORD macros to re-invoke the current function with arguments 
deserialized from the reproducer. This relies on the function being called in 
the exact same order as during replay. It uses the same mechanism to toggle the 
API boundary as during recording, which guarantees that only boundary crossing 
calls are replayed.

Another major change is that before this patch we could ignore the result of an 
API call, because we only cared about the observable behavior. Now we need to 
be able to return the replayed result to the SWIG bindings.

We reuse a lot of the recording infrastructure, which can be a little confusing 
at first. I'm open to renaming these things in a future patch, but I refrained 
from doing so here to make it easier to review.

[1] http://lists.llvm.org/pipermail/lldb-dev/2020-April/016100.html


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D77602

Files:
  lldb/include/lldb/Utility/Reproducer.h
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/API/SBReproducer.cpp
  lldb/source/API/SBReproducerPrivate.h
  lldb/source/Utility/Reproducer.cpp
  lldb/source/Utility/ReproducerInstrumentation.cpp

Index: lldb/source/Utility/ReproducerInstrumentation.cpp
===
--- lldb/source/Utility/ReproducerInstrumentation.cpp
+++ lldb/source/Utility/ReproducerInstrumentation.cpp
@@ -117,6 +117,39 @@
   return m_ids[id].second.ToString();
 }
 
+/// Extract the method name from the given signature.
+static llvm::StringRef GetMethodName(llvm::StringRef signature) {
+  auto pos = signature.find('(');
+  if (pos == llvm::StringRef::npos)
+return {};
+
+  llvm::StringRef temp = signature.substr(0, pos);
+  pos = temp.rfind("::");
+  if (pos == llvm::StringRef::npos)
+return {};
+
+  return temp.substr(pos);
+}
+
+void Registry::CheckSignature(llvm::StringRef expected, unsigned id) {
+  std::string actual = GetSignature(id);
+
+  llvm::StringRef expected_method = GetMethodName(expected);
+  llvm::StringRef actual_method = GetMethodName(actual);
+
+  /// This check is an approximation but will catch divergences eventually.
+  if (expected_method.empty() || actual_method.empty() ||
+  expected_method != actual_method) {
+llvm::errs() << "Reproducer expected signature: '" << expected << "'\n";
+llvm::errs() << "Reproducer actual signature:   '" << actual << "'\n";
+llvm::report_fatal_error(
+"Detected reproducer replay divergence. Refusing to continue.");
+  }
+#ifdef LLDB_REPRO_INSTR_TRACE
+  llvm::errs() << "Replaying " << id << ": " << actual << "\n";
+#endif
+}
+
 Replayer *Registry::GetReplayer(unsigned id) {
   assert(m_ids.count(id) != 0 && "ID not in registry");
   return m_ids[id].first;
Index: lldb/source/Utility/Reproducer.cpp
===
--- lldb/source/Utility/Reproducer.cpp
+++ lldb/source/Utility/Reproducer.cpp
@@ -228,7 +228,8 @@
 }
 
 Loader::Loader(FileSpec root)
-: m_root(MakeAbsolute(std::move(root))), m_loaded(false) {}
+: m_root(MakeAbsolute(std::move(root))), m_loaded(false),
+  m_api_replay(false) {}
 
 llvm::Error Loader::LoadIndex() {
   if (m_loaded)
Index: lldb/source/API/SBReproducerPrivate.h
===
--- lldb/source/API/SBReproducerPrivate.h
+++ lldb/source/API/SBReproducerPrivate.h
@@ -55,6 +55,17 @@
   SBRegistry m_registry;
 };
 
+class ReplayData {
+public:
+  ReplayData(llvm::StringRef buffer) : m_registry(), m_deserializer(buffer) {}
+  Deserializer &GetDeserializer() { return m_deserializer; }
+  Registry &GetRegistry() { return m_registry; }
+
+private:
+  SBRegistry m_registry;
+  Deserializer m_deserializer;
+};
+
 inline InstrumentationData GetInstrumentationData() {
   if (!lldb_private::repro::Reproducer::Initialized())
 return {};
@@ -64,6 +75,17 @@
 return {p.GetSerializer(), p.GetRegistry()};
   }
 
+  if (auto *l = lldb_private::repro::Reproducer::Instance().GetLoader()) {
+if (l->IsAPIReplay()) {
+  FileSpec file = l->GetFile();
+  static auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
+  if (!error_or_file)
+return {};
+  static ReplayData r((*error_or_file)->getBuffer());
+  return {r.GetDeserializer(), r.GetRegistry()};
+}
+  }
+
   return {};
 }
 
Index: lldb/source/API/SBReproducer.cpp
===
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -130,6 +130,8 @@
 error = llvm::toString(std::move(e));
 return error.c_str();
   }
+  repro::Loader *lo

[Lldb-commits] [lldb] 1e05d7b - Remap the target (Xcode) SDK directory to the host SDK directory.

2020-04-06 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-04-06T15:51:30-07:00
New Revision: 1e05d7b3d3c6d29ff6f9493cc478a36244cc32bd

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

LOG: Remap the target (Xcode) SDK directory to the host SDK directory.

This is mostly useful for Swift support; it allows LLDB to substitute
a matching SDK it shipped with instead of the sysroot path that was
used at compile time.

The goal of this is to make the Xcode SDK something that behaves more
like the compiler's resource directory, as in that it ships with LLDB
rather than with the debugged program. This important primarily for
importing Swift and Clang modules in the expression evaluator, and
getting at the APINotes from the SDK in Swift.

For a cross-debugging scenario, this means you have to have an SDK for
your target installed alongside LLDB. In Xcode this will always be the
case.

rdar://problem/60640017

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

Added: 
lldb/include/lldb/Utility/XcodeSDK.h
lldb/source/Utility/XcodeSDK.cpp
lldb/unittests/Utility/XcodeSDKTest.cpp

Modified: 
lldb/include/lldb/Core/Module.h
lldb/include/lldb/Host/HostInfoBase.h
lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
lldb/include/lldb/Target/Platform.h
lldb/source/Core/Module.cpp
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.h
lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Utility/CMakeLists.txt
lldb/unittests/Platform/PlatformDarwinTest.cpp
lldb/unittests/Utility/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 4715961eabf1..35d00d295fa8 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -20,6 +20,7 @@
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
+#include "lldb/Utility/XcodeSDK.h"
 #include "lldb/Utility/UUID.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
@@ -509,6 +510,12 @@ class Module : public std::enable_shared_from_this,
 m_mod_time = mod_time;
   }
 
+  /// This callback will be called by SymbolFile implementations when
+  /// parsing a compile unit that contains SDK information.
+  /// \param sdk will be merged with \p m_sdk.
+  /// \param sysroot will be added to the path remapping dictionary.
+  void RegisterXcodeSDK(llvm::StringRef sdk, llvm::StringRef sysroot);
+
   /// Tells whether this module is capable of being the main executable for a
   /// process.
   ///
@@ -971,6 +978,10 @@ class Module : public std::enable_shared_from_this,
   /// module that doesn't match where the sources currently are.
   PathMappingList m_source_mappings =
   ModuleList::GetGlobalModuleListProperties().GetSymlinkMappings();
+
+  /// The (Xcode) SDK this module was compiled with.
+  XcodeSDK m_xcode_sdk;
+  
   lldb::SectionListUP m_sections_up; ///< Unified section list for module that
  /// is used by the ObjectFile and and
  /// ObjectFile instances for the debug 
info

diff  --git a/lldb/include/lldb/Host/HostInfoBase.h 
b/lldb/include/lldb/Host/HostInfoBase.h
index d7c28061b10c..f195a9a1f021 100644
--- a/lldb/include/lldb/Host/HostInfoBase.h
+++ b/lldb/include/lldb/Host/HostInfoBase.h
@@ -12,6 +12,7 @@
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/UserIDResolver.h"
+#include "lldb/Utility/XcodeSDK.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -91,6 +92,9 @@ class HostInfoBase {
   static bool ComputePathRelativeToLibrary(FileSpec &file_spec,
llvm::StringRef dir);
 
+  /// Return the directory containing a specific Xcode SDK.
+  static std::string GetXcodeSDK(XcodeSDK sdk) { return {}; }
+
 protected:
   static bool ComputeSharedLibraryDirectory(FileSpec &file_spec);
   static bool ComputeSupportExeDirectory(FileSpec &file_spec);

diff  --git a/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h 
b/lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
index cd5161c4159f..fdbe869ea475 100644
--- a/lldb/include/lldb/Host/macosx/HostInfoMac

[Lldb-commits] [lldb] 29beabb - [lldb/API] Add missing LLDB_REGISTER_METHOD macros

2020-04-06 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-04-06T16:09:40-07:00
New Revision: 29beabbe51c222c7ee0f8aead5b45649363cdbea

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

LOG: [lldb/API] Add missing LLDB_REGISTER_METHOD macros

Add LLDB_REGISTER_METHOD macros for GetRetriesWithFixIts and
SetRetriesWithFixIts.

Added: 


Modified: 
lldb/source/API/SBExpressionOptions.cpp

Removed: 




diff  --git a/lldb/source/API/SBExpressionOptions.cpp 
b/lldb/source/API/SBExpressionOptions.cpp
index bbe7cba7012c..217e8ad5c21b 100644
--- a/lldb/source/API/SBExpressionOptions.cpp
+++ b/lldb/source/API/SBExpressionOptions.cpp
@@ -343,6 +343,9 @@ void RegisterMethods(Registry &R) {
   LLDB_REGISTER_METHOD(void, SBExpressionOptions, SetTopLevel, (bool));
   LLDB_REGISTER_METHOD(bool, SBExpressionOptions, GetAllowJIT, ());
   LLDB_REGISTER_METHOD(void, SBExpressionOptions, SetAllowJIT, (bool));
+  LLDB_REGISTER_METHOD(uint64_t, SBExpressionOptions, GetRetriesWithFixIts, 
());
+  LLDB_REGISTER_METHOD(void, SBExpressionOptions, SetRetriesWithFixIts,
+   (uint64_t));
 }
 
 }



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


Re: [Lldb-commits] [lldb] 29beabb - [lldb/API] Add missing LLDB_REGISTER_METHOD macros

2020-04-06 Thread Davidino Italiano via lldb-commits


> On Apr 6, 2020, at 4:09 PM, Jonas Devlieghere via lldb-commits 
>  wrote:
> 
> 
> Author: Jonas Devlieghere
> Date: 2020-04-06T16:09:40-07:00
> New Revision: 29beabbe51c222c7ee0f8aead5b45649363cdbea
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/29beabbe51c222c7ee0f8aead5b45649363cdbea
> DIFF: 
> https://github.com/llvm/llvm-project/commit/29beabbe51c222c7ee0f8aead5b45649363cdbea.diff
> 
> LOG: [lldb/API] Add missing LLDB_REGISTER_METHOD macros
> 
> Add LLDB_REGISTER_METHOD macros for GetRetriesWithFixIts and
> SetRetriesWithFixIts.
> 
> Added: 
> 
> 
> Modified: 
>lldb/source/API/SBExpressionOptions.cpp
> 
> Removed: 
> 
> 

I’m seeing a crash on green dragon for TestFixIts. Will this fix it?

Thanks,

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


[Lldb-commits] [PATCH] D77327: [nfc] [lldb] 2/2: Introduce DWARF callbacks

2020-04-06 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:103
+  // Helper iterator for the 'equal_range' method.
+  class CString_iterator {
+  public:

`CStringIterator`? 



Comment at: lldb/include/lldb/Core/UniqueCStringMap.h:134
+  private:
+const UniqueCStringMap &m_map;
+const Entry *m_entry_ptr;

It feels like this should be a pointer, `m_entry_ptr` is a pointer already and 
most uses in the class use the pointer value already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327



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


[Lldb-commits] [PATCH] D76471: Remap the target SDK directory to the host SDK directory

2020-04-06 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1e05d7b3d3c6: Remap the target (Xcode) SDK directory to the 
host SDK directory. (authored by aprantl).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D76471?vs=254696&id=255535#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76471

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Host/HostInfoBase.h
  lldb/include/lldb/Host/macosx/HostInfoMacOSX.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Utility/XcodeSDK.h
  lldb/source/Core/Module.cpp
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.h
  lldb/source/Plugins/Platform/MacOSX/PlatformAppleWatchSimulator.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h
  lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
  lldb/source/Plugins/Platform/MacOSX/PlatformiOSSimulator.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/XcodeSDK.cpp
  lldb/unittests/Platform/PlatformDarwinTest.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/XcodeSDKTest.cpp

Index: lldb/unittests/Utility/XcodeSDKTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -0,0 +1,86 @@
+//===-- XcodeSDKTest.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/XcodeSDK.h"
+
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
+using namespace lldb_private;
+
+TEST(XcodeSDKTest, ParseTest) {
+  EXPECT_EQ(XcodeSDK::GetAnyMacOS().GetType(), XcodeSDK::MacOSX);
+  EXPECT_EQ(XcodeSDK("MacOSX.sdk").GetType(), XcodeSDK::MacOSX);
+  EXPECT_EQ(XcodeSDK("iPhoneSimulator.sdk").GetType(), XcodeSDK::iPhoneSimulator);
+  EXPECT_EQ(XcodeSDK("iPhoneOS.sdk").GetType(), XcodeSDK::iPhoneOS);
+  EXPECT_EQ(XcodeSDK("AppleTVSimulator.sdk").GetType(), XcodeSDK::AppleTVSimulator);
+  EXPECT_EQ(XcodeSDK("AppleTVOS.sdk").GetType(), XcodeSDK::AppleTVOS);
+  EXPECT_EQ(XcodeSDK("WatchSimulator.sdk").GetType(), XcodeSDK::WatchSimulator);
+  EXPECT_EQ(XcodeSDK("WatchOS.sdk").GetType(), XcodeSDK::watchOS);
+  EXPECT_EQ(XcodeSDK("Linux.sdk").GetType(), XcodeSDK::Linux);
+  EXPECT_EQ(XcodeSDK("MacOSX.sdk").GetVersion(), llvm::VersionTuple());
+  EXPECT_EQ(XcodeSDK("MacOSX10.9.sdk").GetVersion(), llvm::VersionTuple(10, 9));
+  EXPECT_EQ(XcodeSDK("MacOSX10.15.4.sdk").GetVersion(), llvm::VersionTuple(10, 15));
+  EXPECT_EQ(XcodeSDK().GetType(), XcodeSDK::unknown);
+  EXPECT_EQ(XcodeSDK().GetVersion(), llvm::VersionTuple());
+}
+
+TEST(XcodeSDKTest, MergeTest) {
+  XcodeSDK sdk("MacOSX.sdk");
+  sdk.Merge(XcodeSDK("WatchOS.sdk"));
+  // This doesn't make any particular sense and shouldn't happen in practice, we
+  // just want to guarantee a well-defined behavior when choosing one
+  // SDK to fit all CUs in an lldb::Module.
+  // -> The higher number wins.
+  EXPECT_EQ(sdk.GetType(), XcodeSDK::watchOS);
+  sdk.Merge(XcodeSDK("WatchOS1.1.sdk"));
+  EXPECT_EQ(sdk.GetVersion(), llvm::VersionTuple(1, 1));
+  sdk.Merge(XcodeSDK("WatchOS2.0.sdk"));
+  EXPECT_EQ(sdk.GetVersion(), llvm::VersionTuple(2, 0));
+}
+
+TEST(XcodeSDKTest, SDKSupportsModules) {
+  std::string base = "/Applications/Xcode.app/Contents/Developer/Platforms/";
+  EXPECT_TRUE(XcodeSDK::SDKSupportsModules(
+  XcodeSDK::Type::iPhoneSimulator,
+  FileSpec(
+  base +
+  "iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator12.0.sdk")));
+  EXPECT_FALSE(XcodeSDK::SDKSupportsModules(
+  XcodeSDK::Type::iPhoneSimulator,
+  FileSpec(
+  base +
+  "iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator7.2.sdk")));
+  EXPECT_TRUE(XcodeSDK::SDKSupportsModules(
+  XcodeSDK::Type::MacOSX,
+  FileSpec(base + "MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk")));
+  EXPECT_FALSE(XcodeSDK::SDKSupportsModules(
+  XcodeSDK::Type::MacOSX,
+  FileSpec(base + "MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk")));
+}
+
+TEST(XcodeSDKTest, GetSDKNameForType) {
+  EXPECT_EQ("macosx", XcodeSDK::GetSDKNameForType(XcodeSDK::Type::MacOSX));
+  EXPECT_EQ("iphonesimulator",
+XcodeSDK::GetSDKNameForType(XcodeSDK::Type::iPhoneSimulator));
+  EXPECT_EQ

Re: [Lldb-commits] [lldb] 29beabb - [lldb/API] Add missing LLDB_REGISTER_METHOD macros

2020-04-06 Thread Jonas Devlieghere via lldb-commits
Yep!

On Mon, Apr 6, 2020 at 4:18 PM Davidino Italiano 
wrote:

>
>
> > On Apr 6, 2020, at 4:09 PM, Jonas Devlieghere via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> >
> > Author: Jonas Devlieghere
> > Date: 2020-04-06T16:09:40-07:00
> > New Revision: 29beabbe51c222c7ee0f8aead5b45649363cdbea
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/29beabbe51c222c7ee0f8aead5b45649363cdbea
> > DIFF:
> https://github.com/llvm/llvm-project/commit/29beabbe51c222c7ee0f8aead5b45649363cdbea.diff
> >
> > LOG: [lldb/API] Add missing LLDB_REGISTER_METHOD macros
> >
> > Add LLDB_REGISTER_METHOD macros for GetRetriesWithFixIts and
> > SetRetriesWithFixIts.
> >
> > Added:
> >
> >
> > Modified:
> >lldb/source/API/SBExpressionOptions.cpp
> >
> > Removed:
> >
> >
>
> I’m seeing a crash on green dragon for TestFixIts. Will this fix it?
>
> Thanks,
>
> —
> D
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D77608: [lldb/Scripts] Add script to replay multiple reproducers

2020-04-06 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/scripts/reproducer-replay.py:1
+#! /usr/bin/env python3
+

This is needed for the timeout. I'd rather not rely on an external package for 
this unless anyone objects.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D77608



___
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

2020-04-06 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

LGTM, but let's wait for Pavel to sign off on this.


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] D77608: [lldb/Scripts] Add script to replay multiple reproducers

2020-04-06 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/scripts/reproducer-replay.py:1
+#! /usr/bin/env python3
+

This is needed for the timeout. I'd rather not rely on an external package for 
this unless anyone objects.


Script to replay reproducers with the command line driver in parallel. This is 
used for stage 1 as described in the RFC on lldb-dev [1].

[1] http://lists.llvm.org/pipermail/lldb-dev/2020-April/016100.html


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D77608

Files:
  lldb/scripts/reproducer-replay.py

Index: lldb/scripts/reproducer-replay.py
===
--- /dev/null
+++ lldb/scripts/reproducer-replay.py
@@ -0,0 +1,98 @@
+#! /usr/bin/env python3
+
+from multiprocessing import Pool
+import argparse
+import tempfile
+import logging
+import os
+import subprocess
+
+
+def run_reproducer(path):
+proc = subprocess.Popen([LLDB, '--replay', path],
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE)
+reason = None
+try:
+outs, errs = proc.communicate(timeout=TIMEOUT)
+result = 'PASSED' if proc.returncode == 0 else 'FAILED'
+if proc.returncode != 0:
+outs = outs.decode()
+errs = errs.decode()
+# Do some pattern matching to find out the cause of the failure.
+if 'Encountered unexpected packet during replay' in errs:
+reason = 'Unexpected packet'
+elif 'Assertion failed' in errs:
+reason = 'Assertion failed'
+elif 'UNREACHABLE' in errs:
+reason = 'Unreachable executed'
+elif 'Segmentation fault' in errs:
+reason = 'Segmentation fault'
+elif 'Illegal instruction' in errs:
+reason = 'Illegal instruction'
+else:
+reason = f'Exit code {proc.returncode}'
+except subprocess.TimeoutExpired:
+proc.kill()
+outs, errs = proc.communicate()
+result = 'TIMEOUT'
+
+reason_str = f' ({reason})' if reason else ''
+print(f'{result}: {path}{reason_str}')
+
+
+def find_reproducers(path):
+reproducers = []
+for root, dirs, files in os.walk(path):
+for dir in dirs:
+_, extension = os.path.splitext(dir)
+if dir.startswith('Test') and extension == '.py':
+reproducers.append(os.path.join(root, dir))
+return reproducers
+
+
+if __name__ == '__main__':
+parser = argparse.ArgumentParser(description='LLDB Replay Driver')
+parser.add_argument('-j',
+'--threads',
+type=int,
+default=8,
+help='Number of threads')
+parser.add_argument('-t',
+'--timeout',
+type=int,
+default=60,
+help='Replay timeout')
+parser.add_argument('-l',
+'--lldb',
+type=str,
+required=True,
+help='Path to the LLDB command line driver')
+parser.add_argument('-p',
+'--path',
+type=str,
+default=None,
+help='Path to directory containing the reproducers')
+args = parser.parse_args()
+
+global LLDB
+global TIMEOUT
+LLDB = args.lldb
+TIMEOUT = args.timeout
+
+if args.path:
+path = args.path
+else:
+path = tempfile.gettempdir()
+
+print(f'Looking for reproducers in {path}')
+reproducers = find_reproducers(path)
+print(f'Found {len(reproducers)} reproducers')
+print(
+f'Replaying with {args.threads} threads and {args.timeout} seconds timeout'
+)
+try:
+pool = Pool(args.threads)
+pool.map(run_reproducer, reproducers)
+except KeyboardInterrupt:
+print('Interrupted')
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 469580a - Add missing include

2020-04-06 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-04-06T17:29:31-07:00
New Revision: 469580a9677473f4ee19d80861f7a5da4e0f316d

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

LOG: Add missing include

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 029bb590de68..872e5301d984 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -26,6 +26,7 @@
 #include "lldb/Utility/StructuredData.h"
 #include "lldb/Utility/Timeout.h"
 #include "lldb/Utility/UserIDResolver.h"
+#include "lldb/Utility/XcodeSDK.h"
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-public.h"
 #include "llvm/Support/VersionTuple.h"



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


[Lldb-commits] [lldb] 3775be2 - Target: correct the return value for `GetImageAddrFromToken`

2020-04-06 Thread Saleem Abdulrasool via lldb-commits

Author: Saleem Abdulrasool
Date: 2020-04-06T17:37:57-07:00
New Revision: 3775be2d8e17aaeae62ab83ded005867f4bf70ac

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

LOG: Target: correct the return value for `GetImageAddrFromToken`

We would return `LLDB_INVALID_IMAGE_TOKEN` for the address rather than
the correct value of `LLDB_IMAGE_ADDRESS`.  This would result in the
check for the return value to silently pass on x64 as the invalid
address and invalid token are of different sizes (`size_t` vs
`uintprr_t`).  This corrects the return value to `LLDB_INVALID_ADDRESS`
and addresses the rest to reset the mapped address to the invalid value.

This was found by inspection when trying to implement module support for
Windows.

Added: 


Modified: 
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index a3776f95..7797a4c60964 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -5796,12 +5796,12 @@ size_t Process::AddImageToken(lldb::addr_t image_ptr) {
 lldb::addr_t Process::GetImagePtrFromToken(size_t token) const {
   if (token < m_image_tokens.size())
 return m_image_tokens[token];
-  return LLDB_INVALID_IMAGE_TOKEN;
+  return LLDB_INVALID_ADDRESS;
 }
 
 void Process::ResetImageToken(size_t token) {
   if (token < m_image_tokens.size())
-m_image_tokens[token] = LLDB_INVALID_IMAGE_TOKEN;
+m_image_tokens[token] = LLDB_INVALID_ADDRESS;
 }
 
 Address



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


Re: [Lldb-commits] [lldb] 3775be2 - Target: correct the return value for `GetImageAddrFromToken`

2020-04-06 Thread Shafik Yaghmour via lldb-commits
Hello Saleem,

I am not familiar with this part of the code but is there an easy way to test 
this failure? We should add a test to make sure we don’t break this 
accidentally in the future.

Thank you!

> On Apr 6, 2020, at 5:38 PM, Saleem Abdulrasool via lldb-commits 
>  wrote:
> 
> 
> Author: Saleem Abdulrasool
> Date: 2020-04-06T17:37:57-07:00
> New Revision: 3775be2d8e17aaeae62ab83ded005867f4bf70ac
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/3775be2d8e17aaeae62ab83ded005867f4bf70ac
> DIFF: 
> https://github.com/llvm/llvm-project/commit/3775be2d8e17aaeae62ab83ded005867f4bf70ac.diff
> 
> LOG: Target: correct the return value for `GetImageAddrFromToken`
> 
> We would return `LLDB_INVALID_IMAGE_TOKEN` for the address rather than
> the correct value of `LLDB_IMAGE_ADDRESS`.  This would result in the
> check for the return value to silently pass on x64 as the invalid
> address and invalid token are of different sizes (`size_t` vs
> `uintprr_t`).  This corrects the return value to `LLDB_INVALID_ADDRESS`
> and addresses the rest to reset the mapped address to the invalid value.
> 
> This was found by inspection when trying to implement module support for
> Windows.
> 
> Added: 
> 
> 
> Modified: 
>lldb/source/Target/Process.cpp
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
> index a3776f95..7797a4c60964 100644
> --- a/lldb/source/Target/Process.cpp
> +++ b/lldb/source/Target/Process.cpp
> @@ -5796,12 +5796,12 @@ size_t Process::AddImageToken(lldb::addr_t image_ptr) 
> {
> lldb::addr_t Process::GetImagePtrFromToken(size_t token) const {
>   if (token < m_image_tokens.size())
> return m_image_tokens[token];
> -  return LLDB_INVALID_IMAGE_TOKEN;
> +  return LLDB_INVALID_ADDRESS;
> }
> 
> void Process::ResetImageToken(size_t token) {
>   if (token < m_image_tokens.size())
> -m_image_tokens[token] = LLDB_INVALID_IMAGE_TOKEN;
> +m_image_tokens[token] = LLDB_INVALID_ADDRESS;
> }
> 
> Address
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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


Re: [Lldb-commits] [lldb] 3775be2 - Target: correct the return value for `GetImageAddrFromToken`

2020-04-06 Thread Jim Ingham via lldb-commits
This should be easy to test.  If you have a running process, and haven’t called 
“process load” then:

(lldb) process unload 12345

Should return the error:

failed to unload image: invalid image token

But instead on x86_64 it returns the error:

failed to unload image: expression failed: "dlclose((void *)0x)”

Should be enough to run process unload with a bogus token and make sure the 
error is “invalid image token”.

Jim



> On Apr 6, 2020, at 6:48 PM, Shafik Yaghmour via lldb-commits 
>  wrote:
> 
> Hello Saleem,
> 
> I am not familiar with this part of the code but is there an easy way to test 
> this failure? We should add a test to make sure we don’t break this 
> accidentally in the future.
> 
> Thank you!
> 
>> On Apr 6, 2020, at 5:38 PM, Saleem Abdulrasool via lldb-commits 
>>  wrote:
>> 
>> 
>> Author: Saleem Abdulrasool
>> Date: 2020-04-06T17:37:57-07:00
>> New Revision: 3775be2d8e17aaeae62ab83ded005867f4bf70ac
>> 
>> URL: 
>> https://github.com/llvm/llvm-project/commit/3775be2d8e17aaeae62ab83ded005867f4bf70ac
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/3775be2d8e17aaeae62ab83ded005867f4bf70ac.diff
>> 
>> LOG: Target: correct the return value for `GetImageAddrFromToken`
>> 
>> We would return `LLDB_INVALID_IMAGE_TOKEN` for the address rather than
>> the correct value of `LLDB_IMAGE_ADDRESS`.  This would result in the
>> check for the return value to silently pass on x64 as the invalid
>> address and invalid token are of different sizes (`size_t` vs
>> `uintprr_t`).  This corrects the return value to `LLDB_INVALID_ADDRESS`
>> and addresses the rest to reset the mapped address to the invalid value.
>> 
>> This was found by inspection when trying to implement module support for
>> Windows.
>> 
>> Added: 
>> 
>> 
>> Modified: 
>>   lldb/source/Target/Process.cpp
>> 
>> Removed: 
>> 
>> 
>> 
>> 
>> diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
>> index a3776f95..7797a4c60964 100644
>> --- a/lldb/source/Target/Process.cpp
>> +++ b/lldb/source/Target/Process.cpp
>> @@ -5796,12 +5796,12 @@ size_t Process::AddImageToken(lldb::addr_t 
>> image_ptr) {
>> lldb::addr_t Process::GetImagePtrFromToken(size_t token) const {
>>  if (token < m_image_tokens.size())
>>return m_image_tokens[token];
>> -  return LLDB_INVALID_IMAGE_TOKEN;
>> +  return LLDB_INVALID_ADDRESS;
>> }
>> 
>> void Process::ResetImageToken(size_t token) {
>>  if (token < m_image_tokens.size())
>> -m_image_tokens[token] = LLDB_INVALID_IMAGE_TOKEN;
>> +m_image_tokens[token] = LLDB_INVALID_ADDRESS;
>> }
>> 
>> Address
>> 
>> 
>> 
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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


[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

2020-04-06 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp:70
 const dw_tag_t die_tag = die_info_array[i].tag;
-if (die_tag != 0 && die_tag != DW_TAG_class_type &&
-die_tag != DW_TAG_structure_type)
+if (!(die_tag == 0 || die_tag == DW_TAG_class_type ||
+  die_tag == DW_TAG_structure_type))

I also feel like the previous version was more readable. Is there another gain 
that I am missing?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2065
 
-case DW_TAG_variable: {
-  auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
-  if (!dwarf_cu)
-continue;
-  sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
-
-  if (parent_decl_ctx) {
-if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
-  CompilerDeclContext actual_parent_decl_ctx =
-  dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
-  if (!actual_parent_decl_ctx ||
-  actual_parent_decl_ctx != parent_decl_ctx)
-continue;
-}
-  }
+switch (die.Tag()) {
+default:

This whole switch feels overly clever, how about:

```
if (die.Tag() != DW_TAG_variable)
  continue;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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


[Lldb-commits] [lldb] 06ea05a - [lldb/test] Fix TestDSYMSourcePathRemapping in the presence of symlnks

2020-04-06 Thread Fred Riss via lldb-commits

Author: Fred Riss
Date: 2020-04-06T19:50:34-07:00
New Revision: 06ea05a3fbc6e70cd4c492cced363a8630d65c6a

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

LOG: [lldb/test] Fix TestDSYMSourcePathRemapping in the presence of symlnks

My main work directory is on a separate partition, but I usually access
it through a symlink in my home directory. When running the tests,
either Clang or make resolves the symlink, and the real path of the
test directory ends up in the debug information.

This confuses this test as LLDB is trying to remap the real path, but
the remapping description uses the path with the symlink in
it. Calling realpath on the source path when constructing the
remapping description fixes it.

Added: 


Modified: 
lldb/test/API/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py

Removed: 




diff  --git 
a/lldb/test/API/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py 
b/lldb/test/API/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
index 0f5daf51e975..5075868e9c1a 100644
--- a/lldb/test/API/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
+++ b/lldb/test/API/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
@@ -42,7 +42,7 @@ def build(self):
 f.write('\n')
 f.write('  DBGSourcePathRemapping\n')
 f.write('  \n')
-f.write('' + botdir + '\n')
+f.write('' + os.path.realpath(botdir) + '\n')
 f.write('' + userdir + '\n')
 f.write('  \n')
 f.write('\n')



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


[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-06 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 255571.
friss added a comment.

Implement null C++ printing at the ValueObjectPrinter level


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/TypeSummary.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -71,6 +71,7 @@
 std::u32string u32_string(U"🍄🍅🍆🍌");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -119,6 +119,8 @@
 
   bool IsNilReference(ValueObject &valobj) override;
 
+  llvm::StringRef NilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return &m_highlighter; }
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -88,6 +88,10 @@
   HardcodedFormatters::HardcodedSyntheticFinder
   GetHardcodedSynthetics() override;
 
+  bool IsNilReference(ValueObject &valobj) override;
+
+  llvm::StringRef NilReferenceSummaryString() override { return "nullptr"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return &m_highlighter; }
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -1133,6 +1133,17 @@
   return g_formatters;
 }
 
+bool CPlusPlusLanguage::IsNilReference(ValueObject &valobj) {
+  const uint32_t mask = eTypeIsCPlusPlus | eTypeIsPointer;
+  bool isCPPpointer =
+  (((valobj.GetCompilerType().GetTypeInfo(nullptr)) & mask) == mask);
+  if (!isCPPpointer)
+return false;
+  bool canReadValue = true;
+  bool isZero = valobj.GetValueAsUnsigned(0, &canReadValue) == 0;
+  return canReadValue && isZero;
+}
+
 bool CPlusPlusLanguage::IsSourceFile(llvm::StringRef file_path) const {
   const auto suffixes = {".cpp", ".cxx", ".c++", ".cc",  ".c",
  ".h",   ".hh",  ".hpp", ".hxx", ".h++"};
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -361,9 +361,11 @@
 error.assign(err_cstr);
 
   if (ShouldPrintValueObject()) {
-if (IsNil())
-  summary.assign("nil");
-else if (IsUninitialized())
+if (IsNil()) {
+  Language *lang_plugin =
+  Language::FindPlug

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-06 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment.

This looks like what I'd like to implement, but unfortunately it breaks other 
tests. Some C tests start printing null pointers as `nullptr` too. I suppose 
this is because the expression evaluator is always in C++ mode. Is there a way 
to get the origin type/language of a variable through a ValueObject?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153



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


[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

2020-04-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
compnerd updated this revision to Diff 255580.

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

https://reviews.llvm.org/D77287

Files:
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.h

Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.h
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.h
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.h
@@ -49,6 +49,15 @@
 
   lldb_private::Status DisconnectRemote() override;
 
+  uint32_t DoLoadImage(lldb_private::Process *process,
+   const lldb_private::FileSpec &remote_file,
+   const std::vector *paths,
+   lldb_private::Status &error,
+   lldb_private::FileSpec *loaded_path) override;
+
+  lldb_private::Status UnloadImage(lldb_private::Process *process,
+   uint32_t image_token) override;
+
   lldb::ProcessSP DebugProcess(lldb_private::ProcessLaunchInfo &launch_info,
lldb_private::Debugger &debugger,
lldb_private::Target *target,
@@ -73,6 +82,10 @@
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformWindows);
+
+  lldb_private::Status EvaluateLoaderExpression(lldb_private::Process *process,
+const char *expression,
+lldb::ValueObjectSP &value);
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -20,7 +20,10 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/Status.h"
 
@@ -308,6 +311,111 @@
   return error;
 }
 
+Status PlatformWindows::EvaluateLoaderExpression(Process *process,
+ const char *expression,
+ ValueObjectSP &value) {
+  // FIXME(compnerd) `-fdeclspec` is not passed to the clang instance?
+  static const char kLoaderDecls[] =
+  R"(
+  // libloaderapi.h
+
+  // WINBASEAPI BOOL WINAPI FreeLibrary(HMODULE);
+  extern "C" /* __declspec(dllimport) */ BOOL __stdcall FreeLibrary(void *hLibModule);
+
+  // WINBASEAPI HMODULE WINAPI LoadLibraryA(LPCSTR);
+  extern "C" /* __declspec(dllimport) */ void * __stdcall LoadLibraryA(const char *);
+)";
+
+  if (DynamicLoader *loader = process->GetDynamicLoader()) {
+Status result = loader->CanLoadImage();
+if (result.Fail())
+  return result;
+  }
+
+  ThreadSP thread = process->GetThreadList().GetExpressionExecutionThread();
+  if (!thread)
+return Status("selected thread is invalid");
+
+  StackFrameSP frame = thread->GetStackFrameAtIndex(0);
+  if (!frame)
+return Status("frame 0 is invalid");
+
+  ExecutionContext context;
+  frame->CalculateExecutionContext(context);
+
+  EvaluateExpressionOptions options;
+  options.SetUnwindOnError(true);
+  options.SetIgnoreBreakpoints(true);
+  options.SetExecutionPolicy(eExecutionPolicyAlways);
+  options.SetLanguage(eLanguageTypeC_plus_plus);
+  // LoadLibrary{A,W}/FreeLibrary cannot raise exceptions which we can handle.
+  // They may potentially throw SEH exceptions which we do not know how to
+  // handle currently.
+  options.SetTrapExceptions(false);
+  options.SetTimeout(process->GetUtilityExpressionTimeout());
+
+  Status error;
+  ExpressionResults result = UserExpression::Evaluate(
+  context, options, expression, kLoaderDecls, value, error);
+  if (result != eExpressionCompleted)
+return error;
+
+  if (value->GetError().Fail())
+return value->GetError();
+
+  return Status();
+}
+
+uint32_t PlatformWindows::DoLoadImage(Process *process,
+  const FileSpec &remote_file,
+  const std::vector *paths,
+  Status &error, FileSpec *loaded_path) {
+  if (loaded_path)
+loaded_path->Clear();
+
+  StreamString expression;
+  expression.Printf("LoadLibraryA(\"%s\")", remote_file.GetPath().c_str());
+
+  ValueObjectSP value;
+  Status result =
+  EvaluateLoaderExpression(process, expression.GetData(), value);
+  if (result.Fail())
+return LLDB_INVALID_IMAGE_TOKEN;
+
+  Scalar scalar;
+  if (value->ResolveValue(scalar)) {
+lldb::addr_t address = scalar.ULongLong();
+if (address == 0)
+  return LLDB_INVALID_IMAGE_TOKEN;
+return proce

[Lldb-commits] [PATCH] D77153: [lldb/DataFormatters] Display null C++ pointers as nullptr

2020-04-06 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 255579.
friss added a comment.

Remove the now useless code in TypeSummary.cpp, and change the C++
implementation of IsNilReference() to return true soemtimes. This
breaks other tests though...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77153

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -71,6 +71,7 @@
 std::u32string u32_string(U"🍄🍅🍆🍌");
 std::u32string u32_empty(U"");
 std::basic_string uchar(5, 'a');
+std::string *null_str = nullptr;
 
 #if _LIBCPP_ABI_VERSION == 1
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -77,6 +77,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 self.runCmd("n")
@@ -114,6 +115,7 @@
 '(%s::u32string) u32_empty = ""'%ns,
 '(%s::basic_string, '
 '%s::allocator >) uchar = "a"'%(ns,ns,ns),
+'(%s::string *) null_str = nullptr'%ns,
 ])
 
 # The test assumes that std::string is in its cap-size-data layout.
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -119,6 +119,8 @@
 
   bool IsNilReference(ValueObject &valobj) override;
 
+  llvm::StringRef NilReferenceSummaryString() override { return "nil"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return &m_highlighter; }
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -88,6 +88,10 @@
   HardcodedFormatters::HardcodedSyntheticFinder
   GetHardcodedSynthetics() override;
 
+  bool IsNilReference(ValueObject &valobj) override;
+
+  llvm::StringRef NilReferenceSummaryString() override { return "nullptr"; }
+
   bool IsSourceFile(llvm::StringRef file_path) const override;
 
   const Highlighter *GetHighlighter() const override { return &m_highlighter; }
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -1133,6 +1133,15 @@
   return g_formatters;
 }
 
+bool CPlusPlusLanguage::IsNilReference(ValueObject &valobj) {
+  if (valobj.GetCompilerType().GetMinimumLanguage() != eLanguageTypeC_plus_plus ||
+  !valobj.IsPointerType())
+return false;
+  bool canReadValue = true;
+  bool isZero = valobj.GetValueAsUnsigned(0, &canReadValue) == 0;
+  return canReadValue && isZero;
+}
+
 bool CPlusPlusLanguage::IsSourceFile(llvm::StringRef file_path) const {
   const auto suffixes = {".cpp", ".cxx", ".c++", ".cc",  ".c",
  ".h",   ".hh",  ".hpp", ".hxx", ".h++"};
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -361,9 +361,11 @@
 error.assign(err_cstr);
 
   if (ShouldPrintValueObject()) {
-if (IsNil())
-  summary.assign("nil");
-else if (IsUninitialized())
+if (IsNil()) {
+  Language *lang_plugin =
+  Language::FindPlugin(m_va

[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`

2020-04-06 Thread Saleem Abdulrasool via Phabricator via lldb-commits
compnerd updated this revision to Diff 255581.

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

https://reviews.llvm.org/D77287

Files:
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.h
  lldb/test/Shell/Process/Windows/process_load.cpp

Index: lldb/test/Shell/Process/Windows/process_load.cpp
===
--- /dev/null
+++ lldb/test/Shell/Process/Windows/process_load.cpp
@@ -0,0 +1,12 @@
+// clang-format off
+
+// REQUIRES: system-windows
+// RUN: %build --compiler=clang-cl -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -o "b main" -o "process launch" -o "process load kernel32.dll" | FileCheck %s
+
+int main(int argc, char *argv[]) {
+  return 0;
+}
+
+// CHECK: "Loading "kernel32.dll"...ok{{.*}}
+// CHECK: Image 0 loaded.
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.h
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.h
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.h
@@ -49,6 +49,15 @@
 
   lldb_private::Status DisconnectRemote() override;
 
+  uint32_t DoLoadImage(lldb_private::Process *process,
+   const lldb_private::FileSpec &remote_file,
+   const std::vector *paths,
+   lldb_private::Status &error,
+   lldb_private::FileSpec *loaded_path) override;
+
+  lldb_private::Status UnloadImage(lldb_private::Process *process,
+   uint32_t image_token) override;
+
   lldb::ProcessSP DebugProcess(lldb_private::ProcessLaunchInfo &launch_info,
lldb_private::Debugger &debugger,
lldb_private::Target *target,
@@ -73,6 +82,10 @@
 
 private:
   DISALLOW_COPY_AND_ASSIGN(PlatformWindows);
+
+  lldb_private::Status EvaluateLoaderExpression(lldb_private::Process *process,
+const char *expression,
+lldb::ValueObjectSP &value);
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -20,7 +20,10 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/Status.h"
 
@@ -308,6 +311,111 @@
   return error;
 }
 
+Status PlatformWindows::EvaluateLoaderExpression(Process *process,
+ const char *expression,
+ ValueObjectSP &value) {
+  // FIXME(compnerd) `-fdeclspec` is not passed to the clang instance?
+  static const char kLoaderDecls[] =
+  R"(
+  // libloaderapi.h
+
+  // WINBASEAPI BOOL WINAPI FreeLibrary(HMODULE);
+  extern "C" /* __declspec(dllimport) */ BOOL __stdcall FreeLibrary(void *hLibModule);
+
+  // WINBASEAPI HMODULE WINAPI LoadLibraryA(LPCSTR);
+  extern "C" /* __declspec(dllimport) */ void * __stdcall LoadLibraryA(const char *);
+)";
+
+  if (DynamicLoader *loader = process->GetDynamicLoader()) {
+Status result = loader->CanLoadImage();
+if (result.Fail())
+  return result;
+  }
+
+  ThreadSP thread = process->GetThreadList().GetExpressionExecutionThread();
+  if (!thread)
+return Status("selected thread is invalid");
+
+  StackFrameSP frame = thread->GetStackFrameAtIndex(0);
+  if (!frame)
+return Status("frame 0 is invalid");
+
+  ExecutionContext context;
+  frame->CalculateExecutionContext(context);
+
+  EvaluateExpressionOptions options;
+  options.SetUnwindOnError(true);
+  options.SetIgnoreBreakpoints(true);
+  options.SetExecutionPolicy(eExecutionPolicyAlways);
+  options.SetLanguage(eLanguageTypeC_plus_plus);
+  // LoadLibrary{A,W}/FreeLibrary cannot raise exceptions which we can handle.
+  // They may potentially throw SEH exceptions which we do not know how to
+  // handle currently.
+  options.SetTrapExceptions(false);
+  options.SetTimeout(process->GetUtilityExpressionTimeout());
+
+  Status error;
+  ExpressionResults result = UserExpression::Evaluate(
+  context, options, expression, kLoaderDecls, value, error);
+  if (result != eExpressionCompleted)
+return error;
+
+  if (value->GetError().Fail())
+return value->GetError();
+
+  return Status();
+}
+
+uint32_t PlatformWindows::DoLoadImage(Process *process,
+  const FileSpec &remote_file,
+   

[Lldb-commits] [PATCH] D77529: Prefer executable files from sysroot over files from local filesystem

2020-04-06 Thread Yuri Per via Phabricator via lldb-commits
yuri updated this revision to Diff 255588.
yuri added a comment.

Do not depend on exitance of /bin/sh anymore


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77529

Files:
  lldb/source/Target/RemoteAwarePlatform.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64.core


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -209,6 +209,43 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# Copy wrong executable to the location outside of sysroot
+exe_outside = os.path.join(self.getBuildDir(), "bin", "a.out")
+lldbutil.mkdir_p(os.path.dirname(exe_outside))
+shutil.copyfile("altmain.out", exe_outside)
+
+# Copy correct executable to the location inside sysroot
+tmp_sysroot = os.path.join(self.getBuildDir(), "mock_sysroot")
+exe_inside = os.path.join(tmp_sysroot, os.path.relpath(exe_outside, 
"/"))
+lldbutil.mkdir_p(os.path.dirname(exe_inside))
+shutil.copyfile("linux-x86_64.out", exe_inside)
+
+# Prepare patched core file
+core_file = os.path.join(self.getBuildDir(), "patched.core")
+with open("linux-x86_64.core", "rb") as f:
+core = f.read()
+core = replace_path(core, "/test" * 817 + "/a.out", exe_outside)
+with open(core_file, "wb") as f:
+f.write(core)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % 
tmp_sysroot)
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore(core_file)
+
+# Check that we found executable from the sysroot
+mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
+self.assertEqual(mod_path, exe_inside)
+self.check_all(process, self._x86_64_pid, self._x86_64_regions, 
"a.out")
+
+self.dbg.DeleteTarget(target)
+
 @skipIf(triple='^mips')
 @skipIfLLVMTargetMissing("ARM")
 def test_arm_core(self):
@@ -369,3 +406,9 @@
 self.check_all(process, pid, region_count, thread_name)
 
 self.dbg.DeleteTarget(target)
+
+def replace_path(binary, replace_from, replace_to):
+src = replace_from.encode()
+dst = replace_to.encode()
+dst += b"\0" * (len(src) - len(dst))
+return binary.replace(src, dst)
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -21,7 +21,7 @@
 return m_remote_platform_sp->GetModuleSpec(module_file_spec, arch,
module_spec);
 
-  return Platform::GetModuleSpec(module_file_spec, arch, module_spec);
+  return false;
 }
 
 Status RemoteAwarePlatform::RunShellCommand(


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -209,6 +209,43 @@
 
 self.dbg.DeleteTarget(target)
 
+@skipIf(triple='^mips')
+@skipIfLLVMTargetMissing("X86")
+def test_x86_64_sysroot(self):
+"""Test that sysroot has more priority then local filesystem."""
+
+# Copy wrong executable to the location outside of sysroot
+exe_outside = os.path.join(self.getBuildDir(), "bin", "a.out")
+lldbutil.mkdir_p(os.path.dirname(exe_outside))
+shutil.copyfile("altmain.out", exe_outside)
+
+# Copy correct executable to the location inside sysroot
+tmp_sysroot = os.path.join(self.getBuildDir(), "mock_sysroot")
+exe_inside = os.path.join(tmp_sysroot, os.path.relpath(exe_outside, "/"))
+lldbutil.mkdir_p(os.path.dirname(exe_inside))
+shutil.copyfile("linux-x86_64.out", exe_inside)
+
+# Prepare patched core file
+core_file = os.path.join(self.getBuildDir(), "patched.core")
+with open("linux-x86_64.core", "rb") as f:
+core = f.read()
+core = replace_path(core, "/test" * 817 + "/a.out", exe_outside)
+with open(core_file, "wb") as f:
+f.write(core)
+
+# Set sysroot and load core
+self.runCmd("platform select remote-linux --sysroot '%s'" % tmp_sysroot)
+target = self.dbg.CreateTarget(None)
+se