[Lldb-commits] [PATCH] D53193: [LLDB] - Add support for DW_RLE_start_end entries (.debug_rnglists)

[George Rimar via Phabricator via lldb-commits]

grimar added a comment.

In https://reviews.llvm.org/D53193#1266080, @JDevlieghere wrote:

> The code and test look correct, so this LGTM but I'll leave it open for now 
> in case someone else wants to have a look too.


Thanks for looking, Jonas!


https://reviews.llvm.org/D53193



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

[Lldb-commits] [PATCH] D52461: [PDB] Introduce `PDBNameParser`

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

In https://reviews.llvm.org/D52461#1265633, @zturner wrote:

> Just handle the `anonymous namespace' thing specially before passing to 
> `CPlusPlusNameParser`.


Yes, it's an interesting idea to somehow preprocess an MSVC demangled name and 
make a GCC demangled name from it (and make an MSVC-like name back after 
parsing). But then we need to handle not only anonymous namespaces, also things 
like this:

  `operator<'::`2'::B::operator>

Such a preprocessing will be comparable to the current implementation of 
`PDBNameParser` by complexity (or even more complex). I'll try to somehow 
estimate the complexity of this approach, thanks.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52461



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

[Lldb-commits] [PATCH] D53086: [PDB] Fix flaky `variables-locations.test` after PR38857

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Yes, I mean exactly the same case. For sequences like you've written yes, the 
unwind should work, but there must be some problems with saved registers. 
`x86AssemblyInspectionEngine` doesn't handle instructions like `and %-8, %esp`, 
so the register save would work only if this instruction hadn't changed the 
`esp` value (e.g. `esp` was already aligned). Otherwise, if `esp` was changed, 
the offset to CFA in `RegisterLocation` of some register will be invalid, 
because it will not take the alignment into account.

Moreover, it is impossible to specify a location for some saved register on a 
such stack with the `CFA + offset` restore type, because we can't know how 
`esp` will be changed after `and %-8, %esp`. So I suggest to introduce one more 
frame address (along with CFA), and make it point to `esp` right after `and 
..., %esp`. So any saved register would have `AFA + offset` restore type (I've 
called for now this frame address as AFA - aligned frame address).

As for MSVC-compiled sources, the things are even more interesting. Consider 
the following program:


https://reviews.llvm.org/D53086



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

[Lldb-commits] [lldb] r344605 - Fix: Assertion failed: (!m_first_die || m_first_die == m_die_array.front()), function ExtractDIEsRWLocked

[Jan Kratochvil via lldb-commits]

Author: jankratochvil
Date: Tue Oct 16 04:38:22 2018
New Revision: 344605

URL: http://llvm.org/viewvc/llvm-project?rev=344605&view=rev
Log:
Fix: Assertion failed: (!m_first_die || m_first_die == m_die_array.front()), 
function ExtractDIEsRWLocked

xbolva00 bugreported $subj in: https://reviews.llvm.org/D46810#1247410
It can happen only from the line:
m_die_array.back().SetEmptyChildren(true);

In the case DW_TAG_compile_unit has DW_CHILDREN_yes but there is only 0 (end of
list, no children present). Therefore the assertion can fortunately happen only
with a hand-crafted DWARF or with DWARF from some suboptimal compilers.

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp?rev=344605&r1=344604&r2=344605&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp Tue Oct 16 
04:38:22 2018
@@ -261,7 +261,11 @@ void DWARFUnit::ExtractDIEsRWLocked() {
   }
 
   if (!m_die_array.empty()) {
-lldbassert(!m_first_die || m_first_die == m_die_array.front());
+if (m_first_die) {
+  // Only needed for the assertion.
+  m_first_die.SetEmptyChildren(m_die_array.front().GetEmptyChildren());
+  lldbassert(m_first_die == m_die_array.front());
+}
 m_first_die = m_die_array.front();
   }
 


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

[Lldb-commits] [PATCH] D53255: Fix: Assertion failed: (!m_first_die || m_first_die == m_die_array.front()), function ExtractDIEsRWLocked

[Jan Kratochvil via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344605: Fix: Assertion failed: (!m_first_die || m_first_die 
== m_die_array.front())… (authored by jankratochvil, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53255?vs=169598&id=169808#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53255

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp


Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -261,7 +261,11 @@
   }
 
   if (!m_die_array.empty()) {
-lldbassert(!m_first_die || m_first_die == m_die_array.front());
+if (m_first_die) {
+  // Only needed for the assertion.
+  m_first_die.SetEmptyChildren(m_die_array.front().GetEmptyChildren());
+  lldbassert(m_first_die == m_die_array.front());
+}
 m_first_die = m_die_array.front();
   }
 


Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -261,7 +261,11 @@
   }
 
   if (!m_die_array.empty()) {
-lldbassert(!m_first_die || m_first_die == m_die_array.front());
+if (m_first_die) {
+  // Only needed for the assertion.
+  m_first_die.SetEmptyChildren(m_die_array.front().GetEmptyChildren());
+  lldbassert(m_first_die == m_die_array.front());
+}
 m_first_die = m_die_array.front();
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Jan Kratochvil via Phabricator via lldb-commits]

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

As @aprantl suggested  I have merged 
`DWARFDebugInfoEntry`'s `m_empty_children` into `m_has_children`.
`m_empty_children` was implemented by https://reviews.llvm.org/rL144983. I do 
not see why @clayborg made it a separate flag, from some point of view it is 
sure cleaner but technically I do not see its purpose.
I have checked all calls of `HasChildren()` that it should not matter to them. 
The code even wants to know if there are any children - it does not matter how 
the children presence is coded in the binary.
The only problematic may be `operator==` (and `!=`) although that one is 
currently only used by my `lldbassert()` in my bugfixed 
https://reviews.llvm.org/D53255.
This patch sure has no regressions on Fedora 28 x86_64.
I do not need/request this patch, just that @aprantl asked about it.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53321

Files:
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp


Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -223,7 +223,7 @@
   // the list (saves up to 25% in C++ code), we need a way to let the
   // DIE know that it actually doesn't have children.
   if (!m_die_array.empty())
-m_die_array.back().SetEmptyChildren(true);
+m_die_array.back().SetHasChildren(false);
 }
   } else {
 die.SetParentIndex(m_die_array.size() - die_index_stack[depth - 1]);
@@ -263,7 +263,7 @@
   if (!m_die_array.empty()) {
 if (m_first_die) {
   // Only needed for the assertion.
-  m_first_die.SetEmptyChildren(m_die_array.front().GetEmptyChildren());
+  m_first_die.SetHasChildren(m_die_array.front().HasChildren());
   lldbassert(m_first_die == m_die_array.front());
 }
 m_first_die = m_die_array.front();
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -57,8 +57,7 @@
 
   DWARFDebugInfoEntry()
   : m_offset(DW_INVALID_OFFSET), m_parent_idx(0), m_sibling_idx(0),
-m_empty_children(false), m_abbr_idx(0), m_has_children(false),
-m_tag(0) {}
+m_abbr_idx(0), m_has_children(false), m_tag(0) {}
 
   explicit operator bool() const { return m_offset != DW_INVALID_OFFSET; }
   bool operator==(const DWARFDebugInfoEntry &rhs) const;
@@ -227,10 +226,10 @@
   // we don't need to store our child pointer, if we have a child it will
   // be the next entry in the list...
   DWARFDebugInfoEntry *GetFirstChild() {
-return (HasChildren() && !m_empty_children) ? this + 1 : NULL;
+return HasChildren() ? this + 1 : NULL;
   }
   const DWARFDebugInfoEntry *GetFirstChild() const {
-return (HasChildren() && !m_empty_children) ? this + 1 : NULL;
+return HasChildren() ? this + 1 : NULL;
   }
 
   void GetDeclContextDIEs(DWARFUnit *cu,
@@ -271,10 +270,6 @@
 
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
-  bool GetEmptyChildren() const { return m_empty_children; }
-
-  void SetEmptyChildren(bool b) { m_empty_children = b; }
-
   static void
   DumpDIECollection(lldb_private::Stream &strm,
 DWARFDebugInfoEntry::collection &die_collection);
@@ -284,11 +279,12 @@
   m_offset; // Offset within the .debug_info of the start of this entry
   uint32_t m_parent_idx; // How many to subtract from "this" to get the parent.
  // If zero this die has no parent
-  uint32_t m_sibling_idx : 31, // How many to add to "this" to get the sibling.
-  m_empty_children : 1;// If a DIE says it had children, yet it just
-   // contained a NULL tag, this will be set.
+  uint32_t m_sibling_idx; // How many to add to "this" to get the sibling.
   uint32_t m_abbr_idx : DIE_ABBR_IDX_BITSIZE,
 m_has_children : 1, // Set to 1 if this DIE has 
children
+// It is zero if a DIE says it had
+// children, yet it just contained
+// a NULL tag.
 m_tag : 16; // A copy of the DW_TAG value so we don't
 // have to go through the compile unit
 // abbrev table
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===

[Lldb-commits] [PATCH] D53255: Fix: Assertion failed: (!m_first_die || m_first_die == m_die_array.front()), function ExtractDIEsRWLocked

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In https://reviews.llvm.org/D53255#1265520, @aprantl wrote:

> Out of curiosity: why do we allow for both an empty array and an empty 
> children sentinel? Is that distinction useful?


Filed for that new https://reviews.llvm.org/D53321.


Repository:
  rL LLVM

https://reviews.llvm.org/D53255



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

[Lldb-commits] [PATCH] D53193: [LLDB] - Add support for DW_RLE_start_end entries (.debug_rnglists)

[Greg Clayton via Phabricator via lldb-commits]

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

LGTM


https://reviews.llvm.org/D53193



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

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

m_has_children was used to represent what was in the DWARF in the byte that 
follows the DW_TAG. m_empty_children was used for DIEs that said they had 
children but actually only contain a single NULL tag. It is fine to not 
differentiate between the two.




Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h:282
  // If zero this die has no parent
-  uint32_t m_sibling_idx : 31, // How many to add to "this" to get the sibling.
-  m_empty_children : 1;// If a DIE says it had children, yet it just
-   // contained a NULL tag, this will be set.
+  uint32_t m_sibling_idx; // How many to add to "this" to get the sibling.
   uint32_t m_abbr_idx : DIE_ABBR_IDX_BITSIZE,

We might want to move m_has_children where m_empty_children used to be and give 
a bit back to m_abbr_idx by increasing DIE_ABBR_IDX_BITSIZE by 1. We have a few 
bits to spare in m_sibling_idx since it is the number of dies to advance by to 
get to the sibling. Since DIEs at the very least are a few bytes, having a 2GB 
advance instead of 4GB is plenty.



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h:285-287
+// It is zero if a DIE says it had
+// children, yet it just contained
+// a NULL tag.

This comment is wrong. The DWARF encodes if a DIE has children in the 
.debug_info, but we may override this value if the DIE only contains an NULL 
terminating DIE.

It should read something like:
```
// If it is zero, then the DIE doesn't have children, or the 
// DWARF claimed it had children but the DIE only contained 
// a single NULL terminating child.
```




Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53321



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

[Lldb-commits] [PATCH] D52461: [PDB] Introduce `PDBNameParser`

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D52461#1266302, @aleksandr.urakov wrote:

>   `operator<'::`2'::B::operator>
>


The reason we had to use clang lexer for parsing itanium names is because 
parsing itanium demangled names is tricky precisely for cases like these. If 
the MSVC demangler makes these cases trivial by enclosing them in quotes, maybe 
a separate (simpler) parser is not such a bad idea.

However, I still think this should be done within the scope of 
`CPlusPlusLanguage::MethodName` otherwise, you'll have to special case MSVC for 
all existing uses of this class.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52461



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

[Lldb-commits] [PATCH] D52461: [PDB] Introduce `PDBNameParser`

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Yes, it's simpler to move it to the `CPlusPlusLanguage::MethodName` (or 
`CPlusPlusNameParser`?) I think. The only question left is how to differentiate 
MSVC demangled names from others? May be it would be ok to treat name as an 
MSVC name if it contains a grave accent? Because we probably already can parse 
MSVC names without grave accents with `CPlusPlusLanguage::MethodName`.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52461



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

[Lldb-commits] [PATCH] D53331: [lldbsuite] Mark the TestScriptedResolver tests as XFAIL on Windows

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova created this revision.
stella.stamenova added reviewers: asmith, zturner.
Herald added subscribers: lldb-commits, abidh.

They fail similarly to some of the other breakpoint tests on Windows, so I 
suspect the cause is the same. I've linked to the same bug.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53331

Files:
  
packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py


Index: 
packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
===
--- 
packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
+++ 
packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
@@ -10,6 +10,7 @@
 import re
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 
@@ -19,17 +20,20 @@
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 def test_scripted_resolver(self):
 """Use a scripted resolver to set a by symbol name breakpoint"""
 self.build()
 self.do_test()
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 def test_search_depths(self):
 """ Make sure we are called at the right depths depending on what we 
return
 from __get_depth__"""
 self.build()
 self.do_test_depths()
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 def test_command_line(self):
 """ Make sure we are called at the right depths depending on what we 
return
 from __get_depth__"""


Index: packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
===
--- packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
+++ packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
@@ -10,6 +10,7 @@
 import re
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 
@@ -19,17 +20,20 @@
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 def test_scripted_resolver(self):
 """Use a scripted resolver to set a by symbol name breakpoint"""
 self.build()
 self.do_test()
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 def test_search_depths(self):
 """ Make sure we are called at the right depths depending on what we return
 from __get_depth__"""
 self.build()
 self.do_test_depths()
 
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 def test_command_line(self):
 """ Make sure we are called at the right depths depending on what we return
 from __get_depth__"""
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r344623 - [lldbsuite] Fix the mac version decorator to work on non-mac platforms

[Stella Stamenova via lldb-commits]

Author: stella.stamenova
Date: Tue Oct 16 10:13:45 2018
New Revision: 344623

URL: http://llvm.org/viewvc/llvm-project?rev=344623&view=rev
Log:
[lldbsuite] Fix the mac version decorator to work on non-mac platforms

Summary: On non-mac platforms, mac_ver returns an empty string which when 
converted to LooseVersion has no "version" property. This causes a failure when 
the decorator executes. Instead, check whether the value returned from mac_ver 
is an empty string and avoid the LooseVersion comparison.

Reviewers: labath, davide, asmith, shafik, jingham

Reviewed By: jingham

Subscribers: jingham, lldb-commits

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

Modified:
lldb/trunk/packages/Python/lldbsuite/test/decorators.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/decorators.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/decorators.py?rev=344623&r1=344622&r2=344623&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/decorators.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/decorators.py Tue Oct 16 10:13:45 
2018
@@ -192,10 +192,10 @@ def _decorateTest(mode,
 py_version is None) or _check_expected_version(
 py_version[0], py_version[1], sys.version_info)
 skip_for_macos_version = (macos_version is None) or (
-_check_expected_version(
+(platform.mac_ver()[0] != "") and (_check_expected_version(
 macos_version[0],
 macos_version[1],
-platform.mac_ver()[0]))
+platform.mac_ver()[0])))
 
 # For the test to be skipped, all specified (e.g. not None) parameters 
must be True.
 # An unspecified parameter means "any", so those are marked skip by 
default.  And we skip


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

[Lldb-commits] [PATCH] D53208: [lldbsuite] Fix the mac version decorator to work on non-mac platforms

[Stella Stamenova via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB344623: [lldbsuite] Fix the mac version decorator to work 
on non-mac platforms (authored by stella.stamenova, committed by ).

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53208

Files:
  packages/Python/lldbsuite/test/decorators.py


Index: packages/Python/lldbsuite/test/decorators.py
===
--- packages/Python/lldbsuite/test/decorators.py
+++ packages/Python/lldbsuite/test/decorators.py
@@ -192,10 +192,10 @@
 py_version is None) or _check_expected_version(
 py_version[0], py_version[1], sys.version_info)
 skip_for_macos_version = (macos_version is None) or (
-_check_expected_version(
+(platform.mac_ver()[0] != "") and (_check_expected_version(
 macos_version[0],
 macos_version[1],
-platform.mac_ver()[0]))
+platform.mac_ver()[0])))
 
 # For the test to be skipped, all specified (e.g. not None) parameters 
must be True.
 # An unspecified parameter means "any", so those are marked skip by 
default.  And we skip


Index: packages/Python/lldbsuite/test/decorators.py
===
--- packages/Python/lldbsuite/test/decorators.py
+++ packages/Python/lldbsuite/test/decorators.py
@@ -192,10 +192,10 @@
 py_version is None) or _check_expected_version(
 py_version[0], py_version[1], sys.version_info)
 skip_for_macos_version = (macos_version is None) or (
-_check_expected_version(
+(platform.mac_ver()[0] != "") and (_check_expected_version(
 macos_version[0],
 macos_version[1],
-platform.mac_ver()[0]))
+platform.mac_ver()[0])))
 
 # For the test to be skipped, all specified (e.g. not None) parameters must be True.
 # An unspecified parameter means "any", so those are marked skip by default.  And we skip
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Jan Kratochvil via Phabricator via lldb-commits]

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



Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h:285-287
+// It is zero if a DIE says it had
+// children, yet it just contained
+// a NULL tag.

clayborg wrote:
> This comment is wrong. The DWARF encodes if a DIE has children in the 
> .debug_info, but we may override this value if the DIE only contains an NULL 
> terminating DIE.
> 
> It should read something like:
> ```
> // If it is zero, then the DIE doesn't have children, or the 
> // DWARF claimed it had children but the DIE only contained 
> // a single NULL terminating child.
> ```
> 
> 
I cannot say it was wrong but I agree your rewording is much better.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53321



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

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 169850.
jankratochvil marked an inline comment as done.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53321

Files:
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -223,7 +223,7 @@
   // the list (saves up to 25% in C++ code), we need a way to let the
   // DIE know that it actually doesn't have children.
   if (!m_die_array.empty())
-m_die_array.back().SetEmptyChildren(true);
+m_die_array.back().SetHasChildren(false);
 }
   } else {
 die.SetParentIndex(m_die_array.size() - die_index_stack[depth - 1]);
@@ -263,7 +263,7 @@
   if (!m_die_array.empty()) {
 if (m_first_die) {
   // Only needed for the assertion.
-  m_first_die.SetEmptyChildren(m_die_array.front().GetEmptyChildren());
+  m_first_die.SetHasChildren(m_die_array.front().HasChildren());
   lldbassert(m_first_die == m_die_array.front());
 }
 m_first_die = m_die_array.front();
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -43,7 +43,7 @@
 class DWARFDeclContext;
 
 #define DIE_SIBLING_IDX_BITSIZE 31
-#define DIE_ABBR_IDX_BITSIZE 15
+#define DIE_ABBR_IDX_BITSIZE 16
 
 class DWARFDebugInfoEntry {
 public:
@@ -57,8 +57,7 @@
 
   DWARFDebugInfoEntry()
   : m_offset(DW_INVALID_OFFSET), m_parent_idx(0), m_sibling_idx(0),
-m_empty_children(false), m_abbr_idx(0), m_has_children(false),
-m_tag(0) {}
+m_abbr_idx(0), m_has_children(false), m_tag(0) {}
 
   explicit operator bool() const { return m_offset != DW_INVALID_OFFSET; }
   bool operator==(const DWARFDebugInfoEntry &rhs) const;
@@ -227,10 +226,10 @@
   // we don't need to store our child pointer, if we have a child it will
   // be the next entry in the list...
   DWARFDebugInfoEntry *GetFirstChild() {
-return (HasChildren() && !m_empty_children) ? this + 1 : NULL;
+return HasChildren() ? this + 1 : NULL;
   }
   const DWARFDebugInfoEntry *GetFirstChild() const {
-return (HasChildren() && !m_empty_children) ? this + 1 : NULL;
+return HasChildren() ? this + 1 : NULL;
   }
 
   void GetDeclContextDIEs(DWARFUnit *cu,
@@ -271,10 +270,6 @@
 
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
-  bool GetEmptyChildren() const { return m_empty_children; }
-
-  void SetEmptyChildren(bool b) { m_empty_children = b; }
-
   static void
   DumpDIECollection(lldb_private::Stream &strm,
 DWARFDebugInfoEntry::collection &die_collection);
@@ -285,10 +280,11 @@
   uint32_t m_parent_idx; // How many to subtract from "this" to get the parent.
  // If zero this die has no parent
   uint32_t m_sibling_idx : 31, // How many to add to "this" to get the sibling.
-  m_empty_children : 1;// If a DIE says it had children, yet it just
-   // contained a NULL tag, this will be set.
+  // If it is zero, then the DIE doesn't have children, or the
+  // DWARF claimed it had children but the DIE only contained
+  // a single NULL terminating child.
+  m_has_children : 1;
   uint32_t m_abbr_idx : DIE_ABBR_IDX_BITSIZE,
-m_has_children : 1, // Set to 1 if this DIE has children
 m_tag : 16; // A copy of the DW_TAG value so we don't
 // have to go through the compile unit
 // abbrev table
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -40,7 +40,6 @@
   m_offset = *offset_ptr;
   m_parent_idx = 0;
   m_sibling_idx = 0;
-  m_empty_children = false;
   const uint64_t abbr_idx = debug_info_data.GetULEB128(offset_ptr);
   assert(abbr_idx < (1 << DIE_ABBR_IDX_BITSIZE));
   m_abbr_idx = abbr_idx;
@@ -1836,7 +1835,6 @@
 bool DWARFDebugInfoEntry::operator==(const DWARFDebugInfoEntry &rhs) const {
   return m_offset == rhs.m_offset && m_parent_idx == rhs.m_parent_idx &&
  m_sibling_idx == rhs.m_sibling_idx &&
- m_empty_children == rhs.m_empty_children &&
  m_abbr_idx == rhs.m_abbr_idx && m_has_children == rhs.m_has_children &&
  m_tag == rhs.m_tag;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

See inlined comment.




Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h:287
+  m_has_children : 1;
   uint32_t m_abbr_idx : DIE_ABBR_IDX_BITSIZE,
 m_tag : 16; // A copy of the DW_TAG value so we don't

Might be worth changing these to be uint16_t, removing DIE_ABBR_IDX_BITSIZE and 
changing any code that was using that to make sure that the value is <= 
UINT16_MAX:

```
uint16_t m_abbr_idx;
uint16_t m_tag;
```


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53321



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

[Lldb-commits] [lldb] r344626 - Simplify LocateDSYMInVincinityOfExecutable by moving

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Tue Oct 16 10:26:04 2018
New Revision: 344626

URL: http://llvm.org/viewvc/llvm-project?rev=344626&view=rev
Log:
Simplify LocateDSYMInVincinityOfExecutable by moving
some redundant code into a separate function, 
LookForDsymNextToExecutablePath, and having that function
also look for .dSYM.yaa files in addition to .dSYM
bundles.

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

 


Modified:
lldb/trunk/source/Host/common/Symbols.cpp

Modified: lldb/trunk/source/Host/common/Symbols.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Symbols.cpp?rev=344626&r1=344625&r2=344626&view=diff
==
--- lldb/trunk/source/Host/common/Symbols.cpp (original)
+++ lldb/trunk/source/Host/common/Symbols.cpp Tue Oct 16 10:26:04 2018
@@ -65,96 +65,134 @@ static bool FileAtPathContainsArchAndUUI
   return false;
 }
 
+// Given a binary exec_fspec, and a ModuleSpec with an architecture/uuid,
+// return true if there is a matching dSYM bundle next to the exec_fspec,
+// and return that value in dsym_fspec.  
+// If there is a .dSYM.yaa compressed archive next to the exec_fspec, 
+// call through Symbols::DownloadObjectAndSymbolFile to download the
+// expanded/uncompressed dSYM and return that filepath in dsym_fspec.
+
+static bool LookForDsymNextToExecutablePath(const ModuleSpec &mod_spec,
+const FileSpec &exec_fspec,
+FileSpec &dsym_fspec) {
+  ConstString filename = exec_fspec.GetFilename();
+  FileSpec dsym_directory = exec_fspec;
+  dsym_directory.RemoveLastPathComponent();
+
+  std::string dsym_filename = filename.AsCString();
+  dsym_filename += ".dSYM";
+  dsym_directory.AppendPathComponent(dsym_filename);
+  dsym_directory.AppendPathComponent("Contents");
+  dsym_directory.AppendPathComponent("Resources");
+  dsym_directory.AppendPathComponent("DWARF");
+  
+  if (dsym_directory.Exists()) {
+
+// See if the binary name exists in the dSYM DWARF
+// subdir.
+FileSpec dsym_fspec = dsym_directory;
+dsym_fspec.AppendPathComponent(filename.AsCString());
+if (dsym_fspec.Exists()
+&& FileAtPathContainsArchAndUUID(dsym_fspec, 
+ mod_spec.GetArchitecturePtr(),
+ mod_spec.GetUUIDPtr())) {
+  return true;
+}
+
+// See if we have "../CF.framework" - so we'll look for
+// CF.framework.dSYM/Contents/Resources/DWARF/CF
+// We need to drop the last suffix after '.' to match 
+// 'CF' in the DWARF subdir.
+std::string binary_name (filename.AsCString());
+auto last_dot = binary_name.find_last_of('.');
+if (last_dot != std::string::npos) {
+  binary_name.erase(last_dot);
+  dsym_fspec = dsym_directory;
+  dsym_fspec.AppendPathComponent(binary_name);
+  if (dsym_fspec.Exists()
+  && FileAtPathContainsArchAndUUID(dsym_fspec, 
+   mod_spec.GetArchitecturePtr(),
+   mod_spec.GetUUIDPtr())) {
+return true;
+  }
+}
+  } 
+  
+  // See if we have a .dSYM.yaa next to this executable path.
+  FileSpec dsym_yaa_fspec = exec_fspec;
+  dsym_yaa_fspec.RemoveLastPathComponent();
+  std::string dsym_yaa_filename = filename.AsCString();
+  dsym_yaa_filename += ".dSYM.yaa";
+  dsym_yaa_fspec.AppendPathComponent(dsym_yaa_filename);
+
+  if (dsym_yaa_fspec.Exists()) {
+ModuleSpec mutable_mod_spec = mod_spec;
+if (Symbols::DownloadObjectAndSymbolFile (mutable_mod_spec, true)
+&& mutable_mod_spec.GetSymbolFileSpec().Exists()) {
+  dsym_fspec = mutable_mod_spec.GetSymbolFileSpec();
+  return true;
+}
+  }
+
+  return false;
+}
+
+// Given a ModuleSpec with a FileSpec and optionally uuid/architecture
+// filled in, look for a .dSYM bundle next to that binary.  Returns true
+// if a .dSYM bundle is found, and that path is returned in the dsym_fspec
+// FileSpec.
+//
+// This routine looks a few directory layers above the given exec_path -
+// exec_path might be /System/Library/Frameworks/CF.framework/CF and the
+// dSYM might be /System/Library/Frameworks/CF.framework.dSYM.
+//
+// If there is a .dSYM.yaa compressed archive found next to the binary,
+// we'll call DownloadObjectAndSymbolFile to expand it into a plain .dSYM
+
 static bool LocateDSYMInVincinityOfExecutable(const ModuleSpec &module_spec,
   FileSpec &dsym_fspec) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
-  const FileSpec *exec_fspec = module_spec.GetFileSpecPtr();
+  const FileSpec &exec_fspec = module_spec.GetFileSpec();
   if (exec_fspec) {
-char path[PATH_MAX];
-if (exec_fspec->GetPath(path, sizeof(path))) {
-  // Make sure the module isn't already just a dSYM file...
-  if (strcasestr(path, ".dSYM/Contents/Resources/DWARF") == NULL) {
+if (::LookForDsymNextToExe

[Lldb-commits] [PATCH] D53305: Simplify LocateDSYMInVincinityOfExecutable a bit, and call Symbols::DownloadObjectAndSymbolFile for .dSYM.yaa archives

[Phabricator 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 rLLDB344626: Simplify LocateDSYMInVincinityOfExecutable by 
moving (authored by jmolenda, committed by ).

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53305

Files:
  source/Host/common/Symbols.cpp

Index: source/Host/common/Symbols.cpp
===
--- source/Host/common/Symbols.cpp
+++ source/Host/common/Symbols.cpp
@@ -65,96 +65,134 @@
   return false;
 }
 
+// Given a binary exec_fspec, and a ModuleSpec with an architecture/uuid,
+// return true if there is a matching dSYM bundle next to the exec_fspec,
+// and return that value in dsym_fspec.  
+// If there is a .dSYM.yaa compressed archive next to the exec_fspec, 
+// call through Symbols::DownloadObjectAndSymbolFile to download the
+// expanded/uncompressed dSYM and return that filepath in dsym_fspec.
+
+static bool LookForDsymNextToExecutablePath(const ModuleSpec &mod_spec,
+const FileSpec &exec_fspec,
+FileSpec &dsym_fspec) {
+  ConstString filename = exec_fspec.GetFilename();
+  FileSpec dsym_directory = exec_fspec;
+  dsym_directory.RemoveLastPathComponent();
+
+  std::string dsym_filename = filename.AsCString();
+  dsym_filename += ".dSYM";
+  dsym_directory.AppendPathComponent(dsym_filename);
+  dsym_directory.AppendPathComponent("Contents");
+  dsym_directory.AppendPathComponent("Resources");
+  dsym_directory.AppendPathComponent("DWARF");
+  
+  if (dsym_directory.Exists()) {
+
+// See if the binary name exists in the dSYM DWARF
+// subdir.
+FileSpec dsym_fspec = dsym_directory;
+dsym_fspec.AppendPathComponent(filename.AsCString());
+if (dsym_fspec.Exists()
+&& FileAtPathContainsArchAndUUID(dsym_fspec, 
+ mod_spec.GetArchitecturePtr(),
+ mod_spec.GetUUIDPtr())) {
+  return true;
+}
+
+// See if we have "../CF.framework" - so we'll look for
+// CF.framework.dSYM/Contents/Resources/DWARF/CF
+// We need to drop the last suffix after '.' to match 
+// 'CF' in the DWARF subdir.
+std::string binary_name (filename.AsCString());
+auto last_dot = binary_name.find_last_of('.');
+if (last_dot != std::string::npos) {
+  binary_name.erase(last_dot);
+  dsym_fspec = dsym_directory;
+  dsym_fspec.AppendPathComponent(binary_name);
+  if (dsym_fspec.Exists()
+  && FileAtPathContainsArchAndUUID(dsym_fspec, 
+   mod_spec.GetArchitecturePtr(),
+   mod_spec.GetUUIDPtr())) {
+return true;
+  }
+}
+  } 
+  
+  // See if we have a .dSYM.yaa next to this executable path.
+  FileSpec dsym_yaa_fspec = exec_fspec;
+  dsym_yaa_fspec.RemoveLastPathComponent();
+  std::string dsym_yaa_filename = filename.AsCString();
+  dsym_yaa_filename += ".dSYM.yaa";
+  dsym_yaa_fspec.AppendPathComponent(dsym_yaa_filename);
+
+  if (dsym_yaa_fspec.Exists()) {
+ModuleSpec mutable_mod_spec = mod_spec;
+if (Symbols::DownloadObjectAndSymbolFile (mutable_mod_spec, true)
+&& mutable_mod_spec.GetSymbolFileSpec().Exists()) {
+  dsym_fspec = mutable_mod_spec.GetSymbolFileSpec();
+  return true;
+}
+  }
+
+  return false;
+}
+
+// Given a ModuleSpec with a FileSpec and optionally uuid/architecture
+// filled in, look for a .dSYM bundle next to that binary.  Returns true
+// if a .dSYM bundle is found, and that path is returned in the dsym_fspec
+// FileSpec.
+//
+// This routine looks a few directory layers above the given exec_path -
+// exec_path might be /System/Library/Frameworks/CF.framework/CF and the
+// dSYM might be /System/Library/Frameworks/CF.framework.dSYM.
+//
+// If there is a .dSYM.yaa compressed archive found next to the binary,
+// we'll call DownloadObjectAndSymbolFile to expand it into a plain .dSYM
+
 static bool LocateDSYMInVincinityOfExecutable(const ModuleSpec &module_spec,
   FileSpec &dsym_fspec) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
-  const FileSpec *exec_fspec = module_spec.GetFileSpecPtr();
+  const FileSpec &exec_fspec = module_spec.GetFileSpec();
   if (exec_fspec) {
-char path[PATH_MAX];
-if (exec_fspec->GetPath(path, sizeof(path))) {
-  // Make sure the module isn't already just a dSYM file...
-  if (strcasestr(path, ".dSYM/Contents/Resources/DWARF") == NULL) {
+if (::LookForDsymNextToExecutablePath (module_spec, exec_fspec, dsym_fspec)) {
 if (log) {
-  if (module_spec.GetUUIDPtr() && module_spec.GetUUIDPtr()->IsValid()) {
-log->Printf(
-"Searching for dSYM bundle next to executable %s, UUID %s",
-path, module_spec.GetUUIDPtr()->GetAsString().c_str());
-

[Lldb-commits] [lldb] r344628 - For a built & test bot, add an environment variable PLATFORM_SDK_DIRECTORY,

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Tue Oct 16 10:31:33 2018
New Revision: 344628

URL: http://llvm.org/viewvc/llvm-project?rev=344628&view=rev
Log:
For a built & test bot, add an environment variable PLATFORM_SDK_DIRECTORY,
which PlatformRemoteDarwinDevice::UpdateSDKDirectoryInfosIfNeeded
which examine for any additional SDK directories when it is
constructing its list.





Modified:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp

Modified: 
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp?rev=344628&r1=344627&r2=344628&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp 
(original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp 
Tue Oct 16 10:31:33 2018
@@ -231,6 +231,29 @@ bool PlatformRemoteDarwinDevice::UpdateS
   }
 }
   }
+
+  const char *addtional_platform_dirs = getenv("PLATFORM_SDK_DIRECTORY");
+  if (addtional_platform_dirs) {
+SDKDirectoryInfoCollection env_var_sdk_directory_infos;
+FileSpec::EnumerateDirectory(addtional_platform_dirs, find_directories,
+ find_files, find_other,
+ 
GetContainedFilesIntoVectorOfStringsCallback,
+ &env_var_sdk_directory_infos);
+FileSpec sdk_symbols_symlink_fspec;
+for (const auto &sdk_directory_info : env_var_sdk_directory_infos) {
+  sdk_symbols_symlink_fspec = sdk_directory_info.directory;
+  sdk_symbols_symlink_fspec.AppendPathComponent("Symbols");
+  if (sdk_symbols_symlink_fspec.Exists()) {
+m_sdk_directory_infos.push_back(sdk_directory_info);
+if (log) {
+  
log->Printf("PlatformRemoteDarwinDevice::UpdateSDKDirectoryInfosIfNeeded "
+  "added env var SDK directory %s",
+  sdk_symbols_symlink_fspec.GetPath().c_str());
+}
+  }
+}
+  }
+
 }
   }
   return !m_sdk_directory_infos.empty();


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

Re: [Lldb-commits] [lldb] r344628 - For a built & test bot, add an environment variable PLATFORM_SDK_DIRECTORY,

[Greg Clayton via lldb-commits]

Would it be possible to use a setting instead of an environment variable? We 
could make some settings initialize or append addition values  from an 
environment variables by building that logic into the code?

> On Oct 16, 2018, at 10:31 AM, Jason Molenda via lldb-commits 
>  wrote:
> 
> Author: jmolenda
> Date: Tue Oct 16 10:31:33 2018
> New Revision: 344628
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=344628&view=rev
> Log:
> For a built & test bot, add an environment variable PLATFORM_SDK_DIRECTORY,
> which PlatformRemoteDarwinDevice::UpdateSDKDirectoryInfosIfNeeded
> which examine for any additional SDK directories when it is
> constructing its list.
> 
> 
> 
> 
> 
> Modified:
>lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
> 
> Modified: 
> lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp?rev=344628&r1=344627&r2=344628&view=diff
> ==
> --- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp 
> (original)
> +++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp 
> Tue Oct 16 10:31:33 2018
> @@ -231,6 +231,29 @@ bool PlatformRemoteDarwinDevice::UpdateS
>   }
> }
>   }
> +
> +  const char *addtional_platform_dirs = getenv("PLATFORM_SDK_DIRECTORY");
> +  if (addtional_platform_dirs) {
> +SDKDirectoryInfoCollection env_var_sdk_directory_infos;
> +FileSpec::EnumerateDirectory(addtional_platform_dirs, 
> find_directories,
> + find_files, find_other,
> + 
> GetContainedFilesIntoVectorOfStringsCallback,
> + &env_var_sdk_directory_infos);
> +FileSpec sdk_symbols_symlink_fspec;
> +for (const auto &sdk_directory_info : env_var_sdk_directory_infos) {
> +  sdk_symbols_symlink_fspec = sdk_directory_info.directory;
> +  sdk_symbols_symlink_fspec.AppendPathComponent("Symbols");
> +  if (sdk_symbols_symlink_fspec.Exists()) {
> +m_sdk_directory_infos.push_back(sdk_directory_info);
> +if (log) {
> +  
> log->Printf("PlatformRemoteDarwinDevice::UpdateSDKDirectoryInfosIfNeeded "
> +  "added env var SDK directory %s",
> +  sdk_symbols_symlink_fspec.GetPath().c_str());
> +}
> +  }
> +}
> +  }
> +
> }
>   }
>   return !m_sdk_directory_infos.empty();
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

Re: [Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Stella Stamenova via lldb-commits]

The windows error is because the names are different, as you expected:

AssertionError: 'void sink(void)' != 'sink()'

You can probably update the test to look for a different name on Windows 
(though if I recall correctly, different versions of the DIA sdk provide 
different detail on the names, so that might not be robust either) or look for 
a substring in the full name.

I’ll look into the Linux error as well and let you know what I find.


From: v...@apple.com 
Sent: Monday, October 15, 2018 8:34 PM
To: Frédéric Riss 
Cc: reviews+d50478+public+7e86b794a0909...@reviews.llvm.org; Adrian Prantl 
; paul.robin...@sony.com; jdevliegh...@apple.com; Jim Ingham 
; ztur...@google.com; Stella Stamenova 
; abidh@gmail.com; teempe...@gmail.com; 
sgraen...@apple.com; mgr...@codeaurora.org; dblai...@gmail.com; 
lldb-commits@lists.llvm.org
Subject: Re: [PATCH] D50478: Add support for artificial tail call frames




On Oct 15, 2018, at 4:46 PM, Frédéric Riss 
mailto:fr...@apple.com>> wrote:




On Oct 15, 2018, at 4:40 PM, Vedant Kumar 
mailto:v...@apple.com>> wrote:




On Oct 15, 2018, at 3:47 PM, Stella Stamenova via Phabricator 
mailto:revi...@reviews.llvm.org>> wrote:

stella.stamenova added a comment.

In 
https://reviews.llvm.org/D50478#1262717,
 @vsk wrote:


In 
https://reviews.llvm.org/D50478#1262710,
 @stella.stamenova wrote:


Unfortunately, the bots are broken because of the FileCheck issue, so I can't 
confirm with them, but I see a number of these tests fail in our local testing. 
Some fail on both Windows and Linux and some just fail on Linux. Here are the 
failing tests:

Linux:
lldb-Suite :: 
functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
lldb-Suite :: 
functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
lldb-Suite :: 
functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
lldb-Suite :: 
functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
lldb-Suite :: 
functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
lldb-Suite :: 
functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
lldb-Suite :: 
functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py

Windows:
lldb-Suite :: 
functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
lldb-Suite :: 
functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py


Let me know what you need to investigate.


Strange, I didn't get any bot failure notifications in the days after this 
landed. Could you share the output from the failing tests?


All the failures on Windows are happening when validating the function name. 
For example:

==

FAIL: test_tail_call_frame_sbapi (TestTailCallFrameSBAPI.TestTailCallFrameSBAPI)

--

Traceback (most recent call last):

  File 
"E:\_work\55\s\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\tail_call_frames\sbapi_support\TestTailCallFrameSBAPI.py",
 line 19, in test_tail_call_frame_sbapi

self.do_test()

  File 
"E:\_work\55\s\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\tail_call_frames\sbapi_support\TestTailCallFrameSBAPI.py",
 line 64, in do_test

self.assertTrue(frame.GetDisplayFunctionName() == name)

It could be that the display name of a function is formatted differently on 
Windows. Do you have an easy way of determining what 
frame.GetDisplayFunctionName() is?

If you use assertEqual(a,b) instead of assertTrue, it will print out the values 
and make it easier to debug.

Thanks, done in r344581.

vedant



Fred





AssertionError: False is not True

Config=x86_64-E:\_work\55\b\LLVMBuild\Release\bin\clang.exe

--

There are several different failures on Linux. Here's the first one:

FAIL: LLDB (/vstsdrive/_work/38/b/LLVMBuild/bin/clang-8-x86_64) :: test_dwarf 
(lldbsuite.test.lldbtest.TestDisambiguateCallSite)

--- FileCheck trace (code=1) ---
/vstsdrive/_work/38/b/LLVMBuild/bin/FileCheck 
/vstsdrive/_work/38/s/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_fr

[Lldb-commits] [lldb] r344633 - Don't run TestBreakpointIt.py on arm64 devices;

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Tue Oct 16 11:11:17 2018
New Revision: 344633

URL: http://llvm.org/viewvc/llvm-project?rev=344633&view=rev
Log:
Don't run TestBreakpointIt.py on arm64 devices;
it is armv7 specific.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py?rev=344633&r1=344632&r2=344633&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py 
Tue Oct 16 11:11:17 2018
@@ -19,6 +19,7 @@ class TestBreakpointIt(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIf(archs=no_match(["arm"]))
+@skipIf(archs=["arm64", "arm64e"])
 def test_false(self):
 self.build()
 exe = self.getBuildArtifact("a.out")
@@ -32,6 +33,7 @@ class TestBreakpointIt(TestBase):
 "Breakpoint does not get hit")
 
 @skipIf(archs=no_match(["arm"]))
+@skipIf(archs=["arm64", "arm64e"])
 def test_true(self):
 self.build()
 exe = self.getBuildArtifact("a.out")


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

[Lldb-commits] [lldb] r344634 - Use a relaxed substring check for function names in a test

[Vedant Kumar via lldb-commits]

Author: vedantk
Date: Tue Oct 16 11:13:42 2018
New Revision: 344634

URL: http://llvm.org/viewvc/llvm-project?rev=344634&view=rev
Log:
Use a relaxed substring check for function names in a test

The TestTailCallFrameSBAPI.py test checks that function names in a
backtrace are equal to an expected value.

Use a relaxed substring check because function dislpay names are
platform-dependent. E.g we see "void sink(void)" on Windows, but "sink()" on
Darwin. This seems like a bug -- just work around it for now.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py?rev=344634&r1=344633&r2=344634&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
 Tue Oct 16 11:13:42 2018
@@ -57,9 +57,14 @@ class TestTailCallFrameSBAPI(TestBase):
 #   frame #2: ... a.out`func2() at main.cpp:18:62 [opt]
 #   frame #3: ... a.out`func1() at main.cpp:18:85 [opt] [artificial]
 #   frame #4: ... a.out`main at main.cpp:23:3 [opt]
-names = ["sink()", "func3()", "func2()", "func1()", "main"]
+names = ["sink", "func3", "func2", "func1", "main"]
 artificiality = [False, True, False, True, False]
 for idx, (name, is_artificial) in enumerate(zip(names, artificiality)):
 frame = thread.GetFrameAtIndex(idx)
-self.assertEqual(frame.GetDisplayFunctionName(), name)
+
+# Use a relaxed substring check because function dislpay names are
+# platform-dependent. E.g we see "void sink(void)" on Windows, but
+# "sink()" on Darwin. This seems like a bug -- just work around it
+# for now.
+self.assertTrue(name in frame.GetDisplayFunctionName())
 self.assertEqual(frame.IsArtificial(), is_artificial)


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

[Lldb-commits] [lldb] r344635 - Tiny testsuite tweaks. Don't run the apple simulator

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Tue Oct 16 11:14:30 2018
New Revision: 344635

URL: http://llvm.org/viewvc/llvm-project?rev=344635&view=rev
Log:
Tiny testsuite tweaks.  Don't run the apple simulator
tests when targetting a device.  Add an include to 
safe-to-call-func to work around a modules issue with
a certain combination of header files.  Add rules for
Darwin systems to ad-hoc codesign binaries that the
testsuite builds.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/macosx/safe-to-func-call/main.c
lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/macosx/safe-to-func-call/main.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/safe-to-func-call/main.c?rev=344635&r1=344634&r2=344635&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/safe-to-func-call/main.c 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/safe-to-func-call/main.c 
Tue Oct 16 11:14:30 2018
@@ -1,3 +1,4 @@
+#include   // work around module map issue with iOS sdk, 
 
 #include 
 #include 
 #include 

Modified: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules?rev=344635&r1=344634&r2=344635&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Tue Oct 16 
11:14:30 2018
@@ -153,6 +153,7 @@ ifeq "$(OS)" "Darwin"
DSYM = $(EXE).dSYM
AR := $(CROSS_COMPILE)libtool
ARFLAGS := -static -o
+   CODESIGN = codesign
 else
AR := $(CROSS_COMPILE)ar
# On non-Apple platforms, -arch becomes -m
@@ -513,6 +514,9 @@ endif
 else
 $(EXE) : $(OBJECTS) $(ARCHIVE_NAME)
$(LD) $(OBJECTS) $(LDFLAGS) $(ARCHIVE_NAME) -o "$(EXE)"
+ifneq "$(CODESIGN)" ""
+   $(CODESIGN) -s - "$(EXE)"
+endif
 endif
 
 #--
@@ -557,6 +561,9 @@ endif
 $(DYLIB_FILENAME) : $(DYLIB_OBJECTS)
 ifeq "$(OS)" "Darwin"
$(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -install_name 
"$(DYLIB_EXECUTABLE_PATH)/$(DYLIB_FILENAME)" -dynamiclib -o "$(DYLIB_FILENAME)"
+ifneq "$(CODESIGN)" ""
+   $(CODESIGN) -s - "$(DYLIB_FILENAME)"
+endif
 ifneq "$(MAKE_DSYM)" "NO"
 ifneq "$(DS)" ""
"$(DS)" $(DSFLAGS) "$(DYLIB_FILENAME)"

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py?rev=344635&r1=344634&r2=344635&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py
 Tue Oct 16 11:14:30 2018
@@ -95,18 +95,21 @@ class TestAppleSimulatorOSType(gdbremote
 
 @apple_simulator_test('iphone')
 @debugserver_test
+@skipIfDarwinEmbedded
 def test_simulator_ostype_ios(self):
 self.check_simulator_ostype(sdk='iphonesimulator',
 platform='ios')
 
 @apple_simulator_test('appletv')
 @debugserver_test
+@skipIfDarwinEmbedded
 def test_simulator_ostype_tvos(self):
 self.check_simulator_ostype(sdk='appletvsimulator',
 platform='tvos')
 
 @apple_simulator_test('watch')
 @debugserver_test
+@skipIfDarwinEmbedded
 def test_simulator_ostype_watchos(self):
 self.check_simulator_ostype(sdk='watchsimulator',
 platform='watchos', arch='i386')


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

Re: [Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Vedant Kumar via lldb-commits]

> On Oct 16, 2018, at 10:59 AM, Stella Stamenova  wrote:
> 
> The windows error is because the names are different, as you expected:
>  
> AssertionError: 'void sink(void)' != 'sink()'
>  
> You can probably update the test to look for a different name on Windows 
> (though if I recall correctly, different versions of the DIA sdk provide 
> different detail on the names, so that might not be robust either) or look 
> for a substring in the full name.

I used a substring check in r344634.


> I’ll look into the Linux error as well and let you know what I find.

Thank you very much! I really appreciate your help and patience with this.

The "step" logging channel should provide detailed information about what goes 
wrong when parsing the DWARF for call site information and creating artificial 
frames.

vedant

>  
>  
> From: v...@apple.com  
> Sent: Monday, October 15, 2018 8:34 PM
> To: Frédéric Riss 
> Cc: reviews+d50478+public+7e86b794a0909...@reviews.llvm.org; Adrian Prantl 
> ; paul.robin...@sony.com; jdevliegh...@apple.com; Jim 
> Ingham ; ztur...@google.com; Stella Stamenova 
> ; abidh@gmail.com; teempe...@gmail.com; 
> sgraen...@apple.com; mgr...@codeaurora.org; dblai...@gmail.com; 
> lldb-commits@lists.llvm.org
> Subject: Re: [PATCH] D50478: Add support for artificial tail call frames
>  
>  
> 
> 
> On Oct 15, 2018, at 4:46 PM, Frédéric Riss  > wrote:
>  
> 
> 
> 
> On Oct 15, 2018, at 4:40 PM, Vedant Kumar  > wrote:
>  
> 
> 
> 
> On Oct 15, 2018, at 3:47 PM, Stella Stamenova via Phabricator 
> mailto:revi...@reviews.llvm.org>> wrote:
> 
> stella.stamenova added a comment.
> 
> In https://reviews.llvm.org/D50478#1262717 
> ,
>  @vsk wrote:
> 
> 
> In https://reviews.llvm.org/D50478#1262710 
> ,
>  @stella.stamenova wrote:
> 
> 
> Unfortunately, the bots are broken because of the FileCheck issue, so I can't 
> confirm with them, but I see a number of these tests fail in our local 
> testing. Some fail on both Windows and Linux and some just fail on Linux. 
> Here are the failing tests:
> 
> Linux:
> lldb-Suite :: 
> functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
> lldb-Suite :: 
> functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
> lldb-Suite :: 
> functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
> lldb-Suite :: 
> functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
> lldb-Suite :: 
> functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
> lldb-Suite :: 
> functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
> lldb-Suite :: 
> functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
> 
> Windows:
> lldb-Suite :: 
> functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
> lldb-Suite :: 
> functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
> 
> 
> Let me know what you need to investigate.
> 
> 
> Strange, I didn't get any bot failure notifications in the days after this 
> landed. Could you share the output from the failing tests?
> 
> 
> All the failures on Windows are happening when validating the function name. 
> For example:
> 
> ==
> 
> FAIL: test_tail_call_frame_sbapi 
> (TestTailCallFrameSBAPI.TestTailCallFrameSBAPI)
> 
> --
> 
> Traceback (most recent call last):
> 
>   File 
> "E:\_work\55\s\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\tail_call_frames\sbapi_support\TestTailCallFrameSBAPI.py",
>  line 19, in test_tail_call_frame_sbapi
> 
> self.do_test()
> 
>   File 
> "E:\_work\55\s\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\tail_call_frames\sbapi_support\TestTailCallFrameSBAPI.py",
>  line 64, in do_test
> 
> self.assertTrue(frame.GetDisplayFunctionName() == name)
> 
> It could be that the display name of a function is formatted differently on 
> Windows. Do you have an easy way of determining what 
> frame.GetDisplayFunctionName() is?
>  
> If you use assertEqual(a,b) instead of assertTrue, it will print out the 
> values and make it easier to 

Re: [Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

[Zachary Turner via lldb-commits]

Once we’re no longer using DIA this should be a lot easier to control on
our side
On Tue, Oct 16, 2018 at 11:17 AM Vedant Kumar  wrote:

>
>
> On Oct 16, 2018, at 10:59 AM, Stella Stamenova 
> wrote:
>
> The windows error is because the names are different, as you expected:
>
> AssertionError: 'void sink(void)' != 'sink()'
>
> You can probably update the test to look for a different name on Windows
> (though if I recall correctly, different versions of the DIA sdk provide
> different detail on the names, so that might not be robust either) or look
> for a substring in the full name.
>
>
> I used a substring check in r344634.
>
>
> I’ll look into the Linux error as well and let you know what I find.
>
>
> Thank you very much! I really appreciate your help and patience with this.
>
> The "step" logging channel should provide detailed information about what
> goes wrong when parsing the DWARF for call site information and creating
> artificial frames.
>
> vedant
>
>
>
> *From:* v...@apple.com 
> *Sent:* Monday, October 15, 2018 8:34 PM
> *To:* Frédéric Riss 
> *Cc:* reviews+d50478+public+7e86b794a0909...@reviews.llvm.org; Adrian
> Prantl ; paul.robin...@sony.com; jdevliegh...@apple.com;
> Jim Ingham ; ztur...@google.com; Stella Stamenova <
> sti...@microsoft.com>; abidh@gmail.com; teempe...@gmail.com;
> sgraen...@apple.com; mgr...@codeaurora.org; dblai...@gmail.com;
> lldb-commits@lists.llvm.org
> *Subject:* Re: [PATCH] D50478: Add support for artificial tail call frames
>
>
>
>
> On Oct 15, 2018, at 4:46 PM, Frédéric Riss  wrote:
>
>
>
>
> On Oct 15, 2018, at 4:40 PM, Vedant Kumar  wrote:
>
>
>
>
> On Oct 15, 2018, at 3:47 PM, Stella Stamenova via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
> stella.stamenova added a comment.
>
> In https://reviews.llvm.org/D50478#1262717
> ,
> @vsk wrote:
>
>
> In https://reviews.llvm.org/D50478#1262710
> ,
> @stella.stamenova wrote:
>
>
> Unfortunately, the bots are broken because of the FileCheck issue, so I
> can't confirm with them, but I see a number of these tests fail in our
> local testing. Some fail on both Windows and Linux and some just fail on
> Linux. Here are the failing tests:
>
> Linux:
> lldb-Suite ::
> functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
> lldb-Suite ::
> functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
> lldb-Suite ::
> functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
> lldb-Suite ::
> functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
> lldb-Suite ::
> functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
> lldb-Suite ::
> functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
> lldb-Suite ::
> functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
>
> Windows:
> lldb-Suite ::
> functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
> lldb-Suite ::
> functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
>
>
> Let me know what you need to investigate.
>
>
>
> Strange, I didn't get any bot failure notifications in the days after this
> landed. Could you share the output from the failing tests?
>
>
>
> All the failures on Windows are happening when validating the function
> name. For example:
>
> ==
>
> FAIL: test_tail_call_frame_sbapi
> (TestTailCallFrameSBAPI.TestTailCallFrameSBAPI)
>
> --
>
> Traceback (most recent call last):
>
>   File
> "E:\_work\55\s\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\tail_call_frames\sbapi_support\TestTailCallFrameSBAPI.py",
> line 19, in test_tail_call_frame_sbapi
>
> self.do_test()
>
>   File
> "E:\_work\55\s\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\tail_call_frames\sbapi_support\TestTailCallFrameSBAPI.py",
> line 64, in do_test
>
> self.assertTrue(frame.GetDisplayFunctionName() == name)
>
>
> It could be that the display name of a function is formatted differently
> on Windows. Do you have an easy way of determining what
> frame.GetDisplayFunctionName() is?
>
>
> If you use assertEqual(a,b) instead of asser

[Lldb-commits] [lldb] r344636 - Revert r344626 while I address a testsuite failure from a bot.

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Tue Oct 16 11:25:46 2018
New Revision: 344636

URL: http://llvm.org/viewvc/llvm-project?rev=344636&view=rev
Log:
Revert r344626 while I address a testsuite failure from a bot.


Modified:
lldb/trunk/source/Host/common/Symbols.cpp

Modified: lldb/trunk/source/Host/common/Symbols.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Symbols.cpp?rev=344636&r1=344635&r2=344636&view=diff
==
--- lldb/trunk/source/Host/common/Symbols.cpp (original)
+++ lldb/trunk/source/Host/common/Symbols.cpp Tue Oct 16 11:25:46 2018
@@ -65,134 +65,96 @@ static bool FileAtPathContainsArchAndUUI
   return false;
 }
 
-// Given a binary exec_fspec, and a ModuleSpec with an architecture/uuid,
-// return true if there is a matching dSYM bundle next to the exec_fspec,
-// and return that value in dsym_fspec.  
-// If there is a .dSYM.yaa compressed archive next to the exec_fspec, 
-// call through Symbols::DownloadObjectAndSymbolFile to download the
-// expanded/uncompressed dSYM and return that filepath in dsym_fspec.
-
-static bool LookForDsymNextToExecutablePath(const ModuleSpec &mod_spec,
-const FileSpec &exec_fspec,
-FileSpec &dsym_fspec) {
-  ConstString filename = exec_fspec.GetFilename();
-  FileSpec dsym_directory = exec_fspec;
-  dsym_directory.RemoveLastPathComponent();
-
-  std::string dsym_filename = filename.AsCString();
-  dsym_filename += ".dSYM";
-  dsym_directory.AppendPathComponent(dsym_filename);
-  dsym_directory.AppendPathComponent("Contents");
-  dsym_directory.AppendPathComponent("Resources");
-  dsym_directory.AppendPathComponent("DWARF");
-  
-  if (dsym_directory.Exists()) {
-
-// See if the binary name exists in the dSYM DWARF
-// subdir.
-FileSpec dsym_fspec = dsym_directory;
-dsym_fspec.AppendPathComponent(filename.AsCString());
-if (dsym_fspec.Exists()
-&& FileAtPathContainsArchAndUUID(dsym_fspec, 
- mod_spec.GetArchitecturePtr(),
- mod_spec.GetUUIDPtr())) {
-  return true;
-}
-
-// See if we have "../CF.framework" - so we'll look for
-// CF.framework.dSYM/Contents/Resources/DWARF/CF
-// We need to drop the last suffix after '.' to match 
-// 'CF' in the DWARF subdir.
-std::string binary_name (filename.AsCString());
-auto last_dot = binary_name.find_last_of('.');
-if (last_dot != std::string::npos) {
-  binary_name.erase(last_dot);
-  dsym_fspec = dsym_directory;
-  dsym_fspec.AppendPathComponent(binary_name);
-  if (dsym_fspec.Exists()
-  && FileAtPathContainsArchAndUUID(dsym_fspec, 
-   mod_spec.GetArchitecturePtr(),
-   mod_spec.GetUUIDPtr())) {
-return true;
-  }
-}
-  } 
-  
-  // See if we have a .dSYM.yaa next to this executable path.
-  FileSpec dsym_yaa_fspec = exec_fspec;
-  dsym_yaa_fspec.RemoveLastPathComponent();
-  std::string dsym_yaa_filename = filename.AsCString();
-  dsym_yaa_filename += ".dSYM.yaa";
-  dsym_yaa_fspec.AppendPathComponent(dsym_yaa_filename);
-
-  if (dsym_yaa_fspec.Exists()) {
-ModuleSpec mutable_mod_spec = mod_spec;
-if (Symbols::DownloadObjectAndSymbolFile (mutable_mod_spec, true)
-&& mutable_mod_spec.GetSymbolFileSpec().Exists()) {
-  dsym_fspec = mutable_mod_spec.GetSymbolFileSpec();
-  return true;
-}
-  }
-
-  return false;
-}
-
-// Given a ModuleSpec with a FileSpec and optionally uuid/architecture
-// filled in, look for a .dSYM bundle next to that binary.  Returns true
-// if a .dSYM bundle is found, and that path is returned in the dsym_fspec
-// FileSpec.
-//
-// This routine looks a few directory layers above the given exec_path -
-// exec_path might be /System/Library/Frameworks/CF.framework/CF and the
-// dSYM might be /System/Library/Frameworks/CF.framework.dSYM.
-//
-// If there is a .dSYM.yaa compressed archive found next to the binary,
-// we'll call DownloadObjectAndSymbolFile to expand it into a plain .dSYM
-
 static bool LocateDSYMInVincinityOfExecutable(const ModuleSpec &module_spec,
   FileSpec &dsym_fspec) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
-  const FileSpec &exec_fspec = module_spec.GetFileSpec();
+  const FileSpec *exec_fspec = module_spec.GetFileSpecPtr();
   if (exec_fspec) {
-if (::LookForDsymNextToExecutablePath (module_spec, exec_fspec, 
dsym_fspec)) {
+char path[PATH_MAX];
+if (exec_fspec->GetPath(path, sizeof(path))) {
+  // Make sure the module isn't already just a dSYM file...
+  if (strcasestr(path, ".dSYM/Contents/Resources/DWARF") == NULL) {
 if (log) {
-  log->Printf("dSYM with matching UUID & arch found at %s", 
dsym_fspec.GetPath().c_str());
+  if (module_spec.GetUUIDPtr()

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 169873.
jankratochvil added a comment.

Done the conversion of bitfield to uint16_t.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53321

Files:
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -223,7 +223,7 @@
   // the list (saves up to 25% in C++ code), we need a way to let the
   // DIE know that it actually doesn't have children.
   if (!m_die_array.empty())
-m_die_array.back().SetEmptyChildren(true);
+m_die_array.back().SetHasChildren(false);
 }
   } else {
 die.SetParentIndex(m_die_array.size() - die_index_stack[depth - 1]);
@@ -263,7 +263,7 @@
   if (!m_die_array.empty()) {
 if (m_first_die) {
   // Only needed for the assertion.
-  m_first_die.SetEmptyChildren(m_die_array.front().GetEmptyChildren());
+  m_first_die.SetHasChildren(m_die_array.front().HasChildren());
   lldbassert(m_first_die == m_die_array.front());
 }
 m_first_die = m_die_array.front();
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -43,7 +43,6 @@
 class DWARFDeclContext;
 
 #define DIE_SIBLING_IDX_BITSIZE 31
-#define DIE_ABBR_IDX_BITSIZE 15
 
 class DWARFDebugInfoEntry {
 public:
@@ -57,8 +56,7 @@
 
   DWARFDebugInfoEntry()
   : m_offset(DW_INVALID_OFFSET), m_parent_idx(0), m_sibling_idx(0),
-m_empty_children(false), m_abbr_idx(0), m_has_children(false),
-m_tag(0) {}
+m_has_children(false), m_abbr_idx(0), m_tag(0) {}
 
   explicit operator bool() const { return m_offset != DW_INVALID_OFFSET; }
   bool operator==(const DWARFDebugInfoEntry &rhs) const;
@@ -227,10 +225,10 @@
   // we don't need to store our child pointer, if we have a child it will
   // be the next entry in the list...
   DWARFDebugInfoEntry *GetFirstChild() {
-return (HasChildren() && !m_empty_children) ? this + 1 : NULL;
+return HasChildren() ? this + 1 : NULL;
   }
   const DWARFDebugInfoEntry *GetFirstChild() const {
-return (HasChildren() && !m_empty_children) ? this + 1 : NULL;
+return HasChildren() ? this + 1 : NULL;
   }
 
   void GetDeclContextDIEs(DWARFUnit *cu,
@@ -271,10 +269,6 @@
 
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
-  bool GetEmptyChildren() const { return m_empty_children; }
-
-  void SetEmptyChildren(bool b) { m_empty_children = b; }
-
   static void
   DumpDIECollection(lldb_private::Stream &strm,
 DWARFDebugInfoEntry::collection &die_collection);
@@ -285,13 +279,13 @@
   uint32_t m_parent_idx; // How many to subtract from "this" to get the parent.
  // If zero this die has no parent
   uint32_t m_sibling_idx : 31, // How many to add to "this" to get the sibling.
-  m_empty_children : 1;// If a DIE says it had children, yet it just
-   // contained a NULL tag, this will be set.
-  uint32_t m_abbr_idx : DIE_ABBR_IDX_BITSIZE,
-m_has_children : 1, // Set to 1 if this DIE has children
-m_tag : 16; // A copy of the DW_TAG value so we don't
-// have to go through the compile unit
-// abbrev table
+  // If it is zero, then the DIE doesn't have children, or the
+  // DWARF claimed it had children but the DIE only contained
+  // a single NULL terminating child.
+  m_has_children : 1;
+  uint16_t m_abbr_idx;
+  uint16_t m_tag; // A copy of the DW_TAG value so we don't have to go through
+  // the compile unit abbrev table
 };
 
 #endif // SymbolFileDWARF_DWARFDebugInfoEntry_h_
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -40,9 +40,8 @@
   m_offset = *offset_ptr;
   m_parent_idx = 0;
   m_sibling_idx = 0;
-  m_empty_children = false;
   const uint64_t abbr_idx = debug_info_data.GetULEB128(offset_ptr);
-  assert(abbr_idx < (1 << DIE_ABBR_IDX_BITSIZE));
+  lldbassert(abbr_idx <= UINT16_MAX);
   m_abbr_idx = abbr_idx;
 
   // assert (fixed_form_sizes);  // For best performance this should be
@@ -220,7 +219,7 @@
 m_offset = offset;
 
 const uint64_t abbr_idx = debug_info_data.GetULEB128(&offset);
-assert(abbr_idx < (1 << DIE_ABBR_IDX_BITSIZE));
+lldbassert(abbr_idx <= U

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Jan Kratochvil via Phabricator via lldb-commits]

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

Also changed `assert()`->`lldbassert()` for `m_abbr_idx` 16-bit overflow as 
that could be a tough bug to catch if it ever happens.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53321



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

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Greg Clayton via Phabricator via lldb-commits]

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

Looks good. Nice cleanup.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53321



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

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Jan Kratochvil via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB344644: Code cleanup: Remove 
DWARFDebugInfoEntry::m_empty_children (authored by jankratochvil, committed by 
).

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53321

Files:
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -223,7 +223,7 @@
   // the list (saves up to 25% in C++ code), we need a way to let the
   // DIE know that it actually doesn't have children.
   if (!m_die_array.empty())
-m_die_array.back().SetEmptyChildren(true);
+m_die_array.back().SetHasChildren(false);
 }
   } else {
 die.SetParentIndex(m_die_array.size() - die_index_stack[depth - 1]);
@@ -263,7 +263,7 @@
   if (!m_die_array.empty()) {
 if (m_first_die) {
   // Only needed for the assertion.
-  m_first_die.SetEmptyChildren(m_die_array.front().GetEmptyChildren());
+  m_first_die.SetHasChildren(m_die_array.front().HasChildren());
   lldbassert(m_first_die == m_die_array.front());
 }
 m_first_die = m_die_array.front();
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -40,9 +40,8 @@
   m_offset = *offset_ptr;
   m_parent_idx = 0;
   m_sibling_idx = 0;
-  m_empty_children = false;
   const uint64_t abbr_idx = debug_info_data.GetULEB128(offset_ptr);
-  assert(abbr_idx < (1 << DIE_ABBR_IDX_BITSIZE));
+  lldbassert(abbr_idx <= UINT16_MAX);
   m_abbr_idx = abbr_idx;
 
   // assert (fixed_form_sizes);  // For best performance this should be
@@ -220,7 +219,7 @@
 m_offset = offset;
 
 const uint64_t abbr_idx = debug_info_data.GetULEB128(&offset);
-assert(abbr_idx < (1 << DIE_ABBR_IDX_BITSIZE));
+lldbassert(abbr_idx <= UINT16_MAX);
 m_abbr_idx = abbr_idx;
 if (abbr_idx) {
   const DWARFAbbreviationDeclaration *abbrevDecl =
@@ -1836,7 +1835,6 @@
 bool DWARFDebugInfoEntry::operator==(const DWARFDebugInfoEntry &rhs) const {
   return m_offset == rhs.m_offset && m_parent_idx == rhs.m_parent_idx &&
  m_sibling_idx == rhs.m_sibling_idx &&
- m_empty_children == rhs.m_empty_children &&
  m_abbr_idx == rhs.m_abbr_idx && m_has_children == rhs.m_has_children &&
  m_tag == rhs.m_tag;
 }
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -43,7 +43,6 @@
 class DWARFDeclContext;
 
 #define DIE_SIBLING_IDX_BITSIZE 31
-#define DIE_ABBR_IDX_BITSIZE 15
 
 class DWARFDebugInfoEntry {
 public:
@@ -57,8 +56,7 @@
 
   DWARFDebugInfoEntry()
   : m_offset(DW_INVALID_OFFSET), m_parent_idx(0), m_sibling_idx(0),
-m_empty_children(false), m_abbr_idx(0), m_has_children(false),
-m_tag(0) {}
+m_has_children(false), m_abbr_idx(0), m_tag(0) {}
 
   explicit operator bool() const { return m_offset != DW_INVALID_OFFSET; }
   bool operator==(const DWARFDebugInfoEntry &rhs) const;
@@ -227,10 +225,10 @@
   // we don't need to store our child pointer, if we have a child it will
   // be the next entry in the list...
   DWARFDebugInfoEntry *GetFirstChild() {
-return (HasChildren() && !m_empty_children) ? this + 1 : NULL;
+return HasChildren() ? this + 1 : NULL;
   }
   const DWARFDebugInfoEntry *GetFirstChild() const {
-return (HasChildren() && !m_empty_children) ? this + 1 : NULL;
+return HasChildren() ? this + 1 : NULL;
   }
 
   void GetDeclContextDIEs(DWARFUnit *cu,
@@ -271,10 +269,6 @@
 
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
-  bool GetEmptyChildren() const { return m_empty_children; }
-
-  void SetEmptyChildren(bool b) { m_empty_children = b; }
-
   static void
   DumpDIECollection(lldb_private::Stream &strm,
 DWARFDebugInfoEntry::collection &die_collection);
@@ -285,13 +279,13 @@
   uint32_t m_parent_idx; // How many to subtract from "this" to get the parent.
  // If zero this die has no parent
   uint32_t m_sibling_idx : 31, // How many to add to "this" to get the sibling.
-  m_empty_children : 1;// If a DIE says it had children, yet it just
-   // contained a NULL tag, this will be set.
-  uint32_t m_abbr_idx : DIE_ABBR_IDX_BITSIZE,
-m_has_children : 1, // Set to 1 if this DIE has children
-m_tag : 16; /

[Lldb-commits] [lldb] r344644 - Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Jan Kratochvil via lldb-commits]

Author: jankratochvil
Date: Tue Oct 16 13:49:15 2018
New Revision: 344644

URL: http://llvm.org/viewvc/llvm-project?rev=344644&view=rev
Log:
Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

It merges DWARFDebugInfoEntry's m_empty_children into m_has_children.
m_empty_children was implemented by rL144983.

As Greg confirmed m_has_children was used to represent what was in the DWARF in
the byte that follows the DW_TAG. m_empty_children was used for DIEs that said
they had children but actually only contain a single NULL tag. It is fine to
not differentiate between the two.

Also changed assert()->lldbassert() for m_abbr_idx 16-bit overflow check as
that could be a tough bug to catch if it ever happens.

I have checked all calls of HasChildren() that this change should not matter to
them. The code even wants to know if there are any children - it does not
matter how the children presence is coded in the binary.

Patch written based on suggestions by Greg Clayton.

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp?rev=344644&r1=344643&r2=344644&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp Tue Oct 
16 13:49:15 2018
@@ -40,9 +40,8 @@ bool DWARFDebugInfoEntry::FastExtract(
   m_offset = *offset_ptr;
   m_parent_idx = 0;
   m_sibling_idx = 0;
-  m_empty_children = false;
   const uint64_t abbr_idx = debug_info_data.GetULEB128(offset_ptr);
-  assert(abbr_idx < (1 << DIE_ABBR_IDX_BITSIZE));
+  lldbassert(abbr_idx <= UINT16_MAX);
   m_abbr_idx = abbr_idx;
 
   // assert (fixed_form_sizes);  // For best performance this should be
@@ -220,7 +219,7 @@ bool DWARFDebugInfoEntry::Extract(Symbol
 m_offset = offset;
 
 const uint64_t abbr_idx = debug_info_data.GetULEB128(&offset);
-assert(abbr_idx < (1 << DIE_ABBR_IDX_BITSIZE));
+lldbassert(abbr_idx <= UINT16_MAX);
 m_abbr_idx = abbr_idx;
 if (abbr_idx) {
   const DWARFAbbreviationDeclaration *abbrevDecl =
@@ -1836,7 +1835,6 @@ void DWARFDebugInfoEntry::DumpDIECollect
 bool DWARFDebugInfoEntry::operator==(const DWARFDebugInfoEntry &rhs) const {
   return m_offset == rhs.m_offset && m_parent_idx == rhs.m_parent_idx &&
  m_sibling_idx == rhs.m_sibling_idx &&
- m_empty_children == rhs.m_empty_children &&
  m_abbr_idx == rhs.m_abbr_idx && m_has_children == rhs.m_has_children 
&&
  m_tag == rhs.m_tag;
 }

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h?rev=344644&r1=344643&r2=344644&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h Tue Oct 16 
13:49:15 2018
@@ -43,7 +43,6 @@ typedef UInt32ToDIEMMap::const_iterator
 class DWARFDeclContext;
 
 #define DIE_SIBLING_IDX_BITSIZE 31
-#define DIE_ABBR_IDX_BITSIZE 15
 
 class DWARFDebugInfoEntry {
 public:
@@ -57,8 +56,7 @@ public:
 
   DWARFDebugInfoEntry()
   : m_offset(DW_INVALID_OFFSET), m_parent_idx(0), m_sibling_idx(0),
-m_empty_children(false), m_abbr_idx(0), m_has_children(false),
-m_tag(0) {}
+m_has_children(false), m_abbr_idx(0), m_tag(0) {}
 
   explicit operator bool() const { return m_offset != DW_INVALID_OFFSET; }
   bool operator==(const DWARFDebugInfoEntry &rhs) const;
@@ -227,10 +225,10 @@ public:
   // we don't need to store our child pointer, if we have a child it will
   // be the next entry in the list...
   DWARFDebugInfoEntry *GetFirstChild() {
-return (HasChildren() && !m_empty_children) ? this + 1 : NULL;
+return HasChildren() ? this + 1 : NULL;
   }
   const DWARFDebugInfoEntry *GetFirstChild() const {
-return (HasChildren() && !m_empty_children) ? this + 1 : NULL;
+return HasChildren() ? this + 1 : NULL;
   }
 
   void GetDeclContextDIEs(DWARFUnit *cu,
@@ -271,10 +269,6 @@ public:
 
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
-  bool GetEmptyChildren() const { return m_empty_children; }
-
-  void SetEmptyChildren(bool b) { m_empty_children = b; }
-
   static void
   DumpDIECollection(lldb_private::Stream &strm,
 DWARFDebugInfoEntry::collection &die_collection);
@@ -285,13 +279,13 @@ protected:
   uint32_t m_parent_idx; // How many to subtract from "this" 

[Lldb-commits] [PATCH] D53321: Code cleanup: Remove DWARFDebugInfoEntry::m_empty_children

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In https://reviews.llvm.org/D53321#1266912, @clayborg wrote:

> Looks good. Nice cleanup.


Thanks for the cleanups and fixes.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53321



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

[Lldb-commits] [lldb] r344646 - Fixed an issue that a bot found with my changes

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Tue Oct 16 14:49:31 2018
New Revision: 344646

URL: http://llvm.org/viewvc/llvm-project?rev=344646&view=rev
Log:
Fixed an issue that a bot found with my changes
in r344626 & recommitting.  Original commit msg:


Simplify LocateDSYMInVincinityOfExecutable by moving
some redundant code into a separate function, 
LookForDsymNextToExecutablePath, and having that function
also look for .dSYM.yaa files in addition to .dSYM
bundles.

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

 

Modified:
lldb/trunk/source/Host/common/Symbols.cpp

Modified: lldb/trunk/source/Host/common/Symbols.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Symbols.cpp?rev=344646&r1=344645&r2=344646&view=diff
==
--- lldb/trunk/source/Host/common/Symbols.cpp (original)
+++ lldb/trunk/source/Host/common/Symbols.cpp Tue Oct 16 14:49:31 2018
@@ -65,96 +65,134 @@ static bool FileAtPathContainsArchAndUUI
   return false;
 }
 
+// Given a binary exec_fspec, and a ModuleSpec with an architecture/uuid,
+// return true if there is a matching dSYM bundle next to the exec_fspec,
+// and return that value in dsym_fspec.  
+// If there is a .dSYM.yaa compressed archive next to the exec_fspec, 
+// call through Symbols::DownloadObjectAndSymbolFile to download the
+// expanded/uncompressed dSYM and return that filepath in dsym_fspec.
+
+static bool LookForDsymNextToExecutablePath(const ModuleSpec &mod_spec,
+const FileSpec &exec_fspec,
+FileSpec &dsym_fspec) {
+  ConstString filename = exec_fspec.GetFilename();
+  FileSpec dsym_directory = exec_fspec;
+  dsym_directory.RemoveLastPathComponent();
+
+  std::string dsym_filename = filename.AsCString();
+  dsym_filename += ".dSYM";
+  dsym_directory.AppendPathComponent(dsym_filename);
+  dsym_directory.AppendPathComponent("Contents");
+  dsym_directory.AppendPathComponent("Resources");
+  dsym_directory.AppendPathComponent("DWARF");
+  
+  if (dsym_directory.Exists()) {
+
+// See if the binary name exists in the dSYM DWARF
+// subdir.
+dsym_fspec = dsym_directory;
+dsym_fspec.AppendPathComponent(filename.AsCString());
+if (dsym_fspec.Exists()
+&& FileAtPathContainsArchAndUUID(dsym_fspec, 
+ mod_spec.GetArchitecturePtr(),
+ mod_spec.GetUUIDPtr())) {
+  return true;
+}
+
+// See if we have "../CF.framework" - so we'll look for
+// CF.framework.dSYM/Contents/Resources/DWARF/CF
+// We need to drop the last suffix after '.' to match 
+// 'CF' in the DWARF subdir.
+std::string binary_name (filename.AsCString());
+auto last_dot = binary_name.find_last_of('.');
+if (last_dot != std::string::npos) {
+  binary_name.erase(last_dot);
+  dsym_fspec = dsym_directory;
+  dsym_fspec.AppendPathComponent(binary_name);
+  if (dsym_fspec.Exists()
+  && FileAtPathContainsArchAndUUID(dsym_fspec, 
+   mod_spec.GetArchitecturePtr(),
+   mod_spec.GetUUIDPtr())) {
+return true;
+  }
+}
+  } 
+  
+  // See if we have a .dSYM.yaa next to this executable path.
+  FileSpec dsym_yaa_fspec = exec_fspec;
+  dsym_yaa_fspec.RemoveLastPathComponent();
+  std::string dsym_yaa_filename = filename.AsCString();
+  dsym_yaa_filename += ".dSYM.yaa";
+  dsym_yaa_fspec.AppendPathComponent(dsym_yaa_filename);
+
+  if (dsym_yaa_fspec.Exists()) {
+ModuleSpec mutable_mod_spec = mod_spec;
+if (Symbols::DownloadObjectAndSymbolFile (mutable_mod_spec, true)
+&& mutable_mod_spec.GetSymbolFileSpec().Exists()) {
+  dsym_fspec = mutable_mod_spec.GetSymbolFileSpec();
+  return true;
+}
+  }
+
+  return false;
+}
+
+// Given a ModuleSpec with a FileSpec and optionally uuid/architecture
+// filled in, look for a .dSYM bundle next to that binary.  Returns true
+// if a .dSYM bundle is found, and that path is returned in the dsym_fspec
+// FileSpec.
+//
+// This routine looks a few directory layers above the given exec_path -
+// exec_path might be /System/Library/Frameworks/CF.framework/CF and the
+// dSYM might be /System/Library/Frameworks/CF.framework.dSYM.
+//
+// If there is a .dSYM.yaa compressed archive found next to the binary,
+// we'll call DownloadObjectAndSymbolFile to expand it into a plain .dSYM
+
 static bool LocateDSYMInVincinityOfExecutable(const ModuleSpec &module_spec,
   FileSpec &dsym_fspec) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
-  const FileSpec *exec_fspec = module_spec.GetFileSpecPtr();
+  const FileSpec &exec_fspec = module_spec.GetFileSpec();
   if (exec_fspec) {
-char path[PATH_MAX];
-if (exec_fspec->GetPath(path, sizeof(path))) {
-  // Make sure the module isn't already just a dSYM file...
-  if (strc

[Lldb-commits] [lldb] r344647 - Return a named error in the result object of an expression with no result

[Jim Ingham via lldb-commits]

Author: jingham
Date: Tue Oct 16 14:58:40 2018
New Revision: 344647

URL: http://llvm.org/viewvc/llvm-project?rev=344647&view=rev
Log:
Return a named error in the result object of an expression with no result

Before we returned an error that was not exposed in the SB API and no useful
error message.  This change returns eExpressionProducedNoResult and an
appropriate error string.



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

Added:
lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/

lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/Makefile

lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/TestNoResult.py

lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/main.c
Modified:
lldb/trunk/include/lldb/Expression/UserExpression.h
lldb/trunk/include/lldb/lldb-enumerations.h
lldb/trunk/source/Commands/CommandObjectExpression.cpp
lldb/trunk/source/Expression/ExpressionSourceCode.cpp
lldb/trunk/source/Expression/REPL.cpp
lldb/trunk/source/Expression/UserExpression.cpp

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp

Modified: lldb/trunk/include/lldb/Expression/UserExpression.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/UserExpression.h?rev=344647&r1=344646&r2=344647&view=diff
==
--- lldb/trunk/include/lldb/Expression/UserExpression.h (original)
+++ lldb/trunk/include/lldb/Expression/UserExpression.h Tue Oct 16 14:58:40 2018
@@ -288,10 +288,6 @@ public:
uint32_t line_offset = 0, std::string *fixed_expression = nullptr,
lldb::ModuleSP *jit_module_sp_ptr = nullptr);
 
-  static const Status::ValueType kNoResult =
-  0x1001; ///< ValueObject::GetError() returns this if there is no result
-  /// from the expression.
-
   const char *GetFixedText() {
 if (m_fixed_text.empty())
   return nullptr;

Modified: lldb/trunk/include/lldb/lldb-enumerations.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb-enumerations.h?rev=344647&r1=344646&r2=344647&view=diff
==
--- lldb/trunk/include/lldb/lldb-enumerations.h (original)
+++ lldb/trunk/include/lldb/lldb-enumerations.h Tue Oct 16 14:58:40 2018
@@ -254,7 +254,8 @@ enum ExpressionResults {
   eExpressionHitBreakpoint,
   eExpressionTimedOut,
   eExpressionResultUnavailable,
-  eExpressionStoppedForDebug
+  eExpressionStoppedForDebug,
+  eExpressionProducedNoResult
 };
 
 enum SearchDepth {

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/Makefile?rev=344647&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/Makefile 
(added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/Makefile 
Tue Oct 16 14:58:40 2018
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+C_SOURCES := main.c
+CFLAGS_EXTRAS += -std=c99
+
+include $(LEVEL)/Makefile.rules

Added: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/TestNoResult.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/TestNoResult.py?rev=344647&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/TestNoResult.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/no-result/TestNoResult.py
 Tue Oct 16 14:58:40 2018
@@ -0,0 +1,45 @@
+"""
+Test that an expression that returns no result returns a sensible error.
+"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import re
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestExprNoResult(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+# If your test case doesn't stress debug info, the
+# set this to true.  That way it won't be run once for
+# each debug info format.
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_no_result(self):
+"""Run an expression that has no result, check the error."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.sample_test()
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+def sample_test(self):
+(target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
+   "Set a breakpoint here", 
self.main_source_file)
+
+frame = thread.GetFrameAtIndex(0)
+result = frame.

[Lldb-commits] [PATCH] D53309: Return a useful "Error" for an expression that completes but produces no result

[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 rLLDB344647: Return a named error in the result object of an 
expression with no result (authored by jingham, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53309?vs=169778&id=169899#toc

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53309

Files:
  include/lldb/Expression/UserExpression.h
  include/lldb/lldb-enumerations.h
  packages/Python/lldbsuite/test/expression_command/no-result/Makefile
  packages/Python/lldbsuite/test/expression_command/no-result/TestNoResult.py
  packages/Python/lldbsuite/test/expression_command/no-result/main.c
  source/Commands/CommandObjectExpression.cpp
  source/Expression/ExpressionSourceCode.cpp
  source/Expression/REPL.cpp
  source/Expression/UserExpression.cpp
  
source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp

Index: include/lldb/Expression/UserExpression.h
===
--- include/lldb/Expression/UserExpression.h
+++ include/lldb/Expression/UserExpression.h
@@ -288,10 +288,6 @@
uint32_t line_offset = 0, std::string *fixed_expression = nullptr,
lldb::ModuleSP *jit_module_sp_ptr = nullptr);
 
-  static const Status::ValueType kNoResult =
-  0x1001; ///< ValueObject::GetError() returns this if there is no result
-  /// from the expression.
-
   const char *GetFixedText() {
 if (m_fixed_text.empty())
   return nullptr;
Index: include/lldb/lldb-enumerations.h
===
--- include/lldb/lldb-enumerations.h
+++ include/lldb/lldb-enumerations.h
@@ -254,7 +254,8 @@
   eExpressionHitBreakpoint,
   eExpressionTimedOut,
   eExpressionResultUnavailable,
-  eExpressionStoppedForDebug
+  eExpressionStoppedForDebug,
+  eExpressionProducedNoResult
 };
 
 enum SearchDepth {
Index: packages/Python/lldbsuite/test/expression_command/no-result/main.c
===
--- packages/Python/lldbsuite/test/expression_command/no-result/main.c
+++ packages/Python/lldbsuite/test/expression_command/no-result/main.c
@@ -0,0 +1,9 @@
+#include 
+
+int
+main()
+{
+  int test_var = 10;
+  printf ("Set a breakpoint here: %d.\n", test_var);
+  return 0;
+}
Index: packages/Python/lldbsuite/test/expression_command/no-result/TestNoResult.py
===
--- packages/Python/lldbsuite/test/expression_command/no-result/TestNoResult.py
+++ packages/Python/lldbsuite/test/expression_command/no-result/TestNoResult.py
@@ -0,0 +1,45 @@
+"""
+Test that an expression that returns no result returns a sensible error.
+"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import re
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestExprNoResult(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+# If your test case doesn't stress debug info, the
+# set this to true.  That way it won't be run once for
+# each debug info format.
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_no_result(self):
+"""Run an expression that has no result, check the error."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.sample_test()
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+def sample_test(self):
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+   "Set a breakpoint here", self.main_source_file)
+
+frame = thread.GetFrameAtIndex(0)
+result = frame.EvaluateExpression("int $x = 10")
+# No result expressions are considered to fail:
+self.assertTrue(result.GetError().Fail(), "An expression with no result is a failure.")
+# But the reason should be eExpressionProducedNoResult
+self.assertEqual(result.GetError().GetError(), lldb.eExpressionProducedNoResult, 
+ "But the right kind of failure")
Index: packages/Python/lldbsuite/test/expression_command/no-result/Makefile
===
--- packages/Python/lldbsuite/test/expression_command/no-result/Makefile
+++ packages/Python/lldbsuite/test/expression_command/no-result/Makefile
@@ -0,0 +1,6 @@
+LEVEL = ../../make
+
+C_SOURCES := main.c
+CFLAGS_EXTRAS += -std=c99
+
+include $(LEVEL)/Makefile.rules
Index: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===
--- source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ source/Plugins/LanguageRuntime/RenderScript/RenderScriptRun

[Lldb-commits] [lldb] r344648 - Delete commented-out code.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Tue Oct 16 15:01:49 2018
New Revision: 344648

URL: http://llvm.org/viewvc/llvm-project?rev=344648&view=rev
Log:
Delete commented-out code.

Modified:
lldb/trunk/source/Core/ValueObject.cpp

Modified: lldb/trunk/source/Core/ValueObject.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObject.cpp?rev=344648&r1=344647&r2=344648&view=diff
==
--- lldb/trunk/source/Core/ValueObject.cpp (original)
+++ lldb/trunk/source/Core/ValueObject.cpp Tue Oct 16 15:01:49 2018
@@ -662,8 +662,6 @@ ValueObject *ValueObject::CreateChildAtI
 child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
 child_is_base_class, child_is_deref_of_parent, eAddressTypeInvalid,
 language_flags);
-// if (valobj)
-//valobj->SetAddressTypeOfChildren(eAddressTypeInvalid);
   }
 
   return valobj;


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