[Lldb-commits] [lldb] f13ce15 - [DebugInfo] Rename getOffset() to getContribution(). NFC.

[Igor Kudrin via lldb-commits]

Author: Igor Kudrin
Date: 2020-04-03T14:15:53+07:00
New Revision: f13ce15d441095493030404ab31eddb0fc08ca42

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

LOG: [DebugInfo] Rename getOffset() to getContribution(). NFC.

The old name was a bit misleading because the functions actually return
contributions to the corresponding sections.

Differential revision: https://reviews.llvm.org/D77302

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 0198a10645c2..8447d4fcea4c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -263,7 +263,8 @@ void DWARFUnit::SetDwoStrOffsetsBase() {
   lldb::offset_t baseOffset = 0;
 
   if (const llvm::DWARFUnitIndex::Entry *entry = m_header.GetIndexEntry()) {
-if (const auto *contribution = entry->getOffset(llvm::DW_SECT_STR_OFFSETS))
+if (const auto *contribution =
+entry->getContribution(llvm::DW_SECT_STR_OFFSETS))
   baseOffset = contribution->Offset;
 else
   return;
@@ -479,7 +480,7 @@ DWARFDataExtractor DWARFUnit::GetLocationData() const {
   const DWARFDataExtractor &data =
   GetVersion() >= 5 ? Ctx.getOrLoadLocListsData() : Ctx.getOrLoadLocData();
   if (const llvm::DWARFUnitIndex::Entry *entry = m_header.GetIndexEntry()) {
-if (const auto *contribution = entry->getOffset(llvm::DW_SECT_LOC))
+if (const auto *contribution = entry->getContribution(llvm::DW_SECT_LOC))
   return DWARFDataExtractor(data, contribution->Offset,
 contribution->Length);
 return DWARFDataExtractor();
@@ -815,12 +816,13 @@ DWARFUnitHeader::extract(const DWARFDataExtractor &data,
   llvm::inconvertibleErrorCode(),
   "Package unit with a non-zero abbreviation offset");
 }
-auto *unit_contrib = header.m_index_entry->getOffset();
+auto *unit_contrib = header.m_index_entry->getContribution();
 if (!unit_contrib || unit_contrib->Length != header.m_length + 4) {
   return llvm::createStringError(llvm::inconvertibleErrorCode(),
  "Inconsistent DWARF package unit index");
 }
-auto *abbr_entry = header.m_index_entry->getOffset(llvm::DW_SECT_ABBREV);
+auto *abbr_entry =
+header.m_index_entry->getContribution(llvm::DW_SECT_ABBREV);
 if (!abbr_entry) {
   return llvm::createStringError(
   llvm::inconvertibleErrorCode(),

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
index da8fec46dba7..9bf3206f1c61 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -38,7 +38,7 @@ SymbolFileDWARFDwo::SymbolFileDWARFDwo(SymbolFileDWARF 
&base_symbol_file,
 DWARFCompileUnit *SymbolFileDWARFDwo::GetDWOCompileUnitForHash(uint64_t hash) {
   if (const llvm::DWARFUnitIndex &index = m_context.GetAsLLVM().getCUIndex()) {
 if (const llvm::DWARFUnitIndex::Entry *entry = index.getFromHash(hash)) {
-  if (auto *unit_contrib = entry->getOffset())
+  if (auto *unit_contrib = entry->getContribution())
 return llvm::dyn_cast_or_null(
 DebugInfo().GetUnitAtOffset(DIERef::Section::DebugInfo,
 unit_contrib->Offset));

diff  --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h 
b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
index 1d313a3cca24..635c437fff10 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
@@ -487,7 +487,7 @@ class DWARFUnit {
 
   uint32_t getLineTableOffset() const {
 if (auto IndexEntry = Header.getIndexEntry())
-  if (const auto *Contrib = IndexEntry->getOffset(DW_SECT_LINE))
+  if (const auto *Contrib = IndexEntry->getContribution(DW_SECT_LINE))
 return Contrib->Offset;
 return 0;
   }

diff  --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h 
b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
index 684103aac2fc..d6b5e72cf16c 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
@@ -56,10 +56,10 @@ class DWARFUnitIndex {
 friend class DWARFUnitIndex;
 
   public:
-const

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D77287#1957472 , @compnerd wrote:

> Thanks for the hint about the string conversion, however, I think that it's 
> going to complicate things as the string is going to be a mixture of UTF-8 
> and UTF-16 content.


That's true. Ok, plan B: have clang do the conversion for us. If you form the 
expression like `printf("LoadLibraryW(L\"%s\")", path_in_utf8)` then the path 
will get transformed to utf-16 by the compiler. However the path should also 
get sanitized/escaped into a form suitable for passing to the C compiler. 
(godbolt link )

> As to the testing, `functionalities/load_using_paths/TestLoadUsingPaths.py` 
> is not applicable to Windows.  In fact, I don't really see a good way to 
> really test much of this outside the context of the swift REPL which forces 
> the loading of a DLL which is in fact how I discovered this.  If there is an 
> easy way to ensure that the dll that is needed is in the user's `PATH`, then 
> I suppose creating an empty dll and loading that is theoretically possible, 
> but that too can have a lot of flakiness due to dependencies to build and all.

Ok, so your patch does not implement that functionality. It does not sound like 
there's a fundamental limitation there, as you could do the same thing as what 
the posix code 

 does ( == iterate over the path), but that's fine -- you don't need to 
implement everything straight away. In that case it would be good to check that 
the `paths` argument is empty and return an error instead of attempting to load 
a random library.

However, I believe you are implementing sufficient functionality for the 
`SBProcess::LoadImage` (aka "process load" in CLI) command. So, maybe you could 
take a look at `API/functionalities/load_unload/TestLoadUnload.py` and see if 
anything there can be enabled ? Or (if the test contains too many posix 
specifics), you could could create a windows version of that test doing 
something similar.


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

https://reviews.llvm.org/D77287



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

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

[Pavel Labath via Phabricator via lldb-commits]

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

Looks fine to me, though I can't say I'm much of a thread plan expert (but then 
again, I don't know if anyone except you is).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76814



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

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

[Pavel Labath via Phabricator via lldb-commits]

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

Ok, I think we've done what we can here.

In D76471#1958814 , @aprantl wrote:

> > Yeah, it seems acceptable. The "sdk type" argument still seems somewhat 
> > redundant. Theoretically we could pass in just the triple component out the 
> > xcodesdk object, but that would just mean the individual platforms would 
> > have to reconstruct the XcodeSDK object, which is already present in the 
> > caller, so it's not ideal either...
>
> It's an entire string, not just the type, because it also contains an 
> (optional) version number.


err.. I meant "just the _version_ component", but it sounds like you are saying 
you need more than the version.


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

https://reviews.llvm.org/D76471



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

[Lldb-commits] [PATCH] D77347: Have lldb-vscode update the currently selecte thread and frame when it receives a "scopes" request.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

looks good




Comment at: lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py:22
+output = response['body']['result']
+self.assertTrue(contains_string in output,
+("""Verify %s by checking the command output:\n"""

if you used `self.assertIn(contains_string, output, assertmsg)`, then you 
wouldn't need to repeat the arguments in the assert message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77347



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

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

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

No need to split this into multiples revisions. It is just a rewrite of some 
odd nesting.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2943
+// Make sure the decl contexts match all the way up
+if (dwarf_decl_ctx != type_dwarf_decl_ctx)
+  continue;

jankratochvil wrote:
> kwk wrote:
> > This looks like a logic change and I wonder if it should go in a separate 
> > patch, maybe?
> This should be NFC, do I miss something?
> From:
> ```
> if (a == b) {
>   c;
> }
> ```
> The patch changes it to:
> ```
> if (a != b)
>   continue;
> c;
> ```
> 
Ah, now I see that it was previously a block on the right side of the diff :).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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

[Lldb-commits] [PATCH] D77336: Findtypes -gmodules fix

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

Could you explain, why this was done please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77336



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

[Lldb-commits] [PATCH] D77336: Findtypes -gmodules fix

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D77336#1959094 , @kwk wrote:

> Could you explain, why this was done please?


As D77327  removes `num_die_matches` as now it 
is a callback so one cannot use it anymore after D77327 
. Exact replacement would be to count the 
number of times the callback was called. But that would keep it buggy, 
apparently the intention was to copy the condition above:

  if (types.GetSize() >= max_matches)
break;

So that if the iteration stopped because of too many matches we do not add even 
more matches in this `Clang modules` block downward.
It was implemented by: SymbolFileDWARF: Unconditionally scan through clang 
modules. NFCish 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77336



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

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77376

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

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -412,17 +412,6 @@
   return FileSpec::Match(m_module_spec, spec);
 }
 
-bool SearchFilterByModule::AddressPasses(Address &address) {
-  // FIXME: Not yet implemented
-  return true;
-}
-
-bool SearchFilterByModule::CompUnitPasses(FileSpec &fileSpec) { return true; }
-
-bool SearchFilterByModule::CompUnitPasses(CompileUnit &compUnit) {
-  return true;
-}
-
 void SearchFilterByModule::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;
@@ -538,19 +527,6 @@
   return m_module_spec_list.FindFileIndex(0, spec, true) != UINT32_MAX;
 }
 
-bool SearchFilterByModuleList::AddressPasses(Address &address) {
-  // FIXME: Not yet implemented
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(FileSpec &fileSpec) {
-  return true;
-}
-
-bool SearchFilterByModuleList::CompUnitPasses(CompileUnit &compUnit) {
-  return true;
-}
-
 void SearchFilterByModuleList::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;
Index: lldb/include/lldb/Core/SearchFilter.h
===
--- lldb/include/lldb/Core/SearchFilter.h
+++ lldb/include/lldb/Core/SearchFilter.h
@@ -98,6 +98,8 @@
   ///The file spec to check against the filter.
   /// \return
   ///\b true if \a spec passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const FileSpec &spec);
 
   /// Call this method with a Module to see if that module passes the filter.
@@ -107,6 +109,8 @@
   ///
   /// \return
   ///\b true if \a module passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const lldb::ModuleSP &module_sp);
 
   /// Call this method with a Address to see if \a address passes the filter.
@@ -116,6 +120,8 @@
   ///
   /// \return
   ///\b true if \a address passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool AddressPasses(Address &addr);
 
   /// Call this method with a FileSpec to see if \a file spec passes the
@@ -126,6 +132,8 @@
   ///
   /// \return
   ///\b true if \a file spec passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool CompUnitPasses(FileSpec &fileSpec);
 
   /// Call this method with a CompileUnit to see if \a comp unit passes the
@@ -136,6 +144,8 @@
   ///
   /// \return
   ///\b true if \a Comp Unit passes, and \b false otherwise.
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool CompUnitPasses(CompileUnit &compUnit);
 
   /// Call this method with a Function to see if \a function passes the
@@ -319,12 +329,6 @@
 
   bool ModulePasses(const FileSpec &spec) override;
 
-  bool AddressPasses(Address &address) override;
-
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -370,12 +374,6 @@
 
   bool ModulePasses(const FileSpec &spec) override;
 
-  bool AddressPasses(Address &address) override;
-
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77377: [lldb][nfc] early exit/continue

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This commit just tries to invert some `if`'s logic to
`return`/`continue` early.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77377

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp

Index: lldb/source/Breakpoint/BreakpointResolverName.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -332,65 +332,66 @@
 
   // Remove any duplicates between the function list and the symbol list
   SymbolContext sc;
-  if (func_list.GetSize()) {
-for (uint32_t i = 0; i < func_list.GetSize(); i++) {
-  if (func_list.GetContextAtIndex(i, sc)) {
-bool is_reexported = false;
-
-if (sc.block && sc.block->GetInlinedFunctionInfo()) {
-  if (!sc.block->GetStartAddress(break_addr))
-break_addr.Clear();
-} else if (sc.function) {
-  break_addr = sc.function->GetAddressRange().GetBaseAddress();
-  if (m_skip_prologue && break_addr.IsValid()) {
-const uint32_t prologue_byte_size =
-sc.function->GetPrologueByteSize();
-if (prologue_byte_size)
-  break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
-  }
-} else if (sc.symbol) {
-  if (sc.symbol->GetType() == eSymbolTypeReExported) {
-const Symbol *actual_symbol =
-sc.symbol->ResolveReExportedSymbol(breakpoint.GetTarget());
-if (actual_symbol) {
-  is_reexported = true;
-  break_addr = actual_symbol->GetAddress();
-}
-  } else {
-break_addr = sc.symbol->GetAddress();
-  }
-
-  if (m_skip_prologue && break_addr.IsValid()) {
-const uint32_t prologue_byte_size =
-sc.symbol->GetPrologueByteSize();
-if (prologue_byte_size)
-  break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
-else {
-  const Architecture *arch =
-  breakpoint.GetTarget().GetArchitecturePlugin();
-  if (arch)
-arch->AdjustBreakpointAddress(*sc.symbol, break_addr);
-}
-  }
+  if (!func_list.GetSize())
+return Searcher::eCallbackReturnContinue;
+
+  for (uint32_t i = 0; i < func_list.GetSize(); i++) {
+if (!func_list.GetContextAtIndex(i, sc))
+  continue;
+
+bool is_reexported = false;
+
+if (sc.block && sc.block->GetInlinedFunctionInfo()) {
+  if (!sc.block->GetStartAddress(break_addr))
+break_addr.Clear();
+} else if (sc.function) {
+  break_addr = sc.function->GetAddressRange().GetBaseAddress();
+  if (m_skip_prologue && break_addr.IsValid()) {
+const uint32_t prologue_byte_size = sc.function->GetPrologueByteSize();
+if (prologue_byte_size)
+  break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
+  }
+} else if (sc.symbol) {
+  if (sc.symbol->GetType() == eSymbolTypeReExported) {
+const Symbol *actual_symbol =
+sc.symbol->ResolveReExportedSymbol(breakpoint.GetTarget());
+if (actual_symbol) {
+  is_reexported = true;
+  break_addr = actual_symbol->GetAddress();
 }
+  } else {
+break_addr = sc.symbol->GetAddress();
+  }
 
-if (break_addr.IsValid()) {
-  if (filter.AddressPasses(break_addr)) {
-bool new_location;
-BreakpointLocationSP bp_loc_sp(
-AddLocation(break_addr, &new_location));
-bp_loc_sp->SetIsReExported(is_reexported);
-if (bp_loc_sp && new_location && !breakpoint.IsInternal()) {
-  if (log) {
-StreamString s;
-bp_loc_sp->GetDescription(&s, lldb::eDescriptionLevelVerbose);
-LLDB_LOGF(log, "Added location: %s\n", s.GetData());
-  }
-}
-  }
+  if (m_skip_prologue && break_addr.IsValid()) {
+const uint32_t prologue_byte_size = sc.symbol->GetPrologueByteSize();
+if (prologue_byte_size)
+  break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
+else {
+  const Architecture *arch =
+  breakpoint.GetTarget().GetArchitecturePlugin();
+  if (arch)
+arch->AdjustBreakpointAddress(*sc.symbol, break_addr);
 }
   }
 }
+
+if (!break_addr.IsValid())
+  continue;
+
+if (!filter.AddressPasses(break_addr))
+  continue;
+
+bool new_location;
+BreakpointLocationSP bp_loc_sp(AddLocation(break_addr, &new_location));
+bp_loc_sp->SetIsReExported(is_reexported);
+if (bp_loc_sp && new_location && !breakpoint.IsInternal()) {
+  if (log) {
+StreamString s;
+bp_loc_sp->GetDescription(&s, ll

[Lldb-commits] [lldb] 107200a - [lldb][nfc] early exit/continue

[Konrad Kleine via lldb-commits]

Author: Konrad Kleine
Date: 2020-04-03T14:50:08+02:00
New Revision: 107200ae0a032f2e6024f45d665a0f2b7c732a62

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

LOG: [lldb][nfc] early exit/continue

Summary:
This commit just tries to invert some `if`'s logic to
`return`/`continue` early.

Reviewers: jankratochvil, teemperor

Reviewed By: jankratochvil, teemperor

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Breakpoint/BreakpointResolverName.cpp

Removed: 




diff  --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp 
b/lldb/source/Breakpoint/BreakpointResolverName.cpp
index 2eb0d3ab5888..25f5bb3f6eed 100644
--- a/lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -332,65 +332,66 @@ BreakpointResolverName::SearchCallback(SearchFilter 
&filter,
 
   // Remove any duplicates between the function list and the symbol list
   SymbolContext sc;
-  if (func_list.GetSize()) {
-for (uint32_t i = 0; i < func_list.GetSize(); i++) {
-  if (func_list.GetContextAtIndex(i, sc)) {
-bool is_reexported = false;
-
-if (sc.block && sc.block->GetInlinedFunctionInfo()) {
-  if (!sc.block->GetStartAddress(break_addr))
-break_addr.Clear();
-} else if (sc.function) {
-  break_addr = sc.function->GetAddressRange().GetBaseAddress();
-  if (m_skip_prologue && break_addr.IsValid()) {
-const uint32_t prologue_byte_size =
-sc.function->GetPrologueByteSize();
-if (prologue_byte_size)
-  break_addr.SetOffset(break_addr.GetOffset() + 
prologue_byte_size);
-  }
-} else if (sc.symbol) {
-  if (sc.symbol->GetType() == eSymbolTypeReExported) {
-const Symbol *actual_symbol =
-sc.symbol->ResolveReExportedSymbol(breakpoint.GetTarget());
-if (actual_symbol) {
-  is_reexported = true;
-  break_addr = actual_symbol->GetAddress();
-}
-  } else {
-break_addr = sc.symbol->GetAddress();
-  }
-
-  if (m_skip_prologue && break_addr.IsValid()) {
-const uint32_t prologue_byte_size =
-sc.symbol->GetPrologueByteSize();
-if (prologue_byte_size)
-  break_addr.SetOffset(break_addr.GetOffset() + 
prologue_byte_size);
-else {
-  const Architecture *arch =
-  breakpoint.GetTarget().GetArchitecturePlugin();
-  if (arch)
-arch->AdjustBreakpointAddress(*sc.symbol, break_addr);
-}
-  }
+  if (!func_list.GetSize())
+return Searcher::eCallbackReturnContinue;
+
+  for (uint32_t i = 0; i < func_list.GetSize(); i++) {
+if (!func_list.GetContextAtIndex(i, sc))
+  continue;
+
+bool is_reexported = false;
+
+if (sc.block && sc.block->GetInlinedFunctionInfo()) {
+  if (!sc.block->GetStartAddress(break_addr))
+break_addr.Clear();
+} else if (sc.function) {
+  break_addr = sc.function->GetAddressRange().GetBaseAddress();
+  if (m_skip_prologue && break_addr.IsValid()) {
+const uint32_t prologue_byte_size = sc.function->GetPrologueByteSize();
+if (prologue_byte_size)
+  break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
+  }
+} else if (sc.symbol) {
+  if (sc.symbol->GetType() == eSymbolTypeReExported) {
+const Symbol *actual_symbol =
+sc.symbol->ResolveReExportedSymbol(breakpoint.GetTarget());
+if (actual_symbol) {
+  is_reexported = true;
+  break_addr = actual_symbol->GetAddress();
 }
+  } else {
+break_addr = sc.symbol->GetAddress();
+  }
 
-if (break_addr.IsValid()) {
-  if (filter.AddressPasses(break_addr)) {
-bool new_location;
-BreakpointLocationSP bp_loc_sp(
-AddLocation(break_addr, &new_location));
-bp_loc_sp->SetIsReExported(is_reexported);
-if (bp_loc_sp && new_location && !breakpoint.IsInternal()) {
-  if (log) {
-StreamString s;
-bp_loc_sp->GetDescription(&s, lldb::eDescriptionLevelVerbose);
-LLDB_LOGF(log, "Added location: %s\n", s.GetData());
-  }
-}
-  }
+  if (m_skip_prologue && break_addr.IsValid()) {
+const uint32_t prologue_byte_size = sc.symbol->GetPrologueByteSize();
+if (prologue_byte_size)
+  break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
+else {
+  const Architecture *arch =
+  breakpoint.Get

[Lldb-commits] [PATCH] D77377: [lldb][nfc] early exit/continue

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG107200ae0a03: [lldb][nfc] early exit/continue (authored by 
kwk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77377

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp

Index: lldb/source/Breakpoint/BreakpointResolverName.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -332,65 +332,66 @@
 
   // Remove any duplicates between the function list and the symbol list
   SymbolContext sc;
-  if (func_list.GetSize()) {
-for (uint32_t i = 0; i < func_list.GetSize(); i++) {
-  if (func_list.GetContextAtIndex(i, sc)) {
-bool is_reexported = false;
-
-if (sc.block && sc.block->GetInlinedFunctionInfo()) {
-  if (!sc.block->GetStartAddress(break_addr))
-break_addr.Clear();
-} else if (sc.function) {
-  break_addr = sc.function->GetAddressRange().GetBaseAddress();
-  if (m_skip_prologue && break_addr.IsValid()) {
-const uint32_t prologue_byte_size =
-sc.function->GetPrologueByteSize();
-if (prologue_byte_size)
-  break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
-  }
-} else if (sc.symbol) {
-  if (sc.symbol->GetType() == eSymbolTypeReExported) {
-const Symbol *actual_symbol =
-sc.symbol->ResolveReExportedSymbol(breakpoint.GetTarget());
-if (actual_symbol) {
-  is_reexported = true;
-  break_addr = actual_symbol->GetAddress();
-}
-  } else {
-break_addr = sc.symbol->GetAddress();
-  }
-
-  if (m_skip_prologue && break_addr.IsValid()) {
-const uint32_t prologue_byte_size =
-sc.symbol->GetPrologueByteSize();
-if (prologue_byte_size)
-  break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
-else {
-  const Architecture *arch =
-  breakpoint.GetTarget().GetArchitecturePlugin();
-  if (arch)
-arch->AdjustBreakpointAddress(*sc.symbol, break_addr);
-}
-  }
+  if (!func_list.GetSize())
+return Searcher::eCallbackReturnContinue;
+
+  for (uint32_t i = 0; i < func_list.GetSize(); i++) {
+if (!func_list.GetContextAtIndex(i, sc))
+  continue;
+
+bool is_reexported = false;
+
+if (sc.block && sc.block->GetInlinedFunctionInfo()) {
+  if (!sc.block->GetStartAddress(break_addr))
+break_addr.Clear();
+} else if (sc.function) {
+  break_addr = sc.function->GetAddressRange().GetBaseAddress();
+  if (m_skip_prologue && break_addr.IsValid()) {
+const uint32_t prologue_byte_size = sc.function->GetPrologueByteSize();
+if (prologue_byte_size)
+  break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
+  }
+} else if (sc.symbol) {
+  if (sc.symbol->GetType() == eSymbolTypeReExported) {
+const Symbol *actual_symbol =
+sc.symbol->ResolveReExportedSymbol(breakpoint.GetTarget());
+if (actual_symbol) {
+  is_reexported = true;
+  break_addr = actual_symbol->GetAddress();
 }
+  } else {
+break_addr = sc.symbol->GetAddress();
+  }
 
-if (break_addr.IsValid()) {
-  if (filter.AddressPasses(break_addr)) {
-bool new_location;
-BreakpointLocationSP bp_loc_sp(
-AddLocation(break_addr, &new_location));
-bp_loc_sp->SetIsReExported(is_reexported);
-if (bp_loc_sp && new_location && !breakpoint.IsInternal()) {
-  if (log) {
-StreamString s;
-bp_loc_sp->GetDescription(&s, lldb::eDescriptionLevelVerbose);
-LLDB_LOGF(log, "Added location: %s\n", s.GetData());
-  }
-}
-  }
+  if (m_skip_prologue && break_addr.IsValid()) {
+const uint32_t prologue_byte_size = sc.symbol->GetPrologueByteSize();
+if (prologue_byte_size)
+  break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size);
+else {
+  const Architecture *arch =
+  breakpoint.GetTarget().GetArchitecturePlugin();
+  if (arch)
+arch->AdjustBreakpointAddress(*sc.symbol, break_addr);
 }
   }
 }
+
+if (!break_addr.IsValid())
+  continue;
+
+if (!filter.AddressPasses(break_addr))
+  continue;
+
+bool new_location;
+BreakpointLocationSP bp_loc_sp(AddLocation(break_addr, &new_location));
+bp_loc_sp->SetIsReExported(is_reexported);
+if (bp_loc_sp && new_location && !breakpoint.IsInternal()) {
+  if (log) {
+StreamString 

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

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added inline comments.



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

I am not against it but FYI in a paragraph above there is already written: `The 
default implementation is "Everything Passes."`
English: IMO missing "returns".



Comment at: lldb/source/Core/SearchFilter.cpp:416
-bool SearchFilterByModule::AddressPasses(Address &address) {
-  // FIXME: Not yet implemented
-  return true;

This comment will get lost. Maybe you could keep this override just due to the 
FIXME comment. Or keep the FIXME comment some other way there. Or file a 
Bugzilla tracking Bug instead.
It is there since: [[ 
https://github.com/llvm/llvm-project/commit/30fdc8d841c9d24ac5f3d452b6ece84ee0ac991c
 | Initial checkin of lldb code from internal Apple repo. ]]



Comment at: lldb/source/Core/SearchFilter.cpp:542
-bool SearchFilterByModuleList::AddressPasses(Address &address) {
-  // FIXME: Not yet implemented
-  return true;

Likewise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77376



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

[Lldb-commits] [lldb] d144087 - [lldb/Support] Treat empty FileSpec as an invalid file.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-03T09:29:22-07:00
New Revision: d144087c963d8189bb4aeaa7800dcb9f93ac84db

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

LOG: [lldb/Support] Treat empty FileSpec as an invalid file.

LLDB relies on empty FileSpecs being invalid files, for example, they
don't exists. Currently this assumption does not always hold during
reproducer replay, because we pass the result of GetPath to the VFS.
This is an empty string, which the VFS converts to an absolute directory
by prepending the current working directory, before looking it up in the
YAML mapping. This means that an empty FileSpec will exist when the
current working directory does. This breaks at least one test
(TestAddDsymCommand.py) when ran from replay.

This patch special cases empty FileSpecs and returns a sensible result
before calling GetPath and forwarding the call.

Differential revision: https://reviews.llvm.org/D77351

Added: 


Modified: 
lldb/source/Host/common/FileSystem.cpp
lldb/unittests/Host/FileSystemTest.cpp

Removed: 




diff  --git a/lldb/source/Host/common/FileSystem.cpp 
b/lldb/source/Host/common/FileSystem.cpp
index dcfa594597a1..7e33eeb2a812 100644
--- a/lldb/source/Host/common/FileSystem.cpp
+++ b/lldb/source/Host/common/FileSystem.cpp
@@ -87,6 +87,11 @@ Optional &FileSystem::InstanceImpl() {
 
 vfs::directory_iterator FileSystem::DirBegin(const FileSpec &file_spec,
  std::error_code &ec) {
+  if (!file_spec) {
+ec = std::error_code(static_cast(errc::no_such_file_or_directory),
+ std::system_category());
+return {};
+  }
   return DirBegin(file_spec.GetPath(), ec);
 }
 
@@ -97,6 +102,9 @@ vfs::directory_iterator FileSystem::DirBegin(const Twine 
&dir,
 
 llvm::ErrorOr
 FileSystem::GetStatus(const FileSpec &file_spec) const {
+  if (!file_spec)
+return std::error_code(static_cast(errc::no_such_file_or_directory),
+   std::system_category());
   return GetStatus(file_spec.GetPath());
 }
 
@@ -106,6 +114,8 @@ llvm::ErrorOr FileSystem::GetStatus(const 
Twine &path) const {
 
 sys::TimePoint<>
 FileSystem::GetModificationTime(const FileSpec &file_spec) const {
+  if (!file_spec)
+return sys::TimePoint<>();
   return GetModificationTime(file_spec.GetPath());
 }
 
@@ -117,6 +127,8 @@ sys::TimePoint<> FileSystem::GetModificationTime(const 
Twine &path) const {
 }
 
 uint64_t FileSystem::GetByteSize(const FileSpec &file_spec) const {
+  if (!file_spec)
+return 0;
   return GetByteSize(file_spec.GetPath());
 }
 
@@ -133,6 +145,8 @@ uint32_t FileSystem::GetPermissions(const FileSpec 
&file_spec) const {
 
 uint32_t FileSystem::GetPermissions(const FileSpec &file_spec,
 std::error_code &ec) const {
+  if (!file_spec)
+return sys::fs::perms::perms_not_known;
   return GetPermissions(file_spec.GetPath(), ec);
 }
 
@@ -154,7 +168,7 @@ uint32_t FileSystem::GetPermissions(const Twine &path,
 bool FileSystem::Exists(const Twine &path) const { return m_fs->exists(path); }
 
 bool FileSystem::Exists(const FileSpec &file_spec) const {
-  return Exists(file_spec.GetPath());
+  return file_spec && Exists(file_spec.GetPath());
 }
 
 bool FileSystem::Readable(const Twine &path) const {
@@ -162,7 +176,7 @@ bool FileSystem::Readable(const Twine &path) const {
 }
 
 bool FileSystem::Readable(const FileSpec &file_spec) const {
-  return Readable(file_spec.GetPath());
+  return file_spec && Readable(file_spec.GetPath());
 }
 
 bool FileSystem::IsDirectory(const Twine &path) const {
@@ -173,7 +187,7 @@ bool FileSystem::IsDirectory(const Twine &path) const {
 }
 
 bool FileSystem::IsDirectory(const FileSpec &file_spec) const {
-  return IsDirectory(file_spec.GetPath());
+  return file_spec && IsDirectory(file_spec.GetPath());
 }
 
 bool FileSystem::IsLocal(const Twine &path) const {
@@ -183,7 +197,7 @@ bool FileSystem::IsLocal(const Twine &path) const {
 }
 
 bool FileSystem::IsLocal(const FileSpec &file_spec) const {
-  return IsLocal(file_spec.GetPath());
+  return file_spec && IsLocal(file_spec.GetPath());
 }
 
 void FileSystem::EnumerateDirectory(Twine path, bool find_directories,
@@ -261,6 +275,9 @@ void FileSystem::Resolve(SmallVectorImpl &path) {
 }
 
 void FileSystem::Resolve(FileSpec &file_spec) {
+  if (!file_spec)
+return;
+
   // Extract path from the FileSpec.
   SmallString<128> path;
   file_spec.GetPath(path);

diff  --git a/lldb/unittests/Host/FileSystemTest.cpp 
b/lldb/unittests/Host/FileSystemTest.cpp
index d8db2c12b0ee..f31c5c42a1e1 100644
--- a/lldb/unittests/Host/FileSystemTest.cpp
+++ b/lldb/unittests/Host/FileSystemTest.cpp
@@ -303,3 +303,29 @@ TEST(FileSystemTest, OpenErrno) {
   EXPECT_EQ(code.value(), ENOENT);
 

[Lldb-commits] [lldb] 63bfb3a - [lldb/Symbol] Reimplement Symbols::FindSymbolFileInBundle to use the VFS

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-04-03T09:29:22-07:00
New Revision: 63bfb3a81ba2aef8d0444b58aafc868af38a623c

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

LOG: [lldb/Symbol] Reimplement Symbols::FindSymbolFileInBundle to use the VFS

This reimplements Symbols::FindSymbolFileInBundle to use the VFS-aware
recursive directory iterator. This is needed for reproducer replay.

Differential revision: https://reviews.llvm.org/D77337

Added: 


Modified: 
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Removed: 




diff  --git a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp 
b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
index 2af32584b9d1..251605085c58 100644
--- a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -253,48 +253,34 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec 
&module_spec,
 FileSpec Symbols::FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
  const lldb_private::UUID *uuid,
  const ArchSpec *arch) {
-  char path[PATH_MAX];
-  if (dsym_bundle_fspec.GetPath(path, sizeof(path)) == 0)
-return {};
-
-  ::strncat(path, "/Contents/Resources/DWARF", sizeof(path) - strlen(path) - 
1);
-
-  DIR *dirp = opendir(path);
-  if (!dirp)
-return {};
-
-  // Make sure we close the directory before exiting this scope.
-  auto cleanup_dir = llvm::make_scope_exit([&]() { closedir(dirp); });
-
-  FileSpec dsym_fspec;
-  dsym_fspec.GetDirectory().SetCString(path);
-  struct dirent *dp;
-  while ((dp = readdir(dirp)) != NULL) {
-// Only search directories
-if (dp->d_type == DT_DIR || dp->d_type == DT_UNKNOWN) {
-  if (dp->d_namlen == 1 && dp->d_name[0] == '.')
-continue;
-
-  if (dp->d_namlen == 2 && dp->d_name[0] == '.' && dp->d_name[1] == '.')
-continue;
-}
-
-if (dp->d_type == DT_REG || dp->d_type == DT_UNKNOWN) {
-  dsym_fspec.GetFilename().SetCString(dp->d_name);
-  ModuleSpecList module_specs;
-  if (ObjectFile::GetModuleSpecifications(dsym_fspec, 0, 0, module_specs)) 
{
-ModuleSpec spec;
-for (size_t i = 0; i < module_specs.GetSize(); ++i) {
-  bool got_spec = module_specs.GetModuleSpecAtIndex(i, spec);
-  UNUSED_IF_ASSERT_DISABLED(got_spec);
-  assert(got_spec);
-  if ((uuid == NULL ||
-   (spec.GetUUIDPtr() && spec.GetUUID() == *uuid)) &&
-  (arch == NULL ||
-   (spec.GetArchitecturePtr() &&
-spec.GetArchitecture().IsCompatibleMatch(*arch {
-return dsym_fspec;
-  }
+  std::string dsym_bundle_path = dsym_bundle_fspec.GetPath();
+  llvm::SmallString<128> buffer(dsym_bundle_path);
+  llvm::sys::path::append(buffer, "Contents", "Resources", "DWARF");
+
+  std::error_code EC;
+  llvm::IntrusiveRefCntPtr vfs =
+  FileSystem::Instance().GetVirtualFileSystem();
+  llvm::vfs::recursive_directory_iterator Iter(*vfs, buffer.str(), EC);
+  llvm::vfs::recursive_directory_iterator End;
+  for (; Iter != End && !EC; Iter.increment(EC)) {
+llvm::ErrorOr Status = vfs->status(Iter->path());
+if (Status->isDirectory())
+  continue;
+
+FileSpec dsym_fspec(Iter->path());
+ModuleSpecList module_specs;
+if (ObjectFile::GetModuleSpecifications(dsym_fspec, 0, 0, module_specs)) {
+  ModuleSpec spec;
+  for (size_t i = 0; i < module_specs.GetSize(); ++i) {
+bool got_spec = module_specs.GetModuleSpecAtIndex(i, spec);
+assert(got_spec); // The call has side-effects so can't be inlined.
+UNUSED_IF_ASSERT_DISABLED(got_spec);
+if ((uuid == nullptr ||
+ (spec.GetUUIDPtr() && spec.GetUUID() == *uuid)) &&
+(arch == nullptr ||
+ (spec.GetArchitecturePtr() &&
+  spec.GetArchitecture().IsCompatibleMatch(*arch {
+  return dsym_fspec;
 }
   }
 }



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

[Lldb-commits] [PATCH] D77337: [lldb/Symbol] Reimplement Symbols::FindSymbolFileInBundle to use the VFS

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG63bfb3a81ba2: [lldb/Symbol] Reimplement 
Symbols::FindSymbolFileInBundle to use the VFS (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D77337?vs=254620&id=254842#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77337

Files:
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp


Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -253,48 +253,34 @@
 FileSpec Symbols::FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
  const lldb_private::UUID *uuid,
  const ArchSpec *arch) {
-  char path[PATH_MAX];
-  if (dsym_bundle_fspec.GetPath(path, sizeof(path)) == 0)
-return {};
-
-  ::strncat(path, "/Contents/Resources/DWARF", sizeof(path) - strlen(path) - 
1);
-
-  DIR *dirp = opendir(path);
-  if (!dirp)
-return {};
-
-  // Make sure we close the directory before exiting this scope.
-  auto cleanup_dir = llvm::make_scope_exit([&]() { closedir(dirp); });
-
-  FileSpec dsym_fspec;
-  dsym_fspec.GetDirectory().SetCString(path);
-  struct dirent *dp;
-  while ((dp = readdir(dirp)) != NULL) {
-// Only search directories
-if (dp->d_type == DT_DIR || dp->d_type == DT_UNKNOWN) {
-  if (dp->d_namlen == 1 && dp->d_name[0] == '.')
-continue;
-
-  if (dp->d_namlen == 2 && dp->d_name[0] == '.' && dp->d_name[1] == '.')
-continue;
-}
-
-if (dp->d_type == DT_REG || dp->d_type == DT_UNKNOWN) {
-  dsym_fspec.GetFilename().SetCString(dp->d_name);
-  ModuleSpecList module_specs;
-  if (ObjectFile::GetModuleSpecifications(dsym_fspec, 0, 0, module_specs)) 
{
-ModuleSpec spec;
-for (size_t i = 0; i < module_specs.GetSize(); ++i) {
-  bool got_spec = module_specs.GetModuleSpecAtIndex(i, spec);
-  UNUSED_IF_ASSERT_DISABLED(got_spec);
-  assert(got_spec);
-  if ((uuid == NULL ||
-   (spec.GetUUIDPtr() && spec.GetUUID() == *uuid)) &&
-  (arch == NULL ||
-   (spec.GetArchitecturePtr() &&
-spec.GetArchitecture().IsCompatibleMatch(*arch {
-return dsym_fspec;
-  }
+  std::string dsym_bundle_path = dsym_bundle_fspec.GetPath();
+  llvm::SmallString<128> buffer(dsym_bundle_path);
+  llvm::sys::path::append(buffer, "Contents", "Resources", "DWARF");
+
+  std::error_code EC;
+  llvm::IntrusiveRefCntPtr vfs =
+  FileSystem::Instance().GetVirtualFileSystem();
+  llvm::vfs::recursive_directory_iterator Iter(*vfs, buffer.str(), EC);
+  llvm::vfs::recursive_directory_iterator End;
+  for (; Iter != End && !EC; Iter.increment(EC)) {
+llvm::ErrorOr Status = vfs->status(Iter->path());
+if (Status->isDirectory())
+  continue;
+
+FileSpec dsym_fspec(Iter->path());
+ModuleSpecList module_specs;
+if (ObjectFile::GetModuleSpecifications(dsym_fspec, 0, 0, module_specs)) {
+  ModuleSpec spec;
+  for (size_t i = 0; i < module_specs.GetSize(); ++i) {
+bool got_spec = module_specs.GetModuleSpecAtIndex(i, spec);
+assert(got_spec); // The call has side-effects so can't be inlined.
+UNUSED_IF_ASSERT_DISABLED(got_spec);
+if ((uuid == nullptr ||
+ (spec.GetUUIDPtr() && spec.GetUUID() == *uuid)) &&
+(arch == nullptr ||
+ (spec.GetArchitecturePtr() &&
+  spec.GetArchitecture().IsCompatibleMatch(*arch {
+  return dsym_fspec;
 }
   }
 }


Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -253,48 +253,34 @@
 FileSpec Symbols::FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
  const lldb_private::UUID *uuid,
  const ArchSpec *arch) {
-  char path[PATH_MAX];
-  if (dsym_bundle_fspec.GetPath(path, sizeof(path)) == 0)
-return {};
-
-  ::strncat(path, "/Contents/Resources/DWARF", sizeof(path) - strlen(path) - 1);
-
-  DIR *dirp = opendir(path);
-  if (!dirp)
-return {};
-
-  // Make sure we close the directory before exiting this scope.
-  auto cleanup_dir = llvm::make_scope_exit([&]() { closedir(dirp); });
-
-  FileSpec dsym_fspec;
-  dsym_fspec.GetDirectory().SetCString(path);
-  struct dirent *dp;
-  while ((dp = readdir(dirp)) != NULL) {
-// Only search directories
-if (dp->d_type == DT_DIR || dp->d_type == DT_UNKNOWN) {
-  if (dp->d_namlen == 1 && dp->d_name[0] == '.')
-continue;

[Lldb-commits] [PATCH] D77351: [lldb/Support] Treat empty FileSpec as an invalid file.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd144087c963d: [lldb/Support] Treat empty FileSpec as an 
invalid file. (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77351

Files:
  lldb/source/Host/common/FileSystem.cpp
  lldb/unittests/Host/FileSystemTest.cpp

Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -303,3 +303,29 @@
   EXPECT_EQ(code.value(), ENOENT);
 }
 
+TEST(FileSystemTest, EmptyTest) {
+  FileSpec spec;
+  FileSystem fs;
+
+  {
+std::error_code ec;
+fs.DirBegin(spec, ec);
+EXPECT_EQ(ec.category(), std::system_category());
+EXPECT_EQ(ec.value(), ENOENT);
+  }
+
+  {
+llvm::ErrorOr status = fs.GetStatus(spec);
+ASSERT_FALSE(status);
+EXPECT_EQ(status.getError().category(), std::system_category());
+EXPECT_EQ(status.getError().value(), ENOENT);
+  }
+
+  EXPECT_EQ(sys::TimePoint<>(), fs.GetModificationTime(spec));
+  EXPECT_EQ(static_cast(0), fs.GetByteSize(spec));
+  EXPECT_EQ(llvm::sys::fs::perms::perms_not_known, fs.GetPermissions(spec));
+  EXPECT_FALSE(fs.Exists(spec));
+  EXPECT_FALSE(fs.Readable(spec));
+  EXPECT_FALSE(fs.IsDirectory(spec));
+  EXPECT_FALSE(fs.IsLocal(spec));
+}
Index: lldb/source/Host/common/FileSystem.cpp
===
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -87,6 +87,11 @@
 
 vfs::directory_iterator FileSystem::DirBegin(const FileSpec &file_spec,
  std::error_code &ec) {
+  if (!file_spec) {
+ec = std::error_code(static_cast(errc::no_such_file_or_directory),
+ std::system_category());
+return {};
+  }
   return DirBegin(file_spec.GetPath(), ec);
 }
 
@@ -97,6 +102,9 @@
 
 llvm::ErrorOr
 FileSystem::GetStatus(const FileSpec &file_spec) const {
+  if (!file_spec)
+return std::error_code(static_cast(errc::no_such_file_or_directory),
+   std::system_category());
   return GetStatus(file_spec.GetPath());
 }
 
@@ -106,6 +114,8 @@
 
 sys::TimePoint<>
 FileSystem::GetModificationTime(const FileSpec &file_spec) const {
+  if (!file_spec)
+return sys::TimePoint<>();
   return GetModificationTime(file_spec.GetPath());
 }
 
@@ -117,6 +127,8 @@
 }
 
 uint64_t FileSystem::GetByteSize(const FileSpec &file_spec) const {
+  if (!file_spec)
+return 0;
   return GetByteSize(file_spec.GetPath());
 }
 
@@ -133,6 +145,8 @@
 
 uint32_t FileSystem::GetPermissions(const FileSpec &file_spec,
 std::error_code &ec) const {
+  if (!file_spec)
+return sys::fs::perms::perms_not_known;
   return GetPermissions(file_spec.GetPath(), ec);
 }
 
@@ -154,7 +168,7 @@
 bool FileSystem::Exists(const Twine &path) const { return m_fs->exists(path); }
 
 bool FileSystem::Exists(const FileSpec &file_spec) const {
-  return Exists(file_spec.GetPath());
+  return file_spec && Exists(file_spec.GetPath());
 }
 
 bool FileSystem::Readable(const Twine &path) const {
@@ -162,7 +176,7 @@
 }
 
 bool FileSystem::Readable(const FileSpec &file_spec) const {
-  return Readable(file_spec.GetPath());
+  return file_spec && Readable(file_spec.GetPath());
 }
 
 bool FileSystem::IsDirectory(const Twine &path) const {
@@ -173,7 +187,7 @@
 }
 
 bool FileSystem::IsDirectory(const FileSpec &file_spec) const {
-  return IsDirectory(file_spec.GetPath());
+  return file_spec && IsDirectory(file_spec.GetPath());
 }
 
 bool FileSystem::IsLocal(const Twine &path) const {
@@ -183,7 +197,7 @@
 }
 
 bool FileSystem::IsLocal(const FileSpec &file_spec) const {
-  return IsLocal(file_spec.GetPath());
+  return file_spec && IsLocal(file_spec.GetPath());
 }
 
 void FileSystem::EnumerateDirectory(Twine path, bool find_directories,
@@ -261,6 +275,9 @@
 }
 
 void FileSystem::Resolve(FileSpec &file_spec) {
+  if (!file_spec)
+return;
+
   // Extract path from the FileSpec.
   SmallString<128> path;
   file_spec.GetPath(path);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Why did you abandon this?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61548



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

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

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil marked 2 inline comments as done.
jankratochvil added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp:524
   DIEInfoArray die_info_array;
-  if (FindByName(name, die_info_array))
-DWARFMappedHash::ExtractDIEArray(die_info_array, die_offsets);
+  FindByName(name, die_info_array);
+  DWARFMappedHash::ExtractDIEArray(die_info_array, die_offsets);

shafik wrote:
> jankratochvil wrote:
> > kwk wrote:
> > > Why is the `if` no longer needed?
> > It will now run `DWARFMappedHash::ExtractDIEArray` on empty array which is 
> > cheap, it does nothing.
> If we stick with this change, please add a comment explaining this. It is not 
> obvious and someone later on may come and change it back thinking this is a 
> bug.
I have rather dropped this part of this refactorization patch. It will look 
more logical together with changes of the next part D77327. Thanks for the 
comments.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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

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

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added a comment.

Okay, thanks to some help from @JDevlieghere I was able to get a test going for 
this.  I think that the basic test is sufficient for this.  I think that the 
path sanitizing and conversion should be a subsequent change.


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

https://reviews.llvm.org/D77287



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

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

[Raul Tambre via Phabricator via lldb-commits]

tambre added a comment.

In D61548#1960310 , @JDevlieghere 
wrote:

> Why did you abandon this?


It's been lingering for a long time, so I wanted to get it off my dashboard.
The patch also requires updating as the file has moved.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61548



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

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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/test/Shell/Process/Windows/process_load.cpp:3
+
+// REQUIRES: system-windows
+// RUN: %build --compiler=clang-cl -o %t.exe -- %s

We should probably have a `lit.local.cfg` in the Windows directory with 

```
if 'system-windows' not in config.available_features:
  config.unsupported = True

```


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

https://reviews.llvm.org/D77287



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

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

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd updated this revision to Diff 254883.

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

https://reviews.llvm.org/D77287

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

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

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

[Saleem Abdulrasool via Phabricator via lldb-commits]

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



Comment at: lldb/test/Shell/Process/Windows/process_load.cpp:3
+
+// REQUIRES: system-windows
+// RUN: %build --compiler=clang-cl -o %t.exe -- %s

JDevlieghere wrote:
> We should probably have a `lit.local.cfg` in the Windows directory with 
> 
> ```
> if 'system-windows' not in config.available_features:
>   config.unsupported = True
> 
> ```
I think that's a good idea, but, should be a separate change - it isn't related 
to the load/unload functionality.


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

https://reviews.llvm.org/D77287



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

[Lldb-commits] [lldb] 8023752 - [nfc] [lldb] Unindent code - obvious part

[Jan Kratochvil via lldb-commits]

Author: Jan Kratochvil
Date: 2020-04-03T21:58:11+02:00
New Revision: 80237523193d1311e8744b84faa077a1295d80da

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

LOG: [nfc] [lldb] Unindent code - obvious part

It is an obvious part of D77326.

It removes some needless deep indentation and some redundant statements.
It prepares the code for a more clean next patch - DWARF index callbacks
in D77327.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
index 50b3a2cba61f..027eb08e5621 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -53,8 +53,9 @@ std::unique_ptr AppleDWARFIndex::Create(
 }
 
 void AppleDWARFIndex::GetGlobalVariables(ConstString basename, DIEArray 
&offsets) {
-  if (m_apple_names_up)
-m_apple_names_up->FindByName(basename.GetStringRef(), offsets);
+  if (!m_apple_names_up)
+return;
+  m_apple_names_up->FindByName(basename.GetStringRef(), offsets);
 }
 
 void AppleDWARFIndex::GetGlobalVariables(const RegularExpression ®ex,
@@ -80,22 +81,24 @@ void AppleDWARFIndex::GetGlobalVariables(const DWARFUnit 
&cu,
 
 void AppleDWARFIndex::GetObjCMethods(ConstString class_name,
  DIEArray &offsets) {
-  if (m_apple_objc_up)
-m_apple_objc_up->FindByName(class_name.GetStringRef(), offsets);
+  if (!m_apple_objc_up)
+return;
+  m_apple_objc_up->FindByName(class_name.GetStringRef(), offsets);
 }
 
 void AppleDWARFIndex::GetCompleteObjCClass(ConstString class_name,
bool must_be_implementation,
DIEArray &offsets) {
-  if (m_apple_types_up) {
-m_apple_types_up->FindCompleteObjCClassByName(
-class_name.GetStringRef(), offsets, must_be_implementation);
-  }
+  if (!m_apple_types_up)
+return;
+  m_apple_types_up->FindCompleteObjCClassByName(
+  class_name.GetStringRef(), offsets, must_be_implementation);
 }
 
 void AppleDWARFIndex::GetTypes(ConstString name, DIEArray &offsets) {
-  if (m_apple_types_up)
-m_apple_types_up->FindByName(name.GetStringRef(), offsets);
+  if (!m_apple_types_up)
+return;
+  m_apple_types_up->FindByName(name.GetStringRef(), offsets);
 }
 
 void AppleDWARFIndex::GetTypes(const DWARFDeclContext &context,
@@ -149,8 +152,9 @@ void AppleDWARFIndex::GetTypes(const DWARFDeclContext 
&context,
 }
 
 void AppleDWARFIndex::GetNamespaces(ConstString name, DIEArray &offsets) {
-  if (m_apple_namespaces_up)
-m_apple_namespaces_up->FindByName(name.GetStringRef(), offsets);
+  if (!m_apple_namespaces_up)
+return;
+  m_apple_namespaces_up->FindByName(name.GetStringRef(), offsets);
 }
 
 void AppleDWARFIndex::GetFunctions(ConstString name, SymbolFileDWARF &dwarf,

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 5a2fa70138dd..d3a4b92b7e3e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -165,9 +165,8 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(ConstString 
class_name,
   // If we find the complete version we're done.
   offsets.push_back(*ref);
   return;
-} else {
-  incomplete_types.push_back(*ref);
 }
+incomplete_types.push_back(*ref);
   }
 
   offsets.insert(offsets.end(), incomplete_types.begin(),

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
index aa01668ba05c..5f01b8176b98 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
@@ -21,19 +21,19 @@ void DWARFMappedHash::ExtractDIEArray(const DIEInfoArray 
&die_info_array,
   DIEArray &die_offsets) {
   if (tag == 0) {
 ExtractDIEArray(die_info_array, die_offsets);
-  } else {
-const size_t count = die_info_array.size();
-for (size_t i = 0; i < count; ++i) {
-  const dw_tag_t die_tag = die_info_array[i].tag;
-  bool tag_matches = die_tag == 0 || tag == die_tag;
-  if (!tag_matches) {
-if (die_tag == DW_TAG_class_type || die_tag == DW_TAG_structure_type)
-  tag_matches =
-  tag =

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

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 254900.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327

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

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

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

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 254898.
jankratochvil marked an inline comment as done.
jankratochvil added a comment.

I have checked in the really obvious parts as rG80237523193d 
.
The remainder is left here - although I do not see there more problematic parts 
now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2045,64 +2045,61 @@
   DIEArray die_offsets;
   m_index->GetGlobalVariables(ConstString(basename), die_offsets);
   const size_t num_die_matches = die_offsets.size();
-  if (num_die_matches) {
-SymbolContext sc;
-sc.module_sp = m_objfile_sp->GetModule();
-assert(sc.module_sp);
 
-// Loop invariant: Variables up to this index have been checked for context
-// matches.
-uint32_t pruned_idx = original_size;
+  SymbolContext sc;
+  sc.module_sp = m_objfile_sp->GetModule();
+  assert(sc.module_sp);
 
-bool done = false;
-for (size_t i = 0; i < num_die_matches && !done; ++i) {
-  const DIERef &die_ref = die_offsets[i];
-  DWARFDIE die = GetDIE(die_ref);
+  // Loop invariant: Variables up to this index have been checked for context
+  // matches.
+  uint32_t pruned_idx = original_size;
 
-  if (die) {
-switch (die.Tag()) {
-default:
-case DW_TAG_subprogram:
-case DW_TAG_inlined_subroutine:
-case DW_TAG_try_block:
-case DW_TAG_catch_block:
-  break;
+  for (size_t i = 0; i < num_die_matches; ++i) {
+const DIERef &die_ref = die_offsets[i];
+DWARFDIE die = GetDIE(die_ref);
+if (!die) {
+  m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+  continue;
+}
 
-case DW_TAG_variable: {
-  auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
-  if (!dwarf_cu)
-continue;
-  sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
-
-  if (parent_decl_ctx) {
-if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
-  CompilerDeclContext actual_parent_decl_ctx =
-  dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
-  if (!actual_parent_decl_ctx ||
-  actual_parent_decl_ctx != parent_decl_ctx)
-continue;
-}
-  }
+switch (die.Tag()) {
+default:
+case DW_TAG_subprogram:
+case DW_TAG_inlined_subroutine:
+case DW_TAG_try_block:
+case DW_TAG_catch_block:
+  continue;
 
-  ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false,
- &variables);
-  while (pruned_idx < variables.GetSize()) {
-VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
-if (name_is_mangled ||
-var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
-  ++pruned_idx;
-else
-  variables.RemoveVariableAtIndex(pruned_idx);
-  }
+case DW_TAG_variable:
+  break;
+}
+auto *dwarf_cu = llvm::dyn_cast(die.GetCU());
+if (!dwarf_cu)
+  continue;
+sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
-  if (variables.GetSize() - original_size >= max_matches)
-done = true;
-} break;
-}
-  } else {
-m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+if (parent_decl_ctx) {
+  if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
+CompilerDeclContext actual_parent_decl_ctx =
+dwarf_ast->GetDeclContextContainingUIDFromDWARF(die);
+if (!actual_parent_decl_ctx ||
+actual_parent_decl_ctx != parent_decl_ctx)
+  continue;
   }
 }
+
+ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+while (pruned_idx < variables.GetSize()) {
+  VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
+  if (name_is_mangled ||
+  var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
+++pruned_idx;
+  else
+variables.RemoveVariableAtIndex(pruned_idx);
+}
+
+if (variables.GetSize() - original_size >= max_matches)
+  break;
   }
 
   // Return the number of variable that were appended to the list
@@ -2687,54 +2684,53 @@
 
   const size_t num_matches = die_offsets.size();
 
-  if (num_matches) {
-for (size_t i = 0; i < num_matches; ++i) {
-  const DIERef &die_ref = die_o

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D76814#1958988 , @labath wrote:

> Looks fine to me, though I can't say I'm much of a thread plan expert (but 
> then again, I don't know if anyone except you is).


That's not a great situation, but it is where we currently are...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76814



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

[Lldb-commits] [PATCH] D77336: Findtypes -gmodules fix

[Adrian Prantl via Phabricator via lldb-commits]

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

Thanks for catching this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77336



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

[Lldb-commits] [lldb] 2c1c57a - Make ThreadPlanTracers use TID & Process rather than Thread *.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2020-04-03T14:56:28-07:00
New Revision: 2c1c57a1df8187870df114af666bacbd413699f7

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

LOG: Make ThreadPlanTracers use TID & Process rather than Thread *.

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

Added: 


Modified: 
lldb/include/lldb/Target/ThreadPlanTracer.h
lldb/source/Target/ThreadPlanTracer.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/ThreadPlanTracer.h 
b/lldb/include/lldb/Target/ThreadPlanTracer.h
index 9a0bebf560a5..677a2c0dd93c 100644
--- a/lldb/include/lldb/Target/ThreadPlanTracer.h
+++ b/lldb/include/lldb/Target/ThreadPlanTracer.h
@@ -57,9 +57,12 @@ class ThreadPlanTracer {
   }
 
   bool SingleStepEnabled() { return m_single_step; }
+  
+  Thread &GetThread();
 
 protected:
-  Thread &m_thread;
+  Process &m_process;
+  lldb::tid_t m_tid;
 
   Stream *GetLogStream();
 
@@ -71,6 +74,7 @@ class ThreadPlanTracer {
   bool m_single_step;
   bool m_enabled;
   lldb::StreamSP m_stream_sp;
+  Thread *m_thread;
 };
 
 class ThreadPlanAssemblyTracer : public ThreadPlanTracer {

diff  --git a/lldb/source/Target/ThreadPlanTracer.cpp 
b/lldb/source/Target/ThreadPlanTracer.cpp
index 0ca520c5f9ee..c00415f3c1ee 100644
--- a/lldb/source/Target/ThreadPlanTracer.cpp
+++ b/lldb/source/Target/ThreadPlanTracer.cpp
@@ -34,23 +34,32 @@ using namespace lldb_private;
 #pragma mark ThreadPlanTracer
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread, lldb::StreamSP &stream_sp)
-: m_thread(thread), m_single_step(true), m_enabled(false),
-  m_stream_sp(stream_sp) {}
+: m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
+  m_single_step(true), m_enabled(false), m_stream_sp(stream_sp) {}
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread)
-: m_thread(thread), m_single_step(true), m_enabled(false), m_stream_sp() {}
+: m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
+  m_single_step(true), m_enabled(false), m_stream_sp() {}
 
 Stream *ThreadPlanTracer::GetLogStream() {
   if (m_stream_sp)
 return m_stream_sp.get();
   else {
-TargetSP target_sp(m_thread.CalculateTarget());
+TargetSP target_sp(GetThread().CalculateTarget());
 if (target_sp)
   return &(target_sp->GetDebugger().GetOutputStream());
   }
   return nullptr;
 }
 
+Thread &ThreadPlanTracer::GetThread() {
+  if (m_thread)
+return *m_thread;
+
+  ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
+  m_thread = thread_sp.get();
+  return *m_thread;
+}
 void ThreadPlanTracer::Log() {
   SymbolContext sc;
   bool show_frame_index = false;
@@ -58,8 +67,8 @@ void ThreadPlanTracer::Log() {
 
   Stream *stream = GetLogStream();
   if (stream) {
-m_thread.GetStackFrameAtIndex(0)->Dump(stream, show_frame_index,
-   show_fullpaths);
+GetThread().GetStackFrameAtIndex(0)->Dump(stream, show_frame_index,
+  show_fullpaths);
 stream->Printf("\n");
 stream->Flush();
   }
@@ -67,7 +76,7 @@ void ThreadPlanTracer::Log() {
 
 bool ThreadPlanTracer::TracerExplainsStop() {
   if (m_enabled && m_single_step) {
-lldb::StopInfoSP stop_info = m_thread.GetStopInfo();
+lldb::StopInfoSP stop_info = GetThread().GetStopInfo();
 return (stop_info->GetStopReason() == eStopReasonTrace);
   } else
 return false;
@@ -87,13 +96,13 @@ ThreadPlanAssemblyTracer::ThreadPlanAssemblyTracer(Thread 
&thread)
 Disassembler *ThreadPlanAssemblyTracer::GetDisassembler() {
   if (!m_disassembler_sp)
 m_disassembler_sp = Disassembler::FindPlugin(
-m_thread.GetProcess()->GetTarget().GetArchitecture(), nullptr, 
nullptr);
+m_process.GetTarget().GetArchitecture(), nullptr, nullptr);
   return m_disassembler_sp.get();
 }
 
 TypeFromUser ThreadPlanAssemblyTracer::GetIntPointerType() {
   if (!m_intptr_type.IsValid()) {
-if (auto target_sp = m_thread.CalculateTarget()) {
+if (auto target_sp = m_process.CalculateTarget()) {
   auto type_system_or_err =
   target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC);
   if (auto err = type_system_or_err.takeError()) {
@@ -125,29 +134,27 @@ void ThreadPlanAssemblyTracer::Log() {
   if (!stream)
 return;
 
-  RegisterContext *reg_ctx = m_thread.GetRegisterContext().get();
+  RegisterContext *reg_ctx = GetThread().GetRegisterContext().get();
 
   lldb::addr_t pc = reg_ctx->GetPC();
-  ProcessSP process_sp(m_thread.GetProcess());
   Address pc_addr;
   bool addr_valid = false;
   uint8_t buffer[16] = {0}; // Must be big enough for any single instruction
-  addr_valid = process_sp->GetTarget().GetSectionLoadList().ResolveLoadAddress(
+  addr_valid = m_process.GetTarget().GetSectionLoadList().

[Lldb-commits] [lldb] 61e8e68 - Move thread plan stacks into the Process, indexed by TID.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2020-04-03T14:56:28-07:00
New Revision: 61e8e6882de7c89933d3cc4c4d44719679e4b4f0

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

LOG: Move thread plan stacks into the Process, indexed by TID.

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

Added: 
lldb/include/lldb/Target/ThreadPlanStack.h
lldb/source/Target/ThreadPlanStack.cpp

Modified: 
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/Target/ThreadPlan.h
lldb/source/Target/CMakeLists.txt
lldb/source/Target/Process.cpp
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadList.cpp
lldb/source/Target/ThreadPlan.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 87f61c66bb41..02987f9b50b7 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -37,6 +37,7 @@
 #include "lldb/Target/Memory.h"
 #include "lldb/Target/QueueList.h"
 #include "lldb/Target/ThreadList.h"
+#include "lldb/Target/ThreadPlanStack.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
 #include "lldb/Utility/Event.h"
@@ -2197,6 +2198,19 @@ class Process : public 
std::enable_shared_from_this,
   }
 
   void SetDynamicCheckers(DynamicCheckerFunctions *dynamic_checkers);
+  
+  /// Find the thread plan stack associated with thread with \a tid.
+  ///
+  /// \param[in] tid
+  /// The tid whose Plan Stack we are seeking..
+  ///
+  /// \return
+  /// Returns a ThreadPlan if the TID is found or nullptr if not.
+  ThreadPlanStack *FindThreadPlans(lldb::tid_t tid);
+  
+  void AddThreadPlansForThread(Thread &thread);
+  
+  void RemoveThreadPlansForTID(lldb::tid_t tid);
 
   /// Call this to set the lldb in the mode where it breaks on new thread
   /// creations, and then auto-restarts.  This is useful when you are trying
@@ -2533,7 +2547,7 @@ class Process : public 
std::enable_shared_from_this,
 virtual EventActionResult HandleBeingInterrupted() = 0;
 virtual const char *GetExitString() = 0;
 void RequestResume() { m_process->m_resume_requested = true; }
-
+
   protected:
 Process *m_process;
   };
@@ -2667,6 +2681,10 @@ class Process : public 
std::enable_shared_from_this,
 ///see them. This is usually the same as
   ///< m_thread_list_real, but might be 
diff erent if there is an OS plug-in
   ///creating memory threads
+  ThreadPlanStackMap m_thread_plans; ///< This is the list of thread plans for
+ /// threads in m_thread_list, as well as
+ /// threads we knew existed, but haven't
+ /// determined that they have died yet.
   ThreadList m_extended_thread_list; ///< Owner for extended threads that may 
be
  ///generated, cleared on natural stops
   uint32_t m_extended_thread_stop_id; ///< The natural stop id when

diff  --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index b0bc1b29eb78..dda303ff3c46 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -28,6 +28,8 @@
 
 namespace lldb_private {
 
+class ThreadPlanStack;
+
 class ThreadProperties : public Properties {
 public:
   ThreadProperties(bool is_global);
@@ -119,7 +121,7 @@ class Thread : public std::enable_shared_from_this,
// bit of data.
 lldb::StopInfoSP stop_info_sp; // You have to restore the stop info or you
// might continue with the wrong signals.
-std::vector m_completed_plan_stack;
+size_t m_completed_plan_checkpoint;
 lldb::RegisterCheckpointSP
 register_backup_sp; // You need to restore the registers, of course...
 uint32_t current_inlined_depth;
@@ -912,7 +914,7 @@ class Thread : public std::enable_shared_from_this,
   ///
   /// \return
   /// A pointer to the next executed plan.
-  ThreadPlan *GetCurrentPlan();
+  ThreadPlan *GetCurrentPlan() const;
 
   /// Unwinds the thread stack for the innermost expression plan currently
   /// on the thread plan stack.
@@ -927,14 +929,14 @@ class Thread : public 
std::enable_shared_from_this,
   ///
   /// \return
   /// A pointer to the last completed plan.
-  lldb::ThreadPlanSP GetCompletedPlan();
+  lldb::ThreadPlanSP GetCompletedPlan() const;
 
   /// Gets the outer-most return value from the completed plans
   ///
   /// \return
   /// A ValueObjectSP, either empty if there is no return value,
   /// or containing the return value.
-  lldb::ValueObjectSP GetReturnValueObject();
+  lldb::ValueObjectSP GetReturnValueObject() const;
 
   /// Gets the outer

[Lldb-commits] [lldb] 1893065 - Allow the ThreadPlanStackMap to hold the thread plans for threads

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2020-04-03T14:56:28-07:00
New Revision: 1893065d7bf5c41fbd0dbbee0a39933d3a99806b

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

LOG: Allow the ThreadPlanStackMap to hold the thread plans for threads
that were not reported by the OS plugin.  To facilitate this, move
adding/updating the ThreadPlans for a Thread to the ThreadPlanStackMap.
Also move dumping thread plans there as well.

Added some tests for "thread plan list" and "thread plan discard" since
I didn't seem to have written any originally.

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

Added: 

lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/Makefile

lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py

lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/main.cpp

lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/operating_system.py
lldb/test/API/functionalities/thread_plan/Makefile
lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
lldb/test/API/functionalities/thread_plan/main.c

Modified: 
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/Target.h
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/Target/ThreadPlan.h
lldb/include/lldb/Target/ThreadPlanStack.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/Options.td
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadList.cpp
lldb/source/Target/ThreadPlan.cpp
lldb/source/Target/ThreadPlanStack.cpp
lldb/source/Target/ThreadPlanStepOut.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 02987f9b50b7..82c302c08e11 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2198,19 +2198,75 @@ class Process : public 
std::enable_shared_from_this,
   }
 
   void SetDynamicCheckers(DynamicCheckerFunctions *dynamic_checkers);
-  
+
+/// Prune ThreadPlanStacks for unreported threads.
+///
+/// \param[in] tid
+/// The tid whose Plan Stack we are seeking to prune.
+///
+/// \return
+/// \b true if the TID is found or \b false if not.
+bool PruneThreadPlansForTID(lldb::tid_t tid);
+
+/// Prune ThreadPlanStacks for all unreported threads.
+void PruneThreadPlans();
+
   /// Find the thread plan stack associated with thread with \a tid.
   ///
   /// \param[in] tid
-  /// The tid whose Plan Stack we are seeking..
+  /// The tid whose Plan Stack we are seeking.
   ///
   /// \return
   /// Returns a ThreadPlan if the TID is found or nullptr if not.
   ThreadPlanStack *FindThreadPlans(lldb::tid_t tid);
-  
-  void AddThreadPlansForThread(Thread &thread);
-  
-  void RemoveThreadPlansForTID(lldb::tid_t tid);
+
+  /// Dump the thread plans associated with thread with \a tid.
+  ///
+  /// \param[in/out] strm
+  /// The stream to which to dump the output
+  ///
+  /// \param[in] tid
+  /// The tid whose Plan Stack we are dumping
+  ///
+  /// \param[in] desc_level
+  /// How much detail to dump
+  ///
+  /// \param[in] internal
+  /// If \b true dump all plans, if false only user initiated plans
+  ///
+  /// \param[in] condense_trivial
+  /// If true, only dump a header if the plan stack is just the base plan.
+  ///
+  /// \param[in] skip_unreported_plans
+  /// If true, only dump a plan if it is currently backed by an
+  /// lldb_private::Thread *.
+  ///
+  /// \return
+  /// Returns \b true if TID was found, \b false otherwise
+  bool DumpThreadPlansForTID(Stream &strm, lldb::tid_t tid,
+ lldb::DescriptionLevel desc_level, bool internal,
+ bool condense_trivial, bool 
skip_unreported_plans);
+
+  /// Dump all the thread plans for this process.
+  ///
+  /// \param[in/out] strm
+  /// The stream to which to dump the output
+  ///
+  /// \param[in] desc_level
+  /// How much detail to dump
+  ///
+  /// \param[in] internal
+  /// If \b true dump all plans, if false only user initiated plans
+  ///
+  /// \param[in] condense_trivial
+  /// If true, only dump a header if the plan stack is just the base plan.
+  ///
+  /// \param[in] skip_unreported_plans
+  /// If true, skip printing all thread plan stacks that don't currently
+  /// have a backing lldb_private::Thread *.
+  void DumpThreadPlans(Stream &strm, lldb::DescriptionLevel desc_level,
+   bool internal, bool condense_trivial,
+   bool skip_unreported_plans);
 
   /// 

[Lldb-commits] [lldb] e4598dc - Make ThreadPlans use TID and Process, rather than Thread *.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2020-04-03T14:56:28-07:00
New Revision: e4598dc04a1f582e18b6721ae8ad15cad3ddd48b

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

LOG: Make ThreadPlans use TID and Process, rather than Thread *.

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

Added: 


Modified: 
lldb/include/lldb/Target/ThreadPlan.h
lldb/include/lldb/Target/ThreadPlanPython.h

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
lldb/source/Target/ThreadPlan.cpp
lldb/source/Target/ThreadPlanBase.cpp
lldb/source/Target/ThreadPlanCallFunction.cpp
lldb/source/Target/ThreadPlanCallFunctionUsingABI.cpp
lldb/source/Target/ThreadPlanCallUserExpression.cpp
lldb/source/Target/ThreadPlanPython.cpp
lldb/source/Target/ThreadPlanRunToAddress.cpp
lldb/source/Target/ThreadPlanStepInRange.cpp
lldb/source/Target/ThreadPlanStepInstruction.cpp
lldb/source/Target/ThreadPlanStepOut.cpp
lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
lldb/source/Target/ThreadPlanStepOverRange.cpp
lldb/source/Target/ThreadPlanStepRange.cpp
lldb/source/Target/ThreadPlanStepThrough.cpp
lldb/source/Target/ThreadPlanStepUntil.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/ThreadPlan.h 
b/lldb/include/lldb/Target/ThreadPlan.h
index 3e9f7c884137..40c15e3387d7 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -369,13 +369,11 @@ class ThreadPlan : public 
std::enable_shared_from_this,
   ///
   /// \return
   ///   A  pointer to the thread plan's owning thread.
-  Thread &GetThread() { return m_thread; }
+  Thread &GetThread();
 
-  const Thread &GetThread() const { return m_thread; }
+  Target &GetTarget() { return m_process.GetTarget(); }
 
-  Target &GetTarget() { return m_thread.GetProcess()->GetTarget(); }
-
-  const Target &GetTarget() const { return m_thread.GetProcess()->GetTarget(); 
}
+  const Target &GetTarget() const { return m_process.GetTarget(); }
 
   /// Print a description of this thread to the stream \a s.
   /// \a thread.
@@ -464,7 +462,7 @@ class ThreadPlan : public 
std::enable_shared_from_this,
   // Also sets the plans to private and not master plans.  A plan pushed by 
   // another thread plan is never either of the above.
   void PushPlan(lldb::ThreadPlanSP &thread_plan_sp) {
-m_thread.PushPlan(thread_plan_sp);
+GetThread().PushPlan(thread_plan_sp);
 thread_plan_sp->SetPrivate(false);
 thread_plan_sp->SetIsMasterPlan(false);
   }
@@ -497,7 +495,9 @@ class ThreadPlan : public 
std::enable_shared_from_this,
   // original stop reason so that stopping and calling a few functions won't
   // lose the history of the run. This call can be implemented to get you back
   // to the real stop info.
-  virtual lldb::StopInfoSP GetRealStopInfo() { return m_thread.GetStopInfo(); }
+  virtual lldb::StopInfoSP GetRealStopInfo() { 
+return GetThread().GetStopInfo();
+  }
 
   // If the completion of the thread plan stepped out of a function, the return
   // value of the function might have been captured by the thread plan
@@ -560,17 +560,17 @@ class ThreadPlan : public 
std::enable_shared_from_this,
   // This is mostly a formal requirement, it allows us to make the Thread's
   // GetPreviousPlan protected, but only friend ThreadPlan to thread.
 
-  ThreadPlan *GetPreviousPlan() { return m_thread.GetPreviousPlan(this); }
+  ThreadPlan *GetPreviousPlan() { return GetThread().GetPreviousPlan(this); }
 
   // This forwards the private Thread::GetPrivateStopInfo which is generally
   // what ThreadPlan's need to know.
 
   lldb::StopInfoSP GetPrivateStopInfo() {
-return m_thread.GetPrivateStopInfo();
+return GetThread().GetPrivateStopInfo();
   }
 
   void SetStopInfo(lldb::StopInfoSP stop_reason_sp) {
-m_thread.SetStopInfo(stop_reason_sp);
+GetThread().SetStopInfo(stop_reason_sp);
   }
 
   void CachePlanExplainsStop(bool does_explain) {
@@ -586,7 +586,8 @@ class ThreadPlan : public 
std::enable_shared_from_this,
   bool IsUsuallyUnexplainedStopReason(lldb::StopReason);
 
   Status m_status;
-  Thread &m_thread;
+  Process &m_process;
+  lldb::tid_t m_tid;
   Vote m_stop_vote;
   Vote m_run_vote;
   bool m_takes_iteration_count;
@@ -597,6 +598,7 @@ class ThreadPlan : public 
std::enable_shared_from_this,
   // For ThreadPlan only
   static lldb::user_id_t GetNextID();
 
+  Thread *m_thread;
   ThreadPlanKind m_kind;
   std::string m_name;
   std::recursive_mutex m_plan_complete_mutex;

diff  --git a/lldb/include/lldb/Target/ThreadPlanPython.h 
b/lldb/include/lldb/Target/ThreadPlanPython.h
index 99108733b9d5..5b8713c328e2 100644
--- a/lldb/include/lldb/Target/ThreadPlanPython.h
+++ b/lldb/include/lldb

[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

[Jim Ingham via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe4598dc04a1f: Make ThreadPlans use TID and Process, rather 
than Thread *. (authored by jingham).

Changed prior to commit:
  https://reviews.llvm.org/D75711?vs=248868&id=254926#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75711

Files:
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanPython.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanBase.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/source/Target/ThreadPlanCallFunctionUsingABI.cpp
  lldb/source/Target/ThreadPlanCallUserExpression.cpp
  lldb/source/Target/ThreadPlanPython.cpp
  lldb/source/Target/ThreadPlanRunToAddress.cpp
  lldb/source/Target/ThreadPlanStepInRange.cpp
  lldb/source/Target/ThreadPlanStepInstruction.cpp
  lldb/source/Target/ThreadPlanStepOut.cpp
  lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
  lldb/source/Target/ThreadPlanStepOverRange.cpp
  lldb/source/Target/ThreadPlanStepRange.cpp
  lldb/source/Target/ThreadPlanStepThrough.cpp
  lldb/source/Target/ThreadPlanStepUntil.cpp

Index: lldb/source/Target/ThreadPlanStepUntil.cpp
===
--- lldb/source/Target/ThreadPlanStepUntil.cpp
+++ lldb/source/Target/ThreadPlanStepUntil.cpp
@@ -34,17 +34,16 @@
   m_should_stop(false), m_ran_analyze(false), m_explains_stop(false),
   m_until_points(), m_stop_others(stop_others) {
   // Stash away our "until" addresses:
-  TargetSP target_sp(m_thread.CalculateTarget());
+  TargetSP target_sp(thread.CalculateTarget());
 
-  StackFrameSP frame_sp(m_thread.GetStackFrameAtIndex(frame_idx));
+  StackFrameSP frame_sp(thread.GetStackFrameAtIndex(frame_idx));
   if (frame_sp) {
 m_step_from_insn = frame_sp->GetStackID().GetPC();
-lldb::user_id_t thread_id = m_thread.GetID();
 
 // Find the return address and set a breakpoint there:
 // FIXME - can we do this more securely if we know first_insn?
 
-StackFrameSP return_frame_sp(m_thread.GetStackFrameAtIndex(frame_idx + 1));
+StackFrameSP return_frame_sp(thread.GetStackFrameAtIndex(frame_idx + 1));
 if (return_frame_sp) {
   // TODO: add inline functionality
   m_return_addr = return_frame_sp->GetStackID().GetPC();
@@ -54,7 +53,7 @@
   if (return_bp != nullptr) {
 if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
   m_could_not_resolve_hw_bp = true;
-return_bp->SetThreadID(thread_id);
+return_bp->SetThreadID(m_tid);
 m_return_bp_id = return_bp->GetID();
 return_bp->SetBreakpointKind("until-return-backstop");
   }
@@ -67,7 +66,7 @@
   Breakpoint *until_bp =
   target_sp->CreateBreakpoint(address_list[i], true, false).get();
   if (until_bp != nullptr) {
-until_bp->SetThreadID(thread_id);
+until_bp->SetThreadID(m_tid);
 m_until_points[address_list[i]] = until_bp->GetID();
 until_bp->SetBreakpointKind("until-target");
   } else {
@@ -80,17 +79,15 @@
 ThreadPlanStepUntil::~ThreadPlanStepUntil() { Clear(); }
 
 void ThreadPlanStepUntil::Clear() {
-  TargetSP target_sp(m_thread.CalculateTarget());
-  if (target_sp) {
-if (m_return_bp_id != LLDB_INVALID_BREAK_ID) {
-  target_sp->RemoveBreakpointByID(m_return_bp_id);
-  m_return_bp_id = LLDB_INVALID_BREAK_ID;
-}
+  Target &target = GetTarget();
+  if (m_return_bp_id != LLDB_INVALID_BREAK_ID) {
+target.RemoveBreakpointByID(m_return_bp_id);
+m_return_bp_id = LLDB_INVALID_BREAK_ID;
+  }
 
-until_collection::iterator pos, end = m_until_points.end();
-for (pos = m_until_points.begin(); pos != end; pos++) {
-  target_sp->RemoveBreakpointByID((*pos).second);
-}
+  until_collection::iterator pos, end = m_until_points.end();
+  for (pos = m_until_points.begin(); pos != end; pos++) {
+target.RemoveBreakpointByID((*pos).second);
   }
   m_until_points.clear();
   m_could_not_resolve_hw_bp = false;
@@ -158,8 +155,7 @@
   // If this is OUR breakpoint, we're fine, otherwise we don't know why
   // this happened...
   BreakpointSiteSP this_site =
-  m_thread.GetProcess()->GetBreakpointSiteList().FindByID(
-  stop_info_sp->GetValue());
+  m_process.GetBreakpointSiteList().FindByID(stop_info_sp->GetValue());
   if (!this_site) {
 m_explains_stop = false;
 return;
@@ -196,17 +192,17 @@
 for (pos = m_until_points.begin(); pos != end; pos++) {
   if (this_site->IsBreakpointAtThisSite((*pos).second)) {
 // If we're at the right stack depth, then we're done.
-
+Thread &thread = GetThread();

[Lldb-commits] [PATCH] D75880: [NFC} Move ThreadPlans stacks into their own class, store it in Process by TID

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG61e8e6882de7: Move thread plan stacks into the Process, 
indexed by TID. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75880

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanStack.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Process.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadList.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanStack.cpp

Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- /dev/null
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -0,0 +1,370 @@
+//===-- ThreadPlanStack.cpp -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Target/ThreadPlanStack.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Target/ThreadPlan.h"
+#include "lldb/Utility/Log.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static void PrintPlanElement(Stream *s, const ThreadPlanSP &plan,
+ lldb::DescriptionLevel desc_level,
+ int32_t elem_idx) {
+  s->IndentMore();
+  s->Indent();
+  s->Printf("Element %d: ", elem_idx);
+  plan->GetDescription(s, desc_level);
+  s->EOL();
+  s->IndentLess();
+}
+
+void ThreadPlanStack::DumpThreadPlans(Stream *s,
+  lldb::DescriptionLevel desc_level,
+  bool include_internal) const {
+
+  uint32_t stack_size;
+
+  s->IndentMore();
+  s->Indent();
+  s->Printf("Active plan stack:\n");
+  int32_t print_idx = 0;
+  for (auto plan : m_plans) {
+PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  if (AnyCompletedPlans()) {
+print_idx = 0;
+s->Indent();
+s->Printf("Completed Plan Stack:\n");
+for (auto plan : m_completed_plans)
+  PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  if (AnyDiscardedPlans()) {
+print_idx = 0;
+s->Indent();
+s->Printf("Discarded Plan Stack:\n");
+for (auto plan : m_discarded_plans)
+  PrintPlanElement(s, plan, desc_level, print_idx++);
+  }
+
+  s->IndentLess();
+}
+
+size_t ThreadPlanStack::CheckpointCompletedPlans() {
+  m_completed_plan_checkpoint++;
+  m_completed_plan_store.insert(
+  std::make_pair(m_completed_plan_checkpoint, m_completed_plans));
+  return m_completed_plan_checkpoint;
+}
+
+void ThreadPlanStack::RestoreCompletedPlanCheckpoint(size_t checkpoint) {
+  auto result = m_completed_plan_store.find(checkpoint);
+  assert(result != m_completed_plan_store.end() &&
+ "Asked for a checkpoint that didn't exist");
+  m_completed_plans.swap((*result).second);
+  m_completed_plan_store.erase(result);
+}
+
+void ThreadPlanStack::DiscardCompletedPlanCheckpoint(size_t checkpoint) {
+  m_completed_plan_store.erase(checkpoint);
+}
+
+void ThreadPlanStack::ThreadDestroyed(Thread *thread) {
+  // Tell the plan stacks that this thread is going away:
+  for (ThreadPlanSP plan : m_plans)
+plan->ThreadDestroyed();
+
+  for (ThreadPlanSP plan : m_discarded_plans)
+plan->ThreadDestroyed();
+
+  for (ThreadPlanSP plan : m_completed_plans)
+plan->ThreadDestroyed();
+
+  // Now clear the current plan stacks:
+  m_plans.clear();
+  m_discarded_plans.clear();
+  m_completed_plans.clear();
+
+  // Push a ThreadPlanNull on the plan stack.  That way we can continue
+  // assuming that the plan stack is never empty, but if somebody errantly asks
+  // questions of a destroyed thread without checking first whether it is
+  // destroyed, they won't crash.
+  if (thread != nullptr) {
+lldb::ThreadPlanSP null_plan_sp(new ThreadPlanNull(*thread));
+m_plans.push_back(null_plan_sp);
+  }
+}
+
+void ThreadPlanStack::EnableTracer(bool value, bool single_stepping) {
+  for (ThreadPlanSP plan : m_plans) {
+if (plan->GetThreadPlanTracer()) {
+  plan->GetThreadPlanTracer()->EnableTracing(value);
+  plan->GetThreadPlanTracer()->EnableSingleStep(single_stepping);
+}
+  }
+}
+
+void ThreadPlanStack::SetTracer(lldb::ThreadPlanTracerSP &tracer_sp) {
+  for (ThreadPlanSP plan : m_plans)
+plan->SetThreadPlanTracer(tracer_sp);
+}
+
+void ThreadPlanStack::PushPlan(lldb::ThreadPlanSP new_plan_sp) {
+  // If the thread plan doesn't already have a tracer, give it its parent's
+  // tracer:
+  // The first plan has to be a base plan:
+  assert(m_plans.si

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Does anyone know why harbormaster is useless?




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2014
 
-if (!method_die_offsets.empty()) {
-  DWARFDebugInfo &debug_info = dwarf->DebugInfo();

kwk wrote:
> I assume this can be removed because you're iterating over `num_matches == 0` 
> when it's empty. But I cannot tell about the `DWARFDebugInfo &debug_info = 
> dwarf->DebugInfo();` and how costly this call is and if it makes sense to run 
> when the offsets are empty.
not costly at all. This will be fine.



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

This logic is easy to understand by reading the code.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp:70-71
 const dw_tag_t die_tag = die_info_array[i].tag;
-if (die_tag == 0 || die_tag == DW_TAG_class_type ||
-die_tag == DW_TAG_structure_type) {
-  if (die_info_array[i].type_flags & eTypeFlagClassIsImplementation) {
-if (return_implementation_only_if_available) {
-  // We found the one true definition for this class, so only return
-  // that
-  die_offsets.clear();
-  die_offsets.emplace_back(die_info_array[i]);
-  return;
-} else {
-  // Put the one true definition as the first entry so it matches first
-  die_offsets.emplace(die_offsets.begin(), die_info_array[i]);
-}
-  } else {
+if (die_tag != 0 && die_tag != DW_TAG_class_type &&
+die_tag != DW_TAG_structure_type)
+  continue;

clayborg wrote:
> This logic is easy to understand by reading the code.
This logic is harder to follow. Maybe:
```
if (!(die_tag == 0 || die_tag == DW_TAG_class_type || die_tag == 
DW_TAG_structure_type))
  continue;
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326



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

[Lldb-commits] [PATCH] D75720: Make ThreadPlanTracer use {Process, TID} rather than Thread to find it's underlying thread...

[Jim Ingham via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c1c57a1df81: Make ThreadPlanTracers use TID & Process 
rather than Thread *. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75720

Files:
  lldb/include/lldb/Target/ThreadPlanTracer.h
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -34,23 +34,32 @@
 #pragma mark ThreadPlanTracer
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread, lldb::StreamSP &stream_sp)
-: m_thread(thread), m_single_step(true), m_enabled(false),
-  m_stream_sp(stream_sp) {}
+: m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
+  m_single_step(true), m_enabled(false), m_stream_sp(stream_sp) {}
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread)
-: m_thread(thread), m_single_step(true), m_enabled(false), m_stream_sp() {}
+: m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
+  m_single_step(true), m_enabled(false), m_stream_sp() {}
 
 Stream *ThreadPlanTracer::GetLogStream() {
   if (m_stream_sp)
 return m_stream_sp.get();
   else {
-TargetSP target_sp(m_thread.CalculateTarget());
+TargetSP target_sp(GetThread().CalculateTarget());
 if (target_sp)
   return &(target_sp->GetDebugger().GetOutputStream());
   }
   return nullptr;
 }
 
+Thread &ThreadPlanTracer::GetThread() {
+  if (m_thread)
+return *m_thread;
+
+  ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
+  m_thread = thread_sp.get();
+  return *m_thread;
+}
 void ThreadPlanTracer::Log() {
   SymbolContext sc;
   bool show_frame_index = false;
@@ -58,8 +67,8 @@
 
   Stream *stream = GetLogStream();
   if (stream) {
-m_thread.GetStackFrameAtIndex(0)->Dump(stream, show_frame_index,
-   show_fullpaths);
+GetThread().GetStackFrameAtIndex(0)->Dump(stream, show_frame_index,
+  show_fullpaths);
 stream->Printf("\n");
 stream->Flush();
   }
@@ -67,7 +76,7 @@
 
 bool ThreadPlanTracer::TracerExplainsStop() {
   if (m_enabled && m_single_step) {
-lldb::StopInfoSP stop_info = m_thread.GetStopInfo();
+lldb::StopInfoSP stop_info = GetThread().GetStopInfo();
 return (stop_info->GetStopReason() == eStopReasonTrace);
   } else
 return false;
@@ -87,13 +96,13 @@
 Disassembler *ThreadPlanAssemblyTracer::GetDisassembler() {
   if (!m_disassembler_sp)
 m_disassembler_sp = Disassembler::FindPlugin(
-m_thread.GetProcess()->GetTarget().GetArchitecture(), nullptr, nullptr);
+m_process.GetTarget().GetArchitecture(), nullptr, nullptr);
   return m_disassembler_sp.get();
 }
 
 TypeFromUser ThreadPlanAssemblyTracer::GetIntPointerType() {
   if (!m_intptr_type.IsValid()) {
-if (auto target_sp = m_thread.CalculateTarget()) {
+if (auto target_sp = m_process.CalculateTarget()) {
   auto type_system_or_err =
   target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC);
   if (auto err = type_system_or_err.takeError()) {
@@ -125,29 +134,27 @@
   if (!stream)
 return;
 
-  RegisterContext *reg_ctx = m_thread.GetRegisterContext().get();
+  RegisterContext *reg_ctx = GetThread().GetRegisterContext().get();
 
   lldb::addr_t pc = reg_ctx->GetPC();
-  ProcessSP process_sp(m_thread.GetProcess());
   Address pc_addr;
   bool addr_valid = false;
   uint8_t buffer[16] = {0}; // Must be big enough for any single instruction
-  addr_valid = process_sp->GetTarget().GetSectionLoadList().ResolveLoadAddress(
+  addr_valid = m_process.GetTarget().GetSectionLoadList().ResolveLoadAddress(
   pc, pc_addr);
 
-  pc_addr.Dump(stream, &m_thread, Address::DumpStyleResolvedDescription,
+  pc_addr.Dump(stream, &GetThread(), Address::DumpStyleResolvedDescription,
Address::DumpStyleModuleWithFileAddress);
   stream->PutCString(" ");
 
   Disassembler *disassembler = GetDisassembler();
   if (disassembler) {
 Status err;
-process_sp->ReadMemory(pc, buffer, sizeof(buffer), err);
+m_process.ReadMemory(pc, buffer, sizeof(buffer), err);
 
 if (err.Success()) {
-  DataExtractor extractor(buffer, sizeof(buffer),
-  process_sp->GetByteOrder(),
-  process_sp->GetAddressByteSize());
+  DataExtractor extractor(buffer, sizeof(buffer), m_process.GetByteOrder(),
+  m_process.GetAddressByteSize());
 
   bool data_from_file = false;
   if (addr_valid)
@@ -167,10 +174,7 @@
 Instruction *instruction =
 instruction_list.GetInstructionAtIndex(0).get();
 const Fo

[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1893065d7bf5: Allow the ThreadPlanStackMap to hold the 
thread plans for threads that were not… (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76814

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanStack.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadList.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanStack.cpp
  lldb/source/Target/ThreadPlanStepOut.cpp
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/Makefile
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/main.cpp
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/operating_system.py
  lldb/test/API/functionalities/thread_plan/Makefile
  lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
  lldb/test/API/functionalities/thread_plan/main.c

Index: lldb/test/API/functionalities/thread_plan/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/thread_plan/main.c
@@ -0,0 +1,16 @@
+#include 
+
+void
+call_me(int value) {
+  printf("called with %d\n", value); // Set another here.
+}
+
+int
+main(int argc, char **argv)
+{
+  call_me(argc); // Set a breakpoint here.
+  printf("This just spaces the two calls\n");
+  call_me(argc); // Run here to step over again.
+  printf("More spacing\n");
+  return 0; // Make sure we get here on last continue
+}
Index: lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
===
--- /dev/null
+++ lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
@@ -0,0 +1,164 @@
+"""
+Test that thread plan listing, and deleting works.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestThreadPlanCommands(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_thread_plan_actions(self):
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.thread_plan_test()
+
+def check_list_output(self, command, active_plans = [], completed_plans = [], discarded_plans = []):
+# Check the "thread plan list" output against a list of active & completed and discarded plans.
+# If all three check arrays are empty, that means the command is expected to fail. 
+
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+
+num_active = len(active_plans)
+num_completed = len(completed_plans)
+num_discarded = len(discarded_plans)
+
+interp.HandleCommand(command, result)
+if self.TraceOn():
+print("Command: %s"%(command))
+print(result.GetOutput())
+
+if num_active == 0 and num_completed == 0 and num_discarded == 0:
+self.assertFalse(result.Succeeded(), "command: '%s' succeeded when it should have failed: '%s'"%
+ (command, result.GetError()))
+return
+
+self.assertTrue(result.Succeeded(), "command: '%s' failed: '%s'"%(command, result.GetError()))
+result_arr = result.GetOutput().splitlines()
+num_results = len(result_arr)
+
+# Match the expected number of elements.
+# Adjust the count for the number of header lines we aren't matching:
+fudge = 0
+
+if num_completed == 0 and num_discarded == 0:
+# The fudge is 3: Thread header, Active Plan header and base plan
+fudge = 3
+elif num_completed == 0 or num_discarded == 0:
+# The fudge is 4: The above plus either the Completed or Discarded Plan header:
+fudge = 4
+else:
+# The fudge is 5 since we have both headers:
+fudge = 5
+
+self.assertEqual(num_results, num_active + num_completed + num_discarded + fudge,
+ "Too many elements in match arrays")
+
+# Now iterate through the results array and pick out the results.
+result_idx = 0
+self.assertIn("thread #", result_arr[result_idx], "Found thread header") ; result_idx += 1
+self.assertIn("Active plan stack", result_arr[result_idx], "Found active header") ; result_idx += 

[Lldb-commits] [lldb] fcab66d - [lldb] Findtypes -gmodules fix for too many matches

[Jan Kratochvil via lldb-commits]

Author: Jan Kratochvil
Date: 2020-04-04T00:15:06+02:00
New Revision: fcab66d5fe53fc3c318b0d0c4ef1bb4a43a7744f

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

LOG: [lldb] Findtypes -gmodules fix for too many matches

Apparently the intention was to copy the condition above:
  if (types.GetSize() >= max_matches)
break;

So that if the iteration stopped because of too many matches we do not
add even more matches in this 'Clang modules' block downward.

It was implemented by:
  SymbolFileDWARF: Unconditionally scan through clang modules. NFCish
  fe9eaadd68307347d97698fd0a1646827eafd290

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 1113ed1195a3..ab271d2364bd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2407,7 +2407,7 @@ void SymbolFileDWARF::FindTypes(
   // Next search through the reachable Clang modules. This only applies for
   // DWARF objects compiled with -gmodules that haven't been processed by
   // dsymutil.
-  if (num_die_matches < max_matches) {
+  if (types.GetSize() < max_matches) {
 UpdateExternalModuleListIfNeeded();
 
 for (const auto &pair : m_external_type_modules)



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

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

[Jan Kratochvil via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2387
   // dsymutil.
-  if (num_die_matches < max_matches) {
+  if (types.GetSize() < max_matches) {
 UpdateExternalModuleListIfNeeded();

jankratochvil wrote:
> This is not NFC but it is required by the refactorization. I have posted this 
> bugfix separately as D77336.
It has been now fixed by checked-in D77336.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327



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

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

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 254931.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77327

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

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

[Lldb-commits] [PATCH] D77336: Findtypes -gmodules fix

[Jan Kratochvil via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfcab66d5fe53: [lldb] Findtypes -gmodules fix for too many 
matches (authored by jankratochvil).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77336

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2407,7 +2407,7 @@
   // Next search through the reachable Clang modules. This only applies for
   // DWARF objects compiled with -gmodules that haven't been processed by
   // dsymutil.
-  if (num_die_matches < max_matches) {
+  if (types.GetSize() < max_matches) {
 UpdateExternalModuleListIfNeeded();
 
 for (const auto &pair : m_external_type_modules)


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2407,7 +2407,7 @@
   // Next search through the reachable Clang modules. This only applies for
   // DWARF objects compiled with -gmodules that haven't been processed by
   // dsymutil.
-  if (num_die_matches < max_matches) {
+  if (types.GetSize() < max_matches) {
 UpdateExternalModuleListIfNeeded();
 
 for (const auto &pair : m_external_type_modules)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 2d658c5 - Disable two new tests on Windows. They are failing but the logs are not helpful.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2020-04-03T17:14:56-07:00
New Revision: 2d658c56d57c4b355635b2bab38ba97a598c7393

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

LOG: Disable two new tests on Windows.  They are failing but the logs are not 
helpful.

Also turn on the command trace unconditionally for TestThreadPlanCommands.py as 
the
tests for the Ubuntu bot don't seem to run with -t making it hard to see why 
this is
failing remotely.

Added: 


Modified: 

lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
 
b/lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
index 5bba48364731..1a625cee5d86 100644
--- 
a/lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
+++ 
b/lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
@@ -8,6 +8,7 @@
 
 import os
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -17,6 +18,7 @@ class TestOSPluginStepping(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfWindows
 def test_python_os_plugin(self):
 """Test that stepping works when the OS Plugin doesn't report all
threads at every stop"""
@@ -24,6 +26,7 @@ def test_python_os_plugin(self):
 self.main_file = lldb.SBFileSpec('main.cpp')
 self.run_python_os_step_missing_thread(False)
 
+@skipIfWindows
 def test_python_os_plugin_prune(self):
 """Test that pruning the unreported PlanStacks works"""
 self.build()

diff  --git 
a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py 
b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
index 30fdb218fc2e..25b184ee406f 100644
--- a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
+++ b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
@@ -5,6 +5,7 @@
 
 
 import lldb
+from lldbsuite.test.decorators import *
 import lldbsuite.test.lldbutil as lldbutil
 from lldbsuite.test.lldbtest import *
 
@@ -15,6 +16,7 @@ class TestThreadPlanCommands(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfWindows
 def test_thread_plan_actions(self):
 self.build()
 self.main_source_file = lldb.SBFileSpec("main.c")
@@ -32,9 +34,8 @@ def check_list_output(self, command, active_plans = [], 
completed_plans = [], di
 num_discarded = len(discarded_plans)
 
 interp.HandleCommand(command, result)
-if self.TraceOn():
-print("Command: %s"%(command))
-print(result.GetOutput())
+print("Command: %s"%(command))
+print(result.GetOutput())
 
 if num_active == 0 and num_completed == 0 and num_discarded == 0:
 self.assertFalse(result.Succeeded(), "command: '%s' succeeded when 
it should have failed: '%s'"%



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

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

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: labath, clayborg.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This adds support for commands created through the API to support autorepeat.
This covers the case of single word and multiword commands.

Comprehensive tests are included as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77444

Files:
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/test/API/api/auto-repeat-command/Makefile
  lldb/test/API/api/auto-repeat-command/TestSBCommandAutoRepeat.py
  lldb/test/API/api/auto-repeat-command/main.cpp

Index: lldb/test/API/api/auto-repeat-command/main.cpp
===
--- /dev/null
+++ lldb/test/API/api/auto-repeat-command/main.cpp
@@ -0,0 +1,109 @@
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+#include 
+
+using namespace lldb;
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  DummyCommand(const char *message) : m_message(message) {}
+
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject &result) {
+result.PutCString(m_message.c_str());
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+
+private:
+  std::string m_message;
+};
+
+void testSingleWordCommand(SBCommandInterpreter &interp, SBDebugger &dbg) {
+  // We first test a command without autorepeat
+  DummyCommand dummy("It worked");
+  interp.AddCommand("dummy", &dummy, nullptr /*help*/);
+  {
+SBCommandReturnObject result;
+dbg.GetCommandInterpreter().HandleCommand("dummy", result,
+  true /*add_to_history*/);
+assert(result.Succeeded());
+assert(strcmp(result.GetOutput(), "It worked\n") == 0);
+  }
+  {
+SBCommandReturnObject result;
+dbg.GetCommandInterpreter().HandleCommand("", result);
+assert(!result.Succeeded() &&
+   "The command should fail as a repeated command");
+assert(strcmp(result.GetError(), "error: No auto repeat.\n") == 0);
+  }
+
+  // Now we test a command with autorepeat
+  interp.AddCommand("dummy_with_autorepeat", &dummy, nullptr /*help*/,
+nullptr /*syntax*/, true /*autorepeat*/);
+  {
+SBCommandReturnObject result;
+dbg.GetCommandInterpreter().HandleCommand("dummy_with_autorepeat", result,
+  true /*add_to_history*/);
+assert(result.Succeeded());
+assert(strcmp(result.GetOutput(), "It worked\n") == 0);
+  }
+  {
+SBCommandReturnObject result;
+dbg.GetCommandInterpreter().HandleCommand("", result);
+assert(result.Succeeded());
+assert(strcmp(result.GetOutput(), "It worked\n") == 0);
+  }
+}
+
+void testMultiWordCommand(SBCommandInterpreter &interp, SBDebugger &dbg) {
+  auto command = interp.AddMultiwordCommand("multicommand", nullptr /*help*/);
+  // We first test a subcommand without autorepeat
+  DummyCommand subcommand("It worked again");
+  command.AddCommand("subcommand", &subcommand, nullptr /*help*/);
+  {
+SBCommandReturnObject result;
+dbg.GetCommandInterpreter().HandleCommand("multicommand subcommand", result,
+  true /*add_to_history*/);
+assert(result.Succeeded());
+assert(strcmp(result.GetOutput(), "It worked again\n") == 0);
+  }
+  {
+SBCommandReturnObject result;
+dbg.GetCommandInterpreter().HandleCommand("", result);
+assert(!result.Succeeded() &&
+   "The command should fail as a repeated command");
+assert(strcmp(result.GetError(), "error: No auto repeat.\n") == 0);
+  }
+
+  // We first test a subcommand with autorepeat
+  command.AddCommand("subcommand_with_autorepeat", &subcommand,
+ nullptr /*help*/, nullptr /*syntax*/, true /*autorepeat*/);
+  {
+SBCommandReturnObject result;
+dbg.GetCommandInterpreter().HandleCommand(
+"multicommand subcommand_with_autorepeat", result,
+true /*add_to_history*/);
+assert(result.Succeeded());
+assert(strcmp(result.GetOutput(), "It worked again\n") == 0);
+  }
+  {
+SBCommandReturnObject result;
+dbg.GetCommandInterpreter().HandleCommand("", result);
+assert(result.Succeeded());
+assert(strcmp(result.GetOutput(), "It worked again\n") == 0);
+  }
+}
+
+int main() {
+  SBDebugger::Initialize();
+  SBDebugger dbg = SBDebugger::Create(false /*source_init_files*/);
+  SBCommandInterpreter interp = dbg.GetCommandInterpreter();
+
+  testSingleWordCommand(interp, dbg);
+  testMultiWordCommand(interp, dbg);
+}
Index: lldb/test/API/api/auto-repeat-command/TestSBCommandAutoRepeat.py
===
--- /dev/null
+++ lldb/test/API/api/auto-repeat-command/TestSBCommandAutoRepeat.py
@@ -0,0 +1,34 @

[Lldb-commits] [lldb] 7255793 - [intel-mpx] Delete an unnecessary license header

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-04-03T17:27:37-07:00
New Revision: 725579379deea21bb4201731fe6d6e70ed98961d

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

LOG: [intel-mpx] Delete an unnecessary license header

Summary:
@labath mentioned to me that test files shouldn't have a license header.
I saw this one some days ago, so I'm doing some cleaning.

Reviewers: labath, clayborg

Subscribers: lldb-commits, labath

Tags: #lldb

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

Added: 


Modified: 
lldb/tools/intel-features/intel-mpx/test/main.cpp

Removed: 




diff  --git a/lldb/tools/intel-features/intel-mpx/test/main.cpp 
b/lldb/tools/intel-features/intel-mpx/test/main.cpp
index 2f5253ed8600..ecc292866ff6 100644
--- a/lldb/tools/intel-features/intel-mpx/test/main.cpp
+++ b/lldb/tools/intel-features/intel-mpx/test/main.cpp
@@ -1,12 +1,3 @@
-//===-- main.cpp *- C++ 
-*-===//
-
- Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
- See https://llvm.org/LICENSE.txt for license information.
- SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-===--===//
-//
-
 const int size = 5;
 
 #include 



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

[Lldb-commits] [PATCH] D77328: [intel-mpx] Delete an unnecessary license header

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG725579379dee: [intel-mpx] Delete an unnecessary license 
header (authored by Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77328

Files:
  lldb/tools/intel-features/intel-mpx/test/main.cpp


Index: lldb/tools/intel-features/intel-mpx/test/main.cpp
===
--- lldb/tools/intel-features/intel-mpx/test/main.cpp
+++ lldb/tools/intel-features/intel-mpx/test/main.cpp
@@ -1,12 +1,3 @@
-//===-- main.cpp *- C++ 
-*-===//
-
- Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
- See https://llvm.org/LICENSE.txt for license information.
- SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-===--===//
-//
-
 const int size = 5;
 
 #include 


Index: lldb/tools/intel-features/intel-mpx/test/main.cpp
===
--- lldb/tools/intel-features/intel-mpx/test/main.cpp
+++ lldb/tools/intel-features/intel-mpx/test/main.cpp
@@ -1,12 +1,3 @@
-//===-- main.cpp *- C++ -*-===//
-
- Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
- See https://llvm.org/LICENSE.txt for license information.
- SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-===--===//
-//
-
 const int size = 5;
 
 #include 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3ccd454 - Fix unused variable, format, and format string warnings.

[Eric Christopher via lldb-commits]

Author: Eric Christopher
Date: 2020-04-03T17:58:59-07:00
New Revision: 3ccd454c102b069d2230a18cfe16b84a5f005fc8

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

LOG: Fix unused variable, format, and format string warnings.
NFC.

Added: 


Modified: 
lldb/include/lldb/Target/ThreadPlanStack.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Target/ThreadPlan.cpp
lldb/source/Target/ThreadPlanStack.cpp
lldb/source/Target/ThreadPlanStepRange.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/ThreadPlanStack.h 
b/lldb/include/lldb/Target/ThreadPlanStack.h
index de52ee3ee198..f1874136cad8 100644
--- a/lldb/include/lldb/Target/ThreadPlanStack.h
+++ b/lldb/include/lldb/Target/ThreadPlanStack.h
@@ -125,7 +125,7 @@ class ThreadPlanStackMap {
 
   void AddThread(Thread &thread) {
 lldb::tid_t tid = thread.GetID();
-auto result = m_plans_list.emplace(tid, thread);
+m_plans_list.emplace(tid, thread);
   }
 
   bool RemoveTID(lldb::tid_t tid) {

diff  --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 579f3362be89..fe86c9d34c6c 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -2061,7 +2061,6 @@ class CommandObjectThreadPlanPrune : public 
CommandObjectParsed {
   return true;  
 }
 
-bool success;
 const size_t num_args = args.GetArgumentCount();
 
 std::lock_guard guard(

diff  --git a/lldb/source/Target/ThreadPlan.cpp 
b/lldb/source/Target/ThreadPlan.cpp
index 52b1a8b29c87..b3ff3232a0ac 100644
--- a/lldb/source/Target/ThreadPlan.cpp
+++ b/lldb/source/Target/ThreadPlan.cpp
@@ -24,7 +24,7 @@ ThreadPlan::ThreadPlan(ThreadPlanKind kind, const char *name, 
Thread &thread,
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
   m_stop_vote(stop_vote), m_run_vote(run_vote),
   m_takes_iteration_count(false), m_could_not_resolve_hw_bp(false),
-  m_kind(kind), m_thread(&thread), m_name(name), m_plan_complete_mutex(),
+  m_thread(&thread), m_kind(kind), m_name(name), m_plan_complete_mutex(),
   m_cached_plan_explains_stop(eLazyBoolCalculate), m_plan_complete(false),
   m_plan_private(false), m_okay_to_discard(true), m_is_master_plan(false),
   m_plan_succeeded(true) {

diff  --git a/lldb/source/Target/ThreadPlanStack.cpp 
b/lldb/source/Target/ThreadPlanStack.cpp
index 6120d0ccefcb..44e47f385a82 100644
--- a/lldb/source/Target/ThreadPlanStack.cpp
+++ b/lldb/source/Target/ThreadPlanStack.cpp
@@ -39,9 +39,6 @@ ThreadPlanStack::ThreadPlanStack(const Thread &thread, bool 
make_null) {
 void ThreadPlanStack::DumpThreadPlans(Stream &s,
   lldb::DescriptionLevel desc_level,
   bool include_internal) const {
-
-  uint32_t stack_size;
-
   s.IndentMore();
   PrintOneStack(s, "Active plan stack", m_plans, desc_level, include_internal);
   PrintOneStack(s, "Completed plan stack", m_completed_plans, desc_level,
@@ -73,7 +70,7 @@ void ThreadPlanStack::PrintOneStack(Stream &s, 
llvm::StringRef stack_name,
   if (include_internal || any_public) {
 int print_idx = 0;
 s.Indent();
-s.Printf("%s:\n", stack_name);
+s << stack_name << ":\n";
 for (auto plan : stack) {
   if (!include_internal && plan->GetPrivate())
 continue;
@@ -270,7 +267,6 @@ lldb::ThreadPlanSP ThreadPlanStack::GetCompletedPlan(bool 
skip_private) const {
 lldb::ThreadPlanSP ThreadPlanStack::GetPlanByIndex(uint32_t plan_idx,
bool skip_private) const {
   uint32_t idx = 0;
-  ThreadPlan *up_to_plan_ptr = nullptr;
 
   for (lldb::ThreadPlanSP plan_sp : m_plans) {
 if (skip_private && plan_sp->GetPrivate())

diff  --git a/lldb/source/Target/ThreadPlanStepRange.cpp 
b/lldb/source/Target/ThreadPlanStepRange.cpp
index 85b89e7a080b..f4b2ee3d08a2 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -86,7 +86,6 @@ void ThreadPlanStepRange::AddRange(const AddressRange 
&new_range) {
 }
 
 void ThreadPlanStepRange::DumpRanges(Stream *s) {
-  Thread &thread = GetThread();
   size_t num_ranges = m_address_ranges.size();
   if (num_ranges == 1) {
 m_address_ranges[0].Dump(s, &GetTarget(), Address::DumpStyleLoadAddress);



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

[Lldb-commits] [lldb] 48ba745 - This test is failing on the Ubuntu bot but the bot log doesn't

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2020-04-03T18:04:02-07:00
New Revision: 48ba745eacf883080152fe16b5b1305657d001ca

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

LOG: This test is failing on the Ubuntu bot but the bot log doesn't
capture the test stdout, so put the info I need to see in the error
message instead.

Added: 


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

Removed: 




diff  --git 
a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py 
b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
index 25b184ee406f..c301e8cf8441 100644
--- a/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
+++ b/lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py
@@ -61,7 +61,7 @@ def check_list_output(self, command, active_plans = [], 
completed_plans = [], di
 fudge = 5
 
 self.assertEqual(num_results, num_active + num_completed + 
num_discarded + fudge,
- "Too many elements in match arrays")
+ "Too many elements in match arrays for: 
\n%s\n"%result.GetOutput())
 
 # Now iterate through the results array and pick out the results.
 result_idx = 0



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

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

LGTM. Pavel?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444



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

[Lldb-commits] [PATCH] D77450: Fix LLDB debug builds

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: clayborg, labath, jingham.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

A recent change in ThreadPlans introduced this little compilation error.
Seems to be related to the work around https://reviews.llvm.org/D76814.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77450

Files:
  lldb/source/Target/ThreadPlan.cpp


Index: lldb/source/Target/ThreadPlan.cpp
===
--- lldb/source/Target/ThreadPlan.cpp
+++ lldb/source/Target/ThreadPlan.cpp
@@ -240,7 +240,7 @@
   fprintf(stderr,
   "error: %s called on thread that has been destroyed (tid = 0x%" 
PRIx64
   ", ptid = 0x%" PRIx64 ")",
-  LLVM_PRETTY_FUNCTION, m_thread.GetID(), m_thread.GetProtocolID());
+  LLVM_PRETTY_FUNCTION, GetThread().GetID(), 
GetThread().GetProtocolID());
 #else
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD));
   if (log)


Index: lldb/source/Target/ThreadPlan.cpp
===
--- lldb/source/Target/ThreadPlan.cpp
+++ lldb/source/Target/ThreadPlan.cpp
@@ -240,7 +240,7 @@
   fprintf(stderr,
   "error: %s called on thread that has been destroyed (tid = 0x%" PRIx64
   ", ptid = 0x%" PRIx64 ")",
-  LLVM_PRETTY_FUNCTION, m_thread.GetID(), m_thread.GetProtocolID());
+  LLVM_PRETTY_FUNCTION, GetThread().GetID(), GetThread().GetProtocolID());
 #else
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD));
   if (log)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ca47ac3 - [source maps] Fix remove, insert-after and replace

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-04-03T19:15:56-07:00
New Revision: ca47ac3d5f6f8483d330c96a63f1cd862e667856

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

LOG: [source maps] Fix remove, insert-after and replace

Summary:
In this diff of mine D77186 I introduce a bug in the replace operation, where I 
was failing fast by mistake.
Besides, a similar problem existed in the insert-after operation, where it was 
failing fast.

Finally, the remove operation was wrong, as it was not using the indices 
provided by the users.

I fixed those issues and added some tests account for cases with multiple 
elements in these requests.

Reviewers: labath, clayborg

Reviewed By: labath

Subscribers: mgrang, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Interpreter/OptionValuePathMappings.cpp
lldb/test/API/functionalities/source-map/TestTargetSourceMap.py

Removed: 




diff  --git a/lldb/source/Interpreter/OptionValuePathMappings.cpp 
b/lldb/source/Interpreter/OptionValuePathMappings.cpp
index 2784279579f0..3b3f43d07293 100644
--- a/lldb/source/Interpreter/OptionValuePathMappings.cpp
+++ b/lldb/source/Interpreter/OptionValuePathMappings.cpp
@@ -61,7 +61,7 @@ Status 
OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
 count);
   } else {
 bool changed = false;
-for (size_t i = 1; i < argc; i += 2) {
+for (size_t i = 1; i < argc; idx++, i += 2) {
   const char *orginal_path = args.GetArgumentAtIndex(i);
   const char *replace_path = args.GetArgumentAtIndex(i + 1);
   if (VerifyPathExists(replace_path)) {
@@ -70,14 +70,12 @@ Status 
OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
 if (!m_path_mappings.Replace(a, b, idx, m_notify_changes))
   m_path_mappings.Append(a, b, m_notify_changes);
 changed = true;
-idx++;
   } else {
 std::string previousError =
 error.Fail() ? std::string(error.AsCString()) + "\n" : "";
 error.SetErrorStringWithFormat(
 "%sthe replacement path doesn't exist: \"%s\"",
 previousError.c_str(), replace_path);
-break;
   }
 }
 if (changed)
@@ -156,7 +154,6 @@ Status 
OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
 error.SetErrorStringWithFormat(
 "%sthe replacement path doesn't exist: \"%s\"",
 previousError.c_str(), replace_path);
-break;
   }
 }
 if (changed)
@@ -171,32 +168,23 @@ Status 
OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
   case eVarSetOperationRemove:
 if (argc > 0) {
   std::vector remove_indexes;
-  bool all_indexes_valid = true;
-  size_t i;
-  for (i = 0; all_indexes_valid && i < argc; ++i) {
-const int idx =
+  for (size_t i = 0; i < argc; ++i) {
+int idx =
 StringConvert::ToSInt32(args.GetArgumentAtIndex(i), INT32_MAX);
-if (idx == INT32_MAX)
-  all_indexes_valid = false;
-else
+if (idx < 0 || idx >= (int)m_path_mappings.GetSize()) {
+  error.SetErrorStringWithFormat(
+  "invalid array index '%s', aborting remove operation",
+  args.GetArgumentAtIndex(i));
+  break;
+} else
   remove_indexes.push_back(idx);
   }
 
-  if (all_indexes_valid) {
-size_t num_remove_indexes = remove_indexes.size();
-if (num_remove_indexes) {
-  // Sort and then erase in reverse so indexes are always valid
-  llvm::sort(remove_indexes.begin(), remove_indexes.end());
-  for (size_t j = num_remove_indexes - 1; j < num_remove_indexes; ++j) 
{
-m_path_mappings.Remove(j, m_notify_changes);
-  }
-}
-NotifyValueChanged();
-  } else {
-error.SetErrorStringWithFormat(
-"invalid array index '%s', aborting remove operation",
-args.GetArgumentAtIndex(i));
-  }
+  // Sort and then erase in reverse so indexes are always valid
+  llvm::sort(remove_indexes.begin(), remove_indexes.end());
+  for (auto index : llvm::reverse(remove_indexes))
+m_path_mappings.Remove(index, m_notify_changes);
+  NotifyValueChanged();
 } else {
   error.SetErrorString("remove operation takes one or more array index");
 }

diff  --git a/lldb/test/API/functionalities/source-map/TestTargetSourceMap.py 
b/lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
index c9800e6f199e..6457c766813e 100644
--- a/lldb/test/API/functionalities/source-map/TestTargetSourceMa

[Lldb-commits] [PATCH] D77324: [source maps] Fix remove, insert-after and replace

[walter erquinigo via Phabricator via lldb-commits]

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

address comments`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77324

Files:
  lldb/source/Interpreter/OptionValuePathMappings.cpp
  lldb/test/API/functionalities/source-map/TestTargetSourceMap.py

Index: lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
===
--- lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
+++ lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
@@ -42,67 +42,93 @@
 self.assertEquals(bp.GetNumLocations(), 0,
 "make sure no breakpoints were resolved without map")
 
+valid_path = os.path.dirname(src_dir)
+valid_path2 = os.path.dirname(valid_path)
 invalid_path = src_dir + "invalid_path"
 invalid_path2 = src_dir + "invalid_path2"
 
 # We make sure the error message contains all the invalid paths
 self.expect(
-'settings set target.source-map . "%s" . "%s" . "%s"' % (invalid_path, src_dir, invalid_path2),
+'settings set target.source-map . "%s" . "%s" . "%s" . "%s' \
+% (invalid_path, src_dir, invalid_path2, valid_path),
 substrs=[
-'the replacement path doesn\'t exist: "%s"' % (invalid_path),
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
 'the replacement path doesn\'t exist: "%s"' % (invalid_path2),
 ],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (src_dir)],
+substrs=[
+'[0] "." -> "%s"' % (src_dir),
+'[1] "." -> "%s"' % (valid_path),
+],
 )
 assertBreakpointWithSourceMap(src_path)
 
-# Index 0 is the valid mapping, and modifying it to an invalid one should have no effect
+# Attempts to replace an index to an invalid mapping should have no effect.
+# Modifications to valid mappings should work.
 self.expect(
-'settings replace target.source-map 0 . "%s"' % (invalid_path),
-substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
+'settings replace target.source-map 0 . "%s" . "%s"' % (invalid_path, valid_path2),
+substrs=[
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
+],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (src_dir)]
+substrs=[
+'[0] "." -> "%s"' % (src_dir),
+'[1] "." -> "%s"' % (valid_path2),
+]
 )
 assertBreakpointWithSourceMap(src_path)
 
-# Let's clear and add the mapping in with insert-after
+# Let's clear and add the mapping back with insert-after
 self.runCmd('settings remove target.source-map 0')
 self.expect(
 'settings show target.source-map',
-endstr="target.source-map (path-map) =\n",
+substrs=['[0] "." -> "%s"' % (valid_path2)],
 )
-
-# We add a valid but useless mapping so that we can use insert-after
-another_valid_path = os.path.dirname(src_dir)
-self.runCmd('settings set target.source-map . "%s"' % (another_valid_path))
 
 self.expect(
-'settings replace target.source-map 0 . "%s"' % (invalid_path),
-substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
+'settings insert-after target.source-map 0 . "%s" . "%s" . "%s"' \
+% (invalid_path, invalid_path2, src_dir),
+substrs=[
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
+'the replacement path doesn\'t exist: "%s"' % (invalid_path2),
+],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (another_valid_path)]
+substrs=[
+'[0] "." -> "%s"' % (valid_path2),
+'[1] "." -> "%s"' % (src_dir),
+]
 )
 
-# Let's clear and add the mapping in with append
-self.expect('settings remove target.source-map 0')
+# Let's clear using remove and add the mapping in with append
+self.runCmd('settings remove target.source-map 1')
 self.expect(
 'settings show target.source-map',
-endstr="target.source-map (path-map) =\n",
+substrs=[
+'[0] "." -> "%s"' % (valid_path2),
+]
 )
-
+self.runCmd('settings clear target.source-map')
  

[Lldb-commits] [PATCH] D77324: [source maps] Fix remove, insert-after and replace

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGca47ac3d5f6f: [source maps] Fix remove, insert-after and 
replace (authored by Walter Erquinigo , committed by 
Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77324

Files:
  lldb/source/Interpreter/OptionValuePathMappings.cpp
  lldb/test/API/functionalities/source-map/TestTargetSourceMap.py

Index: lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
===
--- lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
+++ lldb/test/API/functionalities/source-map/TestTargetSourceMap.py
@@ -42,67 +42,93 @@
 self.assertEquals(bp.GetNumLocations(), 0,
 "make sure no breakpoints were resolved without map")
 
+valid_path = os.path.dirname(src_dir)
+valid_path2 = os.path.dirname(valid_path)
 invalid_path = src_dir + "invalid_path"
 invalid_path2 = src_dir + "invalid_path2"
 
 # We make sure the error message contains all the invalid paths
 self.expect(
-'settings set target.source-map . "%s" . "%s" . "%s"' % (invalid_path, src_dir, invalid_path2),
+'settings set target.source-map . "%s" . "%s" . "%s" . "%s' \
+% (invalid_path, src_dir, invalid_path2, valid_path),
 substrs=[
-'the replacement path doesn\'t exist: "%s"' % (invalid_path),
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
 'the replacement path doesn\'t exist: "%s"' % (invalid_path2),
 ],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (src_dir)],
+substrs=[
+'[0] "." -> "%s"' % (src_dir),
+'[1] "." -> "%s"' % (valid_path),
+],
 )
 assertBreakpointWithSourceMap(src_path)
 
-# Index 0 is the valid mapping, and modifying it to an invalid one should have no effect
+# Attempts to replace an index to an invalid mapping should have no effect.
+# Modifications to valid mappings should work.
 self.expect(
-'settings replace target.source-map 0 . "%s"' % (invalid_path),
-substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
+'settings replace target.source-map 0 . "%s" . "%s"' % (invalid_path, valid_path2),
+substrs=[
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
+],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (src_dir)]
+substrs=[
+'[0] "." -> "%s"' % (src_dir),
+'[1] "." -> "%s"' % (valid_path2),
+]
 )
 assertBreakpointWithSourceMap(src_path)
 
-# Let's clear and add the mapping in with insert-after
+# Let's clear and add the mapping back with insert-after
 self.runCmd('settings remove target.source-map 0')
 self.expect(
 'settings show target.source-map',
-endstr="target.source-map (path-map) =\n",
+substrs=['[0] "." -> "%s"' % (valid_path2)],
 )
-
-# We add a valid but useless mapping so that we can use insert-after
-another_valid_path = os.path.dirname(src_dir)
-self.runCmd('settings set target.source-map . "%s"' % (another_valid_path))
 
 self.expect(
-'settings replace target.source-map 0 . "%s"' % (invalid_path),
-substrs=['error: the replacement path doesn\'t exist: "%s"' % (invalid_path)],
+'settings insert-after target.source-map 0 . "%s" . "%s" . "%s"' \
+% (invalid_path, invalid_path2, src_dir),
+substrs=[
+'error: the replacement path doesn\'t exist: "%s"' % (invalid_path),
+'the replacement path doesn\'t exist: "%s"' % (invalid_path2),
+],
 error=True,
 )
 self.expect(
 'settings show target.source-map',
-substrs=['[0] "." -> "%s"' % (another_valid_path)]
+substrs=[
+'[0] "." -> "%s"' % (valid_path2),
+'[1] "." -> "%s"' % (src_dir),
+]
 )
 
-# Let's clear and add the mapping in with append
-self.expect('settings remove target.source-map 0')
+# Let's clear using remove and add the mapping in with append
+self.runCmd('settings remove target.source-map 1')
 self.expect(
 'settings show target.source-map',
-endstr="target.source-map (path-map

[Lldb-commits] [lldb] 9661225 - Fix LLDB debug builds

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-04-03T19:49:38-07:00
New Revision: 966122524b562aab42e7afc8f44c23ea105b27f3

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

LOG: Fix LLDB debug builds

Summary:
A recent change in ThreadPlans introduced this little compilation error.
Seems to be related to the work around https://reviews.llvm.org/D76814.

Reviewers: clayborg, labath, jingham

Reviewed By: jingham

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Target/ThreadPlan.cpp

Removed: 




diff  --git a/lldb/source/Target/ThreadPlan.cpp 
b/lldb/source/Target/ThreadPlan.cpp
index b3ff3232a0ac..d8e92b8fd0de 100644
--- a/lldb/source/Target/ThreadPlan.cpp
+++ b/lldb/source/Target/ThreadPlan.cpp
@@ -240,7 +240,7 @@ bool ThreadPlanNull::DoPlanExplainsStop(Event *event_ptr) {
   fprintf(stderr,
   "error: %s called on thread that has been destroyed (tid = 0x%" 
PRIx64
   ", ptid = 0x%" PRIx64 ")",
-  LLVM_PRETTY_FUNCTION, m_thread.GetID(), m_thread.GetProtocolID());
+  LLVM_PRETTY_FUNCTION, GetThread().GetID(), 
GetThread().GetProtocolID());
 #else
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD));
   if (log)



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

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

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: labath, clayborg.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
wallace updated this revision to Diff 254990.
wallace edited the summary of this revision.
wallace added a comment.

improve description


@labath raised a concern on the way I was skipping this test. I think that was 
fair and I found a better way.
Now I'm skipping if the Intel features library is not present and if the 
processor-trace command doesn't exist.
It's worth mentioning that the Intel library is a collection of two features: 
intel-mpx and intel-pt. Intel PT is optional and has to be enabled at 
configuration step.

I had to add ExistsUserCommand to the CommandInterpreter API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77452

Files:
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py

Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -14,22 +14,26 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+TestBase.setUp(self)
+
+plugin_so = "liblldbIntelFeatures.so"
+plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], plugin_so)
+if not os.path.exists(plugin_path):
+self.skipTest("Intel PT plugin %s missing." % (plugin_so))
+
+self.runCmd("plugin load " + plugin_path)
+if not self.dbg.GetCommandInterpreter().UserCommandExists("processor-trace"):
+self.skipTest("%s was built without Intel PT support" % (plugin_so))
+# This could happen if the user has intel-mpx support but not intel-pt.
+
 @skipIf(oslist=no_match(['linux']))
 @skipIf(archs=no_match(['i386', 'x86_64']))
 @skipIfRemote
 def test_basic_flow(self):
 """Test collection, decoding, and dumping instructions"""
-if os.environ.get('TEST_INTEL_PT') != '1':
-self.skipTest("The environment variable TEST_INTEL_PT=1 is needed to run this test.")
-
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
 
 self.build()
-
-self.runCmd("plugin load " + plugin_file)
-
 exe = self.getBuildArtifact("a.out")
 lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
 # We start tracing from main
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -216,6 +216,14 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_RECORD_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+ (const char *), cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_RECORD_METHOD(bool, SBCommandInterpreter, AliasExists, (const char *),
  cmd);
@@ -874,6 +882,8 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreter, operator bool, ());
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, CommandExists,
(const char *));
+  LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+   (const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, AliasExists,
(const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, IsActive, ());
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -91,8 +91,28 @@
 
   bool IsValid() const;
 
+  /// Determine whether a command exists in this CommandInterpreter.
+  ///
+  /// For commands registered using the LLDB API, see UserCommandExists
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Determine whether a user command exists in this CommandInterpreter.
+  /// Users commands are the ones registered using the LLDB API and are not
+  /// provided by default in LLDB.
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \r

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

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 254990.
wallace edited the summary of this revision.
wallace added a comment.

improve description


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77452

Files:
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py

Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -14,22 +14,26 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+def setUp(self):
+TestBase.setUp(self)
+
+plugin_so = "liblldbIntelFeatures.so"
+plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], plugin_so)
+if not os.path.exists(plugin_path):
+self.skipTest("Intel PT plugin %s missing." % (plugin_so))
+
+self.runCmd("plugin load " + plugin_path)
+if not self.dbg.GetCommandInterpreter().UserCommandExists("processor-trace"):
+self.skipTest("%s was built without Intel PT support" % (plugin_so))
+# This could happen if the user has intel-mpx support but not intel-pt.
+
 @skipIf(oslist=no_match(['linux']))
 @skipIf(archs=no_match(['i386', 'x86_64']))
 @skipIfRemote
 def test_basic_flow(self):
 """Test collection, decoding, and dumping instructions"""
-if os.environ.get('TEST_INTEL_PT') != '1':
-self.skipTest("The environment variable TEST_INTEL_PT=1 is needed to run this test.")
-
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
 
 self.build()
-
-self.runCmd("plugin load " + plugin_file)
-
 exe = self.getBuildArtifact("a.out")
 lldbutil.run_to_name_breakpoint(self, "main", exe_name=exe)
 # We start tracing from main
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -216,6 +216,14 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_RECORD_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+ (const char *), cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_RECORD_METHOD(bool, SBCommandInterpreter, AliasExists, (const char *),
  cmd);
@@ -874,6 +882,8 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreter, operator bool, ());
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, CommandExists,
(const char *));
+  LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+   (const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, AliasExists,
(const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, IsActive, ());
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -91,8 +91,28 @@
 
   bool IsValid() const;
 
+  /// Determine whether a command exists in this CommandInterpreter.
+  ///
+  /// For commands registered using the LLDB API, see UserCommandExists
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Determine whether a user command exists in this CommandInterpreter.
+  /// Users commands are the ones registered using the LLDB API and are not
+  /// provided by default in LLDB.
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
+  bool UserCommandExists(const char *cmd);
+
   bool AliasExists(const char *cmd);
 
   lldb::SBBroadcaster GetBroadcaster();
Index: lldb/bindings/interface/SBCommandInterpreter.i
===
--- lldb/bindings/interface/SBCommandInterpreter.i
+++ lldb/bindings/interface/SBCommandInterpreter.i
@@ -169,6 +169,9 @@
 bool
 CommandExists (const char *cmd);
 
+bool
+UserCommandExists (const 

[Lldb-commits] [PATCH] D77450: Fix LLDB debug builds

[Jim Ingham via Phabricator via lldb-commits]

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

Ah, great, thanks.  Looks like the standalone Xcode debug build doesn't pass 
LDB_CONFIGURATION_DEBUG.  That's a little odd, but the patch is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77450



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

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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

It's great to add this capability.  But if you pass a nullptr, then the exact 
previous command will be repeated.  That's sometimes what you want, but often 
not.  For instance:

(lldb) source list -f foo -l 10
...
(lldb)

You DON'T want those same source lines to be dumped again.

The full solution would be to add an optional method to the Python class 
version of the command that returns the "repeat" command string.  Then if that 
is implemented use the string it produces.

There are only a couple of commands (source list being one) that actually have 
to make up whole new command lines for the repeat command.  For most of them - 
like disassemble - the command with no arguments does "continue from where you 
left off".  So then the you return just your command name for the repeat 
command.  Maybe there's some way to add this convention in without having to go 
whole hog on exposing the repeat string behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444



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

[Lldb-commits] [PATCH] D77450: Fix LLDB debug builds

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG966122524b56: Fix LLDB debug builds (authored by Walter 
Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77450

Files:
  lldb/source/Target/ThreadPlan.cpp


Index: lldb/source/Target/ThreadPlan.cpp
===
--- lldb/source/Target/ThreadPlan.cpp
+++ lldb/source/Target/ThreadPlan.cpp
@@ -240,7 +240,7 @@
   fprintf(stderr,
   "error: %s called on thread that has been destroyed (tid = 0x%" 
PRIx64
   ", ptid = 0x%" PRIx64 ")",
-  LLVM_PRETTY_FUNCTION, m_thread.GetID(), m_thread.GetProtocolID());
+  LLVM_PRETTY_FUNCTION, GetThread().GetID(), 
GetThread().GetProtocolID());
 #else
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD));
   if (log)


Index: lldb/source/Target/ThreadPlan.cpp
===
--- lldb/source/Target/ThreadPlan.cpp
+++ lldb/source/Target/ThreadPlan.cpp
@@ -240,7 +240,7 @@
   fprintf(stderr,
   "error: %s called on thread that has been destroyed (tid = 0x%" PRIx64
   ", ptid = 0x%" PRIx64 ")",
-  LLVM_PRETTY_FUNCTION, m_thread.GetID(), m_thread.GetProtocolID());
+  LLVM_PRETTY_FUNCTION, GetThread().GetID(), GetThread().GetProtocolID());
 #else
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD));
   if (log)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3270748 - The thread plan list test is failing at least on Ubuntu Linux.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2020-04-03T20:06:11-07:00
New Revision: 327074812707d41f492a0e23940430422a9cd237

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

LOG: The thread plan list test is failing at least on Ubuntu Linux.
Mark it expected fail for now.

The test output shows that the "internal" thread listing isn't showing the
step out plan that we use to step back out of a function we're stepping into.
The internal plan listing code has nothing platform specific in it, so that
isn't the problem.

I am pretty sure the difference is that on MacOS we step into the function and 
then need to
step back out again so we push the internal plan the test is checking for.  But 
on Linux we
are able to step past the function without stepping into it.

So nothing is actually going wrong here, I just need to find a better test case 
where I
can ensure we are going to have to push a private plan.  It's probably better 
to test this
using a custom thread plan, then I can control the state of the plan stack 
better.

That's for Monday...

Added: 


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

Removed: 




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



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

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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

For instance, maybe instead of passing in a bool, you could pass in the repeat 
command?  So when you create the command, if you pass in None, you get no 
repeat, if you pass in "" you get the exact command line repeated, and if you 
pass in anything else, that will be the repeat command.  Or maybe use an enum 
with "noRepeat, exactRepeat, commandNameRepeat"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444



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

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

[walter erquinigo via Phabricator via lldb-commits]

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

Very good idea!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77444



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