[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

[Chris Bieneman via Phabricator via lldb-commits]

beanz added a comment.

I conferred off-list with @compnerd. He and I talked through my concerns, and I 
believe this patch is fine as-is, so LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70764



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

[Lldb-commits] [PATCH] D68209: [LiveDebugValues] Introduce entry values of unmodified params

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:159
   // FUNC11-BT: func11_tailcalled{{.*}}
   // FUNC11-BT-NEXT: func12{{.*}} [artificial]
   use(x);

djtodoro wrote:
> vsk wrote:
> > The failure was:
> > ```
> > main.cpp:159:21: error: FUNC11-BT-NEXT: expected string not found in input
> >  // FUNC11-BT-NEXT: func12{{.*}} [artificial]
> > ^
> > :3:2: note: scanning from here
> >  frame #1: 0x0001079eae69 a.out`func12(sink=0x7ffee8215cb4, x=123) 
> > at main.cpp:179:3 [opt]
> > ```
> > 
> > The added `DESTROY_RBX` asm might confuse TailRecursionElimination into 
> > believing that the callee accesses the caller's stack. Could you 
> > double-check that a tail call is actually emitted in `func12` (something 
> > like `jmp *%rax`)? If it //is//, this is a pre-existing lldb bug, so the 
> > func12 test should be disabled.
> @vsk Thanks for the comment!
> 
> The problem here is the fresh change in the code production by using the 
> `-O1` level of optimization. More precisely, at very high level, after the 
> D65410 we do not have a tail call where we expected.
> I am proposing using the `-O2` level of the optimizations, since we are 
> testing printing of the entry values in the test case, rather than tail call 
> frames with particular level of optimization.
> WDYT? 
Sounds good to me, thanks for chasing that down!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68209



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

[Lldb-commits] [lldb] 5312139 - Add a default copy-assignment or copy-constructor for -Wdeprecated-copy warnings.

[Eric Christopher via lldb-commits]

Author: Eric Christopher
Date: 2019-12-04T20:35:32-08:00
New Revision: 5312139f779f9f18cc5fa1c4ce5e5c5c1e854e90

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

LOG: Add a default copy-assignment or copy-constructor for -Wdeprecated-copy 
warnings.

Added: 


Modified: 
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp

Removed: 




diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index cb6fbaa99ea6..a9114aa71b06 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1461,6 +1461,8 @@ class TreeItem {
 return *this;
   }
 
+  TreeItem(const TreeItem &) = default;
+
   size_t GetDepth() const {
 if (m_parent)
   return 1 + m_parent->GetDepth();

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 619c718a1c1b..f6d8d4d9a7eb 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -94,6 +94,8 @@ class MapIterator {
   MapIterator(ValueObject *entry, size_t depth = 0)
   : m_entry(entry), m_max_depth(depth), m_error(false) {}
 
+  MapIterator =(const MapIterator &) = default;
+
   ValueObjectSP value() { return m_entry.GetEntry(); }
 
   ValueObjectSP advance(size_t count) {



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

[Lldb-commits] [PATCH] D70277: [Signal] Allow llvm clients to opt into one-shot SIGPIPE handling

[Vedant Kumar via Phabricator via lldb-commits]

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



Comment at: llvm/lib/Support/Unix/Signals.inc:369
 
-  // Send a special return code that drivers can check for, from 
sysexits.h.
   if (Sig == SIGPIPE)
+if (auto OldOneShotPipeFunction =

aganea wrote:
> @vsk Question question: `SIGPIPE` isn't in the `IntSigs` array anymore 
> (you've removed it at L209), so the above `std::find` will fail when `Sig == 
> SIGPIPE` and this code will never be executed. Most likely this isn't the 
> intended behavior, or I am missing something?
Thanks for catching this, I've fixed this in 9a3f892d018.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70277



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

[Lldb-commits] [lldb] 039d4b3 - [lldb/Reproducers] Don't instrument SBFileSpec::GetPath

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-12-04T18:20:20-08:00
New Revision: 039d4b3aa20a8f36ad058f2b9692b73c6909c612

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

LOG: [lldb/Reproducers] Don't instrument SBFileSpec::GetPath

This method uses a char* and length as output arguments and the
reproducer instrumentation doesn't know how to deal with that (yet).

Added: 


Modified: 
lldb/source/API/SBFileSpec.cpp

Removed: 




diff  --git a/lldb/source/API/SBFileSpec.cpp b/lldb/source/API/SBFileSpec.cpp
index 2f910b9ba294..2e7eba42bc90 100644
--- a/lldb/source/API/SBFileSpec.cpp
+++ b/lldb/source/API/SBFileSpec.cpp
@@ -143,7 +143,7 @@ void SBFileSpec::SetDirectory(const char *directory) {
 }
 
 uint32_t SBFileSpec::GetPath(char *dst_path, size_t dst_len) const {
-  LLDB_RECORD_METHOD_CONST(uint32_t, SBFileSpec, GetPath, (char *, size_t),
+  LLDB_RECORD_DUMMY(uint32_t, SBFileSpec, GetPath, (char *, size_t),
dst_path, dst_len);
 
   uint32_t result = m_opaque_up->GetPath(dst_path, dst_len);



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

[Lldb-commits] [lldb] 6ee96dd - [lldb/Reproducers] Add missing instrumentation for SBFile (2/2)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-12-04T18:20:20-08:00
New Revision: 6ee96ddec89774a534ec93de7266a0bf38de07e8

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

LOG: [lldb/Reproducers] Add missing instrumentation for SBFile (2/2)

Found another issue while running TestDefaultConstructorForAPIObjects.

Added: 


Modified: 
lldb/source/API/SBFile.cpp

Removed: 




diff  --git a/lldb/source/API/SBFile.cpp b/lldb/source/API/SBFile.cpp
index 215e82cd46ac..277402f31abf 100644
--- a/lldb/source/API/SBFile.cpp
+++ b/lldb/source/API/SBFile.cpp
@@ -100,17 +100,17 @@ SBError SBFile::Close() {
 
 SBFile::operator bool() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBFile, operator bool);
-  return LLDB_RECORD_RESULT(IsValid());
+  return IsValid();
 }
 
 bool SBFile::operator!() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBFile, operator!);
-  return LLDB_RECORD_RESULT(!IsValid());
+  return !IsValid();
 }
 
 FileSP SBFile::GetFile() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(FileSP, SBFile, GetFile);
-  return m_opaque_sp;
+  return LLDB_RECORD_RESULT(m_opaque_sp);
 }
 
 namespace lldb_private {



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

[Lldb-commits] [lldb] 3151d7a - Clear out the python class name in OptionParsingStarted for the OptionGroupPythonClassWithDict

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2019-12-04T17:40:57-08:00
New Revision: 3151d7af72bee375c06318195870942d4bc12002

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

LOG: Clear out the python class name in OptionParsingStarted for the 
OptionGroupPythonClassWithDict
options class.  This value was hanging around so for instance if you made a 
scripted breakpoint
resolver, then went to set another breakpoint, it would still think you had 
passed in a class
name and the breakpoint wouldn't do what you expected.

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
 
b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
index 4842bc094551..817d7de6bb96 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
@@ -33,8 +33,7 @@ def test_search_depths(self):
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 def test_command_line(self):
-""" Make sure we are called at the right depths depending on what we 
return
-from __get_depth__"""
+""" Test setting a resolver breakpoint from the command line """
 self.build()
 self.do_test_cli()
 
@@ -202,6 +201,23 @@ def do_test_cli(self):
 
 lldbutil.run_break_set_by_script(self, "resolver.Resolver", 
extra_options="-k symbol -v break_on_me")
 
+# Make sure setting a resolver breakpoint doesn't pollute further 
breakpoint setting
+# by checking the description of a regular file & line breakpoint to 
make sure it
+# doesn't mention the Python Resolver function:
+bkpt_no = lldbutil.run_break_set_by_file_and_line(self, "main.c", 12)
+bkpt = target.FindBreakpointByID(bkpt_no)
+strm = lldb.SBStream()
+bkpt.GetDescription(strm, False)
+used_resolver = "I am a python breakpoint resolver" in strm.GetData()
+self.assertFalse(used_resolver, "Found the resolver description in the 
file & line breakpoint description.")
+
+# Also make sure the breakpoint was where we expected:
+bp_loc = bkpt.GetLocationAtIndex(0)
+bp_sc = 
bp_loc.GetAddress().GetSymbolContext(lldb.eSymbolContextEverything)
+bp_se = bp_sc.GetLineEntry()
+self.assertEqual(bp_se.GetLine(), 12, "Got the right line number")
+self.assertEqual(bp_se.GetFileSpec().GetFilename(), "main.c", "Got the 
right filename")
+
 def do_test_bad_options(self):
 target = self.make_target_and_import()
 

diff  --git a/lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp 
b/lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp
index 20a7ed1f76ca..e41f9d7b40ee 100644
--- a/lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp
+++ b/lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp
@@ -127,6 +127,7 @@ void OptionGroupPythonClassWithDict::OptionParsingStarting(
   // the user didn't pass any -k -v pairs.  We want to be able to warn if these
   // were passed when the function they passed won't use them.
   m_dict_sp.reset();
+  m_name.clear();
 }
 
 Status OptionGroupPythonClassWithDict::OptionParsingFinished(



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

[Lldb-commits] [lldb] fe5ab6d - [lldb/Reproducers] Add missing instrumentation for SBFile

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-12-04T17:37:21-08:00
New Revision: fe5ab6d2cba1ddeb78567ac7c897cd30593ad366

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

LOG: [lldb/Reproducers] Add missing instrumentation for SBFile

This was properly captured by the instrumentation framework when running
TestRunCommandInterpreterAPI.py in capture-mode.

Added: 


Modified: 
lldb/source/API/SBDebugger.cpp
lldb/source/API/SBFile.cpp

Removed: 




diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 82dc60489008..090a3a57a2f4 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -294,7 +294,7 @@ void SBDebugger::SetInputFileHandle(FILE *fh, bool 
transfer_ownership) {
 
 SBError SBDebugger::SetInputFile(FileSP file_sp) {
   LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputFile, (FileSP), file_sp);
-  return SetInputFile(SBFile(file_sp));
+  return LLDB_RECORD_RESULT(SetInputFile(SBFile(file_sp)));
 }
 
 // Shouldn't really be settable after initialization as this could cause lots
@@ -306,7 +306,7 @@ SBError SBDebugger::SetInputFile(SBFile file) {
   SBError error;
   if (!m_opaque_sp) {
 error.ref().SetErrorString("invalid debugger");
-return error;
+return LLDB_RECORD_RESULT(error);
   }
 
   repro::DataRecorder *recorder = nullptr;
@@ -330,16 +330,16 @@ SBError SBDebugger::SetInputFile(SBFile file) {
 
   if (!file_sp || !file_sp->IsValid()) {
 error.ref().SetErrorString("invalid file");
-return error;
+return LLDB_RECORD_RESULT(error);
   }
 
   m_opaque_sp->SetInputFile(file_sp, recorder);
-  return error;
+  return LLDB_RECORD_RESULT(error);
 }
 
 SBError SBDebugger::SetOutputFile(FileSP file_sp) {
   LLDB_RECORD_METHOD(SBError, SBDebugger, SetOutputFile, (FileSP), file_sp);
-  return SetOutputFile(SBFile(file_sp));
+  return LLDB_RECORD_RESULT(SetOutputFile(SBFile(file_sp)));
 }
 
 void SBDebugger::SetOutputFileHandle(FILE *fh, bool transfer_ownership) {
@@ -353,14 +353,14 @@ SBError SBDebugger::SetOutputFile(SBFile file) {
   SBError error;
   if (!m_opaque_sp) {
 error.ref().SetErrorString("invalid debugger");
-return error;
+return LLDB_RECORD_RESULT(error);
   }
   if (!file) {
 error.ref().SetErrorString("invalid file");
-return error;
+return LLDB_RECORD_RESULT(error);
   }
   m_opaque_sp->SetOutputFile(file.m_opaque_sp);
-  return error;
+  return LLDB_RECORD_RESULT(error);
 }
 
 void SBDebugger::SetErrorFileHandle(FILE *fh, bool transfer_ownership) {
@@ -371,7 +371,7 @@ void SBDebugger::SetErrorFileHandle(FILE *fh, bool 
transfer_ownership) {
 
 SBError SBDebugger::SetErrorFile(FileSP file_sp) {
   LLDB_RECORD_METHOD(SBError, SBDebugger, SetErrorFile, (FileSP), file_sp);
-  return SetErrorFile(SBFile(file_sp));
+  return LLDB_RECORD_RESULT(SetErrorFile(SBFile(file_sp)));
 }
 
 SBError SBDebugger::SetErrorFile(SBFile file) {
@@ -379,14 +379,14 @@ SBError SBDebugger::SetErrorFile(SBFile file) {
   SBError error;
   if (!m_opaque_sp) {
 error.ref().SetErrorString("invalid debugger");
-return error;
+return LLDB_RECORD_RESULT(error);
   }
   if (!file) {
 error.ref().SetErrorString("invalid file");
-return error;
+return LLDB_RECORD_RESULT(error);
   }
   m_opaque_sp->SetErrorFile(file.m_opaque_sp);
-  return error;
+  return LLDB_RECORD_RESULT(error);
 }
 
 FILE *SBDebugger::GetInputFileHandle() {
@@ -395,7 +395,7 @@ FILE *SBDebugger::GetInputFileHandle() {
 File _sp = m_opaque_sp->GetInputFile();
 return LLDB_RECORD_RESULT(file_sp.GetStream());
   }
-  return nullptr;
+  return LLDB_RECORD_RESULT(nullptr);
 }
 
 SBFile SBDebugger::GetInputFile() {
@@ -412,7 +412,7 @@ FILE *SBDebugger::GetOutputFileHandle() {
 StreamFile _file = m_opaque_sp->GetOutputStream();
 return LLDB_RECORD_RESULT(stream_file.GetFile().GetStream());
   }
-  return nullptr;
+  return LLDB_RECORD_RESULT(nullptr);
 }
 
 SBFile SBDebugger::GetOutputFile() {
@@ -431,7 +431,7 @@ FILE *SBDebugger::GetErrorFileHandle() {
 StreamFile _file = m_opaque_sp->GetErrorStream();
 return LLDB_RECORD_RESULT(stream_file.GetFile().GetStream());
   }
-  return nullptr;
+  return LLDB_RECORD_RESULT(nullptr);
 }
 
 SBFile SBDebugger::GetErrorFile() {

diff  --git a/lldb/source/API/SBFile.cpp b/lldb/source/API/SBFile.cpp
index f5a38efe4a77..215e82cd46ac 100644
--- a/lldb/source/API/SBFile.cpp
+++ b/lldb/source/API/SBFile.cpp
@@ -117,7 +117,10 @@ namespace lldb_private {
 namespace repro {
 
 template <> void RegisterMethods(Registry ) {
-
+  LLDB_REGISTER_CONSTRUCTOR(SBFile, ());
+  LLDB_REGISTER_CONSTRUCTOR(SBFile, (FileSP));
+  LLDB_REGISTER_CONSTRUCTOR(SBFile, (FILE *, bool));
+  LLDB_REGISTER_CONSTRUCTOR(SBFile, (int, const char *, bool));
   

[Lldb-commits] [lldb] dfe9a79 - [lldb/Reproducers] Override capture with LLDB_CAPTURE_REPRODUCER env var

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-12-04T16:49:11-08:00
New Revision: dfe9a7943bf7a926e51b319a2c2f73e4b4b4cf43

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

LOG: [lldb/Reproducers] Override capture with LLDB_CAPTURE_REPRODUCER env var

Make it possible to override reproducer capture with the
LLDB_CAPTURE_REPRODUCER environment variable.

The goal of this change is twofold.

(1) I want to be able to enable capturing reproducers during regular
test runs, both locally and on the bots. To do so I need a way to
force capture. I cannot do this through the Python API, because
reproducer capture must be enabled *before* the debugger
initialized, which happens automatically when doing `import lldb`.

(2) I want to provide an escape hatch for when reproducers are enabled
by default. Downstream we have reproducer capture enabled by default
in the driver.

This patch solves both problems by overriding the reproducer mode based
on the environment variable. Acceptable values are 0/1 and ON/OFF.

Added: 
lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test

Modified: 
lldb/source/Utility/Reproducer.cpp
lldb/test/Shell/Reproducer/lit.local.cfg

Removed: 




diff  --git a/lldb/source/Utility/Reproducer.cpp 
b/lldb/source/Utility/Reproducer.cpp
index e0806f5f5981..8a28e9b13675 100644
--- a/lldb/source/Utility/Reproducer.cpp
+++ b/lldb/source/Utility/Reproducer.cpp
@@ -25,6 +25,16 @@ llvm::Error Reproducer::Initialize(ReproducerMode mode,
   lldbassert(!InstanceImpl() && "Already initialized.");
   InstanceImpl().emplace();
 
+  // The environment can override the capture mode.
+  if (mode != ReproducerMode::Replay) {
+std::string env =
+llvm::StringRef(getenv("LLDB_CAPTURE_REPRODUCER")).lower();
+if (env == "0" || env == "off")
+  mode = ReproducerMode::Off;
+else if (env == "1" || env == "on")
+  mode = ReproducerMode::Capture;
+  }
+
   switch (mode) {
   case ReproducerMode::Capture: {
 if (!root) {

diff  --git a/lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test 
b/lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test
new file mode 100644
index ..a8e7bdec250e
--- /dev/null
+++ b/lldb/test/Shell/Reproducer/TestCaptureEnvOverride.test
@@ -0,0 +1,20 @@
+# UNSUPPORTED: system-windows
+# This tests the LLDB_CAPTURE_REPRODUCER override.
+
+# RUN: %lldb -b -o 'reproducer status' --capture --capture-path %t.repro 
/bin/ls | FileCheck %s --check-prefix CAPTURE
+# RUN: %lldb -b -o 'reproducer status' --capture | FileCheck %s --check-prefix 
CAPTURE
+
+# RUN: LLDB_CAPTURE_REPRODUCER=1 %lldb -b -o 'reproducer status' | FileCheck 
%s --check-prefix CAPTURE
+# RUN: LLDB_CAPTURE_REPRODUCER=ON %lldb -b -o 'reproducer status' | FileCheck 
%s --check-prefix CAPTURE
+# RUN: LLDB_CAPTURE_REPRODUCER=on %lldb -b -o 'reproducer status' | FileCheck 
%s --check-prefix CAPTURE
+
+# RUN: LLDB_CAPTURE_REPRODUCER=0 %lldb -b -o 'reproducer status' --capture 
--capture-path %t.repro | FileCheck %s --check-prefix OFF
+# RUN: LLDB_CAPTURE_REPRODUCER=0 %lldb -b -o 'reproducer status' --capture | 
FileCheck %s --check-prefix OFF
+# RUN: LLDB_CAPTURE_REPRODUCER=OFF %lldb -b -o 'reproducer status' --capture 
--capture-path %t.repro | FileCheck %s --check-prefix OFF
+# RUN: LLDB_CAPTURE_REPRODUCER=off %lldb -b -o 'reproducer status' --capture | 
FileCheck %s --check-prefix OFF
+
+# RUN: LLDB_CAPTURE_REPRODUCER=bogus %lldb -b -o 'reproducer status' --capture 
| FileCheck %s --check-prefix CAPTURE
+# RUN: LLDB_CAPTURE_REPRODUCER=bogus %lldb -b -o 'reproducer status' | 
FileCheck %s --check-prefix OFF
+
+# CAPTURE: Reproducer is in capture mode.
+# OFF: Reproducer is off.

diff  --git a/lldb/test/Shell/Reproducer/lit.local.cfg 
b/lldb/test/Shell/Reproducer/lit.local.cfg
index 5659f1baa06d..7386b8655d1e 100644
--- a/lldb/test/Shell/Reproducer/lit.local.cfg
+++ b/lldb/test/Shell/Reproducer/lit.local.cfg
@@ -1,2 +1,3 @@
 # Enable crash reports for the reproducer tests.
 del config.environment['LLVM_DISABLE_CRASH_REPORT']
+del config.environment['LLDB_CAPTURE_REPRODUCER']



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

[Lldb-commits] [lldb] acda2bc - [lldb/Reproducers] Propagate LLDB_CAPTURE_REPRODUCER to the test suite

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-12-04T16:49:11-08:00
New Revision: acda2bc0adf62d9e54bf6d284f91e6cfa5b3cc6e

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

LOG: [lldb/Reproducers] Propagate LLDB_CAPTURE_REPRODUCER to the test suite

Added: 


Modified: 
lldb/test/API/lit.cfg.py
lldb/test/Shell/Reproducer/lit.local.cfg
lldb/test/Shell/lit.cfg.py

Removed: 




diff  --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index 06125a1aaedd..9b1c3c12f172 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -54,6 +54,11 @@ def find_shlibpath_var():
 lit_config.warning("unable to inject shared library path on '{}'".format(
 platform.system()))
 
+# Propagate LLDB_CAPTURE_REPRODUCER
+if 'LLDB_CAPTURE_REPRODUCER' in os.environ:
+  config.environment['LLDB_CAPTURE_REPRODUCER'] = os.environ[
+  'LLDB_CAPTURE_REPRODUCER']
+
 # 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.

diff  --git a/lldb/test/Shell/Reproducer/lit.local.cfg 
b/lldb/test/Shell/Reproducer/lit.local.cfg
index 7386b8655d1e..dbb37b199d78 100644
--- a/lldb/test/Shell/Reproducer/lit.local.cfg
+++ b/lldb/test/Shell/Reproducer/lit.local.cfg
@@ -1,3 +1,6 @@
 # Enable crash reports for the reproducer tests.
-del config.environment['LLVM_DISABLE_CRASH_REPORT']
-del config.environment['LLDB_CAPTURE_REPRODUCER']
+if 'LLVM_DISABLE_CRASH_REPORT' in config.environment:
+  del config.environment['LLVM_DISABLE_CRASH_REPORT']
+
+if 'LLDB_CAPTURE_REPRODUCER' in config.environment:
+  del config.environment['LLDB_CAPTURE_REPRODUCER']

diff  --git a/lldb/test/Shell/lit.cfg.py b/lldb/test/Shell/lit.cfg.py
index 84c5b730dd31..68891e600169 100644
--- a/lldb/test/Shell/lit.cfg.py
+++ b/lldb/test/Shell/lit.cfg.py
@@ -38,6 +38,10 @@
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.lldb_obj_root, 'test')
 
+# Propagate LLDB_CAPTURE_REPRODUCER
+if 'LLDB_CAPTURE_REPRODUCER' in os.environ:
+  config.environment['LLDB_CAPTURE_REPRODUCER'] = os.environ[
+  'LLDB_CAPTURE_REPRODUCER']
 
 llvm_config.use_default_substitutions()
 toolchain.use_lldb_substitutions(config)



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

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Nice!




Comment at: lldb/source/Expression/DWARFExpression.cpp:57
+
+static std::unique_ptr
+GetLocationTable(DWARFExpression::LocationListFormat format, const 
DataExtractor ) {

Doxygen comment?



Comment at: lldb/source/Expression/DWARFExpression.cpp:132
+namespace {
+class DummyDWARFObject final: public llvm::DWARFObject {
+public:

Doxygen comment: what is this used for?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71003



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

[Lldb-commits] [lldb] e1a7d04 - Add parray example for lldb, vrs. *ptr@count gdb cmd.

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2019-12-04T15:44:15-08:00
New Revision: e1a7d042c36509a385668ee04ddb3dad3241f503

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

LOG: Add parray example for lldb, vrs. *ptr@count gdb cmd.

Added: 


Modified: 
lldb/docs/use/map.rst

Removed: 




diff  --git a/lldb/docs/use/map.rst b/lldb/docs/use/map.rst
index d878b5633e83..3c6c6e6ffc62 100644
--- a/lldb/docs/use/map.rst
+++ b/lldb/docs/use/map.rst
@@ -880,6 +880,20 @@ Examining Variables

  
 
+ 
+   Print an array of integers in 
memory, assuming we have a pointer like "int *ptr".
+ 
+ 
+   
+  (gdb) p *ptr@10
+  
+   
+   
+  (lldb) parray 10 ptr
+  
+   
+ 
+
   

 



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

[Lldb-commits] [lldb] e001bf6 - Add help text for parray and poarray aliases.

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2019-12-04T15:33:54-08:00
New Revision: e001bf6330bb0e935b17c8a619e71bbded67e2eb

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

LOG: Add help text for parray and poarray aliases.

Added: 


Modified: 
lldb/source/Interpreter/CommandInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index e02248148413..5a4e466144a6 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -362,10 +362,23 @@ void CommandInterpreter::Initialize() {
   "controlled by the type's author.");
   po->SetHelpLong("");
 }
-AddAlias("parray", cmd_obj_sp, "--element-count %1 --")->SetHelpLong("");
-AddAlias("poarray", cmd_obj_sp,
- "--object-description --element-count %1 --")
-->SetHelpLong("");
+CommandAlias *parray_alias = AddAlias("parray", cmd_obj_sp, 
+"--element-count %1 --");
+if (parray_alias) {
+parray_alias->SetHelp
+  ("parray   -- lldb will evaluate EXPRESSION "
+   "to get a typed-pointer-to-an-array in memory, and will display "
+   "COUNT elements of that type from the array.");
+parray_alias->SetHelpLong("");
+}
+CommandAlias *poarray_alias = AddAlias("poarray", cmd_obj_sp,
+ "--object-description --element-count %1 --");
+if (poarray_alias) {
+  poarray_alias->SetHelp("poarray   -- lldb will "
+  "evaluate EXPRESSION to get the address of an array of COUNT "
+  "objects in memory, and will call po on them.");
+  poarray_alias->SetHelpLong("");
+}
   }
 
   cmd_obj_sp = GetCommandSPExact("process kill", false);



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

[Lldb-commits] [PATCH] D70277: [Signal] Allow llvm clients to opt into one-shot SIGPIPE handling

[Alexandre Ganea via Phabricator via lldb-commits]

aganea added inline comments.



Comment at: llvm/lib/Support/Unix/Signals.inc:369
 
-  // Send a special return code that drivers can check for, from 
sysexits.h.
   if (Sig == SIGPIPE)
+if (auto OldOneShotPipeFunction =

@vsk Question question: `SIGPIPE` isn't in the `IntSigs` array anymore (you've 
removed it at L209), so the above `std::find` will fail when `Sig == SIGPIPE` 
and this code will never be executed. Most likely this isn't the intended 
behavior, or I am missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70277



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

[Lldb-commits] [lldb] e11df58 - Upstream debugserver arm64e support.

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2019-12-04T15:20:56-08:00
New Revision: e11df585800596df2052a475f6191673b8f1a5c1

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

LOG: Upstream debugserver arm64e support.

The changes are minor; primarily debugserver needs to go through
accessor functions/macros when changing pc/fp/sp/lr, and debugserver
needs to clear any existing pointer auth bits from values in two
cases.  debugserver can fetch the number of bits used for addressing
from a sysctl, and will include that in the qHostInfo reply.  Update
qHostInfo documentation to document it.

Added: 


Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index e3f11488df64..06cd09d77c41 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -790,6 +790,13 @@ distribution_id: optional. For linux, specifies 
distribution id (e.g. ubuntu, fe
 osmajor: optional, specifies the major version number of the OS (e.g. for 
macOS 10.12.2, it would be 10)
 osminor: optional, specifies the minor version number of the OS (e.g. for 
macOS 10.12.2, it would be 12)
 ospatch: optional, specifies the patch level number of the OS (e.g. for macOS 
10.12.2, it would be 2)
+addressing_bits: optional, specifies how many bits in addresses are
+significant for addressing, base 10.  If bits 38..0
+in a 64-bit pointer are significant for addressing,
+then the value is 39.  This is needed on e.g. Aarch64
+v8.3 ABIs that use pointer authentication, so lldb
+knows which bits to clear/set to get the actual
+addresses.
 
 //--
 // "qGDBServerVersion"

diff  --git a/lldb/tools/debugserver/source/DNB.cpp 
b/lldb/tools/debugserver/source/DNB.cpp
index c9f2e34e2798..8d9c691f9d33 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -1722,6 +1722,8 @@ nub_bool_t DNBSetArchitecture(const char *arch) {
 else if (strstr(arch, "arm64_32") == arch || 
  strstr(arch, "aarch64_32") == arch)
   return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM64_32);
+else if (strstr(arch, "arm64e") == arch)
+  return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM64);
 else if (strstr(arch, "arm64") == arch || strstr(arch, "armv8") == arch ||
  strstr(arch, "aarch64") == arch)
   return DNBArchProtocol::SetArchitecture(CPU_TYPE_ARM64);

diff  --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp 
b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index 1bf14d97056c..e8c40910567c 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -26,6 +26,10 @@
 #include 
 #include 
 
+#if __has_feature(ptrauth_calls)
+#include 
+#endif
+
 // Break only in privileged or user mode
 // (PAC bits in the DBGWVRn_EL1 watchpoint control register)
 #define S_USER ((uint32_t)(2u << 1))
@@ -93,7 +97,11 @@ uint32_t DNBArchMachARM64::GetCPUType() { return 
CPU_TYPE_ARM64; }
 uint64_t DNBArchMachARM64::GetPC(uint64_t failValue) {
   // Get program counter
   if (GetGPRState(false) == KERN_SUCCESS)
+#if defined(__LP64__)
+return arm_thread_state64_get_pc(m_state.context.gpr);
+#else
 return m_state.context.gpr.__pc;
+#endif
   return failValue;
 }
 
@@ -101,7 +109,17 @@ kern_return_t DNBArchMachARM64::SetPC(uint64_t value) {
   // Get program counter
   kern_return_t err = GetGPRState(false);
   if (err == KERN_SUCCESS) {
+#if defined(__LP64__)
+#if __has_feature(ptrauth_calls)
+// The incoming value could be garbage.  Strip it to avoid
+// trapping when it gets resigned in the thread state.
+value = (uint64_t) ptrauth_strip((void*) value, 
ptrauth_key_function_pointer);
+value = (uint64_t) ptrauth_sign_unauthenticated((void*) value, 
ptrauth_key_function_pointer, 0);
+#endif
+arm_thread_state64_set_pc_fptr (m_state.context.gpr, (void*) value);
+#else
 m_state.context.gpr.__pc = value;
+#endif
 err = SetGPRState();
   }
   return err == KERN_SUCCESS;
@@ -110,7 +128,11 @@ kern_return_t DNBArchMachARM64::SetPC(uint64_t value) {
 uint64_t DNBArchMachARM64::GetSP(uint64_t failValue) {
   // Get stack pointer
   if (GetGPRState(false) == KERN_SUCCESS)
+#if defined(__LP64__)
+return arm_thread_state64_get_sp(m_state.context.gpr);
+#else
 return m_state.context.gpr.__sp;
+#endif
   return failValue;
 }
 
@@ 

[Lldb-commits] [PATCH] D70992: Replacing to use of ul suffix in GetMaxU64Bitfield since it not guarenteed to be 64 bit

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Utility/DataExtractor.cpp:595
+  (bitfield_bit_size == 64
+   ? -static_cast(1)
+   : ((static_cast(1) << bitfield_bit_size) - 1));

teemperor wrote:
> How about `std::numeric_limits::max()`?
Good call, I thought about this change too.


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

https://reviews.llvm.org/D70992



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

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

[Chris Bieneman via Phabricator via lldb-commits]

beanz added a comment.

@labath sorry for not replying on-list. I've actually been discussing offline 
with @compnerd. If `llvm-config` is printing an absolute path, I'm concerned 
about how that will interact with binary package distributions for the llvm-dev 
packages. Not sure if @tstellar has any thoughts on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70764



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

[Lldb-commits] [PATCH] D70764: build: reduce CMake handling for zlib

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

`/usr/lib/x86_64-linux-gnu/libz.so` is definitely better than 
`-l/usr/lib/x86_64-linux-gnu/libz.so`, as it's at least a valid link line. It's 
not completely equivalent, and may not work in all cases -- last time I 
accidentally changed this, I got about a dozen emails from people I broke, and 
I wouldn't be surprised if one of them depended on the subtle differences here. 
However, the fact that libxml is also spelled out this way gives me some hope...

That said, this still leaves us with the "problem" that the default cmake 
configuration will be broken for people who don't have zlib installed (pretty 
much everyone on windows, at least). You can try that out by 
deleting/renaming/uninstalling zlib.h from your system. With this patch you get 
a configuration error whereas previously zlib support was automatically 
disabled. Whether this "problem" is a **problem** or "working as intended" 
depends on what do you expect out of your build system, but it's definitely not 
consistent with how other llvm dependencies are managed.

Anyway, I don't really have a horse in this race, but I figured I should at 
least warn you about the problems you're likely to run into. (and I really wish 
@beanz would say something here, as he's the one who's complaining about build 
system inconsistencies...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70764



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

[Lldb-commits] [PATCH] D70644: [DebugInfo] Support for DW_OP_implicit_pointer (llvm.dbg_derefval)

[Stephen Tozer via Phabricator via lldb-commits]

StephenTozer added inline comments.



Comment at: llvm/docs/SourceLevelDebugging.rst:270
+
+An `llvm.dbg.derefval` intrinsic is usefull when location which the variable
+points to is optimized out, but the dereferenced value is known.

Correct usefull -> useful


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70644



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

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

I'd recommend to be careful when making a change of this size. There are a lot 
of possible configurations.
If you split it up into a few smaller and more dedicated commits, you will make 
your life easier. Otherwise someone might just revert everything if this 
happens to break a bot.

In D69589#1736299 , @aadsm wrote:

> Yeah, weird. I wonder if this is something cmake does implicitly when the 
> target is defined as `FRAMEWORK`: 
> https://cmake.org/cmake/help/latest/prop_tgt/FRAMEWORK.html


This is correct. There is a comment mentioning it here:
https://github.com/llvm/llvm-project/blob/276a5b2d5f1f/lldb/cmake/modules/LLDBFramework.cmake#L44




Comment at: lldb/CMakeLists.txt:204
 
-  function(create_relative_symlink target dest_file output_dir output_name)
+  function(add_relative_symlink_target name dest_file output_dir output_name)
 get_filename_component(dest_file ${dest_file} ABSOLUTE)

This change e.g. is reasonable, but unrelated. It could go into a separate NFC 
commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69589



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

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D70846#1767455 , @labath wrote:

> I guess you were building darwin binaries, right? If that's the case, then 
> you weren't using this code at all, as apple uses AppleIndex by default. The 
> test in question uses all three indexes (it forces their generation by 
> altering compile flags) and the failures you were seeing were most likely 
> coming the "manual" case...


Correct, so I understand now. So with `-accel-tables=Disable` we will get less 
results.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70846



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

[Lldb-commits] [PATCH] D70848: [LLDB] Set the right address size on output DataExtractors from ObjectFile

[Martin Storsjö via Phabricator via lldb-commits]

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



Comment at: lldb/source/Symbol/ObjectFile.cpp:480-486
+  size_t ret = data.SetData(m_data, offset, length);
+  // DataExtractor::SetData copies the address byte size from m_data, but
+  // m_data's address byte size is only set from sizeof(void*), and we can't
+  // access subclasses GetAddressByteSize() when setting up m_data in the
+  // constructor.
+  data.SetAddressByteSize(GetAddressByteSize());
+  return ret;

mstorsjo wrote:
> labath wrote:
> > mstorsjo wrote:
> > > clayborg wrote:
> > > > I would vote to make this happen within DataExtractor::SetData(const 
> > > > DataExtractor &...)
> > > Do you mean that we'd extend `DataExtractor::SetData(const DataExtractor 
> > > &...)` to take a byte address size parameter, or that we'd update 
> > > `m_data`'s byte address size before doing `data.SetData()` here?
> > > 
> > > Ideally I'd set the right byte address size in `m_data` as soon as it is 
> > > known and available. It's not possible to do this in `ObjectFile`'s 
> > > constructor, as that is called before the subclass is constructed and its 
> > > virtual methods are available, but is there a better point in the 
> > > lifetime where we could update it?
> > I too would prefer that, but I couldn't see a way to achieve that (which is 
> > why I stamped this version).
> > 
> > Today, with a fresh set of eyes, I think it may be reasonable to have this 
> > happen in `ObjectFile::ParseHeader`. After the header has been parsed, all 
> > object formats (I hope) should be able to determine their address size and 
> > endianness, and the operating invariant would be that the address size is 
> > only valid after the ParseHeader has been called. WDYT?
> Sounds sensible! I'll give it a shot.
`ObjectFile::ParseHeader` is currently pure virtual, and thus the subclass 
concrete implementations of it don't call the base class version of the method. 
Do you want me to make the base class method non-pure and add calls to it in 
the subclasses, or add calls to 
`m_data.SetAddressByteSize(GetAddressByteSize());` in all of the subclasses 
`ParseHeader`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70848



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

[Lldb-commits] [PATCH] D70155: [LLDB] Avoid triple corruption while merging core info from platform and target triples

[Jason Molenda via Phabricator via lldb-commits]

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

Thanks for the ping, yes this looks good to me.


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

https://reviews.llvm.org/D70155



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

[Lldb-commits] [PATCH] D71022: [lldb] NFC: less nesting in SearchFilter.cpp

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.

I have one minor request (see inline comments), but otherwise LGTM, thanks!




Comment at: lldb/source/Core/SearchFilter.cpp:777
 lldb::ModuleSP module_sp = target_images.GetModuleAtIndexUnlocked(i);
-if (no_modules_in_filter ||
+if (!(no_modules_in_filter ||
 m_module_spec_list.FindFileIndex(0, module_sp->GetFileSpec(), false) !=

Simplify with de Morgan (or maybe we could get rid of the 
`no_modules_in_filter` as this just seems to be an "optimization" to avoid the 
`FindFileIndex` function call, but just getting rid of the `!(...)` is fine).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71022



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

[Lldb-commits] [PATCH] D70848: [LLDB] Set the right address size on output DataExtractors from ObjectFile

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo marked an inline comment as done.
mstorsjo added a comment.

In D70848#1768730 , @labath wrote:

> BTW, I don't know if you've noticed, but the new test is failing on windows: 
> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/11217. It 
> looks like a path separator problem. We actually already have code which 
> tries to guess the path style of a compile unit, but it is keyed off of the 
> DW_AT_comp_dir attribute, which it looks like you stripped from the test 
> case. My guess is that adding this attribute back will fix this problem...


I did notice it, and tried to fix it 
(https://reviews.llvm.org/rG7d019d1a3be252a885e8db1ee7af11c90), but I forgot to 
check that it actually worked - sorry about that. Apparently I got the 
backslash escaping wrong (from some clang test). I'll see if DW_AT_comp_dir 
helps (and remove that failed regex fix), or fix the backslash matching.




Comment at: lldb/source/Symbol/ObjectFile.cpp:480-486
+  size_t ret = data.SetData(m_data, offset, length);
+  // DataExtractor::SetData copies the address byte size from m_data, but
+  // m_data's address byte size is only set from sizeof(void*), and we can't
+  // access subclasses GetAddressByteSize() when setting up m_data in the
+  // constructor.
+  data.SetAddressByteSize(GetAddressByteSize());
+  return ret;

labath wrote:
> mstorsjo wrote:
> > clayborg wrote:
> > > I would vote to make this happen within DataExtractor::SetData(const 
> > > DataExtractor &...)
> > Do you mean that we'd extend `DataExtractor::SetData(const DataExtractor 
> > &...)` to take a byte address size parameter, or that we'd update 
> > `m_data`'s byte address size before doing `data.SetData()` here?
> > 
> > Ideally I'd set the right byte address size in `m_data` as soon as it is 
> > known and available. It's not possible to do this in `ObjectFile`'s 
> > constructor, as that is called before the subclass is constructed and its 
> > virtual methods are available, but is there a better point in the lifetime 
> > where we could update it?
> I too would prefer that, but I couldn't see a way to achieve that (which is 
> why I stamped this version).
> 
> Today, with a fresh set of eyes, I think it may be reasonable to have this 
> happen in `ObjectFile::ParseHeader`. After the header has been parsed, all 
> object formats (I hope) should be able to determine their address size and 
> endianness, and the operating invariant would be that the address size is 
> only valid after the ParseHeader has been called. WDYT?
Sounds sensible! I'll give it a shot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70848



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

[Lldb-commits] [PATCH] D71022: [lldb] NFC: less nesting in SearchFilter.cpp

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Core/SearchFilter.cpp:625
  modules_array);
   FileSpecList modules;
+  if (!success)

I'd move this down and use 
```
  if (!success)
return 
std::make_shared(target.shared_from_this(),{});

```
to make it clear the list of modules is empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71022



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

[Lldb-commits] [PATCH] D70992: Replacing to use of ul suffix in GetMaxU64Bitfield since it not guarenteed to be 64 bit

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.

LGTM minus a small nitpick. Thanks!




Comment at: lldb/source/Utility/DataExtractor.cpp:595
+  (bitfield_bit_size == 64
+   ? -static_cast(1)
+   : ((static_cast(1) << bitfield_bit_size) - 1));

How about `std::numeric_limits::max()`?


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

https://reviews.llvm.org/D70992



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

[Lldb-commits] [PATCH] D71021: [lldb/DWARF] Switch to llvm debug_rnglists parser

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Nice!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71021



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

[Lldb-commits] [PATCH] D70721: [lldb/cpluspluslanguage] Add constructor substitutor

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik accepted this revision.
shafik added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:352
+  void appendUnchangedInput() {
+Result += llvm::StringRef(Written, this->First - Written);
+Written = this->First;

labath wrote:
> shafik wrote:
> > `this->First - Written` feels awkward, I feel like given the names they 
> > should be reversed :-(
> Yeah, it's a bit weird, but I'm not sure what to do about it.. do you have 
> any specific suggestion? `std::distance(Written, First)` ? adding a member 
> function like `currentParserPos()` to wrap the `First`?
Yes, I think `currentParserPos()` would be helpful, it would at least clarify 
the intent and it makes it more obvious it is doing the correct thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70721



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

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Expression/DWARFExpression.h:260
 
-  bool GetLocation(lldb::addr_t func_load_addr, lldb::addr_t pc,
-   lldb::offset_t , lldb::offset_t );
+  void RelocateLowHighPC(lldb::addr_t load_function_start, lldb::addr_t 
_pc,
+ lldb::addr_t _pc) const;

Should this be part of the DWARFExpression API? It feels like a static function 
might be sufficient. Maybe it could take a range directly?



Comment at: lldb/source/Expression/DWARFExpression.cpp:53
+  if (data.ValidOffsetForDataOfSize(offset, index_size))
+return data.GetMaxU64_unchecked(, index_size);
+  return LLDB_INVALID_ADDRESS;

Why the unchecked?



Comment at: lldb/source/Expression/DWARFExpression.cpp:158
+
+llvm::MCRegisterInfo *MRI = nullptr;
+if (abi)

``` llvm::MCRegisterInfo *MRI = abi ? >GetMCRegisterInfo() : nullptr;```



Comment at: lldb/source/Expression/DWARFExpression.cpp:2799
+addr_t _pc) const {
+  // This relocates low_pc and high_pc by adding the difference between the
+  // function file address, and the actual address it is loaded in memory.

Should this be a doxygen comment on the function itself?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71003



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

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:2829
+if (!loc)
+  LLDB_LOG_ERROR(log, loc.takeError(), "{0}");
+if (loc->Range) {

labath wrote:
> labath wrote:
> > dblaikie wrote:
> > > Does LLDB_LOG_ERROR stop the program? If not, then the next line ("if 
> > > (loc->Range)" will invoke UB when it tries to dereference the "loc" when 
> > > it is unset.
> > It doesn't. You're right, there should be a return here, just let me see if 
> > I can tickle this into exploding.
> It turns out triggering this was pretty hard, because the main source of 
> these errors is a failed indirect address resolution, but our callback never 
> returned any errors. I've now fixed it to return errors and included an 
> additional test.
Looks great!

That's about all the thoughts I have on the patch - will leave the rest to LLDB 
developers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71003



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

[Lldb-commits] [PATCH] D71022: [lldb] NFC: less nesting in SearchFilter.cpp

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a reviewer: jdoerfert.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
kwk edited the summary of this revision.

I was working on SearchFilter.cpp and felt it a bit too complex in some cases 
in terms of nesting and logic flow.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71022

Files:
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -208,10 +208,12 @@
 return;
   empty_sc.target_sp = m_target_sp;
 
-  if (searcher.GetDepth() == lldb::eSearchDepthTarget)
+  if (searcher.GetDepth() == lldb::eSearchDepthTarget) {
 searcher.SearchCallback(*this, empty_sc, nullptr);
-  else
-DoModuleIteration(empty_sc, searcher);
+return;
+  }
+  
+  DoModuleIteration(empty_sc, searcher);
 }
 
 void SearchFilter::SearchInModuleList(Searcher , ModuleList ) {
@@ -221,20 +223,20 @@
 return;
   empty_sc.target_sp = m_target_sp;
 
-  if (searcher.GetDepth() == lldb::eSearchDepthTarget)
+  if (searcher.GetDepth() == lldb::eSearchDepthTarget) {
 searcher.SearchCallback(*this, empty_sc, nullptr);
-  else {
-std::lock_guard guard(modules.GetMutex());
-const size_t numModules = modules.GetSize();
-
-for (size_t i = 0; i < numModules; i++) {
-  ModuleSP module_sp(modules.GetModuleAtIndexUnlocked(i));
-  if (ModulePasses(module_sp)) {
-if (DoModuleIteration(module_sp, searcher) ==
-Searcher::eCallbackReturnStop)
-  return;
-  }
-}
+return;
+  }
+
+  std::lock_guard guard(modules.GetMutex());
+  const size_t numModules = modules.GetSize();
+
+  for (size_t i = 0; i < numModules; i++) {
+ModuleSP module_sp(modules.GetModuleAtIndexUnlocked(i));
+if (!ModulePasses(module_sp))
+  continue;
+if (DoModuleIteration(module_sp, searcher) == Searcher::eCallbackReturnStop)
+  return;
   }
 }
 
@@ -248,45 +250,47 @@
 Searcher::CallbackReturn
 SearchFilter::DoModuleIteration(const SymbolContext ,
 Searcher ) {
-  if (searcher.GetDepth() >= lldb::eSearchDepthModule) {
-if (context.module_sp) {
-  if (searcher.GetDepth() == lldb::eSearchDepthModule) {
-SymbolContext matchingContext(context.module_sp.get());
-searcher.SearchCallback(*this, matchingContext, nullptr);
-  } else {
-return DoCUIteration(context.module_sp, context, searcher);
-  }
+  if (searcher.GetDepth() < lldb::eSearchDepthModule)
+return Searcher::eCallbackReturnContinue;
+
+  if (context.module_sp) {
+if (searcher.GetDepth() != lldb::eSearchDepthModule)
+  return DoCUIteration(context.module_sp, context, searcher);
+
+SymbolContext matchingContext(context.module_sp.get());
+searcher.SearchCallback(*this, matchingContext, nullptr);
+return Searcher::eCallbackReturnContinue;
+  }
+
+  const ModuleList _images = m_target_sp->GetImages();
+  std::lock_guard guard(target_images.GetMutex());
+
+  size_t n_modules = target_images.GetSize();
+  for (size_t i = 0; i < n_modules; i++) {
+// If this is the last level supplied, then call the callback directly,
+// otherwise descend.
+ModuleSP module_sp(target_images.GetModuleAtIndexUnlocked(i));
+if (!ModulePasses(module_sp))
+  continue;
+
+if (searcher.GetDepth() == lldb::eSearchDepthModule) {
+  SymbolContext matchingContext(m_target_sp, module_sp);
+
+  Searcher::CallbackReturn shouldContinue =
+  searcher.SearchCallback(*this, matchingContext, nullptr);
+  if (shouldContinue == Searcher::eCallbackReturnStop ||
+  shouldContinue == Searcher::eCallbackReturnPop)
+return shouldContinue;
 } else {
-  const ModuleList _images = m_target_sp->GetImages();
-  std::lock_guard guard(target_images.GetMutex());
-
-  size_t n_modules = target_images.GetSize();
-  for (size_t i = 0; i < n_modules; i++) {
-// If this is the last level supplied, then call the callback directly,
-// otherwise descend.
-ModuleSP module_sp(target_images.GetModuleAtIndexUnlocked(i));
-if (!ModulePasses(module_sp))
-  continue;
-
-if (searcher.GetDepth() == lldb::eSearchDepthModule) {
-  SymbolContext matchingContext(m_target_sp, module_sp);
-
-  Searcher::CallbackReturn shouldContinue =
-  searcher.SearchCallback(*this, matchingContext, nullptr);
-  if (shouldContinue == Searcher::eCallbackReturnStop ||
-  shouldContinue == Searcher::eCallbackReturnPop)
-return shouldContinue;
-} else {
-  Searcher::CallbackReturn shouldContinue =
-  DoCUIteration(module_sp, context, searcher);
-  if (shouldContinue == Searcher::eCallbackReturnStop)
-return shouldContinue;
-  else if (shouldContinue == 

[Lldb-commits] [PATCH] D70721: [lldb/cpluspluslanguage] Add constructor substitutor

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 232148.
labath marked 3 inline comments as done.
labath added a comment.

Remove unused return value


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70721

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -191,6 +191,8 @@
   EXPECT_THAT(FindAlternate("_ZN1A1fEx"), Contains("_ZN1A1fEl"));
   EXPECT_THAT(FindAlternate("_ZN1A1fEy"), Contains("_ZN1A1fEm"));
   EXPECT_THAT(FindAlternate("_ZN1A1fEai"), Contains("_ZN1A1fEci"));
+  EXPECT_THAT(FindAlternate("_ZN1AC1Ev"), Contains("_ZN1AC2Ev"));
+  EXPECT_THAT(FindAlternate("_ZN1AD1Ev"), Contains("_ZN1AD2Ev"));
   EXPECT_THAT(FindAlternate("_bogus"), IsEmpty());
 }
 
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -284,46 +284,34 @@
   }
 };
 
-/// Given a mangled function `Mangled`, replace all the primitive function type
-/// arguments of `Search` with type `Replace`.
-class TypeSubstitutor
-: public llvm::itanium_demangle::AbstractManglingParser
+class ManglingSubstitutor
+: public llvm::itanium_demangle::AbstractManglingParser {
-  /// Input character until which we have constructed the respective output
-  /// already
-  const char *Written;
+  using Base =
+  llvm::itanium_demangle::AbstractManglingParser;
 
-  llvm::StringRef Search;
-  llvm::StringRef Replace;
-  llvm::SmallString<128> Result;
+public:
+  ManglingSubstitutor() : Base(nullptr, nullptr) {}
 
-  /// Whether we have performed any substitutions.
-  bool Substituted;
+  template
+  ConstString substitute(llvm::StringRef Mangled, Ts &&... Vals) {
+this->getDerived().reset(Mangled, std::forward(Vals)...);
+return substituteImpl(Mangled);
+  }
 
-  void reset(llvm::StringRef Mangled, llvm::StringRef Search,
- llvm::StringRef Replace) {
-AbstractManglingParser::reset(Mangled.begin(), Mangled.end());
+
+protected:
+  void reset(llvm::StringRef Mangled) {
+Base::reset(Mangled.begin(), Mangled.end());
 Written = Mangled.begin();
-this->Search = Search;
-this->Replace = Replace;
 Result.clear();
 Substituted = false;
   }
 
-  void appendUnchangedInput() {
-Result += llvm::StringRef(Written, First - Written);
-Written = First;
-  }
-
-public:
-  TypeSubstitutor() : AbstractManglingParser(nullptr, nullptr) {}
-
-  ConstString substitute(llvm::StringRef Mangled, llvm::StringRef From,
- llvm::StringRef To) {
+  ConstString substituteImpl(llvm::StringRef Mangled) {
 Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE);
-
-reset(Mangled, From, To);
-if (parse() == nullptr) {
+if (this->parse() == nullptr) {
   LLDB_LOG(log, "Failed to substitute mangling in {0}", Mangled);
   return ConstString();
 }
@@ -336,20 +324,66 @@
 return ConstString(Result);
   }
 
+  void trySubstitute(llvm::StringRef From, llvm::StringRef To) {
+if (!llvm::StringRef(this->First, this->numLeft()).startswith(From))
+  return;
+
+// We found a match. Append unmodified input up to this point.
+appendUnchangedInput();
+
+// And then perform the replacement.
+Result += To;
+Written += From.size();
+Substituted = true;
+  }
+
+private:
+  /// Input character until which we have constructed the respective output
+  /// already.
+  const char *Written;
+
+  llvm::SmallString<128> Result;
+
+  /// Whether we have performed any substitutions.
+  bool Substituted;
+
+  void appendUnchangedInput() {
+Result += llvm::StringRef(Written, this->First - Written);
+Written = this->First;
+  }
+
+};
+
+/// Given a mangled function `Mangled`, replace all the primitive function type
+/// arguments of `Search` with type `Replace`.
+class TypeSubstitutor : public ManglingSubstitutor {
+  llvm::StringRef Search;
+  llvm::StringRef Replace;
+
+public:
+  void reset(llvm::StringRef Mangled, llvm::StringRef Search,
+ llvm::StringRef Replace) {
+ManglingSubstitutor::reset(Mangled);
+this->Search = Search;
+this->Replace = Replace;
+  }
+
   llvm::itanium_demangle::Node *parseType() {
-if (llvm::StringRef(First, numLeft()).startswith(Search)) {
-  // We found a match. Append unmodified input up to this point.
-  appendUnchangedInput();
-
-  // And then perform the replacement.
-  Result += Replace;
-  Written += Search.size();
-  Substituted = true;
-}
-

[Lldb-commits] [PATCH] D70721: [lldb/cpluspluslanguage] Add constructor substitutor

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for taking a look at this.




Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:327
 
+  bool trySubstitute(llvm::StringRef From, llvm::StringRef To) {
+if (!llvm::StringRef(this->First, this->numLeft()).startswith(From))

shafik wrote:
> `trySubstitute` has a return value but it is not used in the code below.
Yeah, I couldn't make up my mind on what to do here. Having no indication of 
success from this function seems weird, but OTOH the callers don't actually 
need to actually vary their behavior based on the result. I've now just removed 
the return value...



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:352
+  void appendUnchangedInput() {
+Result += llvm::StringRef(Written, this->First - Written);
+Written = this->First;

shafik wrote:
> `this->First - Written` feels awkward, I feel like given the names they 
> should be reversed :-(
Yeah, it's a bit weird, but I'm not sure what to do about it.. do you have any 
specific suggestion? `std::distance(Written, First)` ? adding a member function 
like `currentParserPos()` to wrap the `First`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70721



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

[Lldb-commits] [PATCH] D71021: [lldb/DWARF] Switch to llvm debug_rnglists parser

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, aprantl, clayborg.
Herald added a project: LLDB.

Our rnglist support was working only for the trivial cases (one CU),
because we only ever parsed one contribution out of the debug_rnglists
section. This means we were never able to resolve range lists for the
second and subsequent units (DW_FORM_sec_offset references came out
blang, and DW_FORM_rnglistx references always used the ranges lists from
the first unit).

Since both llvm and lldb rnglist parsers are sufficiently
self-contained, and operate similarly, we can fix this problem by
switching to the llvm parser instead. Besides the changes which are due
to variations in the interface, the main thing is that now the range
list object is a member of the DWARFUnit, instead of the entire symbol
file. This ensures that each unit can get it's own private set of range
list indices, and is consistent with how llvm's DWARFUnit does it
(overall, I've tried to structure the code the same way as the llvm
version).

I've also added a test case for the two unit scenario.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71021

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s

Index: lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s
===
--- lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s
+++ lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s
@@ -1,12 +1,19 @@
 # REQUIRES: x86
 
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
-# RUN: %lldb %t -o "image lookup -v -s lookup_rnglists" -o exit | FileCheck %s
+# RUN: %lldb %t -o "image lookup -v -s lookup_rnglists" \
+# RUN:   -o "image lookup -v -s lookup_rnglists2" -o exit | FileCheck %s
 
+# CHECK-LABEL: image lookup -v -s lookup_rnglists
 # CHECK:  Function: id = {0x7fff0030}, name = "rnglists", range = [0x-0x0004)
 # CHECK:Blocks: id = {0x7fff0030}, range = [0x-0x0004)
 # CHECK-NEXT:   id = {0x7fff0046}, ranges = [0x0001-0x0002)[0x0003-0x0004)
 
+# CHECK-LABEL: image lookup -v -s lookup_rnglists2
+# CHECK:  Function: id = {0x7fff007a}, name = "rnglists2", range = [0x0004-0x0007)
+# CHECK:Blocks: id = {0x7fff007a}, range = [0x0004-0x0007)
+# CHECK-NEXT:   id = {0x7fff0091}, range = [0x0005-0x0007)
+
 .text
 .p2align 12
 rnglists:
@@ -21,6 +28,15 @@
 .Lblock2_end:
 .Lrnglists_end:
 
+rnglists2:
+nop
+.Lblock3_begin:
+lookup_rnglists2:
+nop
+nop
+.Lblock3_end:
+.Lrnglists2_end:
+
 .section.debug_abbrev,"",@progbits
 .byte   1   # Abbreviation Code
 .byte   17  # DW_TAG_compile_unit
@@ -78,6 +94,28 @@
 .byte   0   # End Of Children Mark
 .Ldebug_info_end0:
 
+.Lcu_begin1:
+.long   .Ldebug_info_end1-.Ldebug_info_start1 # Length of Unit
+.Ldebug_info_start1:
+.short  5   # DWARF version number
+.byte   1   # DWARF Unit Type
+.byte   8   # Address Size (in bytes)
+.long   .debug_abbrev   # Offset Into Abbrev. Section
+.byte   1   # Abbrev [1] 0xc:0x5f DW_TAG_compile_unit
+.asciz  "Hand-written DWARF"# DW_AT_producer
+.quad   rnglists2   # DW_AT_low_pc
+.long   .Lrnglists2_end-rnglists2 # DW_AT_high_pc
+.long   .Lrnglists_table_base1  # DW_AT_rnglists_base
+.byte   2   # Abbrev [2] 0x2b:0x37 DW_TAG_subprogram
+.quad   rnglists2   # DW_AT_low_pc
+.long   .Lrnglists2_end-rnglists2 # DW_AT_high_pc
+.asciz  "rnglists2" # DW_AT_name
+.byte   5   # Abbrev [5] 0x52:0xf DW_TAG_lexical_block
+.byte   0   # DW_AT_ranges
+.byte   0   # End Of Children Mark
+.byte   0   # End Of Children Mark
+.Ldebug_info_end1:
+
 .section.debug_rnglists,"",@progbits
 .long   .Ldebug_rnglist_table_end0-.Ldebug_rnglist_table_start0 # Length
 .Ldebug_rnglist_table_start0:
@@ -96,3 +134,18 @@
 .uleb128 .Lblock2_end-rnglists#   ending offset
 .byte   0   # DW_RLE_end_of_list
 .Ldebug_rnglist_table_end0:
+
+ 

[Lldb-commits] [lldb] 92cd68f - [lldb] Simplify debug_{rnglists, ranges}.s tests

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-04T17:08:23+01:00
New Revision: 92cd68f48ed14c6ee90b3ad830af3c2d7879428a

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

LOG: [lldb] Simplify debug_{rnglists,ranges}.s tests

Remove things irrelevant to the test.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/debug_ranges.s
lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/debug_ranges.s 
b/lldb/test/Shell/SymbolFile/DWARF/debug_ranges.s
index bbe5cb220c2d..13eea1b80706 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/debug_ranges.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/debug_ranges.s
@@ -3,16 +3,13 @@
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
 # RUN: %lldb %t -o "image lookup -v -s lookup_ranges" -o exit | FileCheck %s
 
-# CHECK:  Function: id = {0x7fff001c}, name = "ranges", range = 
[0x-0x0004)
-# CHECK:Blocks: id = {0x7fff001c}, range = [0x-0x0004)
-# CHECK-NEXT:   id = {0x7fff002d}, ranges = 
[0x0001-0x0002)[0x0003-0x0004)
+# CHECK:  Function: id = {0x7fff002b}, name = "ranges", range = 
[0x-0x0004)
+# CHECK:Blocks: id = {0x7fff002b}, range = [0x-0x0004)
+# CHECK-NEXT:   id = {0x7fff003f}, ranges = 
[0x0001-0x0002)[0x0003-0x0004)
 
 .text
 .p2align 12
-.globl  ranges
-.type   ranges,@function
-ranges:# @ranges
-.Lfoo_begin:
+ranges:
 nop
 .Lblock1_begin:
 lookup_ranges:
@@ -22,21 +19,14 @@ lookup_ranges:
 .Lblock2_begin:
 nop
 .Lblock2_end:
-.Lfunc_end0:
-.size   ranges, .Lfunc_end0-ranges
-# -- End function
-.section.debug_str,"MS",@progbits,1
-.Lproducer:
-.asciz  "Hand-written DWARF"
-.Lranges:
-.asciz  "ranges"
+.Lranges_end:
 
 .section.debug_abbrev,"",@progbits
 .byte   1   # Abbreviation Code
 .byte   17  # DW_TAG_compile_unit
 .byte   1   # DW_CHILDREN_yes
 .byte   37  # DW_AT_producer
-.byte   14  # DW_FORM_strp
+.byte   8   # DW_FORM_string
 .byte   17  # DW_AT_low_pc
 .byte   1   # DW_FORM_addr
 .byte   18  # DW_AT_high_pc
@@ -51,7 +41,7 @@ lookup_ranges:
 .byte   18  # DW_AT_high_pc
 .byte   6   # DW_FORM_data4
 .byte   3   # DW_AT_name
-.byte   14  # DW_FORM_strp
+.byte   8   # DW_FORM_string
 .byte   0   # EOM(1)
 .byte   0   # EOM(2)
 .byte   5   # Abbreviation Code
@@ -71,13 +61,13 @@ lookup_ranges:
 .long   .debug_abbrev   # Offset Into Abbrev. Section
 .byte   8   # Address Size (in bytes)
 .byte   1   # Abbrev [1] 0xb:0x7b 
DW_TAG_compile_unit
-.long   .Lproducer  # DW_AT_producer
-.quad   .Lfoo_begin # DW_AT_low_pc
-.long   .Lfunc_end0-.Lfoo_begin # DW_AT_high_pc
+.asciz  "Hand-written DWARF"# DW_AT_producer
+.quad   ranges  # DW_AT_low_pc
+.long   .Lranges_end-ranges # DW_AT_high_pc
 .byte   2   # Abbrev [2] 0x2a:0x4d 
DW_TAG_subprogram
-.quad   .Lfoo_begin # DW_AT_low_pc
-.long   .Lfunc_end0-.Lfoo_begin # DW_AT_high_pc
-.long   .Lranges# DW_AT_name
+.quad   ranges  # DW_AT_low_pc
+.long   .Lranges_end-ranges # DW_AT_high_pc
+.asciz  "ranges"# DW_AT_name
 .byte   5   # Abbrev [5] 0x61:0x15 
DW_TAG_lexical_block
 .long   .Ldebug_ranges0 # DW_AT_ranges
 .byte   0   # End Of Children Mark
@@ -86,9 +76,9 @@ lookup_ranges:
 
 .section.debug_ranges,"",@progbits
 .Ldebug_ranges0:
-.quad   .Lblock1_begin-.Lfoo_begin  
-.quad   .Lblock1_end-.Lfoo_begin  
-.quad   .Lblock2_begin-.Lfoo_begin  
-.quad   .Lblock2_end-.Lfoo_begin  
+.quad   .Lblock1_begin-ranges
+.quad   .Lblock1_end-ranges
+.quad   .Lblock2_begin-ranges
+.quad   .Lblock2_end-ranges
 .quad   0
 .quad   0

diff  --git 

[Lldb-commits] [PATCH] D70907: Change Target::FindBreakpointsByName to return Expected

[Joseph Tremoulet via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
JosephTremoulet marked an inline comment as done.
Closed by commit rG95b2e516bd3e: Change Target::FindBreakpointsByName to return 
Expectedvector (authored by JosephTremoulet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70907

Files:
  lldb/include/lldb/Breakpoint/BreakpointList.h
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
  lldb/source/API/SBTarget.cpp
  lldb/source/Breakpoint/BreakpointList.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -728,11 +728,17 @@
 }
 
 void Target::ApplyNameToBreakpoints(BreakpointName _name) {
-  BreakpointList bkpts_with_name(false);
-  m_breakpoint_list.FindBreakpointsByName(bp_name.GetName().AsCString(),
-  bkpts_with_name);
+  llvm::Expected> expected_vector =
+  m_breakpoint_list.FindBreakpointsByName(bp_name.GetName().AsCString());
 
-  for (auto bp_sp : bkpts_with_name.Breakpoints())
+  if (!expected_vector) {
+LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS),
+ "invalid breakpoint name: {}",
+ llvm::toString(expected_vector.takeError()));
+return;
+  }
+
+  for (auto bp_sp : *expected_vector)
 bp_name.ConfigureBreakpoint(bp_sp);
 }
 
Index: lldb/source/Breakpoint/BreakpointList.cpp
===
--- lldb/source/Breakpoint/BreakpointList.cpp
+++ lldb/source/Breakpoint/BreakpointList.cpp
@@ -10,6 +10,8 @@
 
 #include "lldb/Target/Target.h"
 
+#include "llvm/Support/Errc.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -128,22 +130,24 @@
   return {};
 }
 
-bool BreakpointList::FindBreakpointsByName(const char *name,
-   BreakpointList _bps) {
-  Status error;
+llvm::Expected>
+BreakpointList::FindBreakpointsByName(const char *name) {
   if (!name)
-return false;
+return llvm::createStringError(llvm::errc::invalid_argument,
+   "FindBreakpointsByName requires a name");
 
+  Status error;
   if (!BreakpointID::StringIsBreakpointName(llvm::StringRef(name), error))
-return false;
+return error.ToError();
 
+  std::vector matching_bps;
   for (BreakpointSP bkpt_sp : Breakpoints()) {
 if (bkpt_sp->MatchesName(name)) {
-  matching_bps.Add(bkpt_sp, false);
+  matching_bps.push_back(bkpt_sp);
 }
   }
 
-  return true;
+  return matching_bps;
 }
 
 void BreakpointList::Dump(Stream *s) const {
Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1176,12 +1176,15 @@
   TargetSP target_sp(GetSP());
   if (target_sp) {
 std::lock_guard guard(target_sp->GetAPIMutex());
-BreakpointList bkpt_list(false);
-bool is_valid =
-target_sp->GetBreakpointList().FindBreakpointsByName(name, bkpt_list);
-if (!is_valid)
+llvm::Expected> expected_vector =
+target_sp->GetBreakpointList().FindBreakpointsByName(name);
+if (!expected_vector) {
+  LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS),
+   "invalid breakpoint name: {}",
+   llvm::toString(expected_vector.takeError()));
   return false;
-for (BreakpointSP bkpt_sp : bkpt_list.Breakpoints()) {
+}
+for (BreakpointSP bkpt_sp : *expected_vector) {
   bkpts.AppendByID(bkpt_sp->GetID());
 }
   }
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
@@ -155,8 +155,13 @@
 def do_check_using_names(self):
 """Use Python APIs to check names work in place of breakpoint ID's."""
 
+# Create a dummy breakpoint to use up ID 1
+_ = self.target.BreakpointCreateByLocation(self.main_file_spec, 30)
+
+# Create a breakpiont to test with
 bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10)
 bkpt_name = "ABreakpoint"
+bkpt_id = bkpt.GetID()
 other_bkpt_name= "_AnotherBreakpoint"
 
 # Add a name and make sure we match it:
@@ -169,6 +174,7 @@
 self.assertTrue(bkpts.GetSize() == 1, "One breakpoint matched.")
 found_bkpt = bkpts.GetBreakpointAtIndex(0)
 self.assertTrue(bkpt.GetID() == found_bkpt.GetID(),"The right breakpoint.")
+

[Lldb-commits] [lldb] 95b2e51 - Change Target::FindBreakpointsByName to return Expected

[Joseph Tremoulet via lldb-commits]

Author: Joseph Tremoulet
Date: 2019-12-04T09:57:15-05:00
New Revision: 95b2e516bd3e4587953e44bf062054ff84f2b057

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

LOG: Change Target::FindBreakpointsByName to return Expected

Summary:
Using a BreakpointList corrupts the breakpoints' IDs because
BreakpointList::Add sets the ID, so use a vector instead, and
update the signature to return the vector wrapped in an
llvm::Expected which can propagate any error from the inner
call to StringIsBreakpointName.

Note that, despite the similar name, SBTarget::FindBreakpointsByName
doesn't suffer the same problem, because it uses a SBBreakpointList,
which is more like a BreakpointIDList than a BreakpointList under the
covers.

Add a check to TestBreakpointNames that, without this fix, notices the
ID getting mutated and fails.

Reviewers: jingham, JDevlieghere

Reviewed By: JDevlieghere

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Breakpoint/BreakpointList.h

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
lldb/source/API/SBTarget.cpp
lldb/source/Breakpoint/BreakpointList.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/include/lldb/Breakpoint/BreakpointList.h 
b/lldb/include/lldb/Breakpoint/BreakpointList.h
index 110e8d41f36b..ad68151fefc7 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointList.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointList.h
@@ -67,8 +67,10 @@ class BreakpointList {
   ///   The breakpoint name for which to search.
   ///
   /// \result
-  ///   \bfalse if the input name was not a legal breakpoint name.
-  bool FindBreakpointsByName(const char *name, BreakpointList _bps);
+  ///   error if the input name was not a legal breakpoint name, vector
+  ///   of breakpoints otherwise.
+  llvm::Expected>
+  FindBreakpointsByName(const char *name);
 
   /// Returns the number of elements in this breakpoint list.
   ///

diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
 
b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
index 4a5ed87e330f..9513278ba084 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
@@ -155,8 +155,13 @@ def do_check_illegal_names(self):
 def do_check_using_names(self):
 """Use Python APIs to check names work in place of breakpoint ID's."""
 
+# Create a dummy breakpoint to use up ID 1
+_ = self.target.BreakpointCreateByLocation(self.main_file_spec, 30)
+
+# Create a breakpiont to test with
 bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10)
 bkpt_name = "ABreakpoint"
+bkpt_id = bkpt.GetID()
 other_bkpt_name= "_AnotherBreakpoint"
 
 # Add a name and make sure we match it:
@@ -169,6 +174,7 @@ def do_check_using_names(self):
 self.assertTrue(bkpts.GetSize() == 1, "One breakpoint matched.")
 found_bkpt = bkpts.GetBreakpointAtIndex(0)
 self.assertTrue(bkpt.GetID() == found_bkpt.GetID(),"The right 
breakpoint.")
+self.assertTrue(bkpt.GetID() == bkpt_id,"With the same ID as before.")
 
 retval = lldb.SBCommandReturnObject()
 self.dbg.GetCommandInterpreter().HandleCommand("break disable 
%s"%(bkpt_name), retval)

diff  --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 7013e2b45e5f..312e4df75863 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1176,12 +1176,15 @@ bool SBTarget::FindBreakpointsByName(const char *name,
   TargetSP target_sp(GetSP());
   if (target_sp) {
 std::lock_guard guard(target_sp->GetAPIMutex());
-BreakpointList bkpt_list(false);
-bool is_valid =
-target_sp->GetBreakpointList().FindBreakpointsByName(name, bkpt_list);
-if (!is_valid)
+llvm::Expected> expected_vector =
+target_sp->GetBreakpointList().FindBreakpointsByName(name);
+if (!expected_vector) {
+  LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS),
+   "invalid breakpoint name: {}",
+   llvm::toString(expected_vector.takeError()));
   return false;
-for (BreakpointSP bkpt_sp : bkpt_list.Breakpoints()) {
+}
+for (BreakpointSP bkpt_sp : *expected_vector) {
   bkpts.AppendByID(bkpt_sp->GetID());
 }
   }

diff  --git a/lldb/source/Breakpoint/BreakpointList.cpp 
b/lldb/source/Breakpoint/BreakpointList.cpp
index 

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:2829
+if (!loc)
+  LLDB_LOG_ERROR(log, loc.takeError(), "{0}");
+if (loc->Range) {

labath wrote:
> dblaikie wrote:
> > Does LLDB_LOG_ERROR stop the program? If not, then the next line ("if 
> > (loc->Range)" will invoke UB when it tries to dereference the "loc" when it 
> > is unset.
> It doesn't. You're right, there should be a return here, just let me see if I 
> can tickle this into exploding.
It turns out triggering this was pretty hard, because the main source of these 
errors is a failed indirect address resolution, but our callback never returned 
any errors. I've now fixed it to return errors and included an additional test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71003



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

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 232106.
labath marked 4 inline comments as done.
labath added a comment.

Feedback @dblaikie.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71003

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug_loc.s

Index: lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
===
--- lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
+++ lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
@@ -1,24 +1,82 @@
-# Test debug_loc parsing, including the cases of invalid input. The exact
+# Test location list handling, including the cases of invalid input. The exact
 # behavior in the invalid cases is not particularly important, but it should be
 # "reasonable".
 
 # REQUIRES: x86
 
-# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
-# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" -o exit \
-# RUN:   | FileCheck %s
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym LOC=0 > %t
+# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" \
+# RUN:   -o "image dump symfile" -o exit | FileCheck %s
+
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym LOCLISTS=0 > %t
+# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" \
+# RUN:   -o "image dump symfile" -o exit | FileCheck %s --check-prefix=CHECK --check-prefix=LOCLISTS
 
 # CHECK-LABEL: image lookup -v -a 0
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg5 RDI,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
-# CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 
 # CHECK-LABEL: image lookup -v -a 2
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg0 RAX,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
-# CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 # CHECK: Variable: {{.*}}, name = "x3", type = "int", location = DW_OP_reg1 RDX,
 
+# CHECK-LABEL: image dump symfile
+# CHECK: CompileUnit{0x}
+# CHECK:   Function{
+# CHECK: Variable{{.*}}, name = "x0", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  [0x, 0x0001): DW_OP_reg5 RDI
+# CHECK-NEXT:  [0x0001, 0x0006): DW_OP_reg0 RAX
+# CHECK: Variable{{.*}}, name = "x1", {{.*}}, scope = parameter
+# CHECK: Variable{{.*}}, name = "x2", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  error: unexpected end of data
+# CHECK: Variable{{.*}}, name = "x3", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  [0x0002, 0x0003): DW_OP_reg1 RDX
+# LOCLISTS:  Variable{{.*}}, name = "x4", {{.*}}, scope = parameter, location =
+# LOCLISTS-NEXT: DW_LLE_startx_length   (0xdead, 0x0001): DW_OP_reg2 RCX
+
+.ifdef LOC
+.macro OFFSET_PAIR lo hi
+.quad \lo
+.quad \hi
+.endm
+
+.macro BASE_ADDRESS base
+.quad -1
+.quad \base
+.endm
+
+.macro EXPR_SIZE sz
+.short \sz
+.endm
+
+.macro END_OF_LIST
+.quad 0
+.quad 0
+.endm
+.endif
+
+.ifdef LOCLISTS
+.macro OFFSET_PAIR lo hi
+.byte   4   # DW_LLE_offset_pair
+.uleb128 \lo
+.uleb128 \hi
+.endm
+
+.macro BASE_ADDRESS base
+.byte   6   # DW_LLE_base_address
+.quad \base
+.endm
+
+.macro EXPR_SIZE sz
+.uleb128 \sz
+.endm
+
+.macro END_OF_LIST
+.byte   0   # DW_LLE_end_of_list
+.endm
+.endif
+
 .type   f,@function
 f:  # @f
 .Lfunc_begin0:
@@ -44,33 +102,48 @@
 .Linfo_string4:
 .asciz  "int"
 
+.ifdef LOC
 .section.debug_loc,"",@progbits
+.endif
+.ifdef LOCLISTS
+.section.debug_loclists,"",@progbits
+.long   .Ldebug_loclist_table_end0-.Ldebug_loclist_table_start0 # Length
+.Ldebug_loclist_table_start0:
+.short  5   # Version
+.byte   8   # Address size
+.byte   0   # Segment selector size
+.long   0   # Offset entry count
+.endif
 .Ldebug_loc0:
-.quad   .Lfunc_begin0-.Lfunc_begin0
-.quad   .Ltmp0-.Lfunc_begin0
-.short  1   # Loc expr size
+OFFSET_PAIR .Lfunc_begin0-.Lfunc_begin0, .Ltmp0-.Lfunc_begin0
+EXPR_SIZE 1
 .byte   85  # super-register DW_OP_reg5
-.quad   .Ltmp0-.Lfunc_begin0
-.quad   .Lfunc_end0-.Lfunc_begin0
-.short  1   # Loc expr size
+OFFSET_PAIR   .Ltmp0-.Lfunc_begin0, .Lfunc_end0-.Lfunc_begin0
+EXPR_SIZE 1
 .byte   80  # super-register DW_OP_reg0
-.quad   0
-  

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lldb/source/Expression/DWARFExpression.cpp:2829
+if (!loc)
+  LLDB_LOG_ERROR(log, loc.takeError(), "{0}");
+if (loc->Range) {

dblaikie wrote:
> Does LLDB_LOG_ERROR stop the program? If not, then the next line ("if 
> (loc->Range)" will invoke UB when it tries to dereference the "loc" when it 
> is unset.
It doesn't. You're right, there should be a return here, just let me see if I 
can tickle this into exploding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71003



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

[Lldb-commits] [PATCH] D70848: [LLDB] Set the right address size on output DataExtractors from ObjectFile

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

BTW, I don't know if you've noticed, but the new test is failing on windows: 
http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/11217. It looks 
like a path separator problem. We actually already have code which tries to 
guess the path style of a compile unit, but it is keyed off of the 
DW_AT_comp_dir attribute, which it looks like you stripped from the test case. 
My guess is that adding this attribute back will fix this problem...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70848



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

[Lldb-commits] [lldb] 5e71356 - [lldb] Fix macOS build by replacing nullptr with FileSpec()

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-04T14:37:10+01:00
New Revision: 5e713563934eadb48b4830ad019bdb91f1d89150

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

LOG: [lldb] Fix macOS build by replacing nullptr with FileSpec()

Before we had a implicit conversion from nullptr to FileSpec
which was thankfully removed.

Added: 


Modified: 
lldb/source/Host/macosx/objcxx/Host.mm
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Removed: 




diff  --git a/lldb/source/Host/macosx/objcxx/Host.mm 
b/lldb/source/Host/macosx/objcxx/Host.mm
index 8c7393739bc6..03880ff433bd 100644
--- a/lldb/source/Host/macosx/objcxx/Host.mm
+++ b/lldb/source/Host/macosx/objcxx/Host.mm
@@ -1130,7 +1130,7 @@ static Status LaunchProcessPosixSpawn(const char 
*exe_path,
   // --arch  as part of the shell invocation
   // to do that job on OSX.
 
-  if (launch_info.GetShell() == nullptr) {
+  if (launch_info.GetShell() == FileSpec()) {
 // We don't need to do this for ARM, and we really shouldn't now that we
 // have multiple CPU subtypes and no posix_spawnattr call that allows us
 // to set which CPU subtype to launch...

diff  --git a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp 
b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
index 74718a8c5e30..5ee632ec2077 100644
--- a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -595,7 +595,7 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec 
_spec,
 }
 Status error = Host::RunShellCommand(
 command.GetData(),
-NULL,// current working directory
+FileSpec(),  // current working directory
 _status,// Exit status
 ,  // Signal int *
 _output, // Command output



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

[Lldb-commits] [PATCH] D70848: [LLDB] Set the right address size on output DataExtractors from ObjectFile

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I just realized I didn't press "sumbit" yesterday...




Comment at: lldb/source/Symbol/ObjectFile.cpp:480-486
+  size_t ret = data.SetData(m_data, offset, length);
+  // DataExtractor::SetData copies the address byte size from m_data, but
+  // m_data's address byte size is only set from sizeof(void*), and we can't
+  // access subclasses GetAddressByteSize() when setting up m_data in the
+  // constructor.
+  data.SetAddressByteSize(GetAddressByteSize());
+  return ret;

mstorsjo wrote:
> clayborg wrote:
> > I would vote to make this happen within DataExtractor::SetData(const 
> > DataExtractor &...)
> Do you mean that we'd extend `DataExtractor::SetData(const DataExtractor 
> &...)` to take a byte address size parameter, or that we'd update `m_data`'s 
> byte address size before doing `data.SetData()` here?
> 
> Ideally I'd set the right byte address size in `m_data` as soon as it is 
> known and available. It's not possible to do this in `ObjectFile`'s 
> constructor, as that is called before the subclass is constructed and its 
> virtual methods are available, but is there a better point in the lifetime 
> where we could update it?
I too would prefer that, but I couldn't see a way to achieve that (which is why 
I stamped this version).

Today, with a fresh set of eyes, I think it may be reasonable to have this 
happen in `ObjectFile::ParseHeader`. After the header has been parsed, all 
object formats (I hope) should be able to determine their address size and 
endianness, and the operating invariant would be that the address size is only 
valid after the ParseHeader has been called. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70848



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

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:72
+  }
+  llvm_unreachable("Fully covered switch!");
+}

I think usually the unreachable comment that's used in this sort of case is 
"Invalid LocationListFormat!" or similar, to give a bit more context about the 
situation.



Comment at: lldb/source/Expression/DWARFExpression.cpp:636
 
-  lldb::offset_t offset = 0;
-  lldb::addr_t base_address = m_loclist_addresses->cu_file_addr;
-  while (m_data.ValidOffset(offset)) {
-// We need to figure out what the value is for the location.
-addr_t lo_pc = LLDB_INVALID_ADDRESS;
-addr_t hi_pc = LLDB_INVALID_ADDRESS;
-if (!AddressRangeForLocationListEntry(m_dwarf_cu, m_data, , lo_pc,
-  hi_pc))
-  break;
-
-if (lo_pc == 0 && hi_pc == 0)
-  break;
-
-if ((m_data.GetAddressByteSize() == 4 && (lo_pc == UINT32_MAX)) ||
-(m_data.GetAddressByteSize() == 8 && (lo_pc == UINT64_MAX))) {
-  base_address = hi_pc;
-  continue;
-}
-RelocateLowHighPC(base_address, func_load_addr, lo_pc, hi_pc);
-
-if (lo_pc <= addr && addr < hi_pc)
-  return true;
-
-offset += m_data.GetU16();
-  }
-  return false;
-}
-
-bool DWARFExpression::GetLocation(addr_t func_load_addr, addr_t pc,
-  lldb::offset_t ,
-  lldb::offset_t ) {
-  offset = 0;
-  if (!IsLocationList()) {
-length = m_data.GetByteSize();
-return true;
-  }
-
-  if (func_load_addr != LLDB_INVALID_ADDRESS && pc != LLDB_INVALID_ADDRESS) {
-addr_t base_address = m_loclist_addresses->cu_file_addr;
-while (m_data.ValidOffset(offset)) {
-  // We need to figure out what the value is for the location.
-  addr_t lo_pc = LLDB_INVALID_ADDRESS;
-  addr_t hi_pc = LLDB_INVALID_ADDRESS;
-  if (!AddressRangeForLocationListEntry(m_dwarf_cu, m_data, , lo_pc,
-hi_pc))
-break;
-
-  if (lo_pc == 0 && hi_pc == 0)
-break;
-
-  if ((m_data.GetAddressByteSize() == 4 && (lo_pc == UINT32_MAX)) ||
-  (m_data.GetAddressByteSize() == 8 && (lo_pc == UINT64_MAX))) {
-base_address = hi_pc;
-continue;
-  }
-
-  RelocateLowHighPC(base_address, func_load_addr, lo_pc, hi_pc);
-  length = m_data.GetU16();
-
-  if (length > 0 && lo_pc <= pc && pc < hi_pc)
-return true;
-
-  offset += length;
-}
-  }
-  offset = LLDB_INVALID_OFFSET;
-  length = 0;
-  return false;
+  return bool(GetLocationExpression(func_load_addr, addr));
 }

I'd probably write this as "... != None" rather than "bool(...)" - would make 
it clearer what the return type of GetLocationExpression is/what conversion is 
being done here.



Comment at: lldb/source/Expression/DWARFExpression.cpp:2829
+if (!loc)
+  LLDB_LOG_ERROR(log, loc.takeError(), "{0}");
+if (loc->Range) {

Does LLDB_LOG_ERROR stop the program? If not, then the next line ("if 
(loc->Range)" will invoke UB when it tries to dereference the "loc" when it is 
unset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71003



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

[Lldb-commits] [PATCH] D71003: [lldb/DWARF] Switch to llvm location list parser

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, aprantl, clayborg.
Herald added a project: LLDB.
labath added a parent revision: D70532: [lldb] Improve/fix base address 
selection in location lists.

This patch deletes the lldb location list parser and teaches the
DWARFExpression class to use the parser in llvm instead. I have
centralized all the places doing the parsing into a single
GetLocationExpression function.

In theory the the actual location list parsing should be covered by llvm
tests, and this glue code by our existing location list tests, but since
we don't have that many location list tests, I've tried to extend the
coverage a bit by adding some explicit dwarf5 loclist handling and a
test of the dumping code.

For DWARF4 location lists this should be NFC (modulo small differences
in error handling which should only show up on invalid inputs). In case
of DWARF5, this fixes various missing bits of functionality, most
notably, the lack of support for DW_LLE_offset_pair.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71003

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug_loc.s

Index: lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
===
--- lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
+++ lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
@@ -1,24 +1,80 @@
-# Test debug_loc parsing, including the cases of invalid input. The exact
+# Test location list handling, including the cases of invalid input. The exact
 # behavior in the invalid cases is not particularly important, but it should be
 # "reasonable".
 
 # REQUIRES: x86
 
-# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
-# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" -o exit \
-# RUN:   | FileCheck %s
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym LOC=0 > %t
+# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" \
+# RUN:   -o "image dump symfile" -o exit | FileCheck %s
+
+# RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s --defsym LOCLISTS=0 > %t
+# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" \
+# RUN:   -o "image dump symfile" -o exit | FileCheck %s
 
 # CHECK-LABEL: image lookup -v -a 0
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg5 RDI,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
-# CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 
 # CHECK-LABEL: image lookup -v -a 2
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg0 RAX,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
-# CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 # CHECK: Variable: {{.*}}, name = "x3", type = "int", location = DW_OP_reg1 RDX,
 
+# CHECK-LABEL: image dump symfile
+# CHECK: CompileUnit{0x}
+# CHECK:   Function{
+# CHECK: Variable{{.*}}, name = "x0", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  [0x, 0x0001): DW_OP_reg5 RDI
+# CHECK-NEXT:  [0x0001, 0x0006): DW_OP_reg0 RAX
+# CHECK: Variable{{.*}}, name = "x1", {{.*}}, scope = parameter
+# CHECK: Variable{{.*}}, name = "x2", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  error: unexpected end of data
+# CHECK: Variable{{.*}}, name = "x3", {{.*}}, scope = parameter, location =
+# CHECK-NEXT:  [0x0002, 0x0003): DW_OP_reg1 RDX
+
+.ifdef LOC
+.macro OFFSET_PAIR lo hi
+.quad \lo
+.quad \hi
+.endm
+
+.macro BASE_ADDRESS base
+.quad -1
+.quad \base
+.endm
+
+.macro EXPR_SIZE sz
+.short \sz
+.endm
+
+.macro END_OF_LIST
+.quad 0
+.quad 0
+.endm
+.endif
+
+.ifdef LOCLISTS
+.macro OFFSET_PAIR lo hi
+.byte   4   # DW_LLE_offset_pair
+.uleb128 \lo
+.uleb128 \hi
+.endm
+
+.macro BASE_ADDRESS base
+.byte   6   # DW_LLE_base_address
+.quad \base
+.endm
+
+.macro EXPR_SIZE sz
+.uleb128 \sz
+.endm
+
+.macro END_OF_LIST
+.byte   0   # DW_LLE_end_of_list
+.endm
+.endif
+
 .type   f,@function
 f:  # @f
 .Lfunc_begin0:
@@ -44,33 +100,38 @@
 .Linfo_string4:
 .asciz  "int"
 
+.ifdef LOC
 .section.debug_loc,"",@progbits
+.endif
+.ifdef LOCLISTS
+.section.debug_loclists,"",@progbits
+.long   .Ldebug_loclist_table_end0-.Ldebug_loclist_table_start0 # Length
+.Ldebug_loclist_table_start0:
+.short  5   # Version
+.byte   8   # Address size
+.byte   0   # Segment selector size
+.long   0   # Offset entry count
+.endif
 .Ldebug_loc0:
-.quad   

[Lldb-commits] [PATCH] D68209: [LiveDebugValues] Introduce entry values of unmodified params

[Djordje Todorovic via Phabricator via lldb-commits]

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



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:159
   // FUNC11-BT: func11_tailcalled{{.*}}
   // FUNC11-BT-NEXT: func12{{.*}} [artificial]
   use(x);

vsk wrote:
> The failure was:
> ```
> main.cpp:159:21: error: FUNC11-BT-NEXT: expected string not found in input
>  // FUNC11-BT-NEXT: func12{{.*}} [artificial]
> ^
> :3:2: note: scanning from here
>  frame #1: 0x0001079eae69 a.out`func12(sink=0x7ffee8215cb4, x=123) at 
> main.cpp:179:3 [opt]
> ```
> 
> The added `DESTROY_RBX` asm might confuse TailRecursionElimination into 
> believing that the callee accesses the caller's stack. Could you double-check 
> that a tail call is actually emitted in `func12` (something like `jmp 
> *%rax`)? If it //is//, this is a pre-existing lldb bug, so the func12 test 
> should be disabled.
@vsk Thanks for the comment!

The problem here is the fresh change in the code production by using the `-O1` 
level of optimization. More precisely, at very high level, after the D65410 we 
do not have a tail call where we expected.
I am proposing using the `-O2` level of the optimizations, since we are testing 
printing of the entry values in the test case, rather than tail call frames 
with particular level of optimization.
WDYT? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68209



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

[Lldb-commits] [PATCH] D70155: [LLDB] Avoid triple corruption while merging core info from platform and target triples

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

Ping @jasonmolenda

Any comments or LGTM?


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

https://reviews.llvm.org/D70155



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

[Lldb-commits] [lldb] 150c8dd - [lldb] Remove some (almost) unused Stream::operator<<'s

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-04T11:07:46+01:00
New Revision: 150c8dd13be4a9d9a9f631a507871359117871ca

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

LOG: [lldb] Remove some (almost) unused Stream::operator<<'s

llvm::raw_ostream provides equivalent functionality.

Added: 


Modified: 
lldb/include/lldb/Utility/Stream.h
lldb/source/Symbol/Variable.cpp
lldb/source/Utility/Stream.cpp
lldb/unittests/Utility/StreamTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Stream.h 
b/lldb/include/lldb/Utility/Stream.h
index 9982d8236a65..a3a33178086e 100644
--- a/lldb/include/lldb/Utility/Stream.h
+++ b/lldb/include/lldb/Utility/Stream.h
@@ -217,46 +217,10 @@ class Stream {
   Stream <<(uint16_t uval) = delete;
   Stream <<(uint32_t uval) = delete;
   Stream <<(uint64_t uval) = delete;
-
-  /// Output a int8_t \a sval to the stream \a s.
-  ///
-  /// \param[in] sval
-  /// A int8_t value.
-  ///
-  /// \return
-  /// A reference to this class so multiple things can be streamed
-  /// in one statement.
-  Stream <<(int8_t sval);
-
-  /// Output a int16_t \a sval to the stream \a s.
-  ///
-  /// \param[in] sval
-  /// A int16_t value.
-  ///
-  /// \return
-  /// A reference to this class so multiple things can be streamed
-  /// in one statement.
-  Stream <<(int16_t sval);
-
-  /// Output a int32_t \a sval to the stream \a s.
-  ///
-  /// \param[in] sval
-  /// A int32_t value.
-  ///
-  /// \return
-  /// A reference to this class so multiple things can be streamed
-  /// in one statement.
-  Stream <<(int32_t sval);
-
-  /// Output a int64_t \a sval to the stream \a s.
-  ///
-  /// \param[in] sval
-  /// A int64_t value.
-  ///
-  /// \return
-  /// A reference to this class so multiple things can be streamed
-  /// in one statement.
-  Stream <<(int64_t sval);
+  Stream <<(int8_t sval) = delete;
+  Stream <<(int16_t sval) = delete;
+  Stream <<(int32_t sval) = delete;
+  Stream <<(int64_t sval) = delete;
 
   /// Output an address value to this stream.
   ///

diff  --git a/lldb/source/Symbol/Variable.cpp b/lldb/source/Symbol/Variable.cpp
index 6e4b87c47700..fc7d127a326f 100644
--- a/lldb/source/Symbol/Variable.cpp
+++ b/lldb/source/Symbol/Variable.cpp
@@ -134,7 +134,7 @@ void Variable::Dump(Stream *s, bool show_context) const {
   s->PutCString("thread local");
   break;
 default:
-  *s << "??? (" << m_scope << ')';
+  s->AsRawOstream() << "??? (" << m_scope << ')';
 }
   }
 

diff  --git a/lldb/source/Utility/Stream.cpp b/lldb/source/Utility/Stream.cpp
index 119d1e0f7964..2ef4cd78ab03 100644
--- a/lldb/source/Utility/Stream.cpp
+++ b/lldb/source/Utility/Stream.cpp
@@ -160,30 +160,6 @@ Stream ::operator<<(const void *p) {
   return *this;
 }
 
-// Stream a int8_t "sval" out to this stream.
-Stream ::operator<<(int8_t sval) {
-  Printf("%i", static_cast(sval));
-  return *this;
-}
-
-// Stream a int16_t "sval" out to this stream.
-Stream ::operator<<(int16_t sval) {
-  Printf("%i", static_cast(sval));
-  return *this;
-}
-
-// Stream a int32_t "sval" out to this stream.
-Stream ::operator<<(int32_t sval) {
-  Printf("%i", static_cast(sval));
-  return *this;
-}
-
-// Stream a int64_t "sval" out to this stream.
-Stream ::operator<<(int64_t sval) {
-  Printf("%" PRIi64, sval);
-  return *this;
-}
-
 // Get the current indentation level
 unsigned Stream::GetIndentLevel() const { return m_indent_level; }
 

diff  --git a/lldb/unittests/Utility/StreamTest.cpp 
b/lldb/unittests/Utility/StreamTest.cpp
index 93c25166cdfd..6e42ac2d11f0 100644
--- a/lldb/unittests/Utility/StreamTest.cpp
+++ b/lldb/unittests/Utility/StreamTest.cpp
@@ -387,15 +387,6 @@ TEST_F(StreamTest, ShiftOperatorStrings) {
   EXPECT_EQ("cstring\nllvm::StringRef\n", TakeValue());
 }
 
-TEST_F(StreamTest, ShiftOperatorInts) {
-  s << std::numeric_limits::max() << " ";
-  s << std::numeric_limits::max() << " ";
-  s << std::numeric_limits::max() << " ";
-  s << std::numeric_limits::max();
-  EXPECT_EQ(40U, s.GetWrittenBytes());
-  EXPECT_EQ("127 32767 2147483647 9223372036854775807", TakeValue());
-}
-
 TEST_F(StreamTest, ShiftOperatorPtr) {
   // This test is a bit tricky because pretty much everything related to
   // pointer printing seems to lead to UB or IB. So let's make the most basic



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

[Lldb-commits] [lldb] 1351672 - [lldb] s/assertTrue/assertEqual in TestStepTarget.py

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-04T10:56:38+01:00
New Revision: 1351672eedbf9716def4fc789d9046c481c7cd25

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

LOG: [lldb] s/assertTrue/assertEqual in TestStepTarget.py

this improves error messages.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py 
b/lldb/packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py
index c694bda97c28..b3786fb94454 100644
--- a/lldb/packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py
+++ b/lldb/packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py
@@ -32,7 +32,7 @@ def get_to_start(self):
 break_in_main = target.BreakpointCreateBySourceRegex(
 'Break here to try targetted stepping', self.main_source_spec)
 self.assertTrue(break_in_main, VALID_BREAKPOINT)
-self.assertTrue(break_in_main.GetNumLocations() > 0, "Has locations.")
+self.assertGreater(break_in_main.GetNumLocations(), 0, "Has 
locations.")
 
 # Now launch the process, and do not stop at entry point.
 process = target.LaunchSimple(
@@ -60,7 +60,7 @@ def test_with_end_line(self):
 thread.StepInto("lotsOfArgs", self.end_line, error)
 frame = thread.frames[0]
 
-self.assertTrue(frame.name == "lotsOfArgs", "Stepped to lotsOfArgs.")
+self.assertEqual(frame.name, "lotsOfArgs", "Stepped to lotsOfArgs.")
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr32343")
 def test_with_end_line_bad_name(self):
@@ -71,8 +71,7 @@ def test_with_end_line_bad_name(self):
 error = lldb.SBError()
 thread.StepInto("lotsOfArg", self.end_line, error)
 frame = thread.frames[0]
-self.assertTrue(
-frame.line_entry.line == self.end_line,
+self.assertEqual(frame.line_entry.line, self.end_line,
 "Stepped to the block end.")
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr32343")
@@ -84,7 +83,7 @@ def test_with_end_line_deeper(self):
 error = lldb.SBError()
 thread.StepInto("modifyInt", self.end_line, error)
 frame = thread.frames[0]
-self.assertTrue(frame.name == "modifyInt", "Stepped to modifyInt.")
+self.assertEqual(frame.name, "modifyInt", "Stepped to modifyInt.")
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr32343")
 def test_with_command_and_block(self):
@@ -100,7 +99,7 @@ def test_with_command_and_block(self):
 "thread step-in command succeeded.")
 
 frame = thread.frames[0]
-self.assertTrue(frame.name == "lotsOfArgs", "Stepped to lotsOfArgs.")
+self.assertEqual(frame.name, "lotsOfArgs", "Stepped to lotsOfArgs.")
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr32343")
 def test_with_command_and_block_and_bad_name(self):
@@ -117,9 +116,8 @@ def test_with_command_and_block_and_bad_name(self):
 
 frame = thread.frames[0]
 
-self.assertTrue(frame.name == "main", "Stepped back out to main.")
+self.assertEqual(frame.name, "main", "Stepped back out to main.")
 # end_line is set to the line after the containing block.  Check that
 # we got there:
-self.assertTrue(
-frame.line_entry.line == self.end_line,
+self.assertEqual(frame.line_entry.line, self.end_line,
 "Got out of the block")



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

[Lldb-commits] [lldb] 28e4942 - [lldb] Remove FileSpec(FileSpec*) constructor

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-04T10:49:25+01:00
New Revision: 28e4942b2c3b8961b91b362b4b76b9ca0f735cc2

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

LOG: [lldb] Remove FileSpec(FileSpec*) constructor

This constructor was the cause of some pretty weird behavior. Remove it,
and update all code to properly dereference the argument instead.

Added: 


Modified: 
lldb/include/lldb/Utility/FileSpec.h
lldb/source/API/SBThread.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/source/Symbol/LocateSymbolFile.cpp
lldb/source/Target/Target.cpp
lldb/source/Utility/FileSpec.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/FileSpec.h 
b/lldb/include/lldb/Utility/FileSpec.h
index 9da6670f1c61..61b6209bb3c0 100644
--- a/lldb/include/lldb/Utility/FileSpec.h
+++ b/lldb/include/lldb/Utility/FileSpec.h
@@ -75,18 +75,6 @@ class FileSpec {
 
   explicit FileSpec(llvm::StringRef path, const llvm::Triple );
 
-  /// Copy constructor
-  ///
-  /// Makes a copy of the uniqued directory and filename strings from \a rhs
-  /// if it is not nullptr.
-  ///
-  /// \param[in] rhs
-  /// A const FileSpec object pointer to copy if non-nullptr.
-  FileSpec(const FileSpec *rhs);
-
-  /// Destructor.
-  ~FileSpec();
-
   bool DirectoryEquals(const FileSpec ) const;
 
   bool FileEquals(const FileSpec ) const;

diff  --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index 2dada9a6118d..f7f748f56832 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -1037,7 +1037,7 @@ SBError SBThread::JumpToLine(lldb::SBFileSpec _spec, 
uint32_t line) {
 
   Thread *thread = exe_ctx.GetThreadPtr();
 
-  Status err = thread->JumpToLine(file_spec.get(), line, true);
+  Status err = thread->JumpToLine(file_spec.ref(), line, true);
   sb_error.SetError(err);
   return LLDB_RECORD_RESULT(sb_error);
 }

diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 6978a31fb2e5..b0ce967a7966 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -114,9 +114,10 @@ const char *ObjectFilePECOFF::GetPluginDescriptionStatic() 
{
 ObjectFile *ObjectFilePECOFF::CreateInstance(const lldb::ModuleSP _sp,
  DataBufferSP _sp,
  lldb::offset_t data_offset,
- const lldb_private::FileSpec 
*file,
+ const lldb_private::FileSpec 
*file_p,
  lldb::offset_t file_offset,
  lldb::offset_t length) {
+  FileSpec file = file_p ? *file_p : FileSpec();
   if (!data_sp) {
 data_sp = MapFileData(file, length, file_offset);
 if (!data_sp)
@@ -135,7 +136,7 @@ ObjectFile *ObjectFilePECOFF::CreateInstance(const 
lldb::ModuleSP _sp,
   }
 
   auto objfile_up = std::make_unique(
-  module_sp, data_sp, data_offset, file, file_offset, length);
+  module_sp, data_sp, data_offset, file_p, file_offset, length);
   if (!objfile_up || !objfile_up->ParseHeader())
 return nullptr;
 

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 6a3e6b4cadef..ae9f20db43cc 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1106,7 +1106,7 @@ static FileSpec GetXcodeSelectPath() {
   std::string command_output;
   Status status =
   Host::RunShellCommand("/usr/bin/xcode-select --print-path",
-nullptr, // current working directory
+FileSpec(), // current working directory
 _status, , _output,
 std::chrono::seconds(2), // short timeout
 false);  // don't run in a 
shell

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
index 95ba81a2ab49..134a4c7c8075 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -180,11 +180,11 @@ ConstString 
PlatformMacOSX::GetSDKDirectory(lldb_private::Target ) {
 std::string output;
 const char *command = "xcrun -sdk macosx 

[Lldb-commits] [PATCH] D70851: [lldb] s/FileSpec::Equal/FileSpec::Match

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
labath marked 2 inline comments as done.
Closed by commit rG532290e69fcb: [lldb] s/FileSpec::Equal/FileSpec::Match 
(authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D70851?vs=231522=232057#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70851

Files:
  lldb/include/lldb/Core/ModuleSpec.h
  lldb/include/lldb/Core/SourceManager.h
  lldb/include/lldb/Utility/FileSpec.h
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/SearchFilter.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/Declaration.cpp
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Target/TargetList.cpp
  lldb/source/Target/ThreadPlanStepInRange.cpp
  lldb/source/Utility/FileSpec.cpp
  lldb/unittests/Utility/FileSpecTest.cpp

Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -399,3 +399,24 @@
   EXPECT_FALSE(Eq("foo", "/bar/foo", true));
   EXPECT_TRUE(Eq("foo", "/bar/foo", false));
 }
+
+TEST(FileSpecTest, Match) {
+  auto Match = [](const char *pattern, const char *file) {
+return FileSpec::Match(PosixSpec(pattern), PosixSpec(file));
+  };
+  EXPECT_TRUE(Match("/foo/bar", "/foo/bar"));
+  EXPECT_FALSE(Match("/foo/bar", "/oof/bar"));
+  EXPECT_FALSE(Match("/foo/bar", "/foo/baz"));
+  EXPECT_FALSE(Match("/foo/bar", "bar"));
+  EXPECT_FALSE(Match("/foo/bar", ""));
+
+  EXPECT_TRUE(Match("bar", "/foo/bar"));
+  EXPECT_FALSE(Match("bar", "/foo/baz"));
+  EXPECT_TRUE(Match("bar", "bar"));
+  EXPECT_FALSE(Match("bar", "baz"));
+  EXPECT_FALSE(Match("bar", ""));
+
+  EXPECT_TRUE(Match("", "/foo/bar"));
+  EXPECT_TRUE(Match("", ""));
+
+}
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -308,6 +308,14 @@
   return a.FileEquals(b);
 }
 
+bool FileSpec::Match(const FileSpec , const FileSpec ) {
+  if (pattern.GetDirectory())
+return pattern == file;
+  if (pattern.GetFilename())
+return pattern.FileEquals(file);
+  return true;
+}
+
 llvm::Optional FileSpec::GuessPathStyle(llvm::StringRef absolute_path) {
   if (absolute_path.startswith("/"))
 return Style::posix;
Index: lldb/source/Target/ThreadPlanStepInRange.cpp
===
--- lldb/source/Target/ThreadPlanStepInRange.cpp
+++ lldb/source/Target/ThreadPlanStepInRange.cpp
@@ -339,7 +339,7 @@
 if (frame_library) {
   for (size_t i = 0; i < num_libraries; i++) {
 const FileSpec _spec(libraries_to_avoid.GetFileSpecAtIndex(i));
-if (FileSpec::Equal(file_spec, frame_library, false)) {
+if (FileSpec::Match(file_spec, frame_library)) {
   libraries_say_avoid = true;
   break;
 }
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -457,15 +457,12 @@
 const FileSpec _file_spec, const ArchSpec *exe_arch_ptr) const {
   std::lock_guard guard(m_target_list_mutex);
   TargetSP target_sp;
-  bool full_match = (bool)exe_file_spec.GetDirectory();
-
   collection::const_iterator pos, end = m_target_list.end();
   for (pos = m_target_list.begin(); pos != end; ++pos) {
 Module *exe_module = (*pos)->GetExecutableModulePointer();
 
 if (exe_module) {
-  if (FileSpec::Equal(exe_file_spec, exe_module->GetFileSpec(),
-  full_match)) {
+  if (FileSpec::Match(exe_file_spec, exe_module->GetFileSpec())) {
 if (exe_arch_ptr) {
   if (!exe_arch_ptr->IsCompatibleMatch(exe_module->GetArchitecture()))
 continue;
Index: lldb/source/Symbol/SymbolContext.cpp
===
--- lldb/source/Symbol/SymbolContext.cpp
+++ lldb/source/Symbol/SymbolContext.cpp
@@ -1026,8 +1026,7 @@
   return false;
   } else {
 FileSpec module_file_spec(m_module_spec);
-if (!FileSpec::Equal(module_file_spec, sc.module_sp->GetFileSpec(),
- false))
+if (!FileSpec::Match(module_file_spec, sc.module_sp->GetFileSpec()))
   return false;
   }
 }
@@ -1046,8 +1045,8 @@
 sc.block->GetInlinedFunctionInfo();
 if (inline_info != nullptr) {
   was_inlined = true;

[Lldb-commits] [lldb] 532290e - [lldb] s/FileSpec::Equal/FileSpec::Match

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-04T10:42:32+01:00
New Revision: 532290e69fcb0e1f2005853241bc2cac6941e0f7

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

LOG: [lldb] s/FileSpec::Equal/FileSpec::Match

Summary:
The FileSpec class is often used as a sort of a pattern -- one specifies
a bare file name to search, and we check if in matches the full file
name of an existing module (for example).

These comparisons used FileSpec::Equal, which had some support for it
(via the full=false argument), but it was not a good fit for this job.

For one, it did a symmetric comparison, which makes sense for a function
called "equal", but not for typical searches (when searching for
"/foo/bar.so", we don't want to find a module whose name is just
"bar.so"). This resulted in patterns like:
if (FileSpec::Equal(pattern, file, pattern.GetDirectory()))
which would request a "full" match only if the pattern really contained
a directory. This worked, but the intended behavior was very unobvious.

On top of that, a lot of the code wanted to handle the case of an
"empty" pattern, and treat it as matching everything. This resulted in
conditions like:
if (pattern && !FileSpec::Equal(pattern, file, pattern.GetDirectory())
which are nearly impossible to decipher.

This patch introduces a FileSpec::Match function, which does exactly
what most of FileSpec::Equal callers want, an asymmetric match between a
"pattern" FileSpec and a an actual FileSpec. Empty paterns match
everything, filename-only patterns match only the filename component.

I've tried to update all callers of FileSpec::Equal to use a simpler
interface. Those that hardcoded full=true have been changed to use
operator==. Those passing full=pattern.GetDirectory() have been changed
to use FileSpec::Match.

There was also a handful of places which hardcoded full=false. I've
changed these to use FileSpec::Match too. This is a slight change in
semantics, but it does not look like that was ever intended, and it was
more likely a result of a misunderstanding of the "proper" way to use
FileSpec::Equal.

[In an ideal world a "FileSpec" and a "FileSpec pattern" would be two
different types, but given how widespread FileSpec is, it is unlikely
we'll get there in one go. This at least provides a good starting point
by centralizing all matching behavior.]

Reviewers: teemperor, JDevlieghere, jdoerfert

Subscribers: emaste, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Core/ModuleSpec.h
lldb/include/lldb/Core/SourceManager.h
lldb/include/lldb/Utility/FileSpec.h
lldb/source/Breakpoint/Breakpoint.cpp
lldb/source/Commands/CommandObjectSource.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Core/Module.cpp
lldb/source/Core/SearchFilter.cpp
lldb/source/Core/SourceManager.cpp
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Symbol/CompileUnit.cpp
lldb/source/Symbol/Declaration.cpp
lldb/source/Symbol/SymbolContext.cpp
lldb/source/Target/TargetList.cpp
lldb/source/Target/ThreadPlanStepInRange.cpp
lldb/source/Utility/FileSpec.cpp
lldb/unittests/Utility/FileSpecTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ModuleSpec.h 
b/lldb/include/lldb/Core/ModuleSpec.h
index 26be59e3f4ae..6d024fe3434b 100644
--- a/lldb/include/lldb/Core/ModuleSpec.h
+++ b/lldb/include/lldb/Core/ModuleSpec.h
@@ -251,24 +251,18 @@ class ModuleSpec {
 if (match_module_spec.GetObjectName() &&
 match_module_spec.GetObjectName() != GetObjectName())
   return false;
-if (match_module_spec.GetFileSpecPtr()) {
-  const FileSpec  = match_module_spec.GetFileSpec();
-  if (!FileSpec::Equal(fspec, GetFileSpec(),
-   !fspec.GetDirectory().IsEmpty()))
-return false;
-}
-if (GetPlatformFileSpec() && match_module_spec.GetPlatformFileSpecPtr()) {
-  const FileSpec  = match_module_spec.GetPlatformFileSpec();
-  if (!FileSpec::Equal(fspec, GetPlatformFileSpec(),
-   !fspec.GetDirectory().IsEmpty()))
-return false;
+if (!FileSpec::Match(match_module_spec.GetFileSpec(), GetFileSpec()))
+  return false;
+if (GetPlatformFileSpec() &&
+!FileSpec::Match(match_module_spec.GetPlatformFileSpec(),
+ GetPlatformFileSpec())) {
+  return false;
 }
 // Only match the symbol file spec if there is one in this ModuleSpec
-if (GetSymbolFileSpec() && match_module_spec.GetSymbolFileSpecPtr()) {
-  const FileSpec  = match_module_spec.GetSymbolFileSpec();

[Lldb-commits] [lldb] 817d618 - [lldb/Editline] Fix a -Wreturn-type warning with gcc

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2019-12-04T10:44:12+01:00
New Revision: 817d6184e75db5eefca907a296e8379d8f570e7e

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

LOG: [lldb/Editline] Fix a -Wreturn-type warning with gcc

Added: 


Modified: 
lldb/source/Host/common/Editline.cpp

Removed: 




diff  --git a/lldb/source/Host/common/Editline.cpp 
b/lldb/source/Host/common/Editline.cpp
index 45d3db24bbc6..b29c218f0369 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -120,6 +120,7 @@ static int GetOperation(HistoryOperation op) {
 case HistoryOperation::Newest:
   return H_LAST;
   }
+  llvm_unreachable("Fully covered switch!");
 }
 
 



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

[Lldb-commits] [lldb] 4d37f18 - [lldb][NFC] Extract single member parsing out of DWARFASTParserClang::ParseChildMembers

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-04T10:05:40+01:00
New Revision: 4d37f18b29cc3fd1abd336ec10baca17694d035f

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

LOG: [lldb][NFC] Extract single member parsing out of 
DWARFASTParserClang::ParseChildMembers

ParseChildMembers does a few things, only one part is actually parsing a single
member. This extracts the member parsing logic into its own function.

This commit just moves the code as-is into its own function and forwards the 
parameters/
local variables to it, which means it should be NFC.

The only actual changes to the code are replacing 'break's (and one very 
curious 'continue'
that behaves like a 'break') with 'return's.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ca1db03b02fa..09f5b28449cb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2448,493 +2448,500 @@ Function 
*DWARFASTParserClang::ParseFunctionFromDWARF(CompileUnit _unit,
   return nullptr;
 }
 
-bool DWARFASTParserClang::ParseChildMembers(
-const DWARFDIE _die, CompilerType _clang_type,
-const LanguageType class_language,
-std::vector> _classes,
+void DWARFASTParserClang::ParseSingleMember(
+const DWARFDIE , const DWARFDIE _die,
+lldb_private::CompilerType _clang_type,
+const lldb::LanguageType class_language,
 std::vector _accessibilities,
-std::vector _function_dies,
-DelayedPropertyList _properties, AccessType _accessibility,
-bool _a_class, ClangASTImporter::LayoutInfo _info) {
-  if (!parent_die)
-return false;
-
+lldb::AccessType _accessibility,
+DelayedPropertyList _properties,
+lldb_private::ClangASTImporter::LayoutInfo _info,
+BitfieldInfo _field_info) {
+  ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
+  const dw_tag_t tag = die.Tag();
   // Get the parent byte size so we can verify any members will fit
   const uint64_t parent_byte_size =
   parent_die.GetAttributeValueAsUnsigned(DW_AT_byte_size, UINT64_MAX);
   const uint64_t parent_bit_size =
   parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
 
-  BitfieldInfo last_field_info;
-
-  ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
-  ClangASTContext *ast =
-  
llvm::dyn_cast_or_null(class_clang_type.GetTypeSystem());
-  if (ast == nullptr)
-return false;
-
-  for (DWARFDIE die = parent_die.GetFirstChild(); die.IsValid();
-   die = die.GetSibling()) {
-dw_tag_t tag = die.Tag();
-
-switch (tag) {
-case DW_TAG_member:
-case DW_TAG_APPLE_property: {
-  DWARFAttributes attributes;
-  const size_t num_attributes = die.GetAttributes(attributes);
-  if (num_attributes > 0) {
-const char *name = nullptr;
-const char *prop_name = nullptr;
-const char *prop_getter_name = nullptr;
-const char *prop_setter_name = nullptr;
-uint32_t prop_attributes = 0;
-
-bool is_artificial = false;
-DWARFFormValue encoding_form;
-AccessType accessibility = eAccessNone;
-uint32_t member_byte_offset =
-(parent_die.Tag() == DW_TAG_union_type) ? 0 : UINT32_MAX;
-llvm::Optional byte_size;
-int64_t bit_offset = 0;
-uint64_t data_bit_offset = UINT64_MAX;
-size_t bit_size = 0;
-bool is_external =
-false; // On DW_TAG_members, this means the member is static
-uint32_t i;
-for (i = 0; i < num_attributes && !is_artificial; ++i) {
-  const dw_attr_t attr = attributes.AttributeAtIndex(i);
-  DWARFFormValue form_value;
-  if (attributes.ExtractFormValueAtIndex(i, form_value)) {
-switch (attr) {
-case DW_AT_name:
-  name = form_value.AsCString();
-  break;
-case DW_AT_type:
-  encoding_form = form_value;
-  break;
-case DW_AT_bit_offset:
-  bit_offset = form_value.Signed();
-  break;
-case DW_AT_bit_size:
-  bit_size = form_value.Unsigned();
-  break;
-case DW_AT_byte_size:
-  byte_size = form_value.Unsigned();
-  break;
-case DW_AT_data_bit_offset:
-  data_bit_offset = form_value.Unsigned();
-  break;
-case DW_AT_data_member_location:
-  if (form_value.BlockData()) {
-

[Lldb-commits] [lldb] c4c464f - [lldb][NFC] Migrate to raw_ostream in Module::GetDescription

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-12-04T09:35:50+01:00
New Revision: c4c464f8a5025ad59733723e1ba1900837760078

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

LOG: [lldb][NFC] Migrate to raw_ostream in Module::GetDescription

Added: 


Modified: 
lldb/include/lldb/Core/Module.h
lldb/source/API/SBModule.cpp
lldb/source/Core/Module.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index bb6c9bdad760..2af18c83f23a 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -190,7 +190,7 @@ class Module : public std::enable_shared_from_this,
   lldb::ModuleSP CalculateSymbolContextModule() override;
 
   void
-  GetDescription(Stream *s,
+  GetDescription(llvm::raw_ostream ,
  lldb::DescriptionLevel level = lldb::eDescriptionLevelFull);
 
   /// Get the module path and object name.

diff  --git a/lldb/source/API/SBModule.cpp b/lldb/source/API/SBModule.cpp
index 7ac189bb4273..4e9dfb0c1e62 100644
--- a/lldb/source/API/SBModule.cpp
+++ b/lldb/source/API/SBModule.cpp
@@ -245,7 +245,7 @@ bool SBModule::GetDescription(SBStream ) {
 
   ModuleSP module_sp(GetSP());
   if (module_sp) {
-module_sp->GetDescription();
+module_sp->GetDescription(strm.AsRawOstream());
   } else
 strm.PutCString("No value");
 

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 360c8c134546..a8a92bb5b1fa 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1061,34 +1061,35 @@ std::string Module::GetSpecificationDescription() const 
{
   return spec;
 }
 
-void Module::GetDescription(Stream *s, lldb::DescriptionLevel level) {
+void Module::GetDescription(llvm::raw_ostream ,
+lldb::DescriptionLevel level) {
   std::lock_guard guard(m_mutex);
 
   if (level >= eDescriptionLevelFull) {
 if (m_arch.IsValid())
-  s->Printf("(%s) ", m_arch.GetArchitectureName());
+  s << llvm::formatv("({0}) ", m_arch.GetArchitectureName());
   }
 
   if (level == eDescriptionLevelBrief) {
 const char *filename = m_file.GetFilename().GetCString();
 if (filename)
-  s->PutCString(filename);
+  s << filename;
   } else {
 char path[PATH_MAX];
 if (m_file.GetPath(path, sizeof(path)))
-  s->PutCString(path);
+  s << path;
   }
 
   const char *object_name = m_object_name.GetCString();
   if (object_name)
-s->Printf("(%s)", object_name);
+s << llvm::formatv("({0})", object_name);
 }
 
 void Module::ReportError(const char *format, ...) {
   if (format && format[0]) {
 StreamString strm;
 strm.PutCString("error: ");
-GetDescription(, lldb::eDescriptionLevelBrief);
+GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelBrief);
 strm.PutChar(' ');
 va_list args;
 va_start(args, format);
@@ -1119,7 +1120,7 @@ void Module::ReportErrorIfModifyDetected(const char 
*format, ...) {
   if (format) {
 StreamString strm;
 strm.PutCString("error: the object file ");
-GetDescription(, lldb::eDescriptionLevelFull);
+GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelFull);
 strm.PutCString(" has been modified\n");
 
 va_list args;
@@ -1145,7 +1146,7 @@ void Module::ReportWarning(const char *format, ...) {
   if (format && format[0]) {
 StreamString strm;
 strm.PutCString("warning: ");
-GetDescription(, lldb::eDescriptionLevelFull);
+GetDescription(strm.AsRawOstream(), lldb::eDescriptionLevelFull);
 strm.PutChar(' ');
 
 va_list args;
@@ -1166,7 +1167,7 @@ void Module::ReportWarning(const char *format, ...) {
 void Module::LogMessage(Log *log, const char *format, ...) {
   if (log != nullptr) {
 StreamString log_message;
-GetDescription(_message, lldb::eDescriptionLevelFull);
+GetDescription(log_message.AsRawOstream(), lldb::eDescriptionLevelFull);
 log_message.PutCString(": ");
 va_list args;
 va_start(args, format);
@@ -1179,7 +1180,7 @@ void Module::LogMessage(Log *log, const char *format, 
...) {
 void Module::LogMessageVerboseBacktrace(Log *log, const char *format, ...) {
   if (log != nullptr) {
 StreamString log_message;
-GetDescription(_message, lldb::eDescriptionLevelFull);
+GetDescription(log_message.AsRawOstream(), lldb::eDescriptionLevelFull);
 log_message.PutCString(": ");
 va_list args;
 va_start(args, format);

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
index 654585cb35eb..fb8b48cc108b 100644
--- 

[Lldb-commits] [PATCH] D70883: [vscode.py] Make read_packet only return None when EOF

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This patch seems really weird to me... We may be able to work around random 
lldb output in the test this way, but that won't help any other DAP client who 
ends up talking to lldb-vscode (or do they have similar workarounds already?). 
In fact it might make things worse -- right now, I guess this is only an issue 
if the user has a noisy .lldbinit file, but if we start ignoring this, other 
random output can appear in the future.

If having a "clean" stdout is important in lldb-vscode, then I think it would 
be better to *not* merge this patch, and try to fix any places which are 
polluting it instead. Since everything should be using the SBDebuggers notion 
of "stdout", anything which prints to the process stdout directly is a bug...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70883



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

[Lldb-commits] [PATCH] D70886: [lldb-vscode] capture the debuggers file handles before lldbinit runs

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D70886#1767654 , @aadsm wrote:

> > Maybe you could do something similar to LocalLLDBInit.test ?
>
> That test uses the `lldb` `-S` (and others) flags that `lldb-vscode` doesn't 
> support :(. These flags should really be added to the initialize packet but 
> they're very specific to lldb and the DAP doesn't support it.. We could 
> implement something like what `Driver::ProcessArgs` does and pass options 
> through envs but, the logic in ProcessArgs itself is sketchy (just like the 
> comment mentions) and I l also find it odd to pass options through env vars...


I was specifically referring to the part where the test sets up an `.lldbinit` 
in a random directory and then points the HOME environment variable there in 
order to "trick" lldb into executing it. It's true that the test also uses 
`-S`/`--source-before-file`, but nothing in there is really needed to test this 
functionality (i.e., making sure that lldb-vscode handles the lldbinit file 
reasonably -- making sure that it does not get confused by ambient lldbinit 
files is a different topic).

As for -no-lldbinit functionality, all of the proposed solutions (cmdline flag, 
environment variable, protocol flag...) seem reasonable to me. The environment 
variable is probably easiest to implement, and should be sufficient if we only 
want to use this for testing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70886



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