[Lldb-commits] [lldb] a497e1b - [lldb] Use CompletionRequest in REPL::CompleteCode and remove translation code to old API

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

Author: Raphael Isemann
Date: 2020-01-29T08:56:32+01:00
New Revision: a497e1b5ea7a681ef1b40b5c11f411bfe0e807d0

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

LOG: [lldb] Use CompletionRequest in REPL::CompleteCode and remove translation 
code to old API

Any REPL client should just move to CompletionRequest instead of relying on
the translation code from the old API, so let's remove that translation code.

Added: 


Modified: 
lldb/include/lldb/Expression/REPL.h
lldb/source/Expression/REPL.cpp

Removed: 




diff  --git a/lldb/include/lldb/Expression/REPL.h 
b/lldb/include/lldb/Expression/REPL.h
index 035ad63271e4..e96dfb0499d9 100644
--- a/lldb/include/lldb/Expression/REPL.h
+++ b/lldb/include/lldb/Expression/REPL.h
@@ -130,8 +130,8 @@ class REPL : public IOHandlerDelegate {
 lldb::ValueObjectSP _sp,
 ExpressionVariable *var = nullptr) = 0;
 
-  virtual int CompleteCode(const std::string _code,
-   StringList ) = 0;
+  virtual void CompleteCode(const std::string _code,
+CompletionRequest ) = 0;
 
   OptionGroupFormat m_format_options = OptionGroupFormat(lldb::eFormatDefault);
   OptionGroupValueObjectDisplay m_varobj_options;

diff  --git a/lldb/source/Expression/REPL.cpp b/lldb/source/Expression/REPL.cpp
index 670c0ec98c29..d49b7644d7f6 100644
--- a/lldb/source/Expression/REPL.cpp
+++ b/lldb/source/Expression/REPL.cpp
@@ -488,14 +488,7 @@ void REPL::IOHandlerComplete(IOHandler _handler,
   current_code.append("\n");
   current_code += request.GetRawLine();
 
-  StringList matches;
-  int result = CompleteCode(current_code, matches);
-  if (result == -2) {
-assert(matches.GetSize() == 1);
-request.AddCompletion(matches.GetStringAtIndex(0), "",
-  CompletionMode::RewriteLine);
-  } else
-request.AddCompletions(matches);
+  CompleteCode(current_code, request);
 }
 
 bool QuitCommandOverrideCallback(void *baton, const char **argv) {



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


[Lldb-commits] [PATCH] D73589: Improve help text for (lldb) target symbols add

2020-01-28 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

Looks good, for the --shlib option I would get rid of the "full path or base 
name" language and just say "name", my two cents.


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

https://reviews.llvm.org/D73589



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


[Lldb-commits] [lldb] ede5cd9 - [lldb/API] Fix bogus copy assignment operator

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

Author: Jonas Devlieghere
Date: 2020-01-28T17:11:12-08:00
New Revision: ede5cd9a45bd12c0676da80472e629801faa37bf

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

LOG: [lldb/API] Fix bogus copy assignment operator

The copy assignment operator is supposed to return the class and not
void. Fix the methods and the reproducer instrumentation macros.

Added: 


Modified: 
lldb/include/lldb/API/SBLaunchInfo.h
lldb/include/lldb/API/SBPlatform.h
lldb/source/API/SBLaunchInfo.cpp
lldb/source/API/SBPlatform.cpp

Removed: 




diff  --git a/lldb/include/lldb/API/SBLaunchInfo.h 
b/lldb/include/lldb/API/SBLaunchInfo.h
index eef6bc78e5f2..d78106abf0c7 100644
--- a/lldb/include/lldb/API/SBLaunchInfo.h
+++ b/lldb/include/lldb/API/SBLaunchInfo.h
@@ -28,7 +28,7 @@ class LLDB_API SBLaunchInfo {
 
   SBLaunchInfo(const SBLaunchInfo );
 
-  void operator=(const SBLaunchInfo );
+  SBLaunchInfo =(const SBLaunchInfo );
 
   lldb::pid_t GetProcessID();
 

diff  --git a/lldb/include/lldb/API/SBPlatform.h 
b/lldb/include/lldb/API/SBPlatform.h
index 9e3d03e9a84f..533bb4dce78a 100644
--- a/lldb/include/lldb/API/SBPlatform.h
+++ b/lldb/include/lldb/API/SBPlatform.h
@@ -28,7 +28,7 @@ class LLDB_API SBPlatformConnectOptions {
 
   ~SBPlatformConnectOptions();
 
-  void operator=(const SBPlatformConnectOptions );
+  SBPlatformConnectOptions =(const SBPlatformConnectOptions );
 
   const char *GetURL();
 
@@ -55,6 +55,8 @@ class LLDB_API SBPlatformShellCommand {
 
   SBPlatformShellCommand(const SBPlatformShellCommand );
 
+  SBPlatformShellCommand =(const SBPlatformShellCommand );
+
   ~SBPlatformShellCommand();
 
   void Clear();
@@ -91,7 +93,7 @@ class LLDB_API SBPlatform {
 
   SBPlatform(const SBPlatform );
 
-  void operator=(const SBPlatform );
+  SBPlatform =(const SBPlatform );
 
   ~SBPlatform();
 

diff  --git a/lldb/source/API/SBLaunchInfo.cpp 
b/lldb/source/API/SBLaunchInfo.cpp
index 9e7159995cc4..9e69b53bb3f8 100644
--- a/lldb/source/API/SBLaunchInfo.cpp
+++ b/lldb/source/API/SBLaunchInfo.cpp
@@ -49,11 +49,12 @@ SBLaunchInfo::SBLaunchInfo(const SBLaunchInfo ) {
   m_opaque_sp = rhs.m_opaque_sp;
 }
 
-void SBLaunchInfo::operator=(const SBLaunchInfo ) {
-  LLDB_RECORD_METHOD(void, SBLaunchInfo, operator=,(const lldb::SBLaunchInfo 
&),
- rhs);
+SBLaunchInfo ::operator=(const SBLaunchInfo ) {
+  LLDB_RECORD_METHOD(SBLaunchInfo &,
+ SBLaunchInfo, operator=,(const lldb::SBLaunchInfo &), 
rhs);
 
   m_opaque_sp = rhs.m_opaque_sp;
+  return LLDB_RECORD_RESULT(*this);
 }
 
 SBLaunchInfo::~SBLaunchInfo() {}
@@ -336,7 +337,7 @@ template <>
 void RegisterMethods(Registry ) {
   LLDB_REGISTER_CONSTRUCTOR(SBLaunchInfo, (const char **));
   LLDB_REGISTER_CONSTRUCTOR(SBLaunchInfo, (const lldb::SBLaunchInfo &));
-  LLDB_REGISTER_METHOD(void,
+  LLDB_REGISTER_METHOD(SBLaunchInfo &,
SBLaunchInfo, operator=,(const lldb::SBLaunchInfo &));
   LLDB_REGISTER_METHOD(lldb::pid_t, SBLaunchInfo, GetProcessID, ());
   LLDB_REGISTER_METHOD(uint32_t, SBLaunchInfo, GetUserID, ());

diff  --git a/lldb/source/API/SBPlatform.cpp b/lldb/source/API/SBPlatform.cpp
index 76b851517e36..2603ac728065 100644
--- a/lldb/source/API/SBPlatform.cpp
+++ b/lldb/source/API/SBPlatform.cpp
@@ -80,14 +80,16 @@ SBPlatformConnectOptions::SBPlatformConnectOptions(
 
 SBPlatformConnectOptions::~SBPlatformConnectOptions() { delete m_opaque_ptr; }
 
-void SBPlatformConnectOptions::operator=(const SBPlatformConnectOptions ) {
+SBPlatformConnectOptions ::
+operator=(const SBPlatformConnectOptions ) {
   LLDB_RECORD_METHOD(
-  void,
+  SBPlatformConnectOptions &,
   SBPlatformConnectOptions, operator=,(
 const lldb::SBPlatformConnectOptions &),
   rhs);
 
   *m_opaque_ptr = *rhs.m_opaque_ptr;
+  return LLDB_RECORD_RESULT(*this);
 }
 
 const char *SBPlatformConnectOptions::GetURL() {
@@ -174,6 +176,18 @@ SBPlatformShellCommand::SBPlatformShellCommand(
   *m_opaque_ptr = *rhs.m_opaque_ptr;
 }
 
+SBPlatformShellCommand ::
+operator=(const SBPlatformShellCommand ) {
+
+  LLDB_RECORD_METHOD(
+  SBPlatformShellCommand &,
+  SBPlatformShellCommand, operator=,(const lldb::SBPlatformShellCommand &),
+  rhs);
+
+  *m_opaque_ptr = *rhs.m_opaque_ptr;
+  return LLDB_RECORD_RESULT(*this);
+}
+
 SBPlatformShellCommand::~SBPlatformShellCommand() { delete m_opaque_ptr; }
 
 void SBPlatformShellCommand::Clear() {
@@ -279,11 +293,12 @@ SBPlatform::SBPlatform(const SBPlatform ) {
   m_opaque_sp = rhs.m_opaque_sp;
 }
 
-void SBPlatform::operator=(const SBPlatform ) {
-  LLDB_RECORD_METHOD(void, SBPlatform, operator=,(const lldb::SBPlatform &),
- rhs);
+SBPlatform ::operator=(const SBPlatform ) {
+  

[Lldb-commits] [PATCH] D73594: Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

2020-01-28 Thread Adrian McCarthy via Phabricator via lldb-commits
amccarth created this revision.
amccarth added reviewers: clayborg, jasonmolenda.

- [NFC] Renamed local `matching_module_list` to `matching_modules` for 
conciseness.

- [NFC] Eliminated redundant local variable `num_matches` to reduce the risk 
that changes get it out of sync with `matching_modules.GetSize()`.

- Used an early return from case where the symbol file specified matches 
multiple modules.  This is a slight behavior change, but it's an improvement: 
It didn't make sense to tell the user that the symbol file simultaneously 
matched multiple modules and no modules.

- [NFC] Used an early return from the case where no matches are found, to 
better align with LLVM coding style.

- [NFC] Simplified call of `AppendWarningWithFormat("%s", stuff)` to 
`AppendWarning(stuff)`.  I don't think this adds any copies.  It does construct 
a StringRef, but it was going to have to scan the string for the length anyway.

- [NFC] Removed unnecessary comments and reworded others for clarity.

- Used an early return if the symbol file could not be loaded.  This is a 
behavior change because previously it could fail silently.

- Used an early return if the object file could not be retrieved from the 
symbol file.  Again, this is a change because now there's an error message.

- If we successfully load a symbol file that seems to match, but then detect a 
file name mismatch, now we only issue a warning.  We've already determined they 
match, so it seems pointless to change our minds.

- [NFC] Eliminated a namespace alias that wasn't particularly helpful.


https://reviews.llvm.org/D73594

Files:
  lldb/source/Commands/CommandObjectTarget.cpp

Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -4054,12 +4054,10 @@
 module_spec.GetFileSpec().GetFilename() = symbol_fspec.GetFilename();
 }
 
-// We now have a module that represents a symbol file that can be used
-// for a module that might exist in the current target, so we need to
-// find that module in the target
-ModuleList matching_module_list;
+// Now module_spec represents a symbol file for a module that might exist
+// in the current target.  Let's find possible matches.
+ModuleList matching_modules;
 
-size_t num_matches = 0;
 // First extract all module specs from the symbol file
 lldb_private::ModuleSpecList symfile_module_specs;
 if (ObjectFile::GetModuleSpecifications(module_spec.GetSymbolFileSpec(),
@@ -4070,34 +4068,30 @@
   target_arch_module_spec.GetArchitecture() = target->GetArchitecture();
   if (symfile_module_specs.FindMatchingModuleSpec(target_arch_module_spec,
   symfile_module_spec)) {
-// See if it has a UUID?
 if (symfile_module_spec.GetUUID().IsValid()) {
   // It has a UUID, look for this UUID in the target modules
   ModuleSpec symfile_uuid_module_spec;
   symfile_uuid_module_spec.GetUUID() = symfile_module_spec.GetUUID();
   target->GetImages().FindModules(symfile_uuid_module_spec,
-  matching_module_list);
-  num_matches = matching_module_list.GetSize();
+  matching_modules);
 }
   }
 
-  if (num_matches == 0) {
-// No matches yet, iterate through the module specs to find a UUID
-// value that we can match up to an image in our target
-const size_t num_symfile_module_specs =
-symfile_module_specs.GetSize();
-for (size_t i = 0; i < num_symfile_module_specs && num_matches == 0;
-  ++i) {
+  if (matching_modules.IsEmpty()) {
+// No matches yet.  Iterate through the module specs to find a UUID
+// value that we can match up to an image in our target.
+const size_t num_symfile_module_specs = symfile_module_specs.GetSize();
+for (size_t i = 0;
+ i < num_symfile_module_specs && matching_modules.IsEmpty(); ++i) {
   if (symfile_module_specs.GetModuleSpecAtIndex(
   i, symfile_module_spec)) {
 if (symfile_module_spec.GetUUID().IsValid()) {
-  // It has a UUID, look for this UUID in the target modules
+  // It has a UUID.  Look for this UUID in the target modules.
   ModuleSpec symfile_uuid_module_spec;
   symfile_uuid_module_spec.GetUUID() =
   symfile_module_spec.GetUUID();
   target->GetImages().FindModules(symfile_uuid_module_spec,
-  matching_module_list);
-  num_matches = matching_module_list.GetSize();
+  matching_modules);
 }
   }
 }
@@ -4105,13 +4099,12 @@
 }
 
 

[Lldb-commits] [lldb] 1dfe7b5 - [lldb/API] Implement the copy (assignment) constructor for SBLaunchInfo

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

Author: Jonas Devlieghere
Date: 2020-01-28T16:44:58-08:00
New Revision: 1dfe7b5be63e9d80e2704255dbeb6813cc7f6e57

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

LOG: [lldb/API] Implement the copy (assignment) constructor for SBLaunchInfo

Currently the constructor is compiler generated which means it doesn't
get instrumented for the reproducers.

Added: 


Modified: 
lldb/include/lldb/API/SBLaunchInfo.h
lldb/source/API/SBLaunchInfo.cpp

Removed: 




diff  --git a/lldb/include/lldb/API/SBLaunchInfo.h 
b/lldb/include/lldb/API/SBLaunchInfo.h
index c7b381ffdf97..eef6bc78e5f2 100644
--- a/lldb/include/lldb/API/SBLaunchInfo.h
+++ b/lldb/include/lldb/API/SBLaunchInfo.h
@@ -26,6 +26,10 @@ class LLDB_API SBLaunchInfo {
 
   ~SBLaunchInfo();
 
+  SBLaunchInfo(const SBLaunchInfo );
+
+  void operator=(const SBLaunchInfo );
+
   lldb::pid_t GetProcessID();
 
   uint32_t GetUserID();

diff  --git a/lldb/source/API/SBLaunchInfo.cpp 
b/lldb/source/API/SBLaunchInfo.cpp
index e98e648edc3d..9e7159995cc4 100644
--- a/lldb/source/API/SBLaunchInfo.cpp
+++ b/lldb/source/API/SBLaunchInfo.cpp
@@ -43,6 +43,19 @@ SBLaunchInfo::SBLaunchInfo(const char **argv)
 m_opaque_sp->GetArguments().SetArguments(argv);
 }
 
+SBLaunchInfo::SBLaunchInfo(const SBLaunchInfo ) {
+  LLDB_RECORD_CONSTRUCTOR(SBLaunchInfo, (const lldb::SBLaunchInfo &), rhs);
+
+  m_opaque_sp = rhs.m_opaque_sp;
+}
+
+void SBLaunchInfo::operator=(const SBLaunchInfo ) {
+  LLDB_RECORD_METHOD(void, SBLaunchInfo, operator=,(const lldb::SBLaunchInfo 
&),
+ rhs);
+
+  m_opaque_sp = rhs.m_opaque_sp;
+}
+
 SBLaunchInfo::~SBLaunchInfo() {}
 
 const lldb_private::ProcessLaunchInfo ::ref() const {
@@ -322,6 +335,9 @@ namespace repro {
 template <>
 void RegisterMethods(Registry ) {
   LLDB_REGISTER_CONSTRUCTOR(SBLaunchInfo, (const char **));
+  LLDB_REGISTER_CONSTRUCTOR(SBLaunchInfo, (const lldb::SBLaunchInfo &));
+  LLDB_REGISTER_METHOD(void,
+   SBLaunchInfo, operator=,(const lldb::SBLaunchInfo &));
   LLDB_REGISTER_METHOD(lldb::pid_t, SBLaunchInfo, GetProcessID, ());
   LLDB_REGISTER_METHOD(uint32_t, SBLaunchInfo, GetUserID, ());
   LLDB_REGISTER_METHOD(uint32_t, SBLaunchInfo, GetGroupID, ());



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


[Lldb-commits] [lldb] 66dc467 - [lldb/API] Implement the copy (assignment) constructor for SBPlatform

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

Author: Jonas Devlieghere
Date: 2020-01-28T16:23:03-08:00
New Revision: 66dc467228789cbe94a125d7fdedf42556052ad3

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

LOG: [lldb/API] Implement the copy (assignment) constructor for SBPlatform

Currently the constructor is compiler generated which means it doesn't
get instrumented for the reproducers.

Added: 


Modified: 
lldb/include/lldb/API/SBPlatform.h
lldb/source/API/SBPlatform.cpp

Removed: 




diff  --git a/lldb/include/lldb/API/SBPlatform.h 
b/lldb/include/lldb/API/SBPlatform.h
index 7207b2e2a78d..9e3d03e9a84f 100644
--- a/lldb/include/lldb/API/SBPlatform.h
+++ b/lldb/include/lldb/API/SBPlatform.h
@@ -89,6 +89,10 @@ class LLDB_API SBPlatform {
 
   SBPlatform(const char *platform_name);
 
+  SBPlatform(const SBPlatform );
+
+  void operator=(const SBPlatform );
+
   ~SBPlatform();
 
   explicit operator bool() const;

diff  --git a/lldb/source/API/SBPlatform.cpp b/lldb/source/API/SBPlatform.cpp
index 8f98d2d5e5a0..76b851517e36 100644
--- a/lldb/source/API/SBPlatform.cpp
+++ b/lldb/source/API/SBPlatform.cpp
@@ -273,6 +273,19 @@ SBPlatform::SBPlatform(const char *platform_name) : 
m_opaque_sp() {
 m_opaque_sp = Platform::Create(ConstString(platform_name), error);
 }
 
+SBPlatform::SBPlatform(const SBPlatform ) {
+  LLDB_RECORD_CONSTRUCTOR(SBPlatform, (const lldb::SBPlatform &), rhs);
+
+  m_opaque_sp = rhs.m_opaque_sp;
+}
+
+void SBPlatform::operator=(const SBPlatform ) {
+  LLDB_RECORD_METHOD(void, SBPlatform, operator=,(const lldb::SBPlatform &),
+ rhs);
+
+  m_opaque_sp = rhs.m_opaque_sp;
+}
+
 SBPlatform::~SBPlatform() {}
 
 bool SBPlatform::IsValid() const {
@@ -666,6 +679,8 @@ template <>
 void RegisterMethods(Registry ) {
   LLDB_REGISTER_CONSTRUCTOR(SBPlatform, ());
   LLDB_REGISTER_CONSTRUCTOR(SBPlatform, (const char *));
+  LLDB_REGISTER_CONSTRUCTOR(SBPlatform, (const lldb::SBPlatform &));
+  LLDB_REGISTER_METHOD(void, SBPlatform, operator=,(const lldb::SBPlatform &));
   LLDB_REGISTER_METHOD_CONST(bool, SBPlatform, IsValid, ());
   LLDB_REGISTER_METHOD_CONST(bool, SBPlatform, operator bool, ());
   LLDB_REGISTER_METHOD(void, SBPlatform, Clear, ());



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


[Lldb-commits] [lldb] 71b022e - [lldb] Remove unused header from ValueObject.cpp

2020-01-28 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2020-01-28T16:13:21-08:00
New Revision: 71b022ee55d4bfc5a3c3539f4c1c22c7b308a605

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

LOG: [lldb] Remove unused header from ValueObject.cpp

In commit 5eaf44f99f0a0a3bdfa892892b8aaca841c8dbe0 I removed the last
instance of TypeSystemClang from ValueObject, so the header is no longer
needed.

Added: 


Modified: 
lldb/source/Core/ValueObject.cpp

Removed: 




diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index f3fa09cc8529..283ab309e314 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -25,7 +25,6 @@
 #include "lldb/DataFormatters/ValueObjectPrinter.h"
 #include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Host/Config.h"
-#include "lldb/Symbol/TypeSystemClang.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Symbol/Declaration.h"



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


[Lldb-commits] [PATCH] D73589: Improve help text for (lldb) target symbols add

2020-01-28 Thread Adrian McCarthy via Phabricator via lldb-commits
amccarth created this revision.
amccarth added a reviewer: aaron.ballman.

There were some missing words and awkward syntax.  I think this is clearer.


https://reviews.llvm.org/D73589

Files:
  lldb/source/Commands/CommandObjectTarget.cpp


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -3999,19 +3999,20 @@
   : CommandObjectParsed(
 interpreter, "target symbols add",
 "Add a debug symbol file to one of the target's current modules by 
"
-"specifying a path to a debug symbols file, or using the options "
-"to specify a module to download symbols for.",
+"specifying a path to a debug symbols file or by using the options 
"
+"to specify a module.",
 "target symbols add  []",
 eCommandRequiresTarget),
 m_option_group(),
 m_file_option(
 LLDB_OPT_SET_1, false, "shlib", 's',
 CommandCompletions::eModuleCompletion, eArgTypeShlibName,
-"Fullpath or basename for module to find debug symbols for."),
+"Locate the debug symbols for the shared library specified by "
+"full path or base name."),
 m_current_frame_option(
 LLDB_OPT_SET_2, false, "frame", 'F',
-"Locate the debug symbols the currently selected frame.", false,
-true)
+"Locate the debug symbols for the currently selected frame.",
+false, true)
 
   {
 m_option_group.Append(_uuid_option_group, LLDB_OPT_SET_ALL,


Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -3999,19 +3999,20 @@
   : CommandObjectParsed(
 interpreter, "target symbols add",
 "Add a debug symbol file to one of the target's current modules by "
-"specifying a path to a debug symbols file, or using the options "
-"to specify a module to download symbols for.",
+"specifying a path to a debug symbols file or by using the options "
+"to specify a module.",
 "target symbols add  []",
 eCommandRequiresTarget),
 m_option_group(),
 m_file_option(
 LLDB_OPT_SET_1, false, "shlib", 's',
 CommandCompletions::eModuleCompletion, eArgTypeShlibName,
-"Fullpath or basename for module to find debug symbols for."),
+"Locate the debug symbols for the shared library specified by "
+"full path or base name."),
 m_current_frame_option(
 LLDB_OPT_SET_2, false, "frame", 'F',
-"Locate the debug symbols the currently selected frame.", false,
-true)
+"Locate the debug symbols for the currently selected frame.",
+false, true)
 
   {
 m_option_group.Append(_uuid_option_group, LLDB_OPT_SET_ALL,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 9a8d42e - [lldb/Plugin] Fix implicit conversion in GDBRemote

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

Author: Jonas Devlieghere
Date: 2020-01-28T15:16:56-08:00
New Revision: 9a8d42e60803ba0b67b3669630530de04cafc079

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

LOG: [lldb/Plugin] Fix implicit conversion in GDBRemote

Added: 


Modified: 

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index cac26745a6bf..abb8f63b8b52 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1228,7 +1228,7 @@ void GDBRemoteCommunicationServerCommon::
 if (cpu_subtype != 0)
   response.Printf("cpusubtype:%" PRIx32 ";", cpu_subtype);
 
-const std::string vendor = proc_triple.getVendorName();
+const std::string vendor = proc_triple.getVendorName().str();
 if (!vendor.empty())
   response.Printf("vendor:%s;", vendor.c_str());
 #else



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


[Lldb-commits] [lldb] e9326ed - [lldb/Reproducer] s/nullptr_t/std::nullptr_t/

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

Author: Jonas Devlieghere
Date: 2020-01-28T15:15:13-08:00
New Revision: e9326ed9067834dca0a1fe752a55c534ed938f8c

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

LOG: [lldb/Reproducer] s/nullptr_t/std::nullptr_t/

Fixes error: unknown type name 'nullptr_t'; did you mean
'std::nullptr_t'.

Added: 


Modified: 
lldb/include/lldb/Utility/ReproducerInstrumentation.h

Removed: 




diff  --git a/lldb/include/lldb/Utility/ReproducerInstrumentation.h 
b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
index 91420a147834..2698338a38bb 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -44,8 +44,8 @@ inline void stringify_append(llvm::raw_string_ostream 
,
 }
 
 template <>
-inline void stringify_append(llvm::raw_string_ostream ,
-const nullptr_t ) {
+inline void stringify_append(llvm::raw_string_ostream ,
+ const std::nullptr_t ) {
   ss << "\"nullptr\"";
 }
 



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


[Lldb-commits] [lldb] 19580c3 - Fix implicit conversion in the lldb Python plugin

2020-01-28 Thread Benjamin Kramer via lldb-commits

Author: Benjamin Kramer
Date: 2020-01-29T00:07:50+01:00
New Revision: 19580c3755a1dc198005839a73a7bad5c108f203

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

LOG: Fix implicit conversion in the lldb Python plugin

Added: 


Modified: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index b659957f8dc4..92060ee08cdc 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -58,7 +58,7 @@ Expected 
python::As(Expected &) {
   auto utf8 = str.AsUTF8();
   if (!utf8)
 return utf8.takeError();
-  return utf8.get();
+  return std::string(utf8.get());
 }
 
 void StructuredPythonObject::Serialize(llvm::json::OStream ) const {



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


[Lldb-commits] [lldb] 620f5fa - [lldb/Reproducer] Include result in recording statements

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

Author: Jonas Devlieghere
Date: 2020-01-28T15:03:13-08:00
New Revision: 620f5faf1f340e594bd9cac39a64d9236a324fb9

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

LOG: [lldb/Reproducer] Include result in recording statements

Include the return value in the recording log statements. This helps
diagnose uninstrumented (copy assignment) constructors.

Added: 


Modified: 
lldb/include/lldb/Utility/ReproducerInstrumentation.h

Removed: 




diff  --git a/lldb/include/lldb/Utility/ReproducerInstrumentation.h 
b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
index eed3c1bfdb95..91420a147834 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -43,6 +43,12 @@ inline void stringify_append(llvm::raw_string_ostream 
,
   ss << '\"' << t << '\"';
 }
 
+template <>
+inline void stringify_append(llvm::raw_string_ostream ,
+const nullptr_t ) {
+  ss << "\"nullptr\"";
+}
+
 template 
 inline void stringify_helper(llvm::raw_string_ostream , const Head ) {
   stringify_append(ss, head);
@@ -648,10 +654,6 @@ class Recorder {
 
 unsigned id = registry.GetID(uintptr_t(f));
 
-#ifdef LLDB_REPRO_INSTR_TRACE
-Log(id);
-#endif
-
 serializer.SerializeAll(id);
 serializer.SerializeAll(args...);
 
@@ -662,6 +664,10 @@ class Recorder {
   serializer.SerializeAll(0);
   m_result_recorded = true;
 }
+
+#ifdef LLDB_REPRO_INSTR_TRACE
+Log(id, m_result_recorded);
+#endif
   }
 
   /// Records a single function call.
@@ -674,16 +680,16 @@ class Recorder {
 
 unsigned id = registry.GetID(uintptr_t(f));
 
-#ifdef LLDB_REPRO_INSTR_TRACE
-Log(id);
-#endif
-
 serializer.SerializeAll(id);
 serializer.SerializeAll(args...);
 
 // Record result.
 serializer.SerializeAll(0);
 m_result_recorded = true;
+
+#ifdef LLDB_REPRO_INSTR_TRACE
+Log(id, true);
+#endif
   }
 
   /// Record the result of a function call.
@@ -693,6 +699,9 @@ class Recorder {
   assert(!m_result_recorded);
   m_serializer->SerializeAll(r);
   m_result_recorded = true;
+#ifdef LLDB_REPRO_INSTR_TRACE
+  llvm::errs() << " -> " << stringify_args(r) << '\n';
+#endif
 }
 return std::forward(r);
   }
@@ -706,9 +715,11 @@ class Recorder {
   bool ShouldCapture() { return m_local_boundary; }
 
 #ifdef LLDB_REPRO_INSTR_TRACE
-  void Log(unsigned id) {
+  void Log(unsigned id, bool newline) {
 llvm::errs() << "Recording " << id << ": " << m_pretty_func << " ("
- << m_pretty_args << ")\n";
+ << m_pretty_args << ")";
+if (newline)
+  llvm::errs() << '\n';
   }
 #endif
 



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


[Lldb-commits] [PATCH] D73517: [lldb] Delete ValueObject::GetBaseClassPath

2020-01-28 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5eaf44f99f0a: [lldb] Delete ValueObject::GetBaseClassPath 
(authored by xiaobai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73517

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


Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2013,23 +2013,6 @@
   return m_synthetic_value != nullptr;
 }
 
-bool ValueObject::GetBaseClassPath(Stream ) {
-  if (IsBaseClass()) {
-bool parent_had_base_class =
-GetParent() && GetParent()->GetBaseClassPath(s);
-CompilerType compiler_type = GetCompilerType();
-llvm::Optional cxx_class_name =
-TypeSystemClang::GetCXXClassName(compiler_type);
-if (cxx_class_name) {
-  if (parent_had_base_class)
-s.PutCString("::");
-  s.PutCString(cxx_class_name.getValue());
-}
-return parent_had_base_class || cxx_class_name;
-  }
-  return false;
-}
-
 ValueObject *ValueObject::GetNonBaseClassParent() {
   if (GetParent()) {
 if (GetParent()->IsBaseClass())
@@ -2139,13 +2122,8 @@
   }
 
   const char *name = GetName().GetCString();
-  if (name) {
-if (qualify_cxx_base_classes) {
-  if (GetBaseClassPath(s))
-s.PutCString("::");
-}
+  if (name)
 s.PutCString(name);
-  }
 }
   }
 
Index: lldb/include/lldb/Core/ValueObject.h
===
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -396,8 +396,6 @@
 
   bool IsIntegerType(bool _signed);
 
-  virtual bool GetBaseClassPath(Stream );
-
   virtual void GetExpressionPath(
   Stream , bool qualify_cxx_base_classes,
   GetExpressionPathFormat = eGetExpressionPathFormatDereferencePointers);


Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2013,23 +2013,6 @@
   return m_synthetic_value != nullptr;
 }
 
-bool ValueObject::GetBaseClassPath(Stream ) {
-  if (IsBaseClass()) {
-bool parent_had_base_class =
-GetParent() && GetParent()->GetBaseClassPath(s);
-CompilerType compiler_type = GetCompilerType();
-llvm::Optional cxx_class_name =
-TypeSystemClang::GetCXXClassName(compiler_type);
-if (cxx_class_name) {
-  if (parent_had_base_class)
-s.PutCString("::");
-  s.PutCString(cxx_class_name.getValue());
-}
-return parent_had_base_class || cxx_class_name;
-  }
-  return false;
-}
-
 ValueObject *ValueObject::GetNonBaseClassParent() {
   if (GetParent()) {
 if (GetParent()->IsBaseClass())
@@ -2139,13 +2122,8 @@
   }
 
   const char *name = GetName().GetCString();
-  if (name) {
-if (qualify_cxx_base_classes) {
-  if (GetBaseClassPath(s))
-s.PutCString("::");
-}
+  if (name)
 s.PutCString(name);
-  }
 }
   }
 
Index: lldb/include/lldb/Core/ValueObject.h
===
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -396,8 +396,6 @@
 
   bool IsIntegerType(bool _signed);
 
-  virtual bool GetBaseClassPath(Stream );
-
   virtual void GetExpressionPath(
   Stream , bool qualify_cxx_base_classes,
   GetExpressionPathFormat = eGetExpressionPathFormatDereferencePointers);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D69820: [Symbol] Add TypeSystem::GetClassName

2020-01-28 Thread Alex Langford via Phabricator via lldb-commits
xiaobai closed this revision.
xiaobai added a comment.

Made obsolete by D73517 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69820



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


[Lldb-commits] [lldb] 5eaf44f - [lldb] Delete ValueObject::GetBaseClassPath

2020-01-28 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2020-01-28T14:11:53-08:00
New Revision: 5eaf44f99f0a0a3bdfa892892b8aaca841c8dbe0

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

LOG: [lldb] Delete ValueObject::GetBaseClassPath

Summary:
This method has exactly one call site, which is only actually executed
if `ValueObject::IsBaseClass` returns false. However, the first thing
that `ValueObject::GetBaseClassPath` does is check if `ValueObject::IsBaseClass`
is true. Because this can never be the case, this method always returns false
and is therefore effectively dead.

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

Added: 


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

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index 1b000e617f0e..aa0ff60b3f7c 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -396,8 +396,6 @@ class ValueObject : public UserID {
 
   bool IsIntegerType(bool _signed);
 
-  virtual bool GetBaseClassPath(Stream );
-
   virtual void GetExpressionPath(
   Stream , bool qualify_cxx_base_classes,
   GetExpressionPathFormat = eGetExpressionPathFormatDereferencePointers);

diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 3a0aecfa646b..f6e7dcc305ac 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -2013,23 +2013,6 @@ bool ValueObject::HasSyntheticValue() {
   return m_synthetic_value != nullptr;
 }
 
-bool ValueObject::GetBaseClassPath(Stream ) {
-  if (IsBaseClass()) {
-bool parent_had_base_class =
-GetParent() && GetParent()->GetBaseClassPath(s);
-CompilerType compiler_type = GetCompilerType();
-llvm::Optional cxx_class_name =
-TypeSystemClang::GetCXXClassName(compiler_type);
-if (cxx_class_name) {
-  if (parent_had_base_class)
-s.PutCString("::");
-  s.PutCString(cxx_class_name.getValue());
-}
-return parent_had_base_class || cxx_class_name;
-  }
-  return false;
-}
-
 ValueObject *ValueObject::GetNonBaseClassParent() {
   if (GetParent()) {
 if (GetParent()->IsBaseClass())
@@ -2139,13 +2122,8 @@ void ValueObject::GetExpressionPath(Stream , bool 
qualify_cxx_base_classes,
   }
 
   const char *name = GetName().GetCString();
-  if (name) {
-if (qualify_cxx_base_classes) {
-  if (GetBaseClassPath(s))
-s.PutCString("::");
-}
+  if (name)
 s.PutCString(name);
-  }
 }
   }
 



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


[Lldb-commits] [PATCH] D72946: [lldb] Remove ClangASTImporter reference from Target

2020-01-28 Thread Alex Langford via Phabricator via lldb-commits
xiaobai closed this revision.
xiaobai added a comment.

Commit c4f6fbe971351273b19a4a819bf6ceae2b70b37e 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72946



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


[Lldb-commits] [PATCH] D72513: Don't fail step out if remote server doesn't implement qMemoryRegionInfo

2020-01-28 Thread Ted Woodward via Phabricator via lldb-commits
ted updated this revision to Diff 240985.
ted added a comment.

Removed the code that sets m_constructor_errors when GetLoadAddressPermissions 
returns False, as requested by @jingham .


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

https://reviews.llvm.org/D72513

Files:
  lldb/source/Target/ThreadPlanStepOut.cpp


Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -130,11 +130,9 @@
 uint32_t permissions = 0;
 if (!m_thread.GetProcess()->GetLoadAddressPermissions(m_return_addr,
   permissions)) {
-  m_constructor_errors.Printf("Return address (0x%" PRIx64
-  ") permissions not found.",
-  m_return_addr);
-  LLDB_LOGF(log, "ThreadPlanStepOut(%p): %s", static_cast(this),
-m_constructor_errors.GetData());
+  LLDB_LOGF(log, "ThreadPlanStepOut(%p): Return address (0x%" PRIx64
+") permissions not found.", static_cast(this),
+m_return_addr);
 } else if (!(permissions & ePermissionsExecutable)) {
   m_constructor_errors.Printf("Return address (0x%" PRIx64
   ") did not point to executable memory.",


Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -130,11 +130,9 @@
 uint32_t permissions = 0;
 if (!m_thread.GetProcess()->GetLoadAddressPermissions(m_return_addr,
   permissions)) {
-  m_constructor_errors.Printf("Return address (0x%" PRIx64
-  ") permissions not found.",
-  m_return_addr);
-  LLDB_LOGF(log, "ThreadPlanStepOut(%p): %s", static_cast(this),
-m_constructor_errors.GetData());
+  LLDB_LOGF(log, "ThreadPlanStepOut(%p): Return address (0x%" PRIx64
+") permissions not found.", static_cast(this),
+m_return_addr);
 } else if (!(permissions & ePermissionsExecutable)) {
   m_constructor_errors.Printf("Return address (0x%" PRIx64
   ") did not point to executable memory.",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] c4f6fbe - [lldb] Remove ClangASTImporter from Target

2020-01-28 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2020-01-28T13:40:49-08:00
New Revision: c4f6fbe971351273b19a4a819bf6ceae2b70b37e

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

LOG: [lldb] Remove ClangASTImporter from Target

Target is one of the classes responsible for vending ClangASTImporter.
Target doesn't need to know anything about ClangASTImporter, so if we
instead have ClangPersistentVariables vend it, we can preserve
existing behavior while improving layering and removing dependencies
from non-plugins to plugins.

Added: 


Modified: 
lldb/include/lldb/Symbol/TypeSystemClang.h
lldb/include/lldb/Target/Target.h
lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
lldb/source/Symbol/TypeSystemClang.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/TypeSystemClang.h 
b/lldb/include/lldb/Symbol/TypeSystemClang.h
index bb47ca4af095..9d4d5e56a4fa 100644
--- a/lldb/include/lldb/Symbol/TypeSystemClang.h
+++ b/lldb/include/lldb/Symbol/TypeSystemClang.h
@@ -26,6 +26,7 @@
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
 
+#include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
 #include "lldb/Core/ClangForward.h"
 #include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Symbol/CompilerType.h"
@@ -1007,7 +1008,7 @@ class TypeSystemClangForExpressions : public 
TypeSystemClang {
   PersistentExpressionState *GetPersistentExpressionState() override;
 private:
   lldb::TargetWP m_target_wp;
-  std::unique_ptr
+  std::unique_ptr
   m_persistent_variables; // These are the persistent variables associated
   // with this process for the expression parser
   std::unique_ptr m_scratch_ast_source_up;

diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 4396f05c75aa..414a66970590 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1056,8 +1056,6 @@ class Target : public 
std::enable_shared_from_this,
  const char *name,
  Status );
 
-  lldb::ClangASTImporterSP GetClangASTImporter();
-
   // Install any files through the platform that need be to installed prior to
   // launching or attaching.
   Status Install(ProcessLaunchInfo *launch_info);
@@ -1304,7 +1302,6 @@ class Target : public 
std::enable_shared_from_this,
   typedef std::map REPLMap;
   REPLMap m_repl_map;
 
-  lldb::ClangASTImporterSP m_ast_importer_sp;
   lldb::ClangModulesDeclVendorUP m_clang_modules_decl_vendor_up;
 
   lldb::SourceManagerUP m_source_manager_up;

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
index f7d4f17bbd35..7b2acb483abd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
@@ -459,7 +459,7 @@ void ASTResultSynthesizer::CommitPersistentDecls() {
 StringRef name = decl->getName();
 ConstString name_cs(name.str().c_str());
 
-Decl *D_scratch = m_target.GetClangASTImporter()->DeportDecl(
+Decl *D_scratch = persistent_vars->GetClangASTImporter()->DeportDecl(
 _ctx->getASTContext(), decl);
 
 if (!D_scratch) {

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
index e3ada1cf017e..f847dc940442 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
@@ -9,6 +9,7 @@
 #include "ClangPersistentVariables.h"
 
 #include "lldb/Core/Value.h"
+#include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Symbol/TypeSystemClang.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataExtractor.h"
@@ -99,3 +100,10 @@ clang::NamedDecl *
 ClangPersistentVariables::GetPersistentDecl(ConstString name) {
   return m_persistent_decls.lookup(name.GetCString()).m_decl;
 }
+
+lldb::ClangASTImporterSP ClangPersistentVariables::GetClangASTImporter() {
+  if (!m_ast_importer_sp) {
+m_ast_importer_sp = std::make_shared();
+  }
+  return m_ast_importer_sp;
+}

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h 

[Lldb-commits] [lldb] 8e21d7b - [lldb/Reproducer] Include deserialized value in log statement

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

Author: Jonas Devlieghere
Date: 2020-01-28T13:24:01-08:00
New Revision: 8e21d7b9249e2e35f12dbbaa18287ce8435dd855

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

LOG: [lldb/Reproducer] Include deserialized value in log statement

Extend the replay log statement to include the deserialized value.

Added: 


Modified: 
lldb/include/lldb/Utility/ReproducerInstrumentation.h

Removed: 




diff  --git a/lldb/include/lldb/Utility/ReproducerInstrumentation.h 
b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
index 5b826bb0b1bf..eed3c1bfdb95 100644
--- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h
+++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h
@@ -280,10 +280,12 @@ class Deserializer {
 
   /// Deserialize and interpret value as T.
   template  T Deserialize() {
+T t = Read(typename serializer_tag::type());
 #ifdef LLDB_REPRO_INSTR_TRACE
-llvm::errs() << "Deserializing with " << LLVM_PRETTY_FUNCTION << "\n";
+llvm::errs() << "Deserializing with " << LLVM_PRETTY_FUNCTION << " -> "
+ << stringify_args(t) << "\n";
 #endif
-return Read(typename serializer_tag::type());
+return t;
   }
 
   /// Store the returned value in the index-to-object mapping.



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


[Lldb-commits] [PATCH] D72513: Don't fail step out if remote server doesn't implement qMemoryRegionInfo

2020-01-28 Thread Ted Woodward via Phabricator via lldb-commits
ted added a comment.

Wow @jingham you posted your comment and I landed the patch at the same time!

I'll remove the m_constructor_errors.Printf line as requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72513



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


[Lldb-commits] [PATCH] D72513: Don't fail step out if remote server doesn't implement qMemoryRegionInfo

2020-01-28 Thread Jim Ingham via Phabricator via lldb-commits
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

I don't think you want to put anything in m_constructor_errors in this case 
either.  It's fine to log something out, but ValidatePlan uses 
m_constructor_errors to help report a problem.  If the breakpoint failed to 
take for some other reason, then we would report a permissions problem which is 
not the reason for the failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72513



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


[Lldb-commits] [PATCH] D72513: Don't fail step out if remote server doesn't implement qMemoryRegionInfo

2020-01-28 Thread Ted Woodward via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs 
Revision".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG04488c485a88: Dont fail step out if remote server 
doesnt implement qMemoryRegionInfo (authored by ted).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72513

Files:
  lldb/source/Target/ThreadPlanStepOut.cpp


Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -135,7 +135,6 @@
   m_return_addr);
   LLDB_LOGF(log, "ThreadPlanStepOut(%p): %s", static_cast(this),
 m_constructor_errors.GetData());
-  return;
 } else if (!(permissions & ePermissionsExecutable)) {
   m_constructor_errors.Printf("Return address (0x%" PRIx64
   ") did not point to executable memory.",


Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -135,7 +135,6 @@
   m_return_addr);
   LLDB_LOGF(log, "ThreadPlanStepOut(%p): %s", static_cast(this),
 m_constructor_errors.GetData());
-  return;
 } else if (!(permissions & ePermissionsExecutable)) {
   m_constructor_errors.Printf("Return address (0x%" PRIx64
   ") did not point to executable memory.",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 04488c4 - Don't fail step out if remote server doesn't implement qMemoryRegionInfo

2020-01-28 Thread Ted Woodward via lldb-commits

Author: Ted Woodward
Date: 2020-01-28T13:36:06-06:00
New Revision: 04488c485a8875ba4bd6d2d004ac778276ae37e0

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

LOG: Don't fail step out if remote server doesn't implement qMemoryRegionInfo

Summary:
The return address validation in D71372 will fail if the memory permissions 
can't be determined. Many embedded stubs either don't implement the 
qMemoryRegionInfo packet, or don't have memory permissions at all.

Remove the return from the if clause that calls GetLoadAddressPermissions, so 
this call failing doesn't cause the step out to abort. Instead, assume that the 
memory permission check doesn't apply to this type of target.

Reviewers: labath, jingham, clayborg, mossberg

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Target/ThreadPlanStepOut.cpp

Removed: 




diff  --git a/lldb/source/Target/ThreadPlanStepOut.cpp 
b/lldb/source/Target/ThreadPlanStepOut.cpp
index 9ca1b3fc03ab..eead322f6b89 100644
--- a/lldb/source/Target/ThreadPlanStepOut.cpp
+++ b/lldb/source/Target/ThreadPlanStepOut.cpp
@@ -135,7 +135,6 @@ ThreadPlanStepOut::ThreadPlanStepOut(
   m_return_addr);
   LLDB_LOGF(log, "ThreadPlanStepOut(%p): %s", static_cast(this),
 m_constructor_errors.GetData());
-  return;
 } else if (!(permissions & ePermissionsExecutable)) {
   m_constructor_errors.Printf("Return address (0x%" PRIx64
   ") did not point to executable memory.",



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


[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2020-01-28 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Speaking to having the LLVM layer strip things out, this should be an option if 
it does get added there. If we have a stripped binary and we have no symbols or 
any other CPU map, and DWARF is the only way to tell if a function was ARM or 
Thumb by looking at bit zero, then we need to somehow be able to get this raw 
information.


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

https://reviews.llvm.org/D70840



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


[Lldb-commits] [lldb] 3065ef0 - [lldb/Bindings] Sort headers in headers.swig

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

Author: Jonas Devlieghere
Date: 2020-01-28T10:58:07-08:00
New Revision: 3065ef0bf85a3e4028811fc64344d86a1cd2e1ae

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

LOG: [lldb/Bindings] Sort headers in headers.swig

Added: 


Modified: 
lldb/bindings/headers.swig

Removed: 




diff  --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig
index 69fd28e33c58..ddd3964beb48 100644
--- a/lldb/bindings/headers.swig
+++ b/lldb/bindings/headers.swig
@@ -25,8 +25,8 @@
 #include "lldb/API/SBEvent.h"
 #include "lldb/API/SBExecutionContext.h"
 #include "lldb/API/SBExpressionOptions.h"
-#include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBFile.h"
+#include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBFileSpecList.h"
 #include "lldb/API/SBFrame.h"
 #include "lldb/API/SBFunction.h"
@@ -68,9 +68,9 @@
 #include "lldb/API/SBTypeNameSpecifier.h"
 #include "lldb/API/SBTypeSummary.h"
 #include "lldb/API/SBTypeSynthetic.h"
+#include "lldb/API/SBUnixSignals.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBValueList.h"
 #include "lldb/API/SBVariablesOptions.h"
 #include "lldb/API/SBWatchpoint.h"
-#include "lldb/API/SBUnixSignals.h"
 %}



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


[Lldb-commits] [PATCH] D73303: [lldb/Target] Add Assert StackFrame Recognizer

2020-01-28 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment.

This generally looks good. I'm still not fond of registering this in the Target 
itself. But I don't have a immediately better idea as we don't have a C 
language runtime.

A couple more comments/questions that can be addressed as followups:




Comment at: lldb/include/lldb/Target/StackFrameRecognizer.h:38
+  virtual lldb::StackFrameSP GetMostRelevantFrame() { return nullptr; };
+  virtual llvm::StringRef GetStopDescription() { return ""; }
   virtual ~RecognizedStackFrame(){};

The fact that this returns a StringRef, means it must return a pointer to 
permanent string storage. In this case, you're returning a constant string, and 
it's fine, but it means that if you wanted to construct a return value (for 
example out of data you extract from the inferior), you need to store the 
string backing the StringRef somewhere.  The concrete `RecognizedStackFrame` 
instance seems like a good fit, but I have no clue about their lifetime 
guarantees compared to the lifetime of the StringRef returned here. Maybe we 
should make this a `std::string`?



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py:113
+
+stop_reason = 'stop reason = ' + thread.GetStopDescription(256)
 

Here, you're now just checking self consistency, I son't think that's valuable. 
This should check for the actual text of the assert recognizer (and potentially 
something else on the systems where the recognizer is not supported).



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py:191
+
+self.assertTrue(thread.GetFrameAtIndex(0) == frame, "Frame #0 
selected")
+

as we just did `self.runCmd` above anyway, I'd replace all of this by 
`self.runCmd('frame select 0')`. Or at least just write it as a single line, no 
need to check for every intermediate result. Such verbosity really distracts 
from what the test is trying to verify. 



Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:110-112
+AssertRecognizedStackFrame::AssertRecognizedStackFrame(
+StackFrameSP most_relevant_frame_sp)
+: m_most_relevant_frame(most_relevant_frame_sp) {}

Can you move this with the other AssertRecognizedStackFrame methods?



Comment at: lldb/source/Target/StackFrameRecognizer.cpp:95
 const SymbolContext  =
-frame->GetSymbolContext(eSymbolContextModule | eSymbolContextFunction);
+frame->GetSymbolContext(eSymbolContextEverything);
 ConstString function_name = symctx.GetFunctionName();

What's the reason of this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73303



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


[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2020-01-28 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:400
+entry->base = arch.GetOpcodeLoadAddress(entry->base);
+  }
+  if (frame_base && frame_base->IsLocationList()) {

Either way, I am not picky.


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

https://reviews.llvm.org/D70840



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


[Lldb-commits] [PATCH] D73539: [AVR] Basic support for remote debugging

2020-01-28 Thread Ayke via Phabricator via lldb-commits
aykevl added a comment.

It's worth noting that I haven't managed to get a line table from a binary 
produced by LLVM (with my own compiler frontend) and linked by avr-gcc. But I 
believe that is a bug in the AVR backend of LLVM as a binary built entirely by 
`avr-gcc` works just fine.


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

https://reviews.llvm.org/D73539



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


[Lldb-commits] [PATCH] D73506: Auto-completion bug fix for dot operator

2020-01-28 Thread Héctor Luis Díaz Aceves via Phabricator via lldb-commits
diazhector98 updated this revision to Diff 240916.
diazhector98 marked an inline comment as done.
diazhector98 added a comment.

Added tests for spaces between member operators


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73506

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -951,9 +951,17 @@
   for (size_t i = 0; i < count; i++) {
 std::string match = matches.GetStringAtIndex(i);
 std::string description = descriptions.GetStringAtIndex(i);
-
+
 llvm::json::Object item;
-EmplaceSafeString(item, "text", match);
+
+llvm::StringRef match_ref = match;
+for(llvm::StringRef commit_point: {".", "->"}) {
+  if (match_ref.contains(commit_point)){
+match_ref = match_ref.rsplit(commit_point).second;
+  }
+}
+EmplaceSafeString(item, "text", match_ref);
+
 if (description.empty())
   EmplaceSafeString(item, "label", match);
 else
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
@@ -1,6 +1,17 @@
 #include 
 #include 
 
+struct bar {
+  int var1;
+};
+
+struct foo {
+  int var1;
+  bar* my_bar_pointer;
+  bar my_bar_object;
+  foo* next_foo;
+};
+
 int fun(std::vector var) {
   return var.size(); // breakpoint 1
 }
@@ -12,5 +23,8 @@
   std::string str2 = "b";
   std::vector vec;
   fun(vec);
+  bar bar1 = {2};
+  bar* bar2 =  
+  foo foo1 = {3,, bar1, NULL};
   return 0; // breakpoint 2
 }
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
@@ -113,3 +113,76 @@
 }
 ],
 )
+
+self.verify_completions(
+self.vscode.get_completions("foo1.v"),
+[
+{
+"text": "var1",
+"label": "foo1.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.my_bar_object.v"),
+[
+{
+"text": "var1",
+"label": "foo1.my_bar_object.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.var1 + foo1.v"),
+[
+{
+"text": "var1",
+"label": "foo1.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.var1 + v"),
+[
+{
+"text": "var1",
+"label": "var1 -- int &"
+}
+]
+)
+
+#should correctly handle spaces between objects and member operators
+self.verify_completions(
+self.vscode.get_completions("foo1 .v"),
+[
+{
+"text": "var1",
+"label": ".var1 -- int"
+}
+],
+[
+{
+"text": "var2",
+"label": ".var2 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1 . v"),
+[
+{
+"text": "var1",
+"label": "var1 -- int"
+}
+], 
+[
+{
+"text": "var2",
+"label": "var2 -- int"
+}
+]
+)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 954d042 - Revert "[lldb/Target] Add Assert StackFrame Recognizer"

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

Author: Med Ismail Bennani
Date: 2020-01-28T18:40:08+01:00
New Revision: 954d04295b9b5447139cb1b9b57b9a2a4dd9b656

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

LOG: Revert "[lldb/Target] Add Assert StackFrame Recognizer"

This reverts commit 03a6b858fde5c644ec16b1fddd8e10aa9ef3f0ad.

The test doesn't pass on Debian.

Added: 


Modified: 
lldb/docs/use/formatting.rst
lldb/include/lldb/Core/FormatEntity.h
lldb/include/lldb/Target/StackFrameRecognizer.h
lldb/include/lldb/Target/Thread.h

lldb/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py

lldb/packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
lldb/source/API/SBThread.cpp
lldb/source/Core/FormatEntity.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Target/CMakeLists.txt
lldb/source/Target/Process.cpp
lldb/source/Target/StackFrameRecognizer.cpp
lldb/source/Target/Thread.cpp

Removed: 
lldb/include/lldb/Target/AssertFrameRecognizer.h
lldb/source/Target/AssertFrameRecognizer.cpp
lldb/test/Shell/Recognizer/Inputs/assert.c
lldb/test/Shell/Recognizer/assert.test



diff  --git a/lldb/docs/use/formatting.rst b/lldb/docs/use/formatting.rst
index e2644d50217b..939c4e18f749 100644
--- a/lldb/docs/use/formatting.rst
+++ b/lldb/docs/use/formatting.rst
@@ -134,9 +134,7 @@ A complete list of currently supported format string 
variables is listed below:
 
+---+-+
 | ``thread.queue``  | The queue name of the 
thread if the target OS supports dispatch queues


  |
 
+---+-+
-| ``thread.stop-reason``| A textual reason why the 
thread stopped. If the thread have a recognized frame, this displays its 
recognized stop reason. Otherwise, gets the stop info description.  

  |
-+---+-+
-| ``thread.stop-reason-raw``| A textual reason why the 
thread stopped. Always returns stop info description.   


   |
+| ``thread.stop-reason``| A textual reason each 
thread stopped  


  |
 
+---+-+
 | ``thread.return-value``   | The return value of the 
latest step operation (currently only for step-out.)


|
 

[Lldb-commits] [PATCH] D73506: Auto-completion bug fix for dot operator

2020-01-28 Thread Héctor Luis Díaz Aceves via Phabricator via lldb-commits
diazhector98 updated this revision to Diff 240908.
diazhector98 added a comment.

Simplifying code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73506

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -951,9 +951,17 @@
   for (size_t i = 0; i < count; i++) {
 std::string match = matches.GetStringAtIndex(i);
 std::string description = descriptions.GetStringAtIndex(i);
-
+
 llvm::json::Object item;
-EmplaceSafeString(item, "text", match);
+
+llvm::StringRef match_ref = match;
+for(llvm::StringRef commit_point: {".", "->"}) {
+  if (match_ref.contains(commit_point)){
+match_ref = match_ref.rsplit(commit_point).second;
+  }
+}
+EmplaceSafeString(item, "text", match_ref);
+
 if (description.empty())
   EmplaceSafeString(item, "label", match);
 else
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/main.cpp
@@ -1,6 +1,17 @@
 #include 
 #include 
 
+struct bar {
+  int var1;
+};
+
+struct foo {
+  int var1;
+  bar* my_bar_pointer;
+  bar my_bar_object;
+  foo* next_foo;
+};
+
 int fun(std::vector var) {
   return var.size(); // breakpoint 1
 }
@@ -12,5 +23,8 @@
   std::string str2 = "b";
   std::vector vec;
   fun(vec);
+  bar bar1 = {2};
+  bar* bar2 =  
+  foo foo1 = {3,, bar1, NULL};
   return 0; // breakpoint 2
 }
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
@@ -113,3 +113,43 @@
 }
 ],
 )
+
+self.verify_completions(
+self.vscode.get_completions("foo1.v"),
+[
+{
+"text": "var1",
+"label": "foo1.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.my_bar_object.v"),
+[
+{
+"text": "var1",
+"label": "foo1.my_bar_object.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.var1 + foo1.v"),
+[
+{
+"text": "var1",
+"label": "foo1.var1 -- int"
+}
+]
+)
+
+self.verify_completions(
+self.vscode.get_completions("foo1.var1 + v"),
+[
+{
+"text": "var1",
+"label": "var1 -- int &"
+}
+]
+)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D73303: [lldb/Target] Add Assert StackFrame Recognizer

2020-01-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG03a6b858fde5: [lldb/Target] Add Assert StackFrame Recognizer 
(authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73303

Files:
  lldb/docs/use/formatting.rst
  lldb/include/lldb/Core/FormatEntity.h
  lldb/include/lldb/Target/AssertFrameRecognizer.h
  lldb/include/lldb/Target/StackFrameRecognizer.h
  lldb/include/lldb/Target/Thread.h
  
lldb/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
  lldb/packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
  lldb/source/API/SBThread.cpp
  lldb/source/Core/FormatEntity.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Target/AssertFrameRecognizer.cpp
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrameRecognizer.cpp
  lldb/source/Target/Thread.cpp
  lldb/test/Shell/Recognizer/Inputs/assert.c
  lldb/test/Shell/Recognizer/assert.test

Index: lldb/test/Shell/Recognizer/assert.test
===
--- /dev/null
+++ lldb/test/Shell/Recognizer/assert.test
@@ -0,0 +1,13 @@
+# UNSUPPORTED: system-windows
+# RUN: %clang_host -g -O0 %S/Inputs/assert.c -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+run
+# CHECK: thread #{{.*}}stop reason = hit program assert
+frame info
+# CHECK: frame #{{.*}}`main at assert.c
+frame recognizer info 0
+# CHECK: frame 0 is recognized by Assert StackFrame Recognizer
+set set thread-format "{${thread.stop-reason-raw}}\n"
+thread info
+# CHECK: signal SIGABRT
+q
Index: lldb/test/Shell/Recognizer/Inputs/assert.c
===
--- /dev/null
+++ lldb/test/Shell/Recognizer/Inputs/assert.c
@@ -0,0 +1,9 @@
+#include 
+
+int main() {
+  int a = 42;
+  assert(a == 42);
+  a--;
+  assert(a == 42);
+  return 0;
+}
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -573,9 +573,64 @@
   m_state = state;
 }
 
+llvm::StringRef Thread::GetStopDescription() {
+  StackFrameSP frame_sp = GetStackFrameAtIndex(0);
+
+  if (!frame_sp)
+return GetStopDescriptionRaw();
+
+  auto recognized_frame_sp = frame_sp->GetRecognizedFrame();
+
+  if (!recognized_frame_sp)
+return GetStopDescriptionRaw();
+
+  llvm::StringRef recognized_stop_description =
+  recognized_frame_sp->GetStopDescription();
+
+  if (!recognized_stop_description.empty())
+return recognized_stop_description;
+
+  return GetStopDescriptionRaw();
+}
+
+llvm::StringRef Thread::GetStopDescriptionRaw() {
+  StopInfoSP stop_info_sp = GetStopInfo();
+  llvm::StringRef raw_stop_description;
+  if (stop_info_sp && stop_info_sp->IsValid())
+raw_stop_description = stop_info_sp->GetDescription();
+  return raw_stop_description;
+}
+
+void Thread::SelectMostRelevantFrame() {
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD);
+
+  auto frames_list_sp = GetStackFrameList();
+
+  // Only the top frame should be recognized.
+  auto frame_sp = frames_list_sp->GetFrameAtIndex(0);
+
+  auto recognized_frame_sp = frame_sp->GetRecognizedFrame();
+
+  if (!recognized_frame_sp) {
+LLDB_LOG(log, "Frame #0 not recognized");
+return;
+  }
+
+  if (StackFrameSP most_relevant_frame_sp =
+  recognized_frame_sp->GetMostRelevantFrame()) {
+LLDB_LOG(log, "Found most relevant frame at index {0}",
+ most_relevant_frame_sp->GetFrameIndex());
+SetSelectedFrame(most_relevant_frame_sp.get());
+  } else {
+LLDB_LOG(log, "No relevant frame!");
+  }
+}
+
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
+  SelectMostRelevantFrame();
+
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.
 
Index: lldb/source/Target/StackFrameRecognizer.cpp
===
--- lldb/source/Target/StackFrameRecognizer.cpp
+++ lldb/source/Target/StackFrameRecognizer.cpp
@@ -92,7 +92,7 @@
 
   StackFrameRecognizerSP GetRecognizerForFrame(StackFrameSP frame) {
 const SymbolContext  =
-frame->GetSymbolContext(eSymbolContextModule | eSymbolContextFunction);
+frame->GetSymbolContext(eSymbolContextEverything);
 ConstString function_name = symctx.GetFunctionName();
 ModuleSP module_sp = symctx.module_sp;
 if (!module_sp) return StackFrameRecognizerSP();
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -38,6 +38,7 @@
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/ABI.h"
+#include 

[Lldb-commits] [lldb] 03a6b85 - [lldb/Target] Add Assert StackFrame Recognizer

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

Author: Med Ismail Bennani
Date: 2020-01-28T18:21:29+01:00
New Revision: 03a6b858fde5c644ec16b1fddd8e10aa9ef3f0ad

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

LOG: [lldb/Target] Add Assert StackFrame Recognizer

When a thread stops, this checks depending on the platform if the top frame is
an abort stack frame. If so, it looks for an assert stack frame in the upper
frames and set it as the most relavant frame when found.

To do so, the StackFrameRecognizer class holds a "Most Relevant Frame" and a
"cooked" stop reason description. When the thread is about to stop, it checks
if the current frame is recognized, and if so, it fetches the recognized frame's
attributes and applies them.

rdar://58528686

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

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/include/lldb/Target/AssertFrameRecognizer.h
lldb/source/Target/AssertFrameRecognizer.cpp
lldb/test/Shell/Recognizer/Inputs/assert.c
lldb/test/Shell/Recognizer/assert.test

Modified: 
lldb/docs/use/formatting.rst
lldb/include/lldb/Core/FormatEntity.h
lldb/include/lldb/Target/StackFrameRecognizer.h
lldb/include/lldb/Target/Thread.h

lldb/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py

lldb/packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
lldb/source/API/SBThread.cpp
lldb/source/Core/FormatEntity.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Target/CMakeLists.txt
lldb/source/Target/Process.cpp
lldb/source/Target/StackFrameRecognizer.cpp
lldb/source/Target/Thread.cpp

Removed: 




diff  --git a/lldb/docs/use/formatting.rst b/lldb/docs/use/formatting.rst
index 939c4e18f749..e2644d50217b 100644
--- a/lldb/docs/use/formatting.rst
+++ b/lldb/docs/use/formatting.rst
@@ -134,7 +134,9 @@ A complete list of currently supported format string 
variables is listed below:
 
+---+-+
 | ``thread.queue``  | The queue name of the 
thread if the target OS supports dispatch queues


  |
 
+---+-+
-| ``thread.stop-reason``| A textual reason each 
thread stopped  


  |
+| ``thread.stop-reason``| A textual reason why the 
thread stopped. If the thread have a recognized frame, this displays its 
recognized stop reason. Otherwise, gets the stop info description.  

  |
++---+-+
+| ``thread.stop-reason-raw``| A textual reason why the 
thread stopped. Always returns stop info description.   


   |
 
+---+-+
 | ``thread.return-value``   

[Lldb-commits] [PATCH] D73303: [lldb/Target] Add Assert StackFrame Recognizer

2020-01-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 240899.
mib added a comment.

Added early return in `AssertFrameRecognizer::RecognizeFrame`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73303

Files:
  lldb/docs/use/formatting.rst
  lldb/include/lldb/Core/FormatEntity.h
  lldb/include/lldb/Target/AssertFrameRecognizer.h
  lldb/include/lldb/Target/StackFrameRecognizer.h
  lldb/include/lldb/Target/Thread.h
  
lldb/packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py
  lldb/packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py
  lldb/source/API/SBThread.cpp
  lldb/source/Core/FormatEntity.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Target/AssertFrameRecognizer.cpp
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrameRecognizer.cpp
  lldb/source/Target/Thread.cpp
  lldb/test/Shell/Recognizer/Inputs/assert.c
  lldb/test/Shell/Recognizer/assert.test

Index: lldb/test/Shell/Recognizer/assert.test
===
--- /dev/null
+++ lldb/test/Shell/Recognizer/assert.test
@@ -0,0 +1,13 @@
+# UNSUPPORTED: system-windows
+# RUN: %clang_host -g -O0 %S/Inputs/assert.c -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+run
+# CHECK: thread #{{.*}}stop reason = hit program assert
+frame info
+# CHECK: frame #{{.*}}`main at assert.c
+frame recognizer info 0
+# CHECK: frame 0 is recognized by Assert StackFrame Recognizer
+set set thread-format "{${thread.stop-reason-raw}}\n"
+thread info
+# CHECK: signal SIGABRT
+q
Index: lldb/test/Shell/Recognizer/Inputs/assert.c
===
--- /dev/null
+++ lldb/test/Shell/Recognizer/Inputs/assert.c
@@ -0,0 +1,9 @@
+#include 
+
+int main() {
+  int a = 42;
+  assert(a == 42);
+  a--;
+  assert(a == 42);
+  return 0;
+}
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -573,9 +573,64 @@
   m_state = state;
 }
 
+llvm::StringRef Thread::GetStopDescription() {
+  StackFrameSP frame_sp = GetStackFrameAtIndex(0);
+
+  if (!frame_sp)
+return GetStopDescriptionRaw();
+
+  auto recognized_frame_sp = frame_sp->GetRecognizedFrame();
+
+  if (!recognized_frame_sp)
+return GetStopDescriptionRaw();
+
+  llvm::StringRef recognized_stop_description =
+  recognized_frame_sp->GetStopDescription();
+
+  if (!recognized_stop_description.empty())
+return recognized_stop_description;
+
+  return GetStopDescriptionRaw();
+}
+
+llvm::StringRef Thread::GetStopDescriptionRaw() {
+  StopInfoSP stop_info_sp = GetStopInfo();
+  llvm::StringRef raw_stop_description;
+  if (stop_info_sp && stop_info_sp->IsValid())
+raw_stop_description = stop_info_sp->GetDescription();
+  return raw_stop_description;
+}
+
+void Thread::SelectMostRelevantFrame() {
+  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD);
+
+  auto frames_list_sp = GetStackFrameList();
+
+  // Only the top frame should be recognized.
+  auto frame_sp = frames_list_sp->GetFrameAtIndex(0);
+
+  auto recognized_frame_sp = frame_sp->GetRecognizedFrame();
+
+  if (!recognized_frame_sp) {
+LLDB_LOG(log, "Frame #0 not recognized");
+return;
+  }
+
+  if (StackFrameSP most_relevant_frame_sp =
+  recognized_frame_sp->GetMostRelevantFrame()) {
+LLDB_LOG(log, "Found most relevant frame at index {0}",
+ most_relevant_frame_sp->GetFrameIndex());
+SetSelectedFrame(most_relevant_frame_sp.get());
+  } else {
+LLDB_LOG(log, "No relevant frame!");
+  }
+}
+
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
+  SelectMostRelevantFrame();
+
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.
 
Index: lldb/source/Target/StackFrameRecognizer.cpp
===
--- lldb/source/Target/StackFrameRecognizer.cpp
+++ lldb/source/Target/StackFrameRecognizer.cpp
@@ -92,7 +92,7 @@
 
   StackFrameRecognizerSP GetRecognizerForFrame(StackFrameSP frame) {
 const SymbolContext  =
-frame->GetSymbolContext(eSymbolContextModule | eSymbolContextFunction);
+frame->GetSymbolContext(eSymbolContextEverything);
 ConstString function_name = symctx.GetFunctionName();
 ModuleSP module_sp = symctx.module_sp;
 if (!module_sp) return StackFrameRecognizerSP();
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -38,6 +38,7 @@
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/ABI.h"
+#include "lldb/Target/AssertFrameRecognizer.h"
 

[Lldb-commits] [PATCH] D73517: [lldb] Delete ValueObject::GetBaseClassPath

2020-01-28 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Doesn't look like it's used in the swift fork downstream either. LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73517



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


[Lldb-commits] [PATCH] D73303: [lldb/Target] Add Assert StackFrame Recognizer

2020-01-28 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked an inline comment as done.
mib added inline comments.



Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:155
+
+  most_relevant_frame_sp = thread_sp->GetStackFrameAtIndex(
+  std::min(frame_index + 1, last_frame_index));

JDevlieghere wrote:
> Why not return here? 
> 
> ```
> StackFrameSP most_relevant_frame_sp = 
> thread_sp->GetStackFrameAtIndex(std::min(frame_index + 1, last_frame_index));
> return lldb::RecognizedStackFrameSP(new 
> AssertRecognizedStackFrame(most_relevant_frame_sp));
> ```
> 
> That way you don't have to check again after you finish the loop and you can 
> just `return RecognizedStackFrameSP();`.
Fair.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73303



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


[Lldb-commits] [PATCH] D72158: [DebugInfo] Make most debug line prologue errors non-fatal to parsing

2020-01-28 Thread James Henderson via Phabricator via lldb-commits
jhenderson requested review of this revision.
jhenderson added a comment.

In D72158#1844534 , @labath wrote:

> If I understand this correctly, this will cause lldb to continue to read the 
> parsed line table contribution after encountering some errors in the 
> prologue, whereas previously we would stop straight away. That sounds 
> reasonable if now or in the future we will be able to get some useful 
> information (at least some subset of file names, or line numbers without file 
> names, etc.) out of these kinds of line tables.


Indeed, that's what this change allows: the parser will continue parsing after 
reporting the Errors via the new callback. In cases where it can't (i.e. 
unsupported versions or reserved unit length values), it will stop and return 
the Error (as it previously did, and also did for the now-recoverable Errors). 
For now I've changed LLDB to record both kinds of Errors in the same way as 
they were before, but the recoverable errors do not prevent it subsequently 
calling `ParseSupportFilesFromPrologue`. I could just as easily change it to 
doing what it always did for all cases, namely log the errors and not call 
`ParseSupportFilesFromPrologue`. That's probably the safer approach on further 
reflection, so I'll update the patch tomorrow (I'm just about to leave the 
office for the day), unless someone thinks changing the behaviour is good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72158



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


[Lldb-commits] [lldb] c4267b7 - Revert "[lldb/PDB] Use the new line table constructor"

2020-01-28 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-01-28T17:15:44+01:00
New Revision: c4267b7b1371cc3ffaf6d4e701ab90c082ef18dd

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

LOG: Revert "[lldb/PDB] Use the new line table constructor"

This reverts commit bb73210ba9f16c1516f564235c86cbddccd1bd6d due to
failures on the windows bot.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 5757efe5fa6a..9ecaa042863f 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1021,7 +1021,7 @@ uint32_t SymbolFileNativePDB::ResolveSymbolContext(
   return 0;
 }
 
-static void AppendLineEntryToSequence(LineSequence ,
+static void AppendLineEntryToSequence(LineTable , LineSequence ,
   const CompilandIndexItem ,
   lldb::addr_t base_addr,
   uint32_t file_number,
@@ -1040,18 +1040,21 @@ static void AppendLineEntryToSequence(LineSequence 
,
 
   uint32_t lno = cur_info.getStartLine();
 
-  LineTable::AppendLineEntryToSequence(, addr, lno, 0, file_number,
-   is_statement, false, is_prologue,
-   is_epilogue, false);
+  table.AppendLineEntryToSequence(, addr, lno, 0, file_number,
+  is_statement, false, is_prologue, 
is_epilogue,
+  false);
 }
 
-static void TerminateLineSequence(const LineFragmentHeader ,
+static void TerminateLineSequence(LineTable ,
+  const LineFragmentHeader ,
   lldb::addr_t base_addr, uint32_t file_number,
-  uint32_t last_line, LineSequence ) {
+  uint32_t last_line,
+  std::unique_ptr seq) {
   // The end is always a terminal entry, so insert it regardless.
-  LineTable::AppendLineEntryToSequence(, base_addr + block.CodeSize,
-   last_line, 0, file_number, false, false,
-   false, false, true);
+  table.AppendLineEntryToSequence(seq.get(), base_addr + block.CodeSize,
+  last_line, 0, file_number, false, false,
+  false, false, true);
+  table.InsertSequence(seq.release());
 }
 
 bool SymbolFileNativePDB::ParseLineTable(CompileUnit _unit) {
@@ -1065,11 +1068,11 @@ bool SymbolFileNativePDB::ParseLineTable(CompileUnit 
_unit) {
   CompilandIndexItem *cci =
   m_index->compilands().GetCompiland(cu_id.asCompiland().modi);
   lldbassert(cci);
+  auto line_table = std::make_unique(_unit);
 
   // This is basically a copy of the .debug$S subsections from all original 
COFF
   // object files merged together with address relocations applied.  We are
   // looking for all DEBUG_S_LINES subsections.
-  std::vector> sequences;
   for (const DebugSubsectionRecord  :
cci->m_debug_stream.getSubsectionsArray()) {
 if (dssr.kind() != DebugSubsectionKind::Lines)
@@ -1108,23 +,20 @@ bool SymbolFileNativePDB::ParseLineTable(CompileUnit 
_unit) {
   lldbassert(fn_iter != cci->m_file_list.end());
   uint32_t file_index = std::distance(cci->m_file_list.begin(), fn_iter);
 
-  std::unique_ptr sequence =
-  LineTable::CreateLineSequenceContainer();
+  std::unique_ptr sequence(
+  line_table->CreateLineSequenceContainer());
   lldbassert(!group.LineNumbers.empty());
 
   for (const LineNumberEntry  : group.LineNumbers) {
-AppendLineEntryToSequence(*sequence, *cci, virtual_addr, file_index,
-  *lfh, entry);
+AppendLineEntryToSequence(*line_table, *sequence, *cci, virtual_addr,
+  file_index, *lfh, entry);
   }
   LineInfo last_line(group.LineNumbers.back().Flags);
-  TerminateLineSequence(*lfh, virtual_addr, file_index,
-last_line.getEndLine(), *sequence);
-  sequences.push_back(std::move(sequence));
+  TerminateLineSequence(*line_table, *lfh, virtual_addr, file_index,
+last_line.getEndLine(), std::move(sequence));
 }
   }
 
-  auto line_table =
-  std::make_unique(_unit, std::move(sequences));
   if (line_table->GetSize() == 0)
 return false;
 

diff  --git 

[Lldb-commits] [PATCH] D73303: [lldb/Target] Add Assert StackFrame Recognizer

2020-01-28 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM with one inline comment, but let's give the other reviewers a chance to 
take another look before landing this.




Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:155
+
+  most_relevant_frame_sp = thread_sp->GetStackFrameAtIndex(
+  std::min(frame_index + 1, last_frame_index));

Why not return here? 

```
StackFrameSP most_relevant_frame_sp = 
thread_sp->GetStackFrameAtIndex(std::min(frame_index + 1, last_frame_index));
return lldb::RecognizedStackFrameSP(new 
AssertRecognizedStackFrame(most_relevant_frame_sp));
```

That way you don't have to check again after you finish the loop and you can 
just `return RecognizedStackFrameSP();`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73303



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


[Lldb-commits] [PATCH] D72158: [DebugInfo] Make most debug line prologue errors non-fatal to parsing

2020-01-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

If I understand this correctly, this will cause lldb to continue to read the 
parsed line table contribution after encountering some errors in the prologue, 
whereas previously we would stop straight away. That sounds reasonable if now 
or in the future we will be able to get some useful information (at least some 
subset of file names, or line numbers without file names, etc.) out of these 
kinds of line tables. If that is not the case, and these errors are recoverable 
only in the sense that they allow you to jump to the next contribution, then it 
might be better to treat these errors as unrecoverable for lldb purposes 
(jumping to the next contribution is not interesting for us since we always use 
DW_AT_stmt_list to locate the line table).

However, I don't think that resolving that needs to hold this patch up, as this 
behavior can be easily adjusted from within lldb.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72158



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


[Lldb-commits] [PATCH] D73539: [AVR] Basic support for remote debugging

2020-01-28 Thread Ayke via Phabricator via lldb-commits
aykevl updated this revision to Diff 240857.
aykevl added a comment.
Herald added subscribers: MaskRay, emaste.
Herald added a reviewer: espindola.

@labath Thank you for the quick review!

I have added a test per your suggestions. I'm not very familiar with lldb but 
this passes the tests.


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

https://reviews.llvm.org/D73539

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml


Index: lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml
@@ -0,0 +1,30 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# CHECK: Plugin name: elf
+# CHECK: Architecture: avr--
+# CHECK: Executable: true
+# CHECK: Stripped: false
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK:   Name: .text
+# CHECK-NEXT:   Type: code
+# CHECK-NEXT:   Permissions: r-x
+# CHECK-NEXT:   Thread specific: no
+# CHECK-NEXT:   VM address: 0x0
+# CHECK-NEXT:   VM size: 4
+# CHECK-NEXT:   File size: 4
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_AVR
+  Flags:   [ EF_AVR_ARCH_AVR1, EF_AVR_ARCH_AVR4 ]
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+AddressAlign:0x0001
+Content: 'FECF'
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -218,6 +218,8 @@
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
 {eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"},
 
+{eByteOrderLittle, 2, 2, 4, llvm::Triple::avr, ArchSpec::eCore_avr, "avr"},
+
 {eByteOrderLittle, 4, 1, 4, llvm::Triple::wasm32, ArchSpec::eCore_wasm32,
  "wasm32"},
 };
@@ -448,6 +450,8 @@
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARC
+{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu}, // AVR
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: lldb/include/lldb/Utility/ArchSpec.h
===
--- lldb/include/lldb/Utility/ArchSpec.h
+++ lldb/include/lldb/Utility/ArchSpec.h
@@ -188,6 +188,8 @@
 
 eCore_arc, // little endian ARC
 
+eCore_avr,
+
 eCore_wasm32,
 
 kNumCores,


Index: lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/avr-basic-info.yaml
@@ -0,0 +1,30 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# CHECK: Plugin name: elf
+# CHECK: Architecture: avr--
+# CHECK: Executable: true
+# CHECK: Stripped: false
+# CHECK: Type: executable
+# CHECK: Strata: user
+# CHECK:   Name: .text
+# CHECK-NEXT:   Type: code
+# CHECK-NEXT:   Permissions: r-x
+# CHECK-NEXT:   Thread specific: no
+# CHECK-NEXT:   VM address: 0x0
+# CHECK-NEXT:   VM size: 4
+# CHECK-NEXT:   File size: 4
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_AVR
+  Flags:   [ EF_AVR_ARCH_AVR1, EF_AVR_ARCH_AVR4 ]
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+AddressAlign:0x0001
+Content: 'FECF'
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -218,6 +218,8 @@
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
 {eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"},
 
+{eByteOrderLittle, 2, 2, 4, llvm::Triple::avr, ArchSpec::eCore_avr, "avr"},
+
 {eByteOrderLittle, 4, 1, 4, llvm::Triple::wasm32, ArchSpec::eCore_wasm32,
  "wasm32"},
 };
@@ -448,6 +450,8 @@
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARC
+{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu}, // AVR
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: lldb/include/lldb/Utility/ArchSpec.h
===
--- lldb/include/lldb/Utility/ArchSpec.h
+++ lldb/include/lldb/Utility/ArchSpec.h
@@ -188,6 +188,8 @@
 
 eCore_arc, // little endian 

Re: [Lldb-commits] [lldb] 2046d72 - build: improve python checks for Windows

2020-01-28 Thread Hans Wennborg via lldb-commits
Should https://llvm.org/docs/GettingStarted.html#requirements and
https://llvm.org/docs/CMake.html#quick-start be updated to reflect the
new requirement? And maybe a release note?

On Sun, Dec 22, 2019 at 2:01 PM Saleem Abdulrasool via lldb-commits
 wrote:
>
>
> Author: Saleem Abdulrasool
> Date: 2019-12-22T13:57:46-08:00
> New Revision: 2046d72e91670114625c87e122db6e013ba089d5
>
> URL: 
> https://github.com/llvm/llvm-project/commit/2046d72e91670114625c87e122db6e013ba089d5
> DIFF: 
> https://github.com/llvm/llvm-project/commit/2046d72e91670114625c87e122db6e013ba089d5.diff
>
> LOG: build: improve python checks for Windows
>
> Require a newer CMake on Windows to use the Python3 support that is
> packaged in CMake. This version is able to check both 32-bit and 64-bit
> versions and will setup everything properly without the user needing to
> specify PYTHON_HOME. This enables building lldb's python bindings on
> Windows under Azure's CI again.
>
> Added:
>
>
> Modified:
> lldb/CMakeLists.txt
> lldb/cmake/modules/LLDBConfig.cmake
>
> Removed:
>
>
>
> 
> diff  --git a/lldb/CMakeLists.txt b/lldb/CMakeLists.txt
> index ff3d8ae70747..6170ab625c54 100644
> --- a/lldb/CMakeLists.txt
> +++ b/lldb/CMakeLists.txt
> @@ -1,4 +1,7 @@
>  cmake_minimum_required(VERSION 3.4.3)
> +if(CMAKE_SYSTEM_NAME STREQUAL Windows)
> +  cmake_minimum_required(VERSION 3.13)
> +endif()
>
>  if(POLICY CMP0075)
>cmake_policy(SET CMP0075 NEW)
>
> diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
> b/lldb/cmake/modules/LLDBConfig.cmake
> index c34ef76cb652..e1da76cf6249 100644
> --- a/lldb/cmake/modules/LLDBConfig.cmake
> +++ b/lldb/cmake/modules/LLDBConfig.cmake
> @@ -138,184 +138,39 @@ if (LLDB_ENABLE_LIBEDIT)
>set(CMAKE_EXTRA_INCLUDE_FILES)
>  endif()
>
> -# On Windows, we can't use the normal FindPythonLibs module that comes with 
> CMake,
> -# for a number of reasons.
> -# 1) Prior to MSVC 2015, it is only possible to embed Python if python 
> itself was
> -#compiled with an identical version (and build configuration) of MSVC as 
> LLDB.
> -#The standard algorithm does not take into account the
> diff erences between
> -#a binary release distribution of python and a custom built distribution.
> -# 2) From MSVC 2015 and onwards, it is only possible to use Python 3.5 or 
> later.
> -# 3) FindPythonLibs queries the registry to locate Python, and when looking 
> for a
> -#64-bit version of Python, since cmake.exe is a 32-bit executable, it 
> will see
> -#a 32-bit view of the registry.  As such, it is impossible for 
> FindPythonLibs to
> -#locate 64-bit Python libraries.
> -# This function is designed to address those limitations.  Currently it only 
> partially
> -# addresses them, but it can be improved and extended on an as-needed basis.
> -function(find_python_libs_windows_helper LOOKUP_DEBUG OUT_EXE_PATH_VARNAME 
> OUT_LIB_PATH_VARNAME OUT_DLL_PATH_VARNAME OUT_VERSION_VARNAME)
> -  if(LOOKUP_DEBUG)
> -  set(POSTFIX "_d")
> -  else()
> -  set(POSTFIX "")
> -  endif()
> -
> -  file(TO_CMAKE_PATH "${PYTHON_HOME}/python${POSTFIX}.exe"   
> PYTHON_EXE)
> -  file(TO_CMAKE_PATH 
> "${PYTHON_HOME}/libs/${PYTHONLIBS_BASE_NAME}${POSTFIX}.lib" PYTHON_LIB)
> -  file(TO_CMAKE_PATH "${PYTHON_HOME}/${PYTHONLIBS_BASE_NAME}${POSTFIX}.dll"  
> PYTHON_DLL)
> -
> -  foreach(component PYTHON_EXE;PYTHON_LIB;PYTHON_DLL)
> -if(NOT EXISTS ${${component}})
> -  message(WARNING "Unable to find ${component}")
> -  unset(${component})
> -endif()
> -  endforeach()
> -
> -  if (NOT PYTHON_EXE OR NOT PYTHON_LIB OR NOT PYTHON_DLL)
> -message(WARNING "Unable to find all Python components.  Python support 
> will be disabled for this build.")
> -set(LLDB_ENABLE_PYTHON 0 PARENT_SCOPE)
> -return()
> -  endif()
> -
> -  # Find the version of the Python interpreter.
> -  execute_process(COMMAND "${PYTHON_EXE}" -c
> -  "import sys; sys.stdout.write('.'.join([str(x) for x in 
> sys.version_info[:3]]))"
> -  OUTPUT_VARIABLE PYTHON_VERSION_OUTPUT
> -  RESULT_VARIABLE PYTHON_VERSION_RESULT
> -  ERROR_QUIET)
> -
> -  if(PYTHON_VERSION_RESULT)
> -message(WARNING "Unable to retrieve Python executable version")
> -set(PYTHON_VERSION_OUTPUT "")
> -  endif()
> -
> -  set(${OUT_EXE_PATH_VARNAME} ${PYTHON_EXE} PARENT_SCOPE)
> -  set(${OUT_LIB_PATH_VARNAME} ${PYTHON_LIB} PARENT_SCOPE)
> -  set(${OUT_DLL_PATH_VARNAME} ${PYTHON_DLL} PARENT_SCOPE)
> -  set(${OUT_VERSION_VARNAME}  ${PYTHON_VERSION_OUTPUT} PARENT_SCOPE)
> -endfunction()
> -
> -function(find_python_libs_windows)
> -  if ("${PYTHON_HOME}" STREQUAL "")
> -message(WARNING "LLDB embedded Python on Windows requires specifying a 
> value for PYTHON_HOME.  Python support disabled.")
> -set(LLDB_ENABLE_PYTHON 0 PARENT_SCOPE)
> -return()
> -  endif()
> -
> -  file(TO_CMAKE_PATH 

[Lldb-commits] [PATCH] D72158: [DebugInfo] Make most debug line prologue errors non-fatal to parsing

2020-01-28 Thread James Henderson via Phabricator via lldb-commits
jhenderson reopened this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

Could somebody please look at the LLDB change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72158



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


[Lldb-commits] [PATCH] D72158: [DebugInfo] Make most debug line prologue errors non-fatal to parsing

2020-01-28 Thread James Henderson via Phabricator via lldb-commits
jhenderson updated this revision to Diff 240852.
jhenderson added a reviewer: labath.
jhenderson added a comment.
Herald added subscribers: lldb-commits, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLDB.

Fix LLD test + fix LLDB build.

I'm uncertain if the LLDB fix is the right fix to make or not, but it does at 
least stop this change breaking the build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72158

Files:
  lld/test/ELF/Inputs/undef-bad-debug.s
  lld/test/ELF/undef.s
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h
  llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
  llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
  llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s
  llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test
  llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

Index: llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
@@ -359,10 +359,15 @@
 
   generate();
 
-  checkGetOrParseLineTableEmitsFatalError(
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+nullptr, RecordRecoverable);
+  EXPECT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
+
+  checkError(
   {"parsing line table prologue at 0x found an invalid directory "
"or file table description at 0x0014",
-   "failed to parse entry content descriptions because no path was found"});
+   "failed to parse entry content descriptions because no path was found"},
+  std::move(Recoverable));
 }
 
 TEST_P(DebugLineParameterisedFixture, ErrorForTooLargePrologueLength) {
@@ -379,14 +384,24 @@
 
   generate();
 
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+nullptr, RecordRecoverable);
+  ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
+  DWARFDebugLine::LineTable Result(**ExpectedLineTable);
+  // Undo the earlier modification so that it can be compared against a
+  // "default" prologue.
+  --Result.Prologue.PrologueLength;
+  checkDefaultPrologue(Version, Format, Result.Prologue, 0);
+
   uint64_t ExpectedEnd =
   Prologue.TotalLength + 1 + Prologue.sizeofTotalLength();
-  checkGetOrParseLineTableEmitsFatalError(
+  checkError(
   (Twine("parsing line table prologue at 0x should have ended at "
  "0x00") +
Twine::utohexstr(ExpectedEnd) + " but it ended at 0x00" +
Twine::utohexstr(ExpectedEnd - 1))
-  .str());
+  .str(),
+  std::move(Recoverable));
 }
 
 TEST_P(DebugLineParameterisedFixture, ErrorForTooShortPrologueLength) {
@@ -408,16 +423,29 @@
 
   generate();
 
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+nullptr, RecordRecoverable);
+  ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
+  DWARFDebugLine::LineTable Result(**ExpectedLineTable);
+  // Undo the earlier modification so that it can be compared against a
+  // "default" prologue.
+  if (Version < 5)
+Result.Prologue.PrologueLength += 2;
+  else
+Result.Prologue.PrologueLength += 1;
+  checkDefaultPrologue(Version, Format, Result.Prologue, 0);
+
   uint64_t ExpectedEnd =
   Prologue.TotalLength - 1 + Prologue.sizeofTotalLength();
   if (Version < 5)
 --ExpectedEnd;
-  checkGetOrParseLineTableEmitsFatalError(
+  checkError(
   (Twine("parsing line table prologue at 0x should have ended at "
  "0x00") +
Twine::utohexstr(ExpectedEnd) + " but it ended at 0x00" +
Twine::utohexstr(ExpectedEnd + 1))
-  .str());
+  .str(),
+  std::move(Recoverable));
 }
 
 INSTANTIATE_TEST_CASE_P(
@@ -636,14 +664,15 @@
   EXPECT_EQ(Parser.getOffset(), 0u);
   ASSERT_FALSE(Parser.done());
 
-  Parser.skip(RecordUnrecoverable);
+  Parser.skip(RecordRecoverable, RecordUnrecoverable);
   EXPECT_EQ(Parser.getOffset(), 62u);
   ASSERT_FALSE(Parser.done());
 
-  Parser.skip(RecordUnrecoverable);
+  Parser.skip(RecordRecoverable, RecordUnrecoverable);
   EXPECT_EQ(Parser.getOffset(), 136u);
   EXPECT_TRUE(Parser.done());
 
+  EXPECT_FALSE(Recoverable);
   EXPECT_FALSE(Unrecoverable);
 }
 
@@ -688,10 +717,11 @@
   generate();
 
   DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
-  Parser.skip(RecordUnrecoverable);
+  Parser.skip(RecordRecoverable, RecordUnrecoverable);
 
   EXPECT_EQ(Parser.getOffset(), 4u);
   EXPECT_TRUE(Parser.done());
+  EXPECT_FALSE(Recoverable);
 
   checkError("parsing line table prologue at offset 0x unsupported "
  "reserved unit length found 

[Lldb-commits] [lldb] bb73210 - [lldb/PDB] Use the new line table constructor

2020-01-28 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-01-28T15:09:14+01:00
New Revision: bb73210ba9f16c1516f564235c86cbddccd1bd6d

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

LOG: [lldb/PDB] Use the new line table constructor

The old method of adding line sequences one by one can easily go
quadratic if the sequences are not perfectly sorted. The equivalent
change in DWARF brought a considerable improvement in line table
parsing. It is not clear if the same will be the case for PDB, but this
does bring us a step closer towards removing the dangerous API.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 9ecaa042863f..5757efe5fa6a 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1021,7 +1021,7 @@ uint32_t SymbolFileNativePDB::ResolveSymbolContext(
   return 0;
 }
 
-static void AppendLineEntryToSequence(LineTable , LineSequence ,
+static void AppendLineEntryToSequence(LineSequence ,
   const CompilandIndexItem ,
   lldb::addr_t base_addr,
   uint32_t file_number,
@@ -1040,21 +1040,18 @@ static void AppendLineEntryToSequence(LineTable , 
LineSequence ,
 
   uint32_t lno = cur_info.getStartLine();
 
-  table.AppendLineEntryToSequence(, addr, lno, 0, file_number,
-  is_statement, false, is_prologue, 
is_epilogue,
-  false);
+  LineTable::AppendLineEntryToSequence(, addr, lno, 0, file_number,
+   is_statement, false, is_prologue,
+   is_epilogue, false);
 }
 
-static void TerminateLineSequence(LineTable ,
-  const LineFragmentHeader ,
+static void TerminateLineSequence(const LineFragmentHeader ,
   lldb::addr_t base_addr, uint32_t file_number,
-  uint32_t last_line,
-  std::unique_ptr seq) {
+  uint32_t last_line, LineSequence ) {
   // The end is always a terminal entry, so insert it regardless.
-  table.AppendLineEntryToSequence(seq.get(), base_addr + block.CodeSize,
-  last_line, 0, file_number, false, false,
-  false, false, true);
-  table.InsertSequence(seq.release());
+  LineTable::AppendLineEntryToSequence(, base_addr + block.CodeSize,
+   last_line, 0, file_number, false, false,
+   false, false, true);
 }
 
 bool SymbolFileNativePDB::ParseLineTable(CompileUnit _unit) {
@@ -1068,11 +1065,11 @@ bool SymbolFileNativePDB::ParseLineTable(CompileUnit 
_unit) {
   CompilandIndexItem *cci =
   m_index->compilands().GetCompiland(cu_id.asCompiland().modi);
   lldbassert(cci);
-  auto line_table = std::make_unique(_unit);
 
   // This is basically a copy of the .debug$S subsections from all original 
COFF
   // object files merged together with address relocations applied.  We are
   // looking for all DEBUG_S_LINES subsections.
+  std::vector> sequences;
   for (const DebugSubsectionRecord  :
cci->m_debug_stream.getSubsectionsArray()) {
 if (dssr.kind() != DebugSubsectionKind::Lines)
@@ -,20 +1108,23 @@ bool SymbolFileNativePDB::ParseLineTable(CompileUnit 
_unit) {
   lldbassert(fn_iter != cci->m_file_list.end());
   uint32_t file_index = std::distance(cci->m_file_list.begin(), fn_iter);
 
-  std::unique_ptr sequence(
-  line_table->CreateLineSequenceContainer());
+  std::unique_ptr sequence =
+  LineTable::CreateLineSequenceContainer();
   lldbassert(!group.LineNumbers.empty());
 
   for (const LineNumberEntry  : group.LineNumbers) {
-AppendLineEntryToSequence(*line_table, *sequence, *cci, virtual_addr,
-  file_index, *lfh, entry);
+AppendLineEntryToSequence(*sequence, *cci, virtual_addr, file_index,
+  *lfh, entry);
   }
   LineInfo last_line(group.LineNumbers.back().Flags);
-  TerminateLineSequence(*line_table, *lfh, virtual_addr, file_index,
-last_line.getEndLine(), std::move(sequence));
+  TerminateLineSequence(*lfh, virtual_addr, file_index,
+last_line.getEndLine(), *sequence);
+  

[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2020-01-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D70840#1844318 , @mstorsjo wrote:

> In D70840#1844279 , @labath wrote:
>
> > This is something that probably should be discussed with the llvm dwarf 
> > owners too (though most of them are on this review already).
> >
> > However, if we assume that we can agree that this logic should be 
> > implemented at a fairly low level in the dwarf parsing code (so that e.g., 
> > when we switch to the llvm parser, it will do this thing for us),
>
>
> I guess the main question here, as follows from my comment above, is who/how 
> is the fixup controlled in the llvm side, where there's (to my knowledge) no 
> `GetOpcodeLoadAddress` that can abstract away the fixup? IIRC the dwarf code 
> itself is happily unaware of what architecture it is describing.


Yes, that is the trickiest part. Dwarf is architecture-agnostic for the most 
part (which makes this behavior of the windows linker very unfortunate), but 
the llvm DWARFContext already knows the architecture of the file it is 
describing (DWARFContext::getArch). The fixup could key off of that, but this 
will need careful consideration.

> 
> 
>> then this patch still seems quite "schizophrenic" to me, because the line 
>> table and location list fixups happen pretty high up. The location list case 
>> is particularly interesting here because we use a high level api 
>> (`llvm::DWARFLocationExpression`) to retrieve the location list, which may 
>> be reasonably expected to return "fixed" addresses, and so this fix should 
>> happen in llvm.
> 
> FWIW, as @clayborg said that `DWARFExpression` owns a Module in these cases, 
> and thus should have access to an ArchSpec, I could just slip the fixup into 
> `DWARFExpression::SetLocationListAddresses`. The reason I was messing around 
> there was primarily to avoid sprinkling fixups in many codepaths in 
> `DWARFDebugInfoEntry::GetDIENamesAndRanges` that call this method.

Here, I wasn't referring to `SetLocationListAddresses`, but rather to the 
addresses you get from the location list section 
(DWARFExpression::GetLocationExpression). I would expect you need to fix those 
do (but you don't do it in this patch). As for `SetLocationListAddresses`, I 
would still consider that high level, as the addresses pass through several 
high-level functions before landing here. If we go with the low-level approach, 
I'd expect this to happen somewhere near `DWARFUnit::GetBaseAddress` and 
`DWARFDIE::getLowPC` (imaginary function inspired by 
`llvm::DWARFDie::getLowAndHighPC`)

> 
> 
>> (The line tables also use llvm code for parsing, but we access them using  a 
>> very low-level api, so it's likely this check would have to done on lldb 
>> side even if llvm provided such a functionality -- but this could happen in 
>> SymbolFileDWARF::ParseLineTable instead of in the LineTable class).
> 
> Sure, I'll look into moving the fixups there.

I wouldn't bother with that too much until we figure out the overall strategy 
here.

>> If you want, I can send the initial email to start the discussion, but I 
>> can't commit to writing any of the patches. :(
> 
> If you can do that, that would be very much appreciated!

OK, I'll try to get the ball rolling there.


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

https://reviews.llvm.org/D70840



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


[Lldb-commits] [PATCH] D70646: Move non-DWARF code: `DWARFUnit` -> `SymbolFileDWARF`

2020-01-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: JDevlieghere.
labath added a comment.

This moves a lot of lldb-specific stuff out of low-level dwarf code, and as 
such, I think this is a great step towards the parser unification.

I'd just like to squeeze one interface tweak to function we are moving around, 
but otherwise, I think this is fine.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4011
+void SymbolFileDWARF::GetDWARFDeclContext(const DWARFDIE ,
+  DWARFDeclContext _decl_ctx) {
+  if (!die.IsValid()) {

please return the DWARFDeclContext by value here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70646



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


Re: [Lldb-commits] [lldb] fcaf5f6 - [LLDB] Fix the handling of unnamed bit-fields when parsing DWARF

2020-01-28 Thread Hans Wennborg via lldb-commits
Cherry-picked that in 54b022d344412fc9f1dfe37ee05de934a01e1ea4

Thanks!

On Tue, Jan 28, 2020 at 1:12 AM Raphael Isemann  wrote:
>
> Quick follow up: apparently Pavel fixed some nondeterminism in the testing 
> code that had made the test fail on some systems. Could you also cherry-pick 
> 77cedb0cdb8623ff9eb22dbf3b9302ee4d9f8a20 ?
>
> Thanks again!
>
> > On 27. Jan 2020, at 15:10, Hans Wennborg  wrote:
> >
> > Sure, b5cf892651812003e64c4a8f0dbf81f74a499016
> >
> > On Mon, Jan 27, 2020 at 12:18 AM Raphael Isemann  wrote:
> >>
> >> This commit fixes a *very* frequent crash when debugging Clang with LLDB 
> >> 10. Could we get that cherry-picked into the 10.x branch?
> >>
> >> I’ve been running the patch since more than a week and I haven’t found any 
> >> issues with it and the bots are also happy.
> >>
> >> Thanks!
> >> - Raphael
> >>
> >>> On 23. Jan 2020, at 23:46, via lldb-commits  
> >>> wrote:
> >>>
> >>>
> >>> Author: shafik
> >>> Date: 2020-01-23T14:46:24-08:00
> >>> New Revision: fcaf5f6c01a09f23b948afb8c91c4dd951d4525e
> >>>
> >>> URL: 
> >>> https://github.com/llvm/llvm-project/commit/fcaf5f6c01a09f23b948afb8c91c4dd951d4525e
> >>> DIFF: 
> >>> https://github.com/llvm/llvm-project/commit/fcaf5f6c01a09f23b948afb8c91c4dd951d4525e.diff
> >>>
> >>> LOG: [LLDB] Fix the handling of unnamed bit-fields when parsing DWARF
> >>>
> >>> We ran into an assert when debugging clang and performing an expression 
> >>> on a class derived from DeclContext. The assert was indicating we were 
> >>> getting the offsets wrong for RecordDeclBitfields. We were getting both 
> >>> the size and offset of unnamed bit-field members wrong. We could fix this 
> >>> case with a quick change but as I extended the test suite to include more 
> >>> combinations we kept finding more cases that were being handled 
> >>> incorrectly. A fix that handled all the new cases as well as the cases 
> >>> already covered required a refactor of the existing technique.
> >>>
> >>> Differential Revision: https://reviews.llvm.org/D72953
> >>>
> >>> Added:
> >>>   lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/Makefile
> >>>   
> >>> lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/TestCppBitfields.py
> >>>   lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/main.cpp
> >>>
> >>> Modified:
> >>>   lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
> >>>   lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
> >>>
> >>> Removed:
> >>>
> >>>
> >>>
> >>> 
> >>> diff  --git 
> >>> a/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/Makefile 
> >>> b/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/Makefile
> >>> new file mode 100644
> >>> index ..8b20bcb0
> >>> --- /dev/null
> >>> +++ b/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/Makefile
> >>> @@ -0,0 +1,3 @@
> >>> +CXX_SOURCES := main.cpp
> >>> +
> >>> +include Makefile.rules
> >>>
> >>> diff  --git 
> >>> a/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/TestCppBitfields.py
> >>>  
> >>> b/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/TestCppBitfields.py
> >>> new file mode 100644
> >>> index ..696e5647f13f
> >>> --- /dev/null
> >>> +++ 
> >>> b/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/TestCppBitfields.py
> >>> @@ -0,0 +1,105 @@
> >>> +"""Show bitfields and check that they display correctly."""
> >>> +
> >>> +import lldb
> >>> +from lldbsuite.test.decorators import *
> >>> +from lldbsuite.test.lldbtest import *
> >>> +from lldbsuite.test import lldbutil
> >>> +
> >>> +
> >>> +class CppBitfieldsTestCase(TestBase):
> >>> +
> >>> +mydir = TestBase.compute_mydir(__file__)
> >>> +
> >>> +def setUp(self):
> >>> +# Call super's setUp().
> >>> +TestBase.setUp(self)
> >>> +# Find the line number to break inside main().
> >>> +self.line = line_number('main.cpp', '// Set break point at this 
> >>> line.')
> >>> +
> >>> +# BitFields exhibit crashes in record layout on Windows
> >>> +# (http://llvm.org/pr21800)
> >>> +@skipIfWindows
> >>> +def test_and_run_command(self):
> >>> +"""Test 'frame variable ...' on a variable with bitfields."""
> >>> +self.build()
> >>> +
> >>> +lldbutil.run_to_source_breakpoint(self, '// Set break point at 
> >>> this line.',
> >>> +  lldb.SBFileSpec("main.cpp", False))
> >>> +
> >>> +# The stop reason of the thread should be breakpoint.
> >>> +self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
> >>> +substrs=['stopped',
> >>> + 'stop reason = breakpoint'])
> >>> +
> >>> +# The breakpoint should have a hit count of 1.
> >>> +self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
> >>> +substrs=[' resolved, hit count = 1'])
> >>> +
> >>> +self.expect("expr (lba.a)", VARIABLES_DISPLAYED_CORRECTLY,

[Lldb-commits] [PATCH] D73191: Only match mangled name in full-name function lookup (with accelerators)

2020-01-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added a subscriber: teemperor.
labath added a comment.

I've had to revert this because of some failures on macos 
(http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/7109/testReport/). 
@jarin, are you able to run the test suite on a mac to investigate.

@teemperor, all of the failures (except TestPrintf, which was actually fixed) 
were in the "import std module" tests, relating to the failure to lookup 
symbols like `__Z12__to_addressP1C`. I have a feeling this could actually be a 
bug in the std module handler -- I know it does something special with symbols 
in the `std` namespace, but this looks like a standard library symbol which is 
not in that namespace (and it's not surprising we can't find it in the binary 
if the program never used it). What do you think is the best way to resolve 
this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73191



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


[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2020-01-28 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo added a comment.

In D70840#1844279 , @labath wrote:

> Right, your answer implicitly assumes (and I do agree with you) that the 
> answer is that `getSubroutineForAddress` should return the function that 
> address is in. That is not unreasonable -- I deliberately chose the most 
> high-level api I could find, to make arguing that case easy. But that case 
> should be argued nonetheless. For example, a counterargument to that might be 
> that one should only use valid PC register values as inputs to this function 
> (so an even value would not be a valid input here, because the function is 
> thumb), and in that case, the there would be no fixups needed. This position 
> could be further strengthened by the fact that the llvm dwarf parser is also 
> used by tools (e.g., llvm-dwarfdump) which want a fairly exact view of the 
> data in the file, without too much smartness, so there may need to be some 
> kind of an api which would provide the "raw" value.


I guess the selection of what kind of fixups to apply could be controlled via 
whatever object controls the fixup. In these patches, it's lldb's ArchSpec - 
the low level dwarf routines doesn't have any ArchSpec but uses the one given 
to them to fix up the addresses.

> This is something that probably should be discussed with the llvm dwarf 
> owners too (though most of them are on this review already).
> 
> However, if we assume that we can agree that this logic should be implemented 
> at a fairly low level in the dwarf parsing code (so that e.g., when we switch 
> to the llvm parser, it will do this thing for us),

I guess the main question here, as follows from my comment above, is who/how is 
the fixup controlled in the llvm side, where there's (to my knowledge) no 
`GetOpcodeLoadAddress` that can abstract away the fixup? IIRC the dwarf code 
itself is happily unaware of what architecture it is describing.

> then this patch still seems quite "schizophrenic" to me, because the line 
> table and location list fixups happen pretty high up. The location list case 
> is particularly interesting here because we use a high level api 
> (`llvm::DWARFLocationExpression`) to retrieve the location list, which may be 
> reasonably expected to return "fixed" addresses, and so this fix should 
> happen in llvm.

FWIW, as @clayborg said that `DWARFExpression` owns a Module in these cases, 
and thus should have access to an ArchSpec, I could just slip the fixup into 
`DWARFExpression::SetLocationListAddresses`. The reason I was messing around 
there was primarily to avoid sprinkling fixups in many codepaths in 
`DWARFDebugInfoEntry::GetDIENamesAndRanges` that call this method.

> (The line tables also use llvm code for parsing, but we access them using  a 
> very low-level api, so it's likely this check would have to done on lldb side 
> even if llvm provided such a functionality -- but this could happen in 
> SymbolFileDWARF::ParseLineTable instead of in the LineTable class).

Sure, I'll look into moving the fixups there.

> So, at the end of the day, I guess what I am saying is that we should send a 
> small RFC to the llvm debug info folks to clarify what should the behavior of 
> the llvm classes be for this kind of dwarf. If the conclusion is that this 
> should be implemented inside these classes (and not by their users), then we 
> should implement it there, and then have the lldb code mirror that. This is 
> the approach we've been trying to do for a while now when modifying or 
> implementing new dwarf features in lldb, but this is probably the first case 
> of wanting to implement a feature in lldb where there is no existing llvm 
> counterpart. (In contrast the DWZ feature is also blocked in part due to 
> concerns about increasing llvm divergence, but there I'm pretty sure the 
> answer is that DWZ should not be implemented in llvm, and the lldb parts 
> should be written in a way that does not require low-level dwarf changes.)
> 
> If you want, I can send the initial email to start the discussion, but I 
> can't commit to writing any of the patches. :(

If you can do that, that would be very much appreciated!


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

https://reviews.llvm.org/D70840



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


[Lldb-commits] [lldb] d8de349 - Revert "[lldb/DWARF] Only match mangled name in full-name function lookup (with accelerators)"

2020-01-28 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-01-28T13:46:43+01:00
New Revision: d8de349951c275af86d67eb3e9c7b1f554531a9b

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

LOG: Revert "[lldb/DWARF] Only match mangled name in full-name function lookup 
(with accelerators)"

This reverts commit 1b12766883006b8aa9d1ff744e57317647aa052a because of
breaking the mac test suite.

I'm not certain this is the cause because of a concurrent build breakage
which masked this problem, but the failure messages are related to
symbol lookup, which makes this very likely.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
index 2b435e8d237e..7b8e499a27b4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
@@ -39,8 +39,8 @@ void DWARFIndex::ProcessFunctionDIE(llvm::StringRef name, 
DIERef ref,
   if (!SymbolFileDWARF::DIEInDeclContext(_decl_ctx, die))
 return;
 
-  // In case of a full match, we insert functions with a matching mangled name.
-  if (name_type_mask & eFunctionNameTypeFull && die.GetMangledName() == name) {
+  // In case of a full match, we just insert everything we find.
+  if (name_type_mask & eFunctionNameTypeFull) {
 dies.push_back(die);
 return;
   }

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp 
b/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
index c4fdee113eab..0adf7b733408 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
@@ -10,10 +10,6 @@
 // RUN:   FileCheck --check-prefix=FULL %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
-// RUN: lldb-test symbols --name=_ZN3bar3baz3fooEv --find=function 
--function-flags=full %t | \
-// RUN:   FileCheck --check-prefix=FULL-MANGLED-NAMESPACE %s
-// RUN: lldb-test symbols --name=_ZN4sbar3fooEi --find=function 
--function-flags=full %t | \
-// RUN:   FileCheck --check-prefix=FULL-MANGLED-METHOD %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=function %t | \
@@ -25,13 +21,9 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck --check-prefix=FULL %s
+// RUN:   FileCheck --check-prefix=FULL-INDEXED %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
-// RUN: lldb-test symbols --name=_ZN3bar3baz3fooEv --find=function 
--function-flags=full %t | \
-// RUN:   FileCheck --check-prefix=FULL-MANGLED-NAMESPACE %s
-// RUN: lldb-test symbols --name=_ZN4sbar3fooEi --find=function 
--function-flags=full %t | \
-// RUN:   FileCheck --check-prefix=FULL-MANGLED-METHOD %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=function %t | \
@@ -45,13 +37,9 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck --check-prefix=FULL %s
+// RUN:   FileCheck --check-prefix=FULL-INDEXED %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
-// RUN: lldb-test symbols --name=_ZN3bar3baz3fooEv --find=function 
--function-flags=full %t | \
-// RUN:   FileCheck --check-prefix=FULL-MANGLED-NAMESPACE %s
-// RUN: lldb-test symbols --name=_ZN4sbar3fooEi --find=function 
--function-flags=full %t | \
-// RUN:   FileCheck --check-prefix=FULL-MANGLED-METHOD %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=function %t | \
@@ -70,17 +58,20 @@
 // METHOD-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
 // METHOD-DAG: name = "ffbar()::sbaz::foo()", mangled = 
"_ZZ5ffbarvEN4sbaz3fooEv"
 
+// FULL-INDEXED: Found 7 functions:
+// FULL-INDEXED-DAG: name = "foo()", mangled = "_Z3foov"
+// 

[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2020-01-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D70840#1838249 , @mstorsjo wrote:

> In D70840#1838186 , @labath wrote:
>
> > The thing that I am not sure we have fully explored is whether there is any 
> > need for this `&~1` business in the llvm dwarf code. For instance, what 
> > should `llvm::DWARFUnit::getSubroutineForAddress` return if you pass it an 
> > address that is equal to the actual memory address of the start of the 
> > thumb function, but the relevant DW_AT_low_pc contains `address|1`? 
> > Answering that might give us an indication on what is the layer at which 
> > this fixup should be applied.
>
>
> From a quick check at the code there, for that particular case, either it'd 
> be done in `llvm::DWARFUnit::updateAddressDieMap` or in 
> `llvm::DWARFDie::getAddressRanges`. Since all levels of the API is public, 
> the fix does become safer the closer to parsing it is, but one also generally 
> has less context to work with in those cases.


Right, your answer implicitly assumes (and I do agree with you) that the answer 
is that `getSubroutineForAddress` should return the function that address is 
in. That is not unreasonable -- I deliberately chose the most high-level api I 
could find, to make arguing that case easy. But that case should be argued 
nonetheless. For example, a counterargument to that might be that one should 
only use valid PC register values as inputs to this function (so an even value 
would not be a valid input here, because the function is thumb), and in that 
case, the there would be no fixups needed. This position could be further 
strengthened by the fact that the llvm dwarf parser is also used by tools 
(e.g., llvm-dwarfdump) which want a fairly exact view of the data in the file, 
without too much smartness, so there may need to be some kind of an api which 
would provide the "raw" value.

This is something that probably should be discussed with the llvm dwarf owners 
too (though most of them are on this review already).

However, if we assume that we can agree that this logic should be implemented 
at a fairly low level in the dwarf parsing code (so that e.g., when we switch 
to the llvm parser, it will do this thing for us), then this patch still seems 
quite "schizophrenic" to me, because the line table and location list fixups 
happen pretty high up. The location list case is particularly interesting here 
because we use a high level api (`llvm::DWARFLocationExpression`) to retrieve 
the location list, which may be reasonably expected to return "fixed" 
addresses, and so this fix should happen in llvm. (The line tables also use 
llvm code for parsing, but we access them using  a very low-level api, so it's 
likely this check would have to done on lldb side even if llvm provided such a 
functionality -- but this could happen in SymbolFileDWARF::ParseLineTable 
instead of in the LineTable class).

So, at the end of the day, I guess what I am saying is that we should send a 
small RFC to the llvm debug info folks to clarify what should the behavior of 
the llvm classes be for this kind of dwarf. If the conclusion is that this 
should be implemented inside these classes (and not by their users), then we 
should implement it there, and then have the lldb code mirror that. This is the 
approach we've been trying to do for a while now when modifying or implementing 
new dwarf features in lldb, but this is probably the first case of wanting to 
implement a feature in lldb where there is no existing llvm counterpart. (In 
contrast the DWZ feature is also blocked in part due to concerns about 
increasing llvm divergence, but there I'm pretty sure the answer is that DWZ 
should not be implemented in llvm, and the lldb parts should be written in a 
way that does not require low-level dwarf changes.)

If you want, I can send the initial email to start the discussion, but I can't 
commit to writing any of the patches. :(


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

https://reviews.llvm.org/D70840



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


[Lldb-commits] [PATCH] D73539: [AVR] Basic support for remote debugging

2020-01-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks for the patch, and in particular for starting with a small increment 
instead of a giant implement-all patch.

Could you please add a simple test case that runs `yaml2obj | lldb-test 
object-file` and verifies that the avr object file is parsed properly. (The 
architecture is the most important part, but you might as well check the 
sections while you're at it.) You can look at the existing tests in 
`test/Shell/ObjectFile` for inspiration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73539



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


[Lldb-commits] [PATCH] D73539: [AVR] Basic support for remote debugging

2020-01-28 Thread Ayke via Phabricator via lldb-commits
aykevl created this revision.
aykevl added reviewers: dylanmckay, deepak2427, Andrzej, clayborg, labath.
Herald added subscribers: lldb-commits, Jim, aprantl.
Herald added a project: LLDB.

Add bare-metal AVR support to lldb. Loading a binary works, but little else.

Things that work:

  $ ./llvm-build.master/bin/lldb tmp/avr.elf
  (lldb) target create "tmp/avr.elf"
  Current executable set to 
'/home/ayke/src/github.com/tinygo-org/tinygo/tmp/avr.elf' (avr).
  (lldb) image lookup -F main
  1 match found in /home/ayke/src/github.com/tinygo-org/tinygo/tmp/avr.elf:
  Address: avr.elf[0x0080] (avr.elf.PT_LOAD[0]..text + 128)
  Summary: avr.elf`main at avr.c:10
  (lldb) image dump line-table avr.c
  Line table for /home/ayke/src/github.com/tinygo-org/tinygo/tmp/avr.c in 
`avr.elf
  0x0080: /home/ayke/src/github.com/tinygo-org/tinygo/tmp/avr.c:10
  0x0084: /home/ayke/src/github.com/tinygo-org/tinygo/tmp/avr.c:13
  0x0088: /usr/lib/avr/include/util/delay.h:163
  0x009a: /home/ayke/src/github.com/tinygo-org/tinygo/tmp/avr.c:15
  0x009c: /usr/lib/avr/include/util/delay.h:163
  0x00b0: /usr/lib/avr/include/util/delay.h:163

Source code (copied from the internet somewhere):

  #ifndef F_CPU
  #define F_CPU 1600UL // 16 MHz clock speed
  #endif
  
  #include 
  #include 
  
  int main(void)
  {
DDRC = 0xFF; //Nakes PORTC as Output
while(1) //infinite loop
{
  PORTC = 0xFF; //Turns ON All LEDs
  _delay_ms(1000); //1 second delay
  PORTC= 0x00; //Turns OFF All LEDs
  _delay_ms(1000); //1 second delay
}
  }

Compiled using avr-gcc (with `-gdwarf-4` as it defaults to the stabs debug 
format in Debian):

  avr-gcc -Og -gdwarf-4 -mmcu=atmega328p -o avr.elf avr.c

Things like disassembling a binary don't work yet, but I think that can be done 
independently by getting llvm-objdump to work with the AVR target (it currently 
results in an assertion failure).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73539

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/Utility/ArchSpec.cpp


Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -218,6 +218,8 @@
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
 {eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"},
 
+{eByteOrderLittle, 2, 2, 4, llvm::Triple::avr, ArchSpec::eCore_avr, "avr"},
+
 {eByteOrderLittle, 4, 1, 4, llvm::Triple::wasm32, ArchSpec::eCore_wasm32,
  "wasm32"},
 };
@@ -448,6 +450,8 @@
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARC
+{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu}, // AVR
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: lldb/include/lldb/Utility/ArchSpec.h
===
--- lldb/include/lldb/Utility/ArchSpec.h
+++ lldb/include/lldb/Utility/ArchSpec.h
@@ -188,6 +188,8 @@
 
 eCore_arc, // little endian ARC
 
+eCore_avr,
+
 eCore_wasm32,
 
 kNumCores,


Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -218,6 +218,8 @@
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
 {eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"},
 
+{eByteOrderLittle, 2, 2, 4, llvm::Triple::avr, ArchSpec::eCore_avr, "avr"},
+
 {eByteOrderLittle, 4, 1, 4, llvm::Triple::wasm32, ArchSpec::eCore_wasm32,
  "wasm32"},
 };
@@ -448,6 +450,8 @@
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARC
+{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu}, // AVR
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: lldb/include/lldb/Utility/ArchSpec.h
===
--- lldb/include/lldb/Utility/ArchSpec.h
+++ lldb/include/lldb/Utility/ArchSpec.h
@@ -188,6 +188,8 @@
 
 eCore_arc, // little endian ARC
 
+eCore_avr,
+
 eCore_wasm32,
 
 kNumCores,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D73191: Only match mangled name in full-name function lookup (with accelerators)

2020-01-28 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1b1276688300: [lldb/DWARF] Only match mangled name in 
full-name function lookup (with… (authored by jarin, committed by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73191

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp


Index: lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
@@ -10,6 +10,10 @@
 // RUN:   FileCheck --check-prefix=FULL %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
+// RUN: lldb-test symbols --name=_ZN3bar3baz3fooEv --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-NAMESPACE %s
+// RUN: lldb-test symbols --name=_ZN4sbar3fooEi --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-METHOD %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=function %t | \
@@ -21,9 +25,13 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck --check-prefix=FULL-INDEXED %s
+// RUN:   FileCheck --check-prefix=FULL %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
+// RUN: lldb-test symbols --name=_ZN3bar3baz3fooEv --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-NAMESPACE %s
+// RUN: lldb-test symbols --name=_ZN4sbar3fooEi --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-METHOD %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=function %t | \
@@ -37,9 +45,13 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck --check-prefix=FULL-INDEXED %s
+// RUN:   FileCheck --check-prefix=FULL %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
+// RUN: lldb-test symbols --name=_ZN3bar3baz3fooEv --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-NAMESPACE %s
+// RUN: lldb-test symbols --name=_ZN4sbar3fooEi --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-METHOD %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=function %t | \
@@ -58,20 +70,17 @@
 // METHOD-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
 // METHOD-DAG: name = "ffbar()::sbaz::foo()", mangled = 
"_ZZ5ffbarvEN4sbaz3fooEv"
 
-// FULL-INDEXED: Found 7 functions:
-// FULL-INDEXED-DAG: name = "foo()", mangled = "_Z3foov"
-// FULL-INDEXED-DAG: name = "foo(int)", mangled = "_Z3fooi"
-// FULL-INDEXED-DAG: name = "bar::foo()", mangled = "_ZN3bar3fooEv"
-// FULL-INDEXED-DAG: name = "bar::baz::foo()", mangled = "_ZN3bar3baz3fooEv"
-// FULL-INDEXED-DAG: name = "sbar::foo()", mangled = "_ZN4sbar3fooEv"
-// FULL-INDEXED-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
-// FULL-INDEXED-DAG: name = "ffbar()::sbaz::foo()", mangled = 
"_ZZ5ffbarvEN4sbaz3fooEv"
-
 // FULL: Found 0 functions:
 
 // FULL-MANGLED: Found 1 functions:
 // FULL-MANGLED-DAG: name = "foo(int)", mangled = "_Z3fooi"
 
+// FULL-MANGLED-NAMESPACE: Found 1 functions:
+// FULL-MANGLED-NAMESPACE-DAG: name = "bar::baz::foo()", mangled = 
"_ZN3bar3baz3fooEv"
+
+// FULL-MANGLED-METHOD: Found 1 functions:
+// FULL-MANGLED-METHOD-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
+
 // CONTEXT: Found 1 functions:
 // CONTEXT-DAG: name = "bar::foo()", mangled = "_ZN3bar3fooEv"
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
@@ -39,8 +39,8 @@
   if (!SymbolFileDWARF::DIEInDeclContext(_decl_ctx, die))
  

[Lldb-commits] [lldb] 1b12766 - [lldb/DWARF] Only match mangled name in full-name function lookup (with accelerators)

2020-01-28 Thread Pavel Labath via lldb-commits

Author: Jaroslav Sevcik
Date: 2020-01-28T12:16:02+01:00
New Revision: 1b12766883006b8aa9d1ff744e57317647aa052a

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

LOG: [lldb/DWARF] Only match mangled name in full-name function lookup (with 
accelerators)

Summary:
In the spirit of https://reviews.llvm.org/D70846, we only return functions with 
matching mangled name from Apple/DebugNamesDWARFIndex::GetFunction if 
eFunctionNameTypeFull is requested.

This speeds up lookup in the presence of large amount of class methods of the 
same name (a typical examples would be constructors of templates with many 
instantiations or overloaded operators).

Reviewers: labath

Reviewed By: labath

Subscribers: aprantl, arphaman, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
index 7b8e499a27b4..2b435e8d237e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
@@ -39,8 +39,8 @@ void DWARFIndex::ProcessFunctionDIE(llvm::StringRef name, 
DIERef ref,
   if (!SymbolFileDWARF::DIEInDeclContext(_decl_ctx, die))
 return;
 
-  // In case of a full match, we just insert everything we find.
-  if (name_type_mask & eFunctionNameTypeFull) {
+  // In case of a full match, we insert functions with a matching mangled name.
+  if (name_type_mask & eFunctionNameTypeFull && die.GetMangledName() == name) {
 dies.push_back(die);
 return;
   }

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp 
b/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
index 0adf7b733408..c4fdee113eab 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
@@ -10,6 +10,10 @@
 // RUN:   FileCheck --check-prefix=FULL %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
+// RUN: lldb-test symbols --name=_ZN3bar3baz3fooEv --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-NAMESPACE %s
+// RUN: lldb-test symbols --name=_ZN4sbar3fooEi --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-METHOD %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=function %t | \
@@ -21,9 +25,13 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck --check-prefix=FULL-INDEXED %s
+// RUN:   FileCheck --check-prefix=FULL %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
+// RUN: lldb-test symbols --name=_ZN3bar3baz3fooEv --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-NAMESPACE %s
+// RUN: lldb-test symbols --name=_ZN4sbar3fooEi --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-METHOD %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=function %t | \
@@ -37,9 +45,13 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck --check-prefix=FULL-INDEXED %s
+// RUN:   FileCheck --check-prefix=FULL %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
+// RUN: lldb-test symbols --name=_ZN3bar3baz3fooEv --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-NAMESPACE %s
+// RUN: lldb-test symbols --name=_ZN4sbar3fooEi --find=function 
--function-flags=full %t | \
+// RUN:   FileCheck --check-prefix=FULL-MANGLED-METHOD %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
 // RUN:   FileCheck --check-prefix=CONTEXT %s
 // RUN: lldb-test symbols --name=not_there --find=function %t | \
@@ -58,20 +70,17 

[Lldb-commits] [PATCH] D73506: Auto-completion bug fix for dot operator

2020-01-28 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

I don't really know the vscode plugin code but I pointed out when this was 
implemented  that the whole 
token replacement mechanism that LLDB's completion API uses is really hard to 
get right. If you're at it, you might also add some tests for the things I 
pointed out back then (e.g., `foo1. v` and `foo1 .v` and `foo1 . v`) where the 
returned tokens might be unexpectedly cut off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73506



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


[Lldb-commits] [lldb] 00efeae - [lldb][NFC] Simplify Materializer/Dematerializer constructors

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

Author: Raphael Isemann
Date: 2020-01-28T11:31:02+01:00
New Revision: 00efeae34f22e81ccbcf8cf9b46f314d8101063b

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

LOG: [lldb][NFC] Simplify Materializer/Dematerializer constructors

Added: 


Modified: 
lldb/include/lldb/Expression/Materializer.h
lldb/source/Expression/Materializer.cpp

Removed: 




diff  --git a/lldb/include/lldb/Expression/Materializer.h 
b/lldb/include/lldb/Expression/Materializer.h
index 70f622e7850b..3bf18de58948 100644
--- a/lldb/include/lldb/Expression/Materializer.h
+++ b/lldb/include/lldb/Expression/Materializer.h
@@ -22,14 +22,12 @@ namespace lldb_private {
 
 class Materializer {
 public:
-  Materializer();
+  Materializer() = default;
   ~Materializer();
 
   class Dematerializer {
   public:
-Dematerializer()
-: m_materializer(nullptr), m_map(nullptr),
-  m_process_address(LLDB_INVALID_ADDRESS) {}
+Dematerializer() = default;
 
 ~Dematerializer() { Wipe(); }
 
@@ -56,11 +54,11 @@ class Materializer {
   }
 }
 
-Materializer *m_materializer;
+Materializer *m_materializer = nullptr;
 lldb::ThreadWP m_thread_wp;
 StackID m_stack_id;
-IRMemoryMap *m_map;
-lldb::addr_t m_process_address;
+IRMemoryMap *m_map = nullptr;
+lldb::addr_t m_process_address = LLDB_INVALID_ADDRESS;
   };
 
   typedef std::shared_ptr DematerializerSP;
@@ -128,8 +126,8 @@ class Materializer {
 
   DematerializerWP m_dematerializer_wp;
   EntityVector m_entities;
-  uint32_t m_current_offset;
-  uint32_t m_struct_alignment;
+  uint32_t m_current_offset = 0;
+  uint32_t m_struct_alignment = 8;
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Expression/Materializer.cpp 
b/lldb/source/Expression/Materializer.cpp
index d0982be6efa4..0d210ca19074 100644
--- a/lldb/source/Expression/Materializer.cpp
+++ b/lldb/source/Expression/Materializer.cpp
@@ -1332,9 +1332,6 @@ uint32_t Materializer::AddRegister(const RegisterInfo 
_info,
   return ret;
 }
 
-Materializer::Materializer()
-: m_dematerializer_wp(), m_current_offset(0), m_struct_alignment(8) {}
-
 Materializer::~Materializer() {
   DematerializerSP dematerializer_sp = m_dematerializer_wp.lock();
 



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


[Lldb-commits] [lldb] 243f52b - [lldb] Cut off unused suffix in CompletionRequest::GetRawLine

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

Author: Raphael Isemann
Date: 2020-01-28T11:12:22+01:00
New Revision: 243f52b58bcefab68fdebefc6d64f7f0c182c0fe

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

LOG: [lldb] Cut off unused suffix in CompletionRequest::GetRawLine

The GetRawLine currently returns the full command line used
to create the CompletionRequest. So for example for "foo b[tab] --arg"
it would return the whole string instead of "foo b". Usually
completion code makes the wrong assumption that the cursor is at
the end of the line and handing out the complete line will cause
that people implement completions that also make this assumption.

This patch makes GetRawLine() return only the string until the
cursor and hides the suffix (so that the cursor is always at the
end of this string) and adds another function GetRawLineWithUnusedSuffix
that is specifically the line with the suffix that isn't used by
the CompletionRequest for argument parsing etc.

There is only one user of this new function that actually needs the
suffix and that is the expression command which needs the suffix to
detect if it is in the raw or argument part of the command (by looking
at the "--" separator).

Added: 


Modified: 
lldb/include/lldb/Utility/CompletionRequest.h
lldb/source/Commands/CommandObjectExpression.cpp
lldb/unittests/Utility/CompletionRequestTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/CompletionRequest.h 
b/lldb/include/lldb/Utility/CompletionRequest.h
index 5418efe180bf..358d88703c71 100644
--- a/lldb/include/lldb/Utility/CompletionRequest.h
+++ b/lldb/include/lldb/Utility/CompletionRequest.h
@@ -115,7 +115,19 @@ class CompletionRequest {
   CompletionRequest(llvm::StringRef command_line, unsigned raw_cursor_pos,
 CompletionResult );
 
-  llvm::StringRef GetRawLine() const { return m_command; }
+  /// Returns the raw user input used to create this CompletionRequest cut off
+  /// at the cursor position. The cursor will be at the end of the raw line.
+  llvm::StringRef GetRawLine() const {
+return m_command.substr(0, GetRawCursorPos());
+  }
+
+  /// Returns the full raw user input used to create this CompletionRequest.
+  /// This string is not cut off at the cursor position and will include
+  /// characters behind the cursor position.
+  ///
+  /// You should most likely *not* use this function unless the characters
+  /// behind the cursor position influence the completion.
+  llvm::StringRef GetRawLineWithUnusedSuffix() const { return m_command; }
 
   unsigned GetRawCursorPos() const { return m_raw_cursor_pos; }
 

diff  --git a/lldb/source/Commands/CommandObjectExpression.cpp 
b/lldb/source/Commands/CommandObjectExpression.cpp
index 6fb6b1df2923..fabc49683b8f 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -311,7 +311,12 @@ void 
CommandObjectExpression::HandleCompletion(CompletionRequest ) {
 target = ();
 
   unsigned cursor_pos = request.GetRawCursorPos();
-  llvm::StringRef code = request.GetRawLine();
+  // Get the full user input including the suffix. The suffix is necessary
+  // as OptionsWithRaw will use it to detect if the cursor is cursor is in the
+  // argument part of in the raw input part of the arguments. If we cut of
+  // of the suffix then "expr -arg[cursor] --" would interpret the "-arg" as
+  // the raw input (as the "--" is hidden in the suffix).
+  llvm::StringRef code = request.GetRawLineWithUnusedSuffix();
 
   const std::size_t original_code_size = code.size();
 

diff  --git a/lldb/unittests/Utility/CompletionRequestTest.cpp 
b/lldb/unittests/Utility/CompletionRequestTest.cpp
index 24d9febcec2d..02a5f5366a42 100644
--- a/lldb/unittests/Utility/CompletionRequestTest.cpp
+++ b/lldb/unittests/Utility/CompletionRequestTest.cpp
@@ -21,7 +21,8 @@ TEST(CompletionRequest, Constructor) {
   CompletionRequest request(command, cursor_pos, result);
   result.GetMatches(matches);
 
-  EXPECT_STREQ(request.GetRawLine().str().c_str(), command.c_str());
+  EXPECT_EQ(request.GetRawLine(), "a b");
+  EXPECT_EQ(request.GetRawLineWithUnusedSuffix(), command);
   EXPECT_EQ(request.GetRawCursorPos(), cursor_pos);
   EXPECT_EQ(request.GetCursorIndex(), arg_index);
 
@@ -38,7 +39,8 @@ TEST(CompletionRequest, FakeLastArg) {
 
   CompletionRequest request(command, cursor_pos, result);
 
-  EXPECT_STREQ(request.GetRawLine().str().c_str(), command.c_str());
+  EXPECT_EQ(request.GetRawLine(), command);
+  EXPECT_EQ(request.GetRawLineWithUnusedSuffix(), command);
   EXPECT_EQ(request.GetRawCursorPos(), cursor_pos);
   EXPECT_EQ(request.GetCursorIndex(), 3U);
 
@@ -93,7 +95,8 @@ TEST(CompletionRequest, ShiftArguments) {
   CompletionRequest request(command, cursor_pos, result);
   

Re: [Lldb-commits] [lldb] fcaf5f6 - [LLDB] Fix the handling of unnamed bit-fields when parsing DWARF

2020-01-28 Thread Raphael Isemann via lldb-commits
Quick follow up: apparently Pavel fixed some nondeterminism in the testing code 
that had made the test fail on some systems. Could you also cherry-pick 
77cedb0cdb8623ff9eb22dbf3b9302ee4d9f8a20 ?

Thanks again!

> On 27. Jan 2020, at 15:10, Hans Wennborg  wrote:
> 
> Sure, b5cf892651812003e64c4a8f0dbf81f74a499016
> 
> On Mon, Jan 27, 2020 at 12:18 AM Raphael Isemann  wrote:
>> 
>> This commit fixes a *very* frequent crash when debugging Clang with LLDB 10. 
>> Could we get that cherry-picked into the 10.x branch?
>> 
>> I’ve been running the patch since more than a week and I haven’t found any 
>> issues with it and the bots are also happy.
>> 
>> Thanks!
>> - Raphael
>> 
>>> On 23. Jan 2020, at 23:46, via lldb-commits  
>>> wrote:
>>> 
>>> 
>>> Author: shafik
>>> Date: 2020-01-23T14:46:24-08:00
>>> New Revision: fcaf5f6c01a09f23b948afb8c91c4dd951d4525e
>>> 
>>> URL: 
>>> https://github.com/llvm/llvm-project/commit/fcaf5f6c01a09f23b948afb8c91c4dd951d4525e
>>> DIFF: 
>>> https://github.com/llvm/llvm-project/commit/fcaf5f6c01a09f23b948afb8c91c4dd951d4525e.diff
>>> 
>>> LOG: [LLDB] Fix the handling of unnamed bit-fields when parsing DWARF
>>> 
>>> We ran into an assert when debugging clang and performing an expression on 
>>> a class derived from DeclContext. The assert was indicating we were getting 
>>> the offsets wrong for RecordDeclBitfields. We were getting both the size 
>>> and offset of unnamed bit-field members wrong. We could fix this case with 
>>> a quick change but as I extended the test suite to include more 
>>> combinations we kept finding more cases that were being handled 
>>> incorrectly. A fix that handled all the new cases as well as the cases 
>>> already covered required a refactor of the existing technique.
>>> 
>>> Differential Revision: https://reviews.llvm.org/D72953
>>> 
>>> Added:
>>>   lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/Makefile
>>>   lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/TestCppBitfields.py
>>>   lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/main.cpp
>>> 
>>> Modified:
>>>   lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
>>>   lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
>>> 
>>> Removed:
>>> 
>>> 
>>> 
>>> 
>>> diff  --git 
>>> a/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/Makefile 
>>> b/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/Makefile
>>> new file mode 100644
>>> index ..8b20bcb0
>>> --- /dev/null
>>> +++ b/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/Makefile
>>> @@ -0,0 +1,3 @@
>>> +CXX_SOURCES := main.cpp
>>> +
>>> +include Makefile.rules
>>> 
>>> diff  --git 
>>> a/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/TestCppBitfields.py
>>>  
>>> b/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/TestCppBitfields.py
>>> new file mode 100644
>>> index ..696e5647f13f
>>> --- /dev/null
>>> +++ 
>>> b/lldb/packages/Python/lldbsuite/test/lang/cpp/bitfields/TestCppBitfields.py
>>> @@ -0,0 +1,105 @@
>>> +"""Show bitfields and check that they display correctly."""
>>> +
>>> +import lldb
>>> +from lldbsuite.test.decorators import *
>>> +from lldbsuite.test.lldbtest import *
>>> +from lldbsuite.test import lldbutil
>>> +
>>> +
>>> +class CppBitfieldsTestCase(TestBase):
>>> +
>>> +mydir = TestBase.compute_mydir(__file__)
>>> +
>>> +def setUp(self):
>>> +# Call super's setUp().
>>> +TestBase.setUp(self)
>>> +# Find the line number to break inside main().
>>> +self.line = line_number('main.cpp', '// Set break point at this 
>>> line.')
>>> +
>>> +# BitFields exhibit crashes in record layout on Windows
>>> +# (http://llvm.org/pr21800)
>>> +@skipIfWindows
>>> +def test_and_run_command(self):
>>> +"""Test 'frame variable ...' on a variable with bitfields."""
>>> +self.build()
>>> +
>>> +lldbutil.run_to_source_breakpoint(self, '// Set break point at 
>>> this line.',
>>> +  lldb.SBFileSpec("main.cpp", False))
>>> +
>>> +# The stop reason of the thread should be breakpoint.
>>> +self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
>>> +substrs=['stopped',
>>> + 'stop reason = breakpoint'])
>>> +
>>> +# The breakpoint should have a hit count of 1.
>>> +self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
>>> +substrs=[' resolved, hit count = 1'])
>>> +
>>> +self.expect("expr (lba.a)", VARIABLES_DISPLAYED_CORRECTLY,
>>> +substrs=['unsigned int', '2'])
>>> +self.expect("expr (lbb.b)", VARIABLES_DISPLAYED_CORRECTLY,
>>> +substrs=['unsigned int', '3'])
>>> +self.expect("expr (lbc.c)", VARIABLES_DISPLAYED_CORRECTLY,
>>> +substrs=['unsigned int', '4'])
>>> +self.expect("expr (lbd.a)", 

[Lldb-commits] [PATCH] D73506: Auto-completion bug fix for dot operator

2020-01-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks for the patch.

I don't know much about the LSP protocol, but I see opportunities for 
simplifying the string chopping code. See the inline comment for what /I think/ 
should be the equivalent code.

It has also occurred to me that this may break for expressions containing 
quoted dots (`foo(".", b`), but I don't know if there's anything we can do 
about that...




Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:957-973
+std::vector commit_points = {".", "->"};
+int max_breakpoint_position = -1;
+int commit_points_index = -1;
+for (size_t breakpoint_index = 0; breakpoint_index < commit_points.size(); 
breakpoint_index++) {
+  int breakpoint_position = match.rfind(commit_points[breakpoint_index]);
+  if (max_breakpoint_position < breakpoint_position){
+commit_points_index = breakpoint_index;

If I correctly understand what this is doing, all of this should boil down to 
something like:
```
llvm::StringRef match_ref = match;
for (StringRef commit_point: {".", "->"}) {
  if (match_ref.contains(commit_point))
match_ref = match_ref.rsplit(commit_point).second;
}
EmplaceSafeString(item, "text", match_ref);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73506



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