[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Note, in both C and C++ converting a `-1` to unsigned will always result in the 
max unsigned value e.g.:

  #include 
  #include 
  
  int main() {
int8_t i8 = -1;
int32_t i32 = -1;
  
unsigned x = i8;
std::cout << x << "\n";
  
x = i32;
std::cout << x << "\n";
  }

output:

  4294967295
  4294967295


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134493

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

[Lldb-commits] [PATCH] D130534: loading a binary at a slide multiple times leaves old entries in the SectionLoadList

[Jason Molenda via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1fee25629d9d: Clear old section-to-addr entry when loading 
Section at new addr (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130534

Files:
  lldb/source/Target/SectionLoadList.cpp
  lldb/test/API/functionalities/multiple-slides/Makefile
  lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py
  lldb/test/API/functionalities/multiple-slides/main.c

Index: lldb/test/API/functionalities/multiple-slides/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/multiple-slides/main.c
@@ -0,0 +1,5 @@
+int first[2048] = { 5 };
+int second[2048] = { 6 };
+int main()  {
+  return first[0] + second[0];
+}
Index: lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py
===
--- /dev/null
+++ lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py
@@ -0,0 +1,79 @@
+"""
+Test that a binary can be slid to different load addresses correctly
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MultipleSlidesTestCase(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+def test_mulitple_slides(self):
+"""Test that a binary can be slid multiple times correctly."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+err = lldb.SBError()
+load_dependent_modules = False
+target = self.dbg.CreateTarget(exe, '', '', load_dependent_modules, err)
+self.assertTrue(target.IsValid())
+module = target.GetModuleAtIndex(0)
+self.assertTrue(module.IsValid())
+
+first_sym = target.FindSymbols("first").GetContextAtIndex(0).GetSymbol()
+second_sym = target.FindSymbols("second").GetContextAtIndex(0).GetSymbol()
+first_size = first_sym.GetEndAddress().GetOffset() - first_sym.GetStartAddress().GetOffset()
+second_size = second_sym.GetEndAddress().GetOffset() - second_sym.GetStartAddress().GetOffset()
+
+# View the first element of `first` and `second` while
+# they have no load address set.
+self.expect("p/d ((int*))[0]", substrs=['= 5'])
+self.expect("p/d ((int*))[0]", substrs=['= 6'])
+self.assertEqual(first_sym.GetStartAddress().GetLoadAddress(target), lldb.LLDB_INVALID_ADDRESS)
+self.assertEqual(second_sym.GetStartAddress().GetLoadAddress(target), lldb.LLDB_INVALID_ADDRESS)
+
+
+# View the first element of `first` and `second` with
+# no slide applied, but with load address set.
+#
+# In memory, we have something like
+#0x1000 - 0x17ff  first[]
+#0x1800 - 0x1fff  second[]
+target.SetModuleLoadAddress(module, 0)
+self.expect("p/d ((int*))[0]", substrs=['= 5'])
+self.expect("p/d ((int*))[0]", substrs=['= 6'])
+self.assertEqual(first_sym.GetStartAddress().GetLoadAddress(target), 
+ first_sym.GetStartAddress().GetFileAddress())
+self.assertEqual(second_sym.GetStartAddress().GetLoadAddress(target),
+ second_sym.GetStartAddress().GetFileAddress())
+
+# Slide it a little bit less than the size of the first array.
+#
+# In memory, we have something like
+#0xfc0 - 0x17bf  first[]
+#0x17c0 - 0x1fbf second[]
+#
+# but if the original entries are still present in lldb, 
+# the beginning address of second[] will get a load address
+# of 0x1800, instead of 0x17c0 (0x1800-64) as we need to get.
+target.SetModuleLoadAddress(module, first_size - 64)
+self.expect("p/d ((int*))[0]", substrs=['= 5'])
+self.expect("p/d ((int*))[0]", substrs=['= 6'])
+self.assertNotEqual(first_sym.GetStartAddress().GetLoadAddress(target), 
+ first_sym.GetStartAddress().GetFileAddress())
+self.assertNotEqual(second_sym.GetStartAddress().GetLoadAddress(target),
+ second_sym.GetStartAddress().GetFileAddress())
+
+# Slide it back to the original vmaddr.
+target.SetModuleLoadAddress(module, 0)
+self.expect("p/d ((int*))[0]", substrs=['= 5'])
+self.expect("p/d ((int*))[0]", substrs=['= 6'])
+self.assertEqual(first_sym.GetStartAddress().GetLoadAddress(target), 
+ first_sym.GetStartAddress().GetFileAddress())
+self.assertEqual(second_sym.GetStartAddress().GetLoadAddress(target),
+ second_sym.GetStartAddress().GetFileAddress())
+
Index: lldb/test/API/functionalities/multiple-slides/Makefile

[Lldb-commits] [lldb] 1fee256 - Clear old section-to-addr entry when loading Section at new addr

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2022-09-27T16:20:26-07:00
New Revision: 1fee25629d9d3f33cc618cb2b61cdf3823bfd092

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

LOG: Clear old section-to-addr entry when loading Section at new addr

SectionLoadList has a section-to-address map (m_sect_to_addr) and
an address-to-section map (m_addr_to_sect).  When the load address
of a section is updated, the old entry from m_addr_to_sect would
never be cleared, resulting in incorrect address-to-section address
lookups from that point forward.

Differential Revision: https://reviews.llvm.org/D130534
rdar://97308773

Added: 
lldb/test/API/functionalities/multiple-slides/Makefile
lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py
lldb/test/API/functionalities/multiple-slides/main.c

Modified: 
lldb/source/Target/SectionLoadList.cpp

Removed: 




diff  --git a/lldb/source/Target/SectionLoadList.cpp 
b/lldb/source/Target/SectionLoadList.cpp
index 03160ee10dd84..a78ca3360f2ec 100644
--- a/lldb/source/Target/SectionLoadList.cpp
+++ b/lldb/source/Target/SectionLoadList.cpp
@@ -116,8 +116,18 @@ bool SectionLoadList::SetSectionLoadAddress(const 
lldb::SectionSP ,
 }
   }
   ats_pos->second = section;
-} else
+} else {
+  // Remove the old address->section entry, if
+  // there is one.
+  for (const auto  : m_addr_to_sect) {
+if (entry.second == section) {
+  const auto _pos = m_addr_to_sect.find(entry.first);
+  m_addr_to_sect.erase(it_pos);
+  break;
+}
+  }
   m_addr_to_sect[load_addr] = section;
+}
 return true; // Changed
 
   } else {

diff  --git a/lldb/test/API/functionalities/multiple-slides/Makefile 
b/lldb/test/API/functionalities/multiple-slides/Makefile
new file mode 100644
index 0..5f83deaa24d77
--- /dev/null
+++ b/lldb/test/API/functionalities/multiple-slides/Makefile
@@ -0,0 +1,12 @@
+C_SOURCES := main.c
+MAKE_DSYM := NO
+
+include Makefile.rules
+
+# lldb has a separate bug where this test case
+# does not work if we have debug info - after
+# sliding the binary, the address of `first` and
+# `second` are not slid for some reason on Darwin.
+main.o: main.c
+   $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@
+

diff  --git 
a/lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py 
b/lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py
new file mode 100644
index 0..d89cc70a94333
--- /dev/null
+++ b/lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py
@@ -0,0 +1,79 @@
+"""
+Test that a binary can be slid to 
diff erent load addresses correctly
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MultipleSlidesTestCase(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+def test_mulitple_slides(self):
+"""Test that a binary can be slid multiple times correctly."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+err = lldb.SBError()
+load_dependent_modules = False
+target = self.dbg.CreateTarget(exe, '', '', load_dependent_modules, 
err)
+self.assertTrue(target.IsValid())
+module = target.GetModuleAtIndex(0)
+self.assertTrue(module.IsValid())
+
+first_sym = 
target.FindSymbols("first").GetContextAtIndex(0).GetSymbol()
+second_sym = 
target.FindSymbols("second").GetContextAtIndex(0).GetSymbol()
+first_size = first_sym.GetEndAddress().GetOffset() - 
first_sym.GetStartAddress().GetOffset()
+second_size = second_sym.GetEndAddress().GetOffset() - 
second_sym.GetStartAddress().GetOffset()
+
+# View the first element of `first` and `second` while
+# they have no load address set.
+self.expect("p/d ((int*))[0]", substrs=['= 5'])
+self.expect("p/d ((int*))[0]", substrs=['= 6'])
+self.assertEqual(first_sym.GetStartAddress().GetLoadAddress(target), 
lldb.LLDB_INVALID_ADDRESS)
+self.assertEqual(second_sym.GetStartAddress().GetLoadAddress(target), 
lldb.LLDB_INVALID_ADDRESS)
+
+
+# View the first element of `first` and `second` with
+# no slide applied, but with load address set.
+#
+# In memory, we have something like
+#0x1000 - 0x17ff  first[]
+#0x1800 - 0x1fff  second[]
+target.SetModuleLoadAddress(module, 0)
+self.expect("p/d ((int*))[0]", substrs=['= 5'])
+self.expect("p/d ((int*))[0]", substrs=['= 6'])
+self.assertEqual(first_sym.GetStartAddress().GetLoadAddress(target), 
+ first_sym.GetStartAddress().GetFileAddress())
+

[Lldb-commits] [PATCH] D134771: [NFCI] Simplify TypeCategoryImpl for-each callbacks.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe created this revision.
jgorbe added reviewers: jingham, labath.
jgorbe added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.

The callback system to iterate over every formatter of a given kind in
a TypeCategoryImpl is only used in one place (the implementation of
`type {formatter_kind} list`), and it's too convoluted for the sake of
unused flexibility.

This change changes it so that only one callback is passed to `ForEach`
(instead of a callback for exact matches and another one for regex
matches), and moves the iteration logic to `TieredFormatterContainer`
to avoid duplication.

If in the future we need different logic in the callback depending on
exact/regex match, the callback can get the type of formatter matching
used from the TypeMatcher argument anyway.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134771

Files:
  lldb/include/lldb/DataFormatters/TypeCategory.h
  lldb/source/Commands/CommandObjectType.cpp

Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -1097,36 +1097,20 @@
   "---\nCategory: %s%s\n---\n",
   category->GetName(), category->IsEnabled() ? "" : " (disabled)");
 
-  TypeCategoryImpl::ForEachCallbacks foreach;
-  foreach
-.SetExact([, _regex, _printed](
-  const TypeMatcher _matcher,
-  const FormatterSharedPointer _sp) -> bool {
-  if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
- formatter_regex.get())) {
-any_printed = true;
-result.GetOutputStream().Printf(
-"%s: %s\n", type_matcher.GetMatchString().GetCString(),
-format_sp->GetDescription().c_str());
-  }
-  return true;
-});
-
-  foreach
-.SetWithRegex([, _regex, _printed](
-  const TypeMatcher _matcher,
-  const FormatterSharedPointer _sp) -> bool {
-  if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
- formatter_regex.get())) {
-any_printed = true;
-result.GetOutputStream().Printf(
-"%s: %s\n", type_matcher.GetMatchString().GetCString(),
-format_sp->GetDescription().c_str());
-  }
-  return true;
-});
-
-  category->ForEach(foreach);
+  TypeCategoryImpl::ForEachCallback print_formatter =
+  [, _regex,
+   _printed](const TypeMatcher _matcher,
+ const FormatterSharedPointer _sp) -> bool {
+if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
+   formatter_regex.get())) {
+  any_printed = true;
+  result.GetOutputStream().Printf(
+  "%s: %s\n", type_matcher.GetMatchString().GetCString(),
+  format_sp->GetDescription().c_str());
+}
+return true;
+  };
+  category->ForEach(print_formatter);
 };
 
 if (m_options.m_category_language.OptionWasSet()) {
Index: lldb/include/lldb/DataFormatters/TypeCategory.h
===
--- lldb/include/lldb/DataFormatters/TypeCategory.h
+++ lldb/include/lldb/DataFormatters/TypeCategory.h
@@ -130,6 +130,16 @@
 return lldb::TypeNameSpecifierImplSP();
   }
 
+  /// Iterates through tiers in order, running `callback` on each element of
+  /// each tier.
+  void ForEach(std::function)>
+   callback) {
+for (int tier = 0; tier <= lldb::eLastFormatterMatchType; ++tier) {
+  m_subcontainers[tier]->ForEach(callback);
+}
+  }
+
  private:
   std::array, lldb::eLastFormatterMatchType + 1>
   m_subcontainers;
@@ -146,120 +156,35 @@
   typedef uint16_t FormatCategoryItems;
   static const uint16_t ALL_ITEM_TYPES = UINT16_MAX;
 
-  template  class ForEachCallbacks {
-  public:
-ForEachCallbacks() = default;
-~ForEachCallbacks() = default;
-
-template 
-typename std::enable_if::value, ForEachCallbacks &>::type
-SetExact(FormatContainer::ForEachCallback callback) {
-  m_format_exact = std::move(callback);
-  return *this;
-}
-template 
-typename std::enable_if::value, ForEachCallbacks &>::type
-SetWithRegex(FormatContainer::ForEachCallback callback) {
-  m_format_regex = std::move(callback);
-  return *this;
-}
-
-template 
-typename std::enable_if::value, ForEachCallbacks &>::type
-SetExact(SummaryContainer::ForEachCallback callback) {
-  m_summary_exact = std::move(callback);
-  return *this;
-}
-template 
-typename std::enable_if::value, ForEachCallbacks &>::type
-

[Lldb-commits] [PATCH] D134508: Track which modules have debug info variable errors.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D134508#381 , @JDevlieghere 
wrote:

> Instead of storing a bool, would it make sense to store the error instead and 
> then have the statistics call `isFail` on it to determine whether there was 
> an issue?

We would need to store a list of unique error strings if we want to go that 
route since we might stop N times in M compile units and currently the errors 
are on the compile unit boundary. We wouldn't want just one string and if we 
stopped 12 times in a the same function we don't want 12 copies of the same 
string in the stats. So if we could use a

  std::set m_var_errors;

I'd be happy to go this route if anyone feels this would be useful. I just want 
to know when there are errors, but I also would love to have the exact errors 
to help us track down.

Please chime in everyone!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134508

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

[Lldb-commits] [PATCH] D134768: [NFCI] More TypeCategoryImpl refactoring.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe abandoned this revision.
jgorbe added a comment.

whoops, I need to commit some other local change before this one. I'm going to 
abandon this revision and I'll re-send this patch when its dependency lands. 
Sorry for the noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134768

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

[Lldb-commits] [PATCH] D134768: [NFCI] More TypeCategoryImpl refactoring.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe created this revision.
jgorbe added reviewers: jingham, labath.
jgorbe added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.

The main aim of this patch is deleting the remaining instances of code
reaching into the internals of `TypeCategoryImpl`. I made the following
changes:

- Add some more methods to `TieredFormatterContainer` and `TypeCategoryImpl` to 
expose functionality that is implemented in `FormattersContainer`.

- Add new overloads of `TypeCategoryImpl::AddTypeXXX` to make it easier to add 
formatters to categories without reaching into the internal 
`FormattersContainer` objects.

- Remove the `GetTypeXXXContainer` and `GetRegexTypeXXXContainer` accessors 
from `TypeCategoryImpl` and update all call sites to use the new methods 
instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134768

Files:
  lldb/include/lldb/DataFormatters/TypeCategory.h
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/DataFormatters/FormattersHelpers.cpp
  lldb/source/DataFormatters/TypeCategory.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -284,12 +284,12 @@
 
   lldb::TypeSummaryImplSP ObjC_BOOL_summary(new CXXFunctionSummaryFormat(
   objc_flags, lldb_private::formatters::ObjCBOOLSummaryProvider, ""));
-  objc_category_sp->GetTypeSummariesContainer()->Add(ConstString("BOOL"),
- ObjC_BOOL_summary);
-  objc_category_sp->GetTypeSummariesContainer()->Add(ConstString("BOOL &"),
- ObjC_BOOL_summary);
-  objc_category_sp->GetTypeSummariesContainer()->Add(ConstString("BOOL *"),
- ObjC_BOOL_summary);
+  objc_category_sp->AddTypeSummary("BOOL", eFormatterMatchExact,
+   ObjC_BOOL_summary);
+  objc_category_sp->AddTypeSummary("BOOL &", eFormatterMatchExact,
+   ObjC_BOOL_summary);
+  objc_category_sp->AddTypeSummary("BOOL *", eFormatterMatchExact,
+   ObjC_BOOL_summary);
 
   // we need to skip pointers here since we are special casing a SEL* when
   // retrieving its value
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -764,8 +764,8 @@
   ConstString("^std::__[[:alnum:]]+::span<.+>(( )?&)?$"), stl_deref_flags,
   true);
 
-  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
-  RegularExpression("^(std::__[[:alnum:]]+::)deque<.+>(( )?&)?$"),
+  cpp_category_sp->AddTypeSynthetic(
+  "^(std::__[[:alnum:]]+::)deque<.+>(( )?&)?$", eFormatterMatchRegex,
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
   stl_synth_flags,
   "lldb.formatters.cpp.libcxx.stddeque_SynthProvider")));
@@ -958,55 +958,52 @@
   stl_summary_flags, LibStdcppWStringSummaryProvider,
   "libstdc++ c++11 std::wstring summary provider"));
 
-  cpp_category_sp->GetTypeSummariesContainer()->Add(ConstString("std::string"),
-std_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::basic_string"), std_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::basic_string,std::"
-  "allocator >"),
-  std_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::basic_string, "
-  "std::allocator >"),
-  std_string_summary_sp);
-
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::__cxx11::string"), cxx11_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::__cxx11::basic_string, "
-  "std::allocator >"),
-  cxx11_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::__cxx11::basic_string, "
-  "std::allocator >"),
-  cxx11_string_summary_sp);
+  cpp_category_sp->AddTypeSummary("std::string", eFormatterMatchExact,
+  std_string_summary_sp);
+  cpp_category_sp->AddTypeSummary("std::basic_string",
+  eFormatterMatchRegex, std_string_summary_sp);
+  cpp_category_sp->AddTypeSummary(
+  "std::basic_string,std::allocator >",
+  

[Lldb-commits] [PATCH] D133910: [NFCI] Refactor FormatterContainerPair into TieredFormatterContainer.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
jgorbe marked 3 inline comments as done.
Closed by commit rG1f2a21820dfa: [NFCI] Refactor FormatterContainerPair into 
TieredFormatterContainer. (authored by jgorbe).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133910

Files:
  lldb/include/lldb/DataFormatters/FormattersContainer.h
  lldb/include/lldb/DataFormatters/TypeCategory.h
  lldb/source/API/SBTypeCategory.cpp
  lldb/source/DataFormatters/TypeCategory.cpp

Index: lldb/source/DataFormatters/TypeCategory.cpp
===
--- lldb/source/DataFormatters/TypeCategory.cpp
+++ lldb/source/DataFormatters/TypeCategory.cpp
@@ -87,10 +87,7 @@
lldb::TypeFormatImplSP ) {
   if (!IsEnabled() || !IsApplicable(lang))
 return false;
-  if (GetTypeFormatsContainer()->Get(candidates, entry))
-return true;
-  bool regex = GetRegexTypeFormatsContainer()->Get(candidates, entry);
-  return regex;
+  return m_format_cont.Get(candidates, entry);
 }
 
 bool TypeCategoryImpl::Get(lldb::LanguageType lang,
@@ -98,10 +95,7 @@
lldb::TypeSummaryImplSP ) {
   if (!IsEnabled() || !IsApplicable(lang))
 return false;
-  if (GetTypeSummariesContainer()->Get(candidates, entry))
-return true;
-  bool regex = GetRegexTypeSummariesContainer()->Get(candidates, entry);
-  return regex;
+  return m_summary_cont.Get(candidates, entry);
 }
 
 bool TypeCategoryImpl::Get(lldb::LanguageType lang,
@@ -109,30 +103,29 @@
lldb::SyntheticChildrenSP ) {
   if (!IsEnabled() || !IsApplicable(lang))
 return false;
-  TypeFilterImpl::SharedPointer filter_sp;
+
   // first find both Filter and Synth, and then check which is most recent
+  bool pick_synth = false;
 
-  if (!GetTypeFiltersContainer()->Get(candidates, filter_sp))
-GetRegexTypeFiltersContainer()->Get(candidates, filter_sp);
+  TypeFilterImpl::SharedPointer filter_sp;
+  m_filter_cont.Get(candidates, filter_sp);
 
-  bool pick_synth = false;
-  ScriptedSyntheticChildren::SharedPointer synth;
-  if (!GetTypeSyntheticsContainer()->Get(candidates, synth))
-GetRegexTypeSyntheticsContainer()->Get(candidates, synth);
-  if (!filter_sp.get() && !synth.get())
+  ScriptedSyntheticChildren::SharedPointer synth_sp;
+  m_synth_cont.Get(candidates, synth_sp);
+
+  if (!filter_sp.get() && !synth_sp.get())
 return false;
-  else if (!filter_sp.get() && synth.get())
+  else if (!filter_sp.get() && synth_sp.get())
 pick_synth = true;
-
-  else if (filter_sp.get() && !synth.get())
+  else if (filter_sp.get() && !synth_sp.get())
 pick_synth = false;
-
-  else /*if (filter_sp.get() && synth.get())*/
+  else /*if (filter_sp.get() && synth_sp.get())*/
   {
-pick_synth = filter_sp->GetRevision() <= synth->GetRevision();
+pick_synth = filter_sp->GetRevision() <= synth_sp->GetRevision();
   }
+
   if (pick_synth) {
-entry = synth;
+entry = synth_sp;
 return true;
   } else {
 entry = filter_sp;
@@ -276,138 +269,62 @@
 
 TypeCategoryImpl::FormatContainer::MapValueType
 TypeCategoryImpl::GetFormatForType(lldb::TypeNameSpecifierImplSP type_sp) {
-  FormatContainer::MapValueType retval;
-
-  if (type_sp) {
-if (type_sp->IsRegex())
-  GetRegexTypeFormatsContainer()->GetExact(ConstString(type_sp->GetName()),
-   retval);
-else
-  GetTypeFormatsContainer()->GetExact(ConstString(type_sp->GetName()),
-  retval);
-  }
-
-  return retval;
+  return m_format_cont.GetForTypeNameSpecifier(type_sp);
 }
 
 TypeCategoryImpl::SummaryContainer::MapValueType
 TypeCategoryImpl::GetSummaryForType(lldb::TypeNameSpecifierImplSP type_sp) {
-  SummaryContainer::MapValueType retval;
-
-  if (type_sp) {
-if (type_sp->IsRegex())
-  GetRegexTypeSummariesContainer()->GetExact(
-  ConstString(type_sp->GetName()), retval);
-else
-  GetTypeSummariesContainer()->GetExact(ConstString(type_sp->GetName()),
-retval);
-  }
-
-  return retval;
+  return m_summary_cont.GetForTypeNameSpecifier(type_sp);
 }
 
 TypeCategoryImpl::FilterContainer::MapValueType
 TypeCategoryImpl::GetFilterForType(lldb::TypeNameSpecifierImplSP type_sp) {
-  FilterContainer::MapValueType retval;
-
-  if (type_sp) {
-if (type_sp->IsRegex())
-  GetRegexTypeFiltersContainer()->GetExact(ConstString(type_sp->GetName()),
-   retval);
-else
-  GetTypeFiltersContainer()->GetExact(ConstString(type_sp->GetName()),
-  retval);
-  }
-
-  return retval;
+  return m_filter_cont.GetForTypeNameSpecifier(type_sp);
 }
 
 TypeCategoryImpl::SynthContainer::MapValueType
 

[Lldb-commits] [lldb] 1f2a218 - [NFCI] Refactor FormatterContainerPair into TieredFormatterContainer.

[Jorge Gorbe Moya via lldb-commits]

Author: Jorge Gorbe Moya
Date: 2022-09-27T14:28:41-07:00
New Revision: 1f2a21820dfa2c97de8cc9e09cd03cf1c1684e31

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

LOG: [NFCI] Refactor FormatterContainerPair into TieredFormatterContainer.

`FormatterContainerPair` is (as its name indicates) a very thin wrapper
over two formatter containers, one for exact matches and another one for
regex matches. The logic to decide which subcontainer to access is
replicated everywhere `FormatterContainerPair`s are used.

So, for example, when we look for a formatter there's some adhoc code
that does a lookup in the exact match formatter container, and if it
fails it does a lookup in the regex match formatter container. The same
logic is then copied and pasted for summaries, filters, and synthetic
child providers.

This change introduces a new `TieredFormatterContainer` that has two
main characteristics:

- It generalizes `FormatterContainerPair` from 2 to any number of
  subcontainers, that are looked up in priority order.
- It centralizes all the logic to choose which subcontainer to use for
  lookups, add/delete, and indexing.

This allows us to have a single copy of the same logic, templatized for
each kind of formatter. It also simplifies the upcoming addition of a
new tier of callback-based matches. See
https://discourse.llvm.org/t/rfc-python-callback-for-data-formatters-type-matching/64204
for more details about this.

The rest of the change is mostly replacing copy-pasted code with calls
to methods of the relevant `TieredFormatterContainer`, and adding some
methods to the `TypeCategoryImpl` class so we can remove some of this
copy-pasted code from `SBTypeCategory`.

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

Added: 


Modified: 
lldb/include/lldb/DataFormatters/FormattersContainer.h
lldb/include/lldb/DataFormatters/TypeCategory.h
lldb/source/API/SBTypeCategory.cpp
lldb/source/DataFormatters/TypeCategory.cpp

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/FormattersContainer.h 
b/lldb/include/lldb/DataFormatters/FormattersContainer.h
index 58df8b9610734..8a93c0345cbe5 100644
--- a/lldb/include/lldb/DataFormatters/FormattersContainer.h
+++ b/lldb/include/lldb/DataFormatters/FormattersContainer.h
@@ -78,6 +78,14 @@ class TypeMatcher {
   TypeMatcher(RegularExpression regex)
   : m_type_name_regex(std::move(regex)),
 m_match_type(lldb::eFormatterMatchRegex) {}
+  /// Creates a matcher using the matching type and string from the given type
+  /// name specifier.
+  TypeMatcher(lldb::TypeNameSpecifierImplSP type_specifier)
+  : m_type_name(type_specifier->GetName()),
+m_match_type(type_specifier->GetMatchType()) {
+if (m_match_type == lldb::eFormatterMatchRegex)
+  m_type_name_regex = RegularExpression(type_specifier->GetName());
+  }
 
   /// True iff this matches the given type name.
   bool Matches(ConstString type_name) const {
@@ -158,6 +166,20 @@ template  class FormattersContainer {
 return false;
   }
 
+  bool Get(const FormattersMatchVector , ValueSP ) {
+for (const FormattersMatchCandidate  : candidates) {
+  if (Get(candidate.GetTypeName(), entry)) {
+if (candidate.IsMatch(entry) == false) {
+  entry.reset();
+  continue;
+} else {
+  return true;
+}
+  }
+}
+return false;
+  }
+
   bool GetExact(TypeMatcher matcher, ValueSP ) {
 std::lock_guard guard(m_map_mutex);
 for (const auto  : m_map)
@@ -219,20 +241,6 @@ template  class FormattersContainer {
   FormattersContainer(const FormattersContainer &) = delete;
   const FormattersContainer =(const FormattersContainer &) = delete;
 
-  bool Get(const FormattersMatchVector , ValueSP ) {
-for (const FormattersMatchCandidate  : candidates) {
-  if (Get(candidate.GetTypeName(), entry)) {
-if (candidate.IsMatch(entry) == false) {
-  entry.reset();
-  continue;
-} else {
-  return true;
-}
-  }
-}
-return false;
-  }
-
   MapType m_map;
   std::recursive_mutex m_map_mutex;
   IFormatChangeListener *listener;

diff  --git a/lldb/include/lldb/DataFormatters/TypeCategory.h 
b/lldb/include/lldb/DataFormatters/TypeCategory.h
index 16255f9488bda..2f0cb6c78472f 100644
--- a/lldb/include/lldb/DataFormatters/TypeCategory.h
+++ b/lldb/include/lldb/DataFormatters/TypeCategory.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_DATAFORMATTERS_TYPECATEGORY_H
 #define LLDB_DATAFORMATTERS_TYPECATEGORY_H
 
+#include 
 #include 
 #include 
 #include 
@@ -23,66 +24,128 @@
 
 namespace lldb_private {
 
-template  class FormatterContainerPair {
+// A formatter container with sub-containers for 
diff erent priority tiers, that
+// also exposes a flat view of all 

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added a comment.

>> For members from parent classes, pdb only has information saying that its 
>> direct parents class are at some offsets for this class. For class without 
>> vtable, it's easy to calculate inherited member offsets by adding parent 
>> class offsets with their member offsets. For class with vtable, it's more 
>> complicated to calculate the offsets.
>
> Yes, so should we take the offsets from the debug info and provide them to 
> clang (so that it does not have to recompute the offsets) ?

Oh, it's already there 
.
 This patch just add the missing bit_size.

>>> If not, how does it handle the case when the definition of some class is 
>>> missing? If that class is a member of some other class, the offsets of all 
>>> subsequent members in the bigger class will be wrong? That will probably be 
>>> true even if we are always able to obtain the size of the smaller class, 
>>> because of things like vtable pointers.
>>
>> I don't quite understand this. vtable pointers are ignored in this visitor 
>> https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp#L136-L139.
>
> Imagine a class like `struct B : A { virtual void foo(); int b; };` and two 
> classes like `struct A1 { virtual void bar(); }; struct A2 { void *a; };`. 
> Both A1 and A2 have the same size (`sizeof(void *)`), but that doesn't mean 
> the layout of B will be the same if we substitute it for the base 
> . If A = A1, then B will reuse the vtable 
> pointer of the base, and `offsetof(B, b)` will be 8. That can't happen with 
> A2, so the compiler will create a new vtable pointer and `b` offset will be 
> 16 (2*sizeof(void)) (at least that's how it works in the itanium ABI, but I'm 
> sure you could create something similar with the MSVC ABI as well). If you 
> don't have the definition of A, then you just can't know which of these two 
> cases  you are in. That's why I think that getting the member locations from 
> the debug info is important.

Yeah, in this case, pdb has the info: offsetof(B, b) == 8 and 
offsetof(B, b) == 16. And they are added into `LayoutInfo::field_offsets`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134656

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

[Lldb-commits] [PATCH] D134636: [lldb][Windows] Always call SetExecutableModule on debugger connected

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

This looks good to me, fwiw.  I agree with Pavel about hardcoding the order 
that libraries might appear unless that ordering is essentially considered API. 
 We could get in a situation where different versions of windows report the 
binaries in different order, and it might be confusing for people to understand 
why a bot is failing when their desktop passes etc.  Not a platform I know 
anything about though, don't have a real objection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134636

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

[Lldb-commits] [lldb] 670cac7 - Rename a duplicate test, also give the test class a useful name.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-09-27T12:38:22-07:00
New Revision: 670cac72804bbeb91fd0653de621aa03f70600e3

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

LOG: Rename a duplicate test, also give the test class a useful name.

Added: 

lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py

Modified: 
lldb/test/API/lang/cpp/function-qualifiers/TestCppFunctionQualifiers.py

Removed: 
lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py



diff  --git 
a/lldb/test/API/lang/cpp/function-qualifiers/TestCppFunctionQualifiers.py 
b/lldb/test/API/lang/cpp/function-qualifiers/TestCppFunctionQualifiers.py
index bc3b6d156046d..13ef2fc30330b 100644
--- a/lldb/test/API/lang/cpp/function-qualifiers/TestCppFunctionQualifiers.py
+++ b/lldb/test/API/lang/cpp/function-qualifiers/TestCppFunctionQualifiers.py
@@ -3,7 +3,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestCase(TestBase):
+class TestFunctionQualifiers(TestBase):
 
 def test(self):
 self.build()

diff  --git 
a/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py 
b/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py
similarity index 96%
rename from 
lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
rename to 
lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py
index 0016ede4937e2..152e8c94a7fc7 100644
--- 
a/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
+++ 
b/lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py
@@ -9,7 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestCase(TestBase):
+class TestFunctionRefQualifiers(TestBase):
 
 def test(self):
 self.build()



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

[Lldb-commits] [PATCH] D134508: Track which modules have debug info variable errors.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Instead of storing a bool, would it make sense to store the error instead and 
then have the statistics call `isFail` on it to determine whether there was an 
issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134508

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

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D134656#3818759 , @zequanwu wrote:

> In D134656#3818234 , @labath wrote:
>
>> That said, I am kinda surprised that this is the whole thing. I would have 
>> expected to see more here. In dwarf we specify the offsets of individual 
>> class members. Does PDB encode that information?
>
> Yes. It has offsets to non-inherited individual class members.

Yes, I only meant direct members. And when I speak of members here, I am also 
thinking about base classes as they behave very similarly here.

> For members from parent classes, pdb only has information saying that its 
> direct parents class are at some offsets for this class. For class without 
> vtable, it's easy to calculate inherited member offsets by adding parent 
> class offsets with their member offsets. For class with vtable, it's more 
> complicated to calculate the offsets.

Yes, so should we take the offsets from the debug info and provide them to 
clang (so that it does not have to recompute the offsets) ?

>> If not, how does it handle the case when the definition of some class is 
>> missing? If that class is a member of some other class, the offsets of all 
>> subsequent members in the bigger class will be wrong? That will probably be 
>> true even if we are always able to obtain the size of the smaller class, 
>> because of things like vtable pointers.
>
> I don't quite understand this. vtable pointers are ignored in this visitor 
> https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp#L136-L139.

Imagine a class like `struct B : A { virtual void foo(); int b; };` and two 
classes like `struct A1 { virtual void bar(); }; struct A2 { void *a; };`. Both 
A1 and A2 have the same size (`sizeof(void *)`), but that doesn't mean the 
layout of B will be the same if we substitute it for the base 
. If A = A1, then B will reuse the vtable 
pointer of the base, and `offsetof(B, b)` will be 8. That can't happen with A2, 
so the compiler will create a new vtable pointer and `b` offset will be 16 
(2*sizeof(void)) (at least that's how it works in the itanium ABI, but I'm sure 
you could create something similar with the MSVC ABI as well). If you don't 
have the definition of A, then you just can't know which of these two cases  
you are in. That's why I think that getting the member locations from the debug 
info is important.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134656

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

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

[jeffrey tan via Phabricator via lldb-commits]

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

LGTM pending Pavel's suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134333

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

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D133858#3818755 , @jingham wrote:

> If we use Target::CreateProcess to handle this task, how do we ensure that 
> that will always get called any time we make a new process?

I'm not really sure how to answer that.

Clearly something like that can happen, but I think the overall risk is low. 
The most likely scenario for that happening would be if someone adds a new, 
fourth, method of initiating a process (in addition to launching, attaching and 
connecting), but I just don't know what would that be. But if one is doing 
that, I think it's at least as likely that he will forget to create the 
`DoWillInitiate` method and call the breakpoint reset function from there.

I think the only way to ensure that can't happen is to store the actual hit 
counts within the Process class itself, but I don't think anyone has an apetite 
to design something like that.

Anyone trying to create a function by bypassing a function called CreateProcess 
should think really hard before proceeding. And the more stuff we put into that 
function, the harder it will be for someone to bypass it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133858

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

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

[Zequan Wu via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd434f437200b: [LLDB][NativePDB] Add class/union layout bit 
size. (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134656

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp


Index: lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
@@ -0,0 +1,39 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Make sure class layout is correct.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe \
+// RUN:   -o "expr a" -o "expr b.c" -o "expr b.u.c" -o "expr b.u.i" -o "exit" 
| FileCheck %s
+
+// CHECK:  (lldb) expr a
+// CHECK-NEXT: (A) $0 = (d1 = 'a', d2 = 1, d3 = 2, d4 = 'b')
+// CHECK-NEXT: (lldb) expr b.c
+// CHECK-NEXT: (char) $1 = 'a'
+// CHECK-NEXT: (lldb) expr b.u.c
+// CHECK-NEXT: (char[2]) $2 = "b"
+// CHECK-NEXT: (lldb) expr b.u.i
+// CHECK-NEXT: (int) $3 = 98
+
+struct __attribute__((packed, aligned(1))) A {
+  char d1;
+  int d2;
+  int d3;
+  char d4;
+};
+
+struct __attribute__((packed, aligned(1))) B {
+  char c;
+  union {
+char c[2];
+int i;
+  } u;
+};
+
+A a = {'a', 1, 2, 'b'};
+B b = {'a', {"b"}};
+
+int main() {
+  return 0;
+}
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -45,10 +45,12 @@
 break;
   case LF_UNION:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, 
m_cvr.ur));
+m_layout.bit_size = m_cvr.ur.getSize() * 8;
 break;
   case LF_CLASS:
   case LF_STRUCTURE:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, 
m_cvr.cr));
+m_layout.bit_size = m_cvr.cr.getSize() * 8;
 break;
   default:
 llvm_unreachable("unreachable!");


Index: lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
@@ -0,0 +1,39 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Make sure class layout is correct.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe \
+// RUN:   -o "expr a" -o "expr b.c" -o "expr b.u.c" -o "expr b.u.i" -o "exit" | FileCheck %s
+
+// CHECK:  (lldb) expr a
+// CHECK-NEXT: (A) $0 = (d1 = 'a', d2 = 1, d3 = 2, d4 = 'b')
+// CHECK-NEXT: (lldb) expr b.c
+// CHECK-NEXT: (char) $1 = 'a'
+// CHECK-NEXT: (lldb) expr b.u.c
+// CHECK-NEXT: (char[2]) $2 = "b"
+// CHECK-NEXT: (lldb) expr b.u.i
+// CHECK-NEXT: (int) $3 = 98
+
+struct __attribute__((packed, aligned(1))) A {
+  char d1;
+  int d2;
+  int d3;
+  char d4;
+};
+
+struct __attribute__((packed, aligned(1))) B {
+  char c;
+  union {
+char c[2];
+int i;
+  } u;
+};
+
+A a = {'a', 1, 2, 'b'};
+B b = {'a', {"b"}};
+
+int main() {
+  return 0;
+}
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -45,10 +45,12 @@
 break;
   case LF_UNION:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, m_cvr.ur));
+m_layout.bit_size = m_cvr.ur.getSize() * 8;
 break;
   case LF_CLASS:
   case LF_STRUCTURE:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, m_cvr.cr));
+m_layout.bit_size = m_cvr.cr.getSize() * 8;
 break;
   default:
 llvm_unreachable("unreachable!");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] d434f43 - [LLDB][NativePDB] Add class/union layout bit size.

[Zequan Wu via lldb-commits]

Author: Zequan Wu
Date: 2022-09-27T11:41:55-07:00
New Revision: d434f437200bdf8d94a47346dd5c64cc3cc5c5fa

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

LOG: [LLDB][NativePDB] Add class/union layout bit size.

Missing it causes lldb to crash or give incorrect result.

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

Added: 
lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp

Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index 16e9990ff3f09..183ee5ad9c07a 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -45,10 +45,12 @@ UdtRecordCompleter::UdtRecordCompleter(
 break;
   case LF_UNION:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, 
m_cvr.ur));
+m_layout.bit_size = m_cvr.ur.getSize() * 8;
 break;
   case LF_CLASS:
   case LF_STRUCTURE:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, 
m_cvr.cr));
+m_layout.bit_size = m_cvr.cr.getSize() * 8;
 break;
   default:
 llvm_unreachable("unreachable!");

diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp 
b/lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
new file mode 100644
index 0..0389ae2a4edde
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
@@ -0,0 +1,39 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Make sure class layout is correct.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe \
+// RUN:   -o "expr a" -o "expr b.c" -o "expr b.u.c" -o "expr b.u.i" -o "exit" 
| FileCheck %s
+
+// CHECK:  (lldb) expr a
+// CHECK-NEXT: (A) $0 = (d1 = 'a', d2 = 1, d3 = 2, d4 = 'b')
+// CHECK-NEXT: (lldb) expr b.c
+// CHECK-NEXT: (char) $1 = 'a'
+// CHECK-NEXT: (lldb) expr b.u.c
+// CHECK-NEXT: (char[2]) $2 = "b"
+// CHECK-NEXT: (lldb) expr b.u.i
+// CHECK-NEXT: (int) $3 = 98
+
+struct __attribute__((packed, aligned(1))) A {
+  char d1;
+  int d2;
+  int d3;
+  char d4;
+};
+
+struct __attribute__((packed, aligned(1))) B {
+  char c;
+  union {
+char c[2];
+int i;
+  } u;
+};
+
+A a = {'a', 1, 2, 'b'};
+B b = {'a', {"b"}};
+
+int main() {
+  return 0;
+}



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

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added a comment.

In D134656#3818234 , @labath wrote:

> That said, I am kinda surprised that this is the whole thing. I would have 
> expected to see more here. In dwarf we specify the offsets of individual 
> class members. Does PDB encode that information?

Yes. It has offsets to non-inherited individual class members. For members from 
parent classes, pdb only has information saying that its direct parents class 
are at some offsets for this class. For class without vtable, it's easy to 
calculate inherited member offsets by adding parent class offsets with their 
member offsets. For class with vtable, it's more complicated to calculate the 
offsets.

> If not, how does it handle the case when the definition of some class is 
> missing? If that class is a member of some other class, the offsets of all 
> subsequent members in the bigger class will be wrong? That will probably be 
> true even if we are always able to obtain the size of the smaller class, 
> because of things like vtable pointers.

I don't quite understand this. vtable pointers are ignored in this visitor 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp#L136-L139.




Comment at: lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp:24
+
+union __attribute__((packed, aligned(1))) U {
+  char c[2];

labath wrote:
> I suspect these attributes do not materially change the layout of the union 
> when it stands on its own. It might be more interesting to take this 
> under-aligned union, place it into a (regular) struct, and then check whether 
> we can print it correctly.
Yeah, it doesn't change layout of the union. Moved to a struct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134656

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

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu updated this revision to Diff 463290.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134656

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp


Index: lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
@@ -0,0 +1,39 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Make sure class layout is correct.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe \
+// RUN:   -o "expr a" -o "expr b.c" -o "expr b.u.c" -o "expr b.u.i" -o "exit" 
| FileCheck %s
+
+// CHECK:  (lldb) expr a
+// CHECK-NEXT: (A) $0 = (d1 = 'a', d2 = 1, d3 = 2, d4 = 'b')
+// CHECK-NEXT: (lldb) expr b.c
+// CHECK-NEXT: (char) $1 = 'a'
+// CHECK-NEXT: (lldb) expr b.u.c
+// CHECK-NEXT: (char[2]) $2 = "b"
+// CHECK-NEXT: (lldb) expr b.u.i
+// CHECK-NEXT: (int) $3 = 98
+
+struct __attribute__((packed, aligned(1))) A {
+  char d1;
+  int d2;
+  int d3;
+  char d4;
+};
+
+struct __attribute__((packed, aligned(1))) B {
+  char c;
+  union {
+char c[2];
+int i;
+  } u;
+};
+
+A a = {'a', 1, 2, 'b'};
+B b = {'a', {"b"}};
+
+int main() {
+  return 0;
+}
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -45,10 +45,12 @@
 break;
   case LF_UNION:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, 
m_cvr.ur));
+m_layout.bit_size = m_cvr.ur.getSize() * 8;
 break;
   case LF_CLASS:
   case LF_STRUCTURE:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, 
m_cvr.cr));
+m_layout.bit_size = m_cvr.cr.getSize() * 8;
 break;
   default:
 llvm_unreachable("unreachable!");


Index: lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
@@ -0,0 +1,39 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Make sure class layout is correct.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe \
+// RUN:   -o "expr a" -o "expr b.c" -o "expr b.u.c" -o "expr b.u.i" -o "exit" | FileCheck %s
+
+// CHECK:  (lldb) expr a
+// CHECK-NEXT: (A) $0 = (d1 = 'a', d2 = 1, d3 = 2, d4 = 'b')
+// CHECK-NEXT: (lldb) expr b.c
+// CHECK-NEXT: (char) $1 = 'a'
+// CHECK-NEXT: (lldb) expr b.u.c
+// CHECK-NEXT: (char[2]) $2 = "b"
+// CHECK-NEXT: (lldb) expr b.u.i
+// CHECK-NEXT: (int) $3 = 98
+
+struct __attribute__((packed, aligned(1))) A {
+  char d1;
+  int d2;
+  int d3;
+  char d4;
+};
+
+struct __attribute__((packed, aligned(1))) B {
+  char c;
+  union {
+char c[2];
+int i;
+  } u;
+};
+
+A a = {'a', 1, 2, 'b'};
+B b = {'a', {"b"}};
+
+int main() {
+  return 0;
+}
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -45,10 +45,12 @@
 break;
   case LF_UNION:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, m_cvr.ur));
+m_layout.bit_size = m_cvr.ur.getSize() * 8;
 break;
   case LF_CLASS:
   case LF_STRUCTURE:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, m_cvr.cr));
+m_layout.bit_size = m_cvr.cr.getSize() * 8;
 break;
   default:
 llvm_unreachable("unreachable!");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

If we use Target::CreateProcess to handle this task, how do we ensure that that 
will always get called any time we make a new process?  That function doesn't 
do all that much special, it's only a couple lines long so it could easily get 
inlined somewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133858

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

[Lldb-commits] [PATCH] D134751: [lldb] Fix running tests when LLVM target triple is different from host

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

Supporting builds with different host and target triples makes sense. However, 
I have a few concerns about the current patch:

- I wouldn't cal it the "host triple". The API tests (`dotest.py`) support 
running the test suite for a different platform/architecture. Take a look at 
the Darwin builder and you'll see a bunch of support for that.
- How does this interact with the `-arch` flag? When the host and target triple 
differ, which part of the triple is different? In theory it can be any part, 
but maybe your use case could be covered by setting the `--arch` flag?
- This doesn't update `getTriple` in `builder.py` which is necessary for some 
of the decorators to work (in particular the ones that invoke the compiler to 
see if something is supported).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134751

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

[Lldb-commits] [PATCH] D134754: [lldb/gdb-server] Better reporting of launch errors

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: mgorny, jasonmolenda.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

Use our "rich error" facility to propagate error reported by the stub to
the user. lldb-server reports rich launch errors as of D133352 
.

To make this easier to implement, and reduce code duplication, I have
moved the vRun/A/qLaunchSuccess handling into a single
GDBRemoteCommunicationClient function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134754

Files:
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -86,7 +86,30 @@
 error = lldb.SBError()
 target.Launch(lldb.SBListener(), None, None, None, None, None,
 None, 0, True, error)
-self.assertEquals("'A' packet returned an error: 71", error.GetCString())
+self.assertRegex(error.GetCString(), "Cannot launch '.*a': Error 71")
+
+def test_launch_rich_error(self):
+class MyResponder(MockGDBServerResponder):
+def qC(self):
+return "E42"
+
+def qfThreadInfo(self):
+return "OK" # No threads.
+
+# Then, when we are asked to attach, error out.
+def vRun(self, packet):
+return "Eff;" + seven.hexlify("I'm a teapot")
+
+self.server.responder = MyResponder()
+
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process, [lldb.eStateConnected])
+
+error = lldb.SBError()
+target.Launch(lldb.SBListener(), None, None, None, None, None,
+None, 0, True, error)
+self.assertRegex(error.GetCString(), "Cannot launch '.*a': I'm a teapot")
 
 def test_read_registers_using_g_packets(self):
 """Test reading registers using 'g' packets (default behavior)"""
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -84,6 +84,7 @@
 
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -799,17 +800,17 @@
   GDBRemoteCommunication::ScopedTimeout timeout(m_gdb_comm,
 std::chrono::seconds(10));
 
-  int arg_packet_err = m_gdb_comm.SendArgumentsPacket(launch_info);
-  if (arg_packet_err == 0) {
-std::string error_str;
-if (m_gdb_comm.GetLaunchSuccess(error_str)) {
-  SetID(m_gdb_comm.GetCurrentProcessID());
-} else {
-  error.SetErrorString(error_str.c_str());
-}
+  // Since we can't send argv0 separate from the executable path, we need to
+  // make sure to use the actual executable path found in the launch_info...
+  Args args = launch_info.GetArguments();
+  if (FileSpec exe_file = launch_info.GetExecutableFile())
+args.ReplaceArgumentAtIndex(0, exe_file.GetPath(false));
+  if (llvm::Error err = m_gdb_comm.LaunchProcess(args)) {
+error.SetErrorStringWithFormatv("Cannot launch '{0}': {1}",
+args.GetArgumentAtIndex(0),
+llvm::fmt_consume(std::move(err)));
   } else {
-error.SetErrorStringWithFormat("'A' packet returned an error: %i",
-   arg_packet_err);
+SetID(m_gdb_comm.GetCurrentProcessID());
   }
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -80,8 +80,6 @@
 
   lldb::pid_t GetCurrentProcessID(bool allow_lazy = true);
 
-  bool GetLaunchSuccess(std::string _str);
-
   bool LaunchGDBServer(const char *remote_accept_hostname, lldb::pid_t ,
uint16_t , std::string _name);
 
@@ -90,19 +88,11 @@
 
   bool KillSpawnedProcess(lldb::pid_t pid);
 
-  /// Sends 

[Lldb-commits] [PATCH] D134509: [LLDB][NativePDB] Let native pdb use class layout in debug info.

[Zequan Wu via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3e60044d758: [LLDB][NativePDB] Let native pdb use class 
layout in debug info. (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134509

Files:
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -443,6 +443,7 @@
   // TypeSystem methods
   DWARFASTParser *GetDWARFParser() override;
   PDBASTParser *GetPDBParser() override;
+  npdb::PdbAstBuilder *GetNativePDBParser() override;
 
   // TypeSystemClang callbacks for external source lookups.
   void CompleteTagDecl(clang::TagDecl *);
@@ -1071,6 +1072,7 @@
   std::unique_ptr m_module_map_up;
   std::unique_ptr m_dwarf_ast_parser_up;
   std::unique_ptr m_pdb_ast_parser_up;
+  std::unique_ptr m_native_pdb_ast_parser_up;
   std::unique_ptr m_mangle_ctx_up;
   uint32_t m_pointer_byte_size = 0;
   bool m_ast_owned = false;
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -73,6 +73,7 @@
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
 #include "Plugins/SymbolFile/DWARF/DWARFASTParserClang.h"
 #include "Plugins/SymbolFile/PDB/PDBASTParser.h"
+#include "Plugins/SymbolFile/NativePDB/PdbAstBuilder.h"
 
 #include 
 
@@ -9364,6 +9365,12 @@
   return m_pdb_ast_parser_up.get();
 }
 
+npdb::PdbAstBuilder *TypeSystemClang::GetNativePDBParser() {
+  if (!m_native_pdb_ast_parser_up)
+m_native_pdb_ast_parser_up = std::make_unique(*this);
+  return m_native_pdb_ast_parser_up.get();
+}
+
 bool TypeSystemClang::LayoutRecordType(
 const clang::RecordDecl *record_decl, uint64_t _size,
 uint64_t ,
@@ -9377,6 +9384,8 @@
 importer = _dwarf_ast_parser_up->GetClangASTImporter();
   if (!importer && m_pdb_ast_parser_up)
 importer = _pdb_ast_parser_up->GetClangASTImporter();
+  if (!importer && m_native_pdb_ast_parser_up)
+importer = _native_pdb_ast_parser_up->GetClangASTImporter();
   if (!importer)
 return false;
 
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -312,6 +312,6 @@
   TypeSystemClang::CompleteTagDeclarationDefinition(m_derived_ct);
 
   if (auto *record_decl = llvm::dyn_cast(_tag_decl)) {
-m_ast_builder.importer().SetRecordLayout(record_decl, m_layout);
+m_ast_builder.GetClangASTImporter().SetRecordLayout(record_decl, m_layout);
   }
 }
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -19,6 +19,7 @@
 
 #include "CompileUnitIndex.h"
 #include "PdbIndex.h"
+#include "PdbAstBuilder.h"
 
 namespace clang {
 class TagDecl;
@@ -37,7 +38,6 @@
 namespace lldb_private {
 
 namespace npdb {
-class PdbAstBuilder;
 
 class SymbolFileNativePDB : public SymbolFileCommon {
   friend class UdtRecordCompleter;
@@ -137,6 +137,8 @@
   void FindFunctions(const RegularExpression , bool include_inlines,
  SymbolContextList _list) override;
 
+  llvm::Optional FindSymbolScope(PdbCompilandSymId id);
+
   void FindTypes(ConstString name, const CompilerDeclContext _decl_ctx,
  uint32_t max_matches,
  llvm::DenseSet _symbol_files,
@@ -158,8 +160,13 @@
   llvm::pdb::PDBFile () { return m_index->pdb(); }
   const llvm::pdb::PDBFile () const { return m_index->pdb(); }
 
+  PdbIndex () { return *m_index; };
+
   void DumpClangAST(Stream ) override;
 
+  llvm::Optional
+  GetParentType(llvm::codeview::TypeIndex ti);
+
 private:
   struct LineTableEntryComparator {
 bool operator()(const lldb_private::LineTable::Entry ,
@@ -180,6 +187,8 @@
 InlineSite(PdbCompilandSymId parent_id) : parent_id(parent_id){};
   };
 
+  void BuildParentMap();
+
   uint32_t CalculateNumCompileUnits() 

[Lldb-commits] [lldb] a3e6004 - [LLDB][NativePDB] Let native pdb use class layout in debug info.

[Zequan Wu via lldb-commits]

Author: Zequan Wu
Date: 2022-09-27T11:01:32-07:00
New Revision: a3e60044d75823a3cd8e35de76b7bde2599fe740

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

LOG: [LLDB][NativePDB] Let native pdb use class layout in debug info.

Before, class layout in native pdb is not hooked up in 
[[https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp#L9375-L9380
 | here]].
This changes hooked it up by refactoring SymbolFileNativePDB and PdbAstBuilder.
PdbIndex (corresponds to a single pdb file) is removed from PdbAstBuilder, so it
can only be accessed via SymbolFileNativePDB and we can construct PdbAstBuilder
with just TypeSystemClang.

Reviewed By: labath

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

Added: 


Modified: 
lldb/include/lldb/Symbol/TypeSystem.h
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Removed: 




diff  --git a/lldb/include/lldb/Symbol/TypeSystem.h 
b/lldb/include/lldb/Symbol/TypeSystem.h
index 769449a4933b2..57bdfa5b626ab 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -31,6 +31,9 @@ class DWARFASTParser;
 class PDBASTParser;
 
 namespace lldb_private {
+namespace npdb {
+  class PdbAstBuilder;
+} // namespace npdb
 
 /// A SmallBitVector that represents a set of source languages (\p
 /// lldb::LanguageType).  Each lldb::LanguageType is represented by
@@ -88,6 +91,7 @@ class TypeSystem : public PluginInterface {
 
   virtual DWARFASTParser *GetDWARFParser() { return nullptr; }
   virtual PDBASTParser *GetPDBParser() { return nullptr; }
+  virtual npdb::PdbAstBuilder *GetNativePDBParser() { return nullptr; }
 
   virtual SymbolFile *GetSymbolFile() const { return m_sym_file; }
 
@@ -366,7 +370,7 @@ class TypeSystem : public PluginInterface {
   LLVM_DUMP_METHOD virtual void
   dump(lldb::opaque_compiler_type_t type) const = 0;
 #endif
-  
+
   virtual void DumpValue(lldb::opaque_compiler_type_t type,
  ExecutionContext *exe_ctx, Stream *s,
  lldb::Format format, const DataExtractor ,

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index ab467f20990c4..2e17aa1260a83 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -24,6 +24,7 @@
 
 #include "PdbUtil.h"
 #include "UdtRecordCompleter.h"
+#include "SymbolFileNativePDB.h"
 
 using namespace lldb_private;
 using namespace lldb_private::npdb;
@@ -94,59 +95,6 @@ struct CreateMethodDecl : public TypeVisitorCallbacks {
 };
 } // namespace
 
-static llvm::Optional FindSymbolScope(PdbIndex ,
- PdbCompilandSymId id) 
{
-  CVSymbol sym = index.ReadSymbolRecord(id);
-  if (symbolOpensScope(sym.kind())) {
-// If this exact symbol opens a scope, we can just directly access its
-// parent.
-id.offset = getScopeParentOffset(sym);
-// Global symbols have parent offset of 0.  Return llvm::None to indicate
-// this.
-if (id.offset == 0)
-  return llvm::None;
-return id;
-  }
-
-  // Otherwise we need to start at the beginning and iterate forward until we
-  // reach (or pass) this particular symbol
-  CompilandIndexItem  = index.compilands().GetOrCreateCompiland(id.modi);
-  const CVSymbolArray  = cii.m_debug_stream.getSymbolArray();
-
-  auto begin = syms.begin();
-  auto end = syms.at(id.offset);
-  std::vector scope_stack;
-
-  while (begin != end) {
-if (begin.offset() > id.offset) {
-  // We passed it.  We couldn't even find this symbol record.
-  lldbassert(false && "Invalid compiland symbol id!");
-  return llvm::None;
-}
-
-// We haven't found the symbol yet.  Check if we need to open or close the
-// scope stack.
-if (symbolOpensScope(begin->kind())) {
-  // We can use the end offset of the scope to determine whether or not
-  // we can just outright skip this entire scope.
-  uint32_t scope_end = getScopeEndOffset(*begin);
-  if (scope_end < id.offset) {
-begin = syms.at(scope_end);
-  } else {
-// The symbol we're looking for is somewhere in this scope.
-scope_stack.emplace_back(id.modi, begin.offset());
-  }
-} else if 

[Lldb-commits] [PATCH] D134636: [lldb][Windows] Always call SetExecutableModule on debugger connected

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added a comment.

Okay, I'll rebase the change to remove the dependency on D134581 
.




Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:21-22
 # CHECK-NEXT: .main.exe
-# CHECK-NEXT: .shlib.dll
+# CHECK-NEXT: ntdll.dll
+# CHECK-NEXT: kernel32.dll
+# CHECK:  .shlib.dll

labath wrote:
> I'm not sure if hardcoding the order of system libraries (something which you 
> have no control of) is such a good idea.
I think it's fine-ish. AFAIK ntdll.dll is pretty much guaranteed to be loaded 
by the loader ahead of other DLLs and kernel32.dll is depended on by almost 
everything else. It is definitely an implementation detail though so I can 
understand the concern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134636

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

[Lldb-commits] [PATCH] D134751: [lldb] Fix running tests when LLVM target triple is different from host

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun created this revision.
alvinhochun added reviewers: labath, JDevlieghere, DavidSpickett, mstorsjo.
Herald added a project: All.
alvinhochun requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This passes the host triple to the lldbsuite test builder used in API
tests and the build helper used in Shell tests, so that Clang can use
the correct host triple when LLVM was configured with a default target
triple different from the host triple.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134751

Files:
  lldb/packages/Python/lldbsuite/test/builders/builder.py
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/test/API/lit.cfg.py
  lldb/test/Shell/BuildScript/script-args.test
  lldb/test/Shell/BuildScript/toolchain-clang.test
  lldb/test/Shell/helper/build.py
  lldb/test/Shell/helper/toolchain.py

Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -36,6 +36,8 @@
 build_script_args = [build_script,
 '--compiler=any', # Default to best compiler
 '--arch=' + str(config.lldb_bitness)]
+if config.host_triple != config.target_triple:
+build_script_args += ['--host-triple', config.host_triple]
 if config.lldb_lit_tools_dir:
 build_script_args.append('--tools-dir={0}'.format(config.lldb_lit_tools_dir))
 if config.lldb_tools_dir:
Index: lldb/test/Shell/helper/build.py
===
--- lldb/test/Shell/helper/build.py
+++ lldb/test/Shell/helper/build.py
@@ -35,6 +35,12 @@
 required=True,
 help='Path to a compiler executable, or one of the values [any, msvc, clang-cl, gcc, clang]')
 
+parser.add_argument('--host-triple',
+metavar='host_triple',
+dest='host_triple',
+required=False,
+help='The host target triple to use instead of the default')
+
 parser.add_argument('--libs-dir',
 metavar='directory',
 dest='libs_dir',
@@ -224,6 +230,7 @@
 self.opt = args.opt
 self.outdir = args.outdir
 self.compiler = args.compiler
+self.host_triple = args.host_triple
 self.clean = args.clean
 self.output = args.output
 self.mode = args.mode
@@ -632,6 +639,8 @@
 args = []
 
 args.append(self.compiler)
+if self.host_triple:
+args.append('--target=' + self.host_triple)
 args.append('-m' + self.arch)
 
 args.append('-g')
@@ -657,6 +666,8 @@
 def _get_link_command(self):
 args = []
 args.append(self.compiler)
+if self.host_triple:
+args.append('--target=' + self.host_triple)
 args.append('-m' + self.arch)
 if self.nodefaultlib:
 args.append('-nostdlib')
@@ -780,6 +791,7 @@
 print('Script Arguments:')
 print('  Arch: ' + args.arch)
 print('  Compiler: ' + args.compiler)
+print('  Host Triple: ' + (args.host_triple or ""))
 print('  Outdir: ' + args.outdir)
 print('  Output: ' + args.output)
 print('  Nodefaultlib: ' + str(args.nodefaultlib))
Index: lldb/test/Shell/BuildScript/toolchain-clang.test
===
--- lldb/test/Shell/BuildScript/toolchain-clang.test
+++ lldb/test/Shell/BuildScript/toolchain-clang.test
@@ -1,14 +1,19 @@
-RUN: %build -n --verbose --arch=32 --compiler=clang --mode=compile-and-link -o %t/foo.exe foobar.c \
+RUN: %build -n --verbose --arch=32 --compiler=clang --host-triple= --mode=compile-and-link -o %t/foo.exe foobar.c \
 RUN:| FileCheck --check-prefix=CHECK --check-prefix=CHECK-32 %s
 
-RUN: %build -n --verbose --arch=64 --compiler=clang --mode=compile-and-link -o %t/foo.exe foobar.c \
+RUN: %build -n --verbose --arch=64 --compiler=clang --host-triple= --mode=compile-and-link -o %t/foo.exe foobar.c \
 RUN:| FileCheck --check-prefix=CHECK --check-prefix=CHECK-64 %s
 
+RUN: %build -n --verbose --arch=64 --compiler=clang --host-triple=x86_64-linux-gnu --mode=compile-and-link -o %t/foo.exe foobar.c \
+RUN:| FileCheck --check-prefix=CHECK --check-prefix=CHECK-TARGET %s
+
 CHECK: Cleaning {{.*}}toolchain-clang.test.tmp{{.}}foo.exe-foobar.o
 CHECK: Cleaning {{.*}}toolchain-clang.test.tmp{{.}}foo.exe
 CHECK: compiling foobar.c -> foo.exe-foobar.o
 CHECK-32: {{.*}}clang++{{(\.EXE)?}} -m32 -g -O0 -c -o {{.*}}foo.exe-foobar.o {{.*}}foobar.c
 CHECK-64: {{.*}}clang++{{(\.EXE)?}} -m64 -g -O0 -c -o {{.*}}foo.exe-foobar.o {{.*}}foobar.c
+CHECK-TARGET: {{.*}}clang++{{(\.EXE)?}} --target=x86_64-linux-gnu -m64 -g -O0 -c -o {{.*}}foo.exe-foobar.o 

[Lldb-commits] [PATCH] D134570: [lldb] Skip check for conflicting filter/synth when adding a new regex.

[Pavel Labath via Phabricator via lldb-commits]

labath added subscribers: jingham, labath.
labath added a reviewer: jingham.
labath added a comment.

The current behavior is clearly not useful, but I don't have any context to be 
able to determine whether there's something else that can be done about that. 
@jingham might have it.


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

https://reviews.llvm.org/D134570

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

[Lldb-commits] [PATCH] D134661: [lldb][TypeSystemClang] Honor DW_AT_rvalue_reference when creating C++ FunctionPrototypes

[Michael Buch via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Michael137 marked an inline comment as done.
Closed by commit rG60eb06be6d23: [lldb][TypeSystemClang] Honor 
DW_AT_rvalue_reference when creating C++… (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134661

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/API/lang/cpp/function-ref-qualifiers/Makefile
  lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
  lldb/test/API/lang/cpp/function-ref-qualifiers/main.cpp

Index: lldb/test/API/lang/cpp/function-ref-qualifiers/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/function-ref-qualifiers/main.cpp
@@ -0,0 +1,19 @@
+#include 
+#include 
+
+struct Foo {
+  uint32_t func() const & { return 0; }
+  int64_t func() const && { return 1; }
+  uint32_t func() & { return 2; }
+  int64_t func() && { return 3; }
+};
+
+int main() {
+  Foo foo;
+  const Foo const_foo;
+  auto res = foo.func() + const_foo.func() + Foo{}.func() +
+ static_cast(Foo{}).func();
+
+  std::puts("Break here");
+  return res;
+}
Index: lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
@@ -0,0 +1,39 @@
+"""
+Tests that C++ expression evaluation can
+disambiguate between rvalue and lvalue
+reference-qualified functions.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "Break here", lldb.SBFileSpec("main.cpp"))
+
+# const lvalue
+self.expect_expr("const_foo.func()", result_type="uint32_t", result_value="0")
+
+# const rvalue
+self.expect_expr("static_cast(Foo{}).func()",
+ result_type="int64_t", result_value="1")
+
+# non-const lvalue
+self.expect_expr("foo.func()", result_type="uint32_t", result_value="2")
+
+# non-const rvalue
+self.expect_expr("Foo{}.func()", result_type="int64_t", result_value="3")
+
+self.filecheck("target modules dump ast", __file__)
+# CHECK:  |-CXXMethodDecl {{.*}} func 'uint32_t () const &'
+# CHECK-NEXT: | `-AsmLabelAttr {{.*}}
+# CHECK-NEXT: |-CXXMethodDecl {{.*}} func 'int64_t () const &&'
+# CHECK-NEXT: | `-AsmLabelAttr {{.*}}
+# CHECK-NEXT: |-CXXMethodDecl {{.*}} func 'uint32_t () &'
+# CHECK-NEXT: | `-AsmLabelAttr {{.*}}
+# CHECK-NEXT: `-CXXMethodDecl {{.*}} func 'int64_t () &&'
+# CHECK-NEXT:   `-AsmLabelAttr {{.*}}
Index: lldb/test/API/lang/cpp/function-ref-qualifiers/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/function-ref-qualifiers/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -23,6 +23,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTFwd.h"
 #include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
@@ -398,10 +399,11 @@
   llvm::StringRef name, const CompilerType _Type,
   clang::StorageClass storage, bool is_inline);
 
-  CompilerType CreateFunctionType(const CompilerType _type,
-  const CompilerType *args, unsigned num_args,
-  bool is_variadic, unsigned type_quals,
-  clang::CallingConv cc = clang::CC_C);
+  CompilerType
+  CreateFunctionType(const CompilerType _type, const CompilerType *args,
+ unsigned num_args, bool is_variadic, unsigned type_quals,
+ clang::CallingConv cc = clang::CC_C,
+ clang::RefQualifierKind ref_qual = clang::RQ_None);
 
   clang::ParmVarDecl *
   CreateParameterDeclaration(clang::DeclContext *decl_ctx,
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ 

[Lldb-commits] [lldb] 60eb06b - [lldb][TypeSystemClang] Honor DW_AT_rvalue_reference when creating C++ FunctionPrototypes

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-09-27T18:03:23+01:00
New Revision: 60eb06be6d23e3c5fd80113143784aac0d962965

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

LOG: [lldb][TypeSystemClang] Honor DW_AT_rvalue_reference when creating C++ 
FunctionPrototypes

Currently funciton lookup in the expression evaluator
fails to disambiguate member functions the are overloaded
on lvalue/rvalue reference-qualifiers. This happens because
we unconditionally set a `FunctionPrototype`s
`ExtProtoInfo::RefQualifier` to `RQ_None`. We lose
the ref-qualifiers in the synthesized AST and `clang::Sema`
fails to pick a correct overload candidate.

DWARF emits information about a function's ref-qualifiers
in the form of a boolean `DW_AT_rvalue_reference` (for rvalues)
and `DW_AT_reference` (for lvalues).

This patch sets the `FunctionPrototype::ExtProtoInfo::RefQualifier`
based on the DWARF attributes above.

**Testing**

* Added API test

llvm/llvm-project issue #57866

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

Added: 
lldb/test/API/lang/cpp/function-ref-qualifiers/Makefile
lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
lldb/test/API/lang/cpp/function-ref-qualifiers/main.cpp

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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 9fe9b2e25a2f8..25f2a37cbab5d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -36,12 +36,12 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
-#include "llvm/Demangle/Demangle.h"
-
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/Type.h"
+#include "llvm/Demangle/Demangle.h"
 
 #include 
 #include 
@@ -412,6 +412,12 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const 
DWARFDIE ) {
 case DW_AT_export_symbols:
   exports_symbols = form_value.Boolean();
   break;
+case DW_AT_rvalue_reference:
+  ref_qual = clang::RQ_RValue;
+  break;
+case DW_AT_reference:
+  ref_qual = clang::RQ_LValue;
+  break;
 }
   }
 }
@@ -974,9 +980,10 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const DWARFDIE 
,
 
   // clang_type will get the function prototype clang type after this
   // call
-  CompilerType clang_type = m_ast.CreateFunctionType(
-  return_clang_type, function_param_types.data(),
-  function_param_types.size(), is_variadic, type_quals, 
calling_convention);
+  CompilerType clang_type =
+  m_ast.CreateFunctionType(return_clang_type, function_param_types.data(),
+   function_param_types.size(), is_variadic,
+   type_quals, calling_convention, attrs.ref_qual);
 
   if (attrs.name) {
 bool type_handled = false;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 735665328fd50..a8640b4a56938 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -10,6 +10,7 @@
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFASTPARSERCLANG_H
 
 #include "clang/AST/CharUnits.h"
+#include "clang/AST/Type.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
@@ -303,6 +304,10 @@ struct ParsedDWARFTypeAttributes {
   uint32_t bit_stride = 0;
   uint32_t byte_stride = 0;
   uint32_t encoding = 0;
+  clang::RefQualifierKind ref_qual =
+  clang::RQ_None; ///< Indicates ref-qualifier of
+  ///< C++ member function if present.
+  ///< Is RQ_None otherwise.
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFASTPARSERCLANG_H

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 6164b978f2f24..7e0a19c6b8bf1 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2168,11 +2168,10 @@ FunctionDecl 
*TypeSystemClang::CreateFunctionDeclaration(
   return func_decl;
 }
 
-CompilerType
-TypeSystemClang::CreateFunctionType(const CompilerType _type,
-const CompilerType *args, unsigned 
num_args,
-   

[Lldb-commits] [PATCH] D134661: [lldb][TypeSystemClang] Honor DW_AT_rvalue_reference when creating C++ FunctionPrototypes

[Michael Buch via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:41
 #include "llvm/Demangle/Demangle.h"
 
 #include "clang/AST/CXXInheritance.h"

labath wrote:
> I guess the `clang/AST/Type.h` include should be grouped with the other clang 
> includes. I'd recommend deleting this empty line and letting clang-format 
> sort the includes for you.
Thanks, done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134661

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

[Lldb-commits] [PATCH] D134661: [lldb][TypeSystemClang] Honor DW_AT_rvalue_reference when creating C++ FunctionPrototypes

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 463268.
Michael137 added a comment.

- Fix includes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134661

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/API/lang/cpp/function-ref-qualifiers/Makefile
  lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
  lldb/test/API/lang/cpp/function-ref-qualifiers/main.cpp

Index: lldb/test/API/lang/cpp/function-ref-qualifiers/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/function-ref-qualifiers/main.cpp
@@ -0,0 +1,19 @@
+#include 
+#include 
+
+struct Foo {
+  uint32_t func() const & { return 0; }
+  int64_t func() const && { return 1; }
+  uint32_t func() & { return 2; }
+  int64_t func() && { return 3; }
+};
+
+int main() {
+  Foo foo;
+  const Foo const_foo;
+  auto res = foo.func() + const_foo.func() + Foo{}.func() +
+ static_cast(Foo{}).func();
+
+  std::puts("Break here");
+  return res;
+}
Index: lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
@@ -0,0 +1,39 @@
+"""
+Tests that C++ expression evaluation can
+disambiguate between rvalue and lvalue
+reference-qualified functions.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "Break here", lldb.SBFileSpec("main.cpp"))
+
+# const lvalue
+self.expect_expr("const_foo.func()", result_type="uint32_t", result_value="0")
+
+# const rvalue
+self.expect_expr("static_cast(Foo{}).func()",
+ result_type="int64_t", result_value="1")
+
+# non-const lvalue
+self.expect_expr("foo.func()", result_type="uint32_t", result_value="2")
+
+# non-const rvalue
+self.expect_expr("Foo{}.func()", result_type="int64_t", result_value="3")
+
+self.filecheck("target modules dump ast", __file__)
+# CHECK:  |-CXXMethodDecl {{.*}} func 'uint32_t () const &'
+# CHECK-NEXT: | `-AsmLabelAttr {{.*}}
+# CHECK-NEXT: |-CXXMethodDecl {{.*}} func 'int64_t () const &&'
+# CHECK-NEXT: | `-AsmLabelAttr {{.*}}
+# CHECK-NEXT: |-CXXMethodDecl {{.*}} func 'uint32_t () &'
+# CHECK-NEXT: | `-AsmLabelAttr {{.*}}
+# CHECK-NEXT: `-CXXMethodDecl {{.*}} func 'int64_t () &&'
+# CHECK-NEXT:   `-AsmLabelAttr {{.*}}
Index: lldb/test/API/lang/cpp/function-ref-qualifiers/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/function-ref-qualifiers/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -23,6 +23,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTFwd.h"
 #include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
@@ -398,10 +399,11 @@
   llvm::StringRef name, const CompilerType _Type,
   clang::StorageClass storage, bool is_inline);
 
-  CompilerType CreateFunctionType(const CompilerType _type,
-  const CompilerType *args, unsigned num_args,
-  bool is_variadic, unsigned type_quals,
-  clang::CallingConv cc = clang::CC_C);
+  CompilerType
+  CreateFunctionType(const CompilerType _type, const CompilerType *args,
+ unsigned num_args, bool is_variadic, unsigned type_quals,
+ clang::CallingConv cc = clang::CC_C,
+ clang::RefQualifierKind ref_qual = clang::RQ_None);
 
   clang::ParmVarDecl *
   CreateParameterDeclaration(clang::DeclContext *decl_ctx,
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2168,11 +2168,10 @@
   return func_decl;
 }
 
-CompilerType
-TypeSystemClang::CreateFunctionType(const CompilerType _type,
-

[Lldb-commits] [PATCH] D134518: [lldb][COFF] Add note to forwarder export symbols in symtab

[Pavel Labath via Phabricator via lldb-commits]

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

In D134518#3811382 , @alvinhochun 
wrote:

> In D134518#3811218 , @labath wrote:
>
>> Ok, so is there any harm in keeping them this way?
>>
>> The symbols may not be very useful, but I would not say that they are 
>> useless. If, for whatever reason, the user finds himself inspecting the part 
>> of the memory covered by the forwarder string, then with this symbol, that 
>> memory would symbolicate as `forwarded_symbol+X`, which seems nice.
>
> I guess you're right, we can keep these symbols. Though do you think it make 
> sense to synthesize a demangled name for the symbol, let's say `ExportName 
> (forwarded to kernel32.LoadLibrary)`?

Possibly. Are these basically the same as "undefined" (i.e., defined by another 
shared library) symbols on ELF? If so, then it may make sense to mark them as 
eSymbolTypeUndefined, in case some generic algorithm wants to do something with 
them. I believe MachO also has this notion of forwarding a symbol to a 
particular shared library (I think they call it "two-level namespacing"), so 
you may want to look at how those are represented.

But overall, I don't think this is particularly important, and we can change 
this later if we have a need for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134518

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

[Lldb-commits] [PATCH] D134426: [lldb][COFF] Match symbols from COFF symbol table to export symbols

[Pavel Labath via Phabricator via lldb-commits]

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

Seems fine to me from a general perspective. I think others have already 
checked the windows parts.




Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:892
   }
+  std::sort(export_list.begin(), export_list.end(), RVASymbolListCompareRVA);
+  return export_list;

alvinhochun wrote:
> labath wrote:
> > Can you have multiple symbols pointing to the same address? Make this 
> > should use `stable_sort` instead?
> Yes, it can happen. The ordering shouldn't affect what 
> AppendFromCOFFSymbolTable does but I suppose stable_sort does make it more 
> deterministic to avoid future issues down the line.
Yeah, it's better to avoid having this kind of nondeterministic behavior that 
can differ from run to run. LLDB is not so string about it as clang/llvm, but 
it's still a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134426

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

[Lldb-commits] [PATCH] D134636: [lldb][Windows] Always call SetExecutableModule on debugger connected

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:21-22
 # CHECK-NEXT: .main.exe
-# CHECK-NEXT: .shlib.dll
+# CHECK-NEXT: ntdll.dll
+# CHECK-NEXT: kernel32.dll
+# CHECK:  .shlib.dll

I'm not sure if hardcoding the order of system libraries (something which you 
have no control of) is such a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134636

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

[Lldb-commits] [PATCH] D134661: [lldb][TypeSystemClang] Honor DW_AT_rvalue_reference when creating C++ FunctionPrototypes

[Pavel Labath via Phabricator via lldb-commits]

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

Looks straight forward enough. Tagging @zequanwu, as he might want to do 
something similar for PDB.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:41
 #include "llvm/Demangle/Demangle.h"
 
 #include "clang/AST/CXXInheritance.h"

I guess the `clang/AST/Type.h` include should be grouped with the other clang 
includes. I'd recommend deleting this empty line and letting clang-format sort 
the includes for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134661

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

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D133858#3816105 , @fdeazeve wrote:

> In D133858#3805630 , @labath wrote:
>
>> I am afraid that this patch misses one method of initiating a debug session 
>> -- connecting to a running debug server (`process connect`, 
>> `SBTarget::ConnectRemote`). The breakpoint hit counts don't get reset in 
>> case of a reconnect. This isn't a particularly common use case (and the only 
>> reason I've noticed it is that for `PlatformQemuUser`, all "launches" are 
>> actually "connects" under the hood 
>> ),
>>  but I've verified that this problem can be reproduced by issuing connect 
>> commands manually (on the regular host platform). I'm pretty sure that was 
>> not intentional.
>>
>> Fixing this by adding another callout to `ResetBreakpointHitCounts` would be 
>> easy enough, but I'm also thinking if there isn't a better place from which 
>> to call this function, one that would capture all three scenarios in a 
>> single statement. I think that one such place could be 
>> `Target::CreateProcess`. This function is called by all three code paths, 
>> and it's a very good indicator that we will be starting a new debug session.
>>
>> What do you think?
>
> My understanding is that there is an obligation of calling the WillXX methods 
> before calling XX, so by placing the reset code in the WillXX functions we 
> can rely on that guarantee. Right now, the same is implicit for "one must 
> call Target::CreateProcess before launching or attaching". We could instead 
> define a WillConnect and have the usual contract for that.

I wouldn't really call it a "usual" contract, but yes, I'm sure this could be 
fixed by adding a WillConnect method. It might be also sufficient to just call 
WillAttach from at some appropriate place, since a "connect" operation looks 
very similar to an "attach", and a lot of the initialization operations are the 
same. I think we're already something like this somewhere (maybe for 
DidAttach?). However, that still leaves us with three (or two) places that need 
to be kept in sync.

> The code is fairly new to me, so I'm not confident enough to make a judgement 
> call here. Thoughts?

I don't see any advantage in doing this particular action "just before" a 
launch, as opposed to doing in on process creation, so I would definitely do it 
that way.

I also find it weird to be going through the Process class just to call another 
Target method, when all of the launch/attach/connect operations already go 
through the Target class, and so it should be perfectly capable of calling that 
method on its own.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133858

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

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The vscode change seems fine to me, but I don't consider myself an authority on 
that.

In D134333#3812653 , @jingham wrote:

> In D134333#3812448 , @clayborg 
> wrote:
>
>> I just did a search through our test sources and we use 
>> GetError().GetCString() 34 times in our test suites python files. So the 
>> SBError::GetCString() is being misused a lot like this already and is 
>> probably making some tests flaky. I would highly recommend we fix 
>> SBError::GetCString() to make sure things work correctly. If people are 
>> already doing this, or if they can do this with our API, then we should make 
>> our API as stable as possible for users even if it costs a bit more memory 
>> in our string pools.
>
> When we return Python strings from the SWIG'ed version of 
> GetError().GetCString() does the Python string we make & return copy the 
> string data, or is it just holding onto the pointer?  In C/C++ if someone 
> returns you a char * unless explicitly specified you assume that string is 
> only going to live as long as the object that handed it out.  But a python 
> string is generally self-contained, so you wouldn't expect it to lose its 
> data when the generating object goes away.  I haven't checked yet, but if 
> this is how SWIG handles making python strings from C strings, then this 
> wouldn't be an error in the testsuite, only in C++ code.  If that's not how 
> it works, that might be a good way to finesse the issue.

The python version is fine because swig SBError::GetCString wrapper gets 
essentially a pointer to the SBError objects (wrapped as PyObject), and returns 
another PyObject with the (copied) string (also a PyObject). All the lifetime 
management happens outside of that function.




Comment at: 
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py:48-53
+contains = verify_value in actual_value
+self.assertTrue(contains,
+('"%s" value "%s" doesn\'t contain'
+ ' "%s")') % (
+key, actual_value,
+verify_value))




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134333

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

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D134581#3817457 , @alvinhochun 
wrote:

> In D134581#3815766 , @jasonmolenda 
> wrote:
>
>> In D134581#3813757 , @alvinhochun 
>> wrote:
>>
>>> In `ProcessWindows::OnDebuggerConnected`, it first checks 
>>> `GetTarget().GetExecutableModule()`. Only when the returned module is null 
>>> (i.e. the binary hasn't already been loaded) then it calls 
>>> `GetTarget().SetExecutableModule(module, eLoadDependentsNo)`. If I 
>>> understood you correctly, then the right thing to do there should be to 
>>> call `GetTarget().SetExecutableModule(module, eLoadDependentsNo)` in all 
>>> cases, without checking `GetExecutableModule`, right?
>>>
>>> It seems to make sense, but I may need some comments from other reviewers 
>>> on this.
>>
>> Just my opinion, but I know how DynamicDarwinLoader works is that when it 
>> starts the actual debug session, it clears the image list entirely (iirc or 
>> maybe it just calls Target::SetExecutableModule - which is effectively the 
>> same thing).  I don't know how Windows works, but on Darwin we pre-load the 
>> binaries we THINK will be loaded, but when the process actually runs, 
>> different binaries may end up getting loaded, and we need to use what the 
>> dynamic linker tells us instead of our original logic.  
>> `GetTarget().SetExecutableModule(module, eLoadDependentsNo)` would be one 
>> option, or clear the list and start adding, effectively the same thing.
>
> Sounds right. On Windows, events from the debug loop will tell us which DLLs 
> are actually being loaded by the process and we add them to the module list.

Seems reasonable to me. I'm not actually sure if we're doing this on 
linux/posix (as I still see duplicated vdso modules occasionaly -- but this 
could be caused by other problems as well), but if we're not -- we probably 
should.

>> I think it would be more straightforward than adding this change to 
>> Target::GetOrAddModule.
>
> Here's the thing, even if we change the Windows debugging support to clear 
> the module list when starting the process, the logic of 
> `Target::GetOrAddModule` still appears to be flawed if it can result in 
> duplicated modules in the target module list, so IMO it needs to be fixed 
> regardless.

I can totally believe that there is a bug in the GetOrAddModule, but I would 
not be able to tell you if this is the right fix or not. As for multiple 
modules, I can say that on linux it is actually possible to load the same 
shared library more than once (into different "linker namespaces"). LLDB does 
not currently support that, but I would like to support it one day. I don't 
know whether this change would make that harder or easier, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

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

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

That said, I am kinda surprised that this is the whole thing. I would have 
expected to see more here. In dwarf we specify the offsets of individual class 
members. Does PDB encode that information? If not, how does it handle the case 
when the definition of some class is missing? If that class is a member of some 
other class, the offsets of all subsequent members in the bigger class will be 
wrong? That will probably be true even if we are always able to obtain the size 
of the smaller class, because of things like vtable pointers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134656

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

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

[Pavel Labath via Phabricator via lldb-commits]

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

LGTM modulo formatting.




Comment at: lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp:24
+
+union __attribute__((packed, aligned(1))) U {
+  char c[2];

I suspect these attributes do not materially change the layout of the union 
when it stands on its own. It might be more interesting to take this 
under-aligned union, place it into a (regular) struct, and then check whether 
we can print it correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134656

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

[Lldb-commits] [PATCH] D134727: [lldb][test] Remove failing ValueCheck on deprecated libcxx entity

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7ccfaecba6b1: [lldb][test] Remove failing ValueCheck on 
deprecated libcxx entity (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134727

Files:
  
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py


Index: 
lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
@@ -10,7 +10,6 @@
 class TestBasicVector(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -29,7 +28,7 @@
 iterator_children = [ValueCheck(name="item")]
 riterator = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 ValueCheck(name="current")
 ]
 
Index: 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -11,7 +11,6 @@
 class TestDbgInfoContentVector(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -30,7 +29,7 @@
 iterator_children = [ValueCheck(name="item")]
 riterator = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 ValueCheck(name="current")
 ]
 
Index: 
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
@@ -10,7 +10,6 @@
 class TestDbgInfoContentDeque(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -33,7 +32,7 @@
 
 riterator_type = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 ValueCheck(name="current")
 ]
 
Index: 
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
@@ -10,7 +10,6 @@
 class TestBasicDeque(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -31,7 +30,7 @@
 ]
 riterator = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 ValueCheck(name="current")
 ]
 


Index: lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
@@ -10,7 +10,6 @@
 class TestBasicVector(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
  

[Lldb-commits] [lldb] 7ccfaec - [lldb][test] Remove failing ValueCheck on deprecated libcxx entity

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-09-27T14:11:02+01:00
New Revision: 7ccfaecba6b1aa80eea1903b2c5ee20d4f00374c

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

LOG: [lldb][test] Remove failing ValueCheck on deprecated libcxx entity

A recent libcxx change renamed all internal variables starting with
`__`. As such, `std::reverse_iterator::__t` was renamed to
`std::reverse_iterator::__t_`. This breaks the `import-std-module`
tests with newer libcxx versions. Since this variable is deprecated
in libcxx anyway, this patch simply removes the explicit `ValueCheck`
on the variable name. We don't lose any interesting test-case here
since the purpose of the test is to see if we can call functions
from the `std` module.

We can now re-enable the tests on Darwin for all buildbot Clang
compiler variants.

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

Added: 


Modified: 

lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py

lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py

lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py

lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
index 5f397ab1218b9..f77ba0d2417a4 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
@@ -10,7 +10,6 @@
 class TestBasicDeque(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -31,7 +30,7 @@ def test(self):
 ]
 riterator = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 ValueCheck(name="current")
 ]
 

diff  --git 
a/lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
index 8b98a527c6012..d375e06c7964c 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
@@ -10,7 +10,6 @@
 class TestDbgInfoContentDeque(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -33,7 +32,7 @@ def test(self):
 
 riterator_type = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 ValueCheck(name="current")
 ]
 

diff  --git 
a/lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
index 8d0e926081661..7da615c3084d5 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -11,7 +11,6 @@
 class TestDbgInfoContentVector(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -30,7 +29,7 @@ def test(self):
 iterator_children = [ValueCheck(name="item")]
 riterator = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 ValueCheck(name="current")
 ]
 

diff  --git 
a/lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
index 44bee3df614c6..143c5216c4d3f 100644
--- 

[Lldb-commits] [PATCH] D134734: [lldb][test] Disable TestSBValueUnsignedEnumBitField.py for old DWARF versions

[Michael Buch via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc97e5adbf7c4: [lldb][test] Disable 
TestSBValueUnsignedEnumBitField.py for old DWARF versions (authored by 
Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134734

Files:
  
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py


Index: 
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
===
--- 
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
+++ 
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
@@ -14,5 +14,7 @@
 """
 
 import lldbsuite.test.lldbinline as lldbinline
+from lldbsuite.test.decorators import *
 
-lldbinline.MakeInlineTest(__file__, globals())
+lldbinline.MakeInlineTest(__file__, globals(),
+  [skipIf(dwarf_version=['<', '3'])])


Index: lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
===
--- lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
+++ lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
@@ -14,5 +14,7 @@
 """
 
 import lldbsuite.test.lldbinline as lldbinline
+from lldbsuite.test.decorators import *
 
-lldbinline.MakeInlineTest(__file__, globals())
+lldbinline.MakeInlineTest(__file__, globals(),
+  [skipIf(dwarf_version=['<', '3'])])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] c97e5ad - [lldb][test] Disable TestSBValueUnsignedEnumBitField.py for old DWARF versions

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-09-27T14:09:12+01:00
New Revision: c97e5adbf7c44ba5c46ec14712d3d8ee8b45aac9

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

LOG: [lldb][test] Disable TestSBValueUnsignedEnumBitField.py for old DWARF 
versions

With older DWARF versions we don't encode the enum's underlying
type in DWARF. In those cases LLDB sign-extends the bitfield as
a signed integer. Without the actual enum type being present in
DWARF there's not much we can do.

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

Added: 


Modified: 

lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py

Removed: 




diff  --git 
a/lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
 
b/lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
index eb01fd6ee63a7..e2e1118ec2b47 100644
--- 
a/lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
+++ 
b/lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
@@ -14,5 +14,7 @@
 """
 
 import lldbsuite.test.lldbinline as lldbinline
+from lldbsuite.test.decorators import *
 
-lldbinline.MakeInlineTest(__file__, globals())
+lldbinline.MakeInlineTest(__file__, globals(),
+  [skipIf(dwarf_version=['<', '3'])])



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

[Lldb-commits] [PATCH] D134734: [lldb][test] Disable TestSBValueUnsignedEnumBitField.py for old DWARF versions

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 463209.
Michael137 added a comment.

- Works on DWARFv3


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134734

Files:
  
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py


Index: 
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
===
--- 
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
+++ 
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
@@ -14,5 +14,7 @@
 """
 
 import lldbsuite.test.lldbinline as lldbinline
+from lldbsuite.test.decorators import *
 
-lldbinline.MakeInlineTest(__file__, globals())
+lldbinline.MakeInlineTest(__file__, globals(),
+  [skipIf(dwarf_version=['<', '3'])])


Index: lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
===
--- lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
+++ lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
@@ -14,5 +14,7 @@
 """
 
 import lldbsuite.test.lldbinline as lldbinline
+from lldbsuite.test.decorators import *
 
-lldbinline.MakeInlineTest(__file__, globals())
+lldbinline.MakeInlineTest(__file__, globals(),
+  [skipIf(dwarf_version=['<', '3'])])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134734: [lldb][test] Disable TestSBValueUnsignedEnumBitField.py for old DWARF versions

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve accepted this revision.
fdeazeve added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: JDevlieghere.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134734

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

[Lldb-commits] [PATCH] D134734: [lldb][test] Disable TestSBValueUnsignedEnumBitField.py for old DWARF versions

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, fdeazeve.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

With older DWARF versions we don't encode the enum's underlying
type in DWARF. In those cases LLDB sign-extends the bitfield as
a signed integer. Without the actual enum type being present in
DWARF there's not much we can do.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134734

Files:
  
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py


Index: 
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
===
--- 
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
+++ 
lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
@@ -14,5 +14,7 @@
 """
 
 import lldbsuite.test.lldbinline as lldbinline
+from lldbsuite.test.decorators import *
 
-lldbinline.MakeInlineTest(__file__, globals())
+lldbinline.MakeInlineTest(__file__, globals(),
+  [skipIf(dwarf_version=['<', '4'])])


Index: lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
===
--- lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
+++ lldb/test/API/python_api/sbvalue_unsigned_enum_bitfield_value/TestSBValueUnsignedEnumBitField.py
@@ -14,5 +14,7 @@
 """
 
 import lldbsuite.test.lldbinline as lldbinline
+from lldbsuite.test.decorators import *
 
-lldbinline.MakeInlineTest(__file__, globals())
+lldbinline.MakeInlineTest(__file__, globals(),
+  [skipIf(dwarf_version=['<', '4'])])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134727: [lldb][test] Remove failing ValueCheck on deprecated libcxx entity

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

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

Thanks for looking into this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134727

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

[Lldb-commits] [PATCH] D134727: [lldb][test] Remove failing ValueCheck on deprecated libcxx entity

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, fdeazeve.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

A recent libcxx change renamed all internal variables starting with
`__`. As such, `std::reverse_iterator::__t` was renamed to
`std::reverse_iterator::__t_`. This breaks the `import-std-module`
tests with newer libcxx versions. Since this variable is deprecated
in libcxx anyway, this patch simply removes the explicit `ValueCheck`
on the variable name. We don't lose any interesting test-case here
since the purpose of the test is to see if we can call functions
from the `std` module.

We can now re-enable the tests on Darwin for all buildbot Clang
compiler variants.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134727

Files:
  
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py


Index: 
lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
@@ -10,7 +10,6 @@
 class TestBasicVector(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -29,7 +28,7 @@
 iterator_children = [ValueCheck(name="item")]
 riterator = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 ValueCheck(name="current")
 ]
 
Index: 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -11,7 +11,6 @@
 class TestDbgInfoContentVector(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -30,7 +29,7 @@
 iterator_children = [ValueCheck(name="item")]
 riterator = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 ValueCheck(name="current")
 ]
 
Index: 
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
@@ -10,7 +10,6 @@
 class TestDbgInfoContentDeque(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -33,7 +32,7 @@
 
 riterator_type = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 ValueCheck(name="current")
 ]
 
Index: 
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
@@ -10,7 +10,6 @@
 class TestBasicDeque(TestBase):
 
 @add_test_categories(["libc++"])
-@expectedFailureDarwin # FIXME: May need to force system libcxx here.
 @skipIf(compiler=no_match("clang"))
 def test(self):
 self.build()
@@ -31,7 +30,7 @@
 ]
 riterator = "reverse_iterator"
 riterator_children = [
-ValueCheck(name="__t"),
+ValueCheck(), # Deprecated __t_ member; no need to check
 

[Lldb-commits] [PATCH] D134516: [lldb] Improve display of absolute symbol lookup

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf4991bfa891e: [lldb] Improve display of absolute symbol 
lookup (authored by alvinhochun, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134516

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/test/Shell/SymbolFile/absolute-symbol.test


Index: lldb/test/Shell/SymbolFile/absolute-symbol.test
===
--- lldb/test/Shell/SymbolFile/absolute-symbol.test
+++ lldb/test/Shell/SymbolFile/absolute-symbol.test
@@ -1,6 +1,7 @@
 # RUN: yaml2obj %s -o %t.o
 # RUN: %lldb -b -o 'target modules lookup -s absolute_symbol' %t.o | FileCheck 
%s
 # CHECK: 1 symbols match 'absolute_symbol'
+# CHECK:   Name: absolute_symbol
 # CHECK:   Value: 0x12345678
 # Created from:
 #   .globl absolute_symbol
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -1549,12 +1549,16 @@
   strm.EOL();
 } else {
   strm.IndentMore();
+  strm.Indent("Name: ");
+  strm.PutCString(symbol->GetDisplayName().GetStringRef());
+  strm.EOL();
   strm.Indent("Value: ");
   strm.Printf("0x%16.16" PRIx64 "\n", symbol->GetRawValue());
   if (symbol->GetByteSizeIsValid()) {
 strm.Indent("Size: ");
 strm.Printf("0x%16.16" PRIx64 "\n", symbol->GetByteSize());
   }
+  strm.IndentLess();
 }
   }
 }


Index: lldb/test/Shell/SymbolFile/absolute-symbol.test
===
--- lldb/test/Shell/SymbolFile/absolute-symbol.test
+++ lldb/test/Shell/SymbolFile/absolute-symbol.test
@@ -1,6 +1,7 @@
 # RUN: yaml2obj %s -o %t.o
 # RUN: %lldb -b -o 'target modules lookup -s absolute_symbol' %t.o | FileCheck %s
 # CHECK: 1 symbols match 'absolute_symbol'
+# CHECK:   Name: absolute_symbol
 # CHECK:   Value: 0x12345678
 # Created from:
 #   .globl absolute_symbol
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -1549,12 +1549,16 @@
   strm.EOL();
 } else {
   strm.IndentMore();
+  strm.Indent("Name: ");
+  strm.PutCString(symbol->GetDisplayName().GetStringRef());
+  strm.EOL();
   strm.Indent("Value: ");
   strm.Printf("0x%16.16" PRIx64 "\n", symbol->GetRawValue());
   if (symbol->GetByteSizeIsValid()) {
 strm.Indent("Size: ");
 strm.Printf("0x%16.16" PRIx64 "\n", symbol->GetByteSize());
   }
+  strm.IndentLess();
 }
   }
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134111: [lldb] Add newline in output of `target modules lookup`

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa426753ef029: [lldb] Add newline in output of `target 
modules lookup` (authored by alvinhochun, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134111

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/test/Shell/Commands/Inputs/symbols.yaml
  lldb/test/Shell/Commands/command-target-modules-lookup.test


Index: lldb/test/Shell/Commands/command-target-modules-lookup.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-target-modules-lookup.test
@@ -0,0 +1,12 @@
+# RUN: yaml2obj %S/Inputs/symbols.yaml -o %t
+
+# RUN: %lldb %t -b -o "target modules lookup -A -r -s some" | FileCheck %s 
-DMODULE=%basename_t.tmp --implicit-check-not ignoreThisFunction
+# CHECK:  4 symbols match the regular expression 'some' in 
{{.*}}[[MODULE]]:
+# CHECK-NEXT: Address: [[MODULE]][0x] ([[MODULE]]..text + 0)
+# CHECK-NEXT: Summary: [[MODULE]]`someFunc(int, int, int)
+# CHECK-NEXT: Address: [[MODULE]][0x001c] ([[MODULE]]..text + 28)
+# CHECK-NEXT: Summary: [[MODULE]]`someFunc(char, int)
+# CHECK-NEXT: Address: [[MODULE]][0x0034] ([[MODULE]]..text + 52)
+# CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc()
+# CHECK-NEXT: Address: [[MODULE]][0x0038] ([[MODULE]]..text + 56)
+# CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc(double)
Index: lldb/test/Shell/Commands/Inputs/symbols.yaml
===
--- /dev/null
+++ lldb/test/Shell/Commands/Inputs/symbols.yaml
@@ -0,0 +1,48 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_AARCH64
+  SectionHeaderStringTable: .strtab
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+AddressAlign:0x4
+  - Type:SectionHeaderTable
+Sections:
+  - Name:.text
+  - Name:.strtab
+  - Name:.symtab
+Symbols:
+  - Name:_Z8someFunciii
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Size:0x1C
+  - Name:_Z8someFuncci
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x1C
+Size:0x18
+  - Name:_Z13someOtherFuncv
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x34
+Size:0x4
+  - Name:_Z13someOtherFuncd
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x38
+Size:0x10
+  - Name:_Z18ignoreThisFunctionv
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x48
+Size:0x8
+...
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -1546,6 +1546,7 @@
   DumpAddress(
   interpreter.GetExecutionContext().GetBestExecutionContextScope(),
   symbol->GetAddressRef(), verbose, all_ranges, strm);
+  strm.EOL();
 } else {
   strm.IndentMore();
   strm.Indent("Value: ");


Index: lldb/test/Shell/Commands/command-target-modules-lookup.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-target-modules-lookup.test
@@ -0,0 +1,12 @@
+# RUN: yaml2obj %S/Inputs/symbols.yaml -o %t
+
+# RUN: %lldb %t -b -o "target modules lookup -A -r -s some" | FileCheck %s -DMODULE=%basename_t.tmp --implicit-check-not ignoreThisFunction
+# CHECK:  4 symbols match the regular expression 'some' in {{.*}}[[MODULE]]:
+# CHECK-NEXT: Address: [[MODULE]][0x] ([[MODULE]]..text + 0)
+# CHECK-NEXT: Summary: [[MODULE]]`someFunc(int, int, int)
+# CHECK-NEXT: Address: [[MODULE]][0x001c] ([[MODULE]]..text + 28)
+# CHECK-NEXT: Summary: [[MODULE]]`someFunc(char, int)
+# CHECK-NEXT: Address: [[MODULE]][0x0034] ([[MODULE]]..text + 52)
+# CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc()
+# CHECK-NEXT: Address: [[MODULE]][0x0038] ([[MODULE]]..text + 56)
+# CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc(double)
Index: lldb/test/Shell/Commands/Inputs/symbols.yaml
===
--- /dev/null
+++ lldb/test/Shell/Commands/Inputs/symbols.yaml
@@ -0,0 +1,48 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data: 

[Lldb-commits] [lldb] a426753 - [lldb] Add newline in output of `target modules lookup`

[Martin Storsjö via lldb-commits]

Author: Alvin Wong
Date: 2022-09-27T13:09:45+03:00
New Revision: a426753ef0290362a0268f917dba46d6b9744b02

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

LOG: [lldb] Add newline in output of `target modules lookup`

This adds a line break between each result address in the output of the
lldb command `target modules lookup`. Before this change, a new address
result will be printed on the same line as the summary of the last
result, making the output difficult to view.

Also adds a test for this command.

Reviewed By: labath

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

Added: 
lldb/test/Shell/Commands/Inputs/symbols.yaml
lldb/test/Shell/Commands/command-target-modules-lookup.test

Modified: 
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 08bdb856d9161..deb7f84f70b70 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1546,6 +1546,7 @@ static uint32_t LookupSymbolInModule(CommandInterpreter 
,
   DumpAddress(
   interpreter.GetExecutionContext().GetBestExecutionContextScope(),
   symbol->GetAddressRef(), verbose, all_ranges, strm);
+  strm.EOL();
 } else {
   strm.IndentMore();
   strm.Indent("Value: ");

diff  --git a/lldb/test/Shell/Commands/Inputs/symbols.yaml 
b/lldb/test/Shell/Commands/Inputs/symbols.yaml
new file mode 100644
index 0..64200ecd28fef
--- /dev/null
+++ b/lldb/test/Shell/Commands/Inputs/symbols.yaml
@@ -0,0 +1,48 @@
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_AARCH64
+  SectionHeaderStringTable: .strtab
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+AddressAlign:0x4
+  - Type:SectionHeaderTable
+Sections:
+  - Name:.text
+  - Name:.strtab
+  - Name:.symtab
+Symbols:
+  - Name:_Z8someFunciii
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Size:0x1C
+  - Name:_Z8someFuncci
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x1C
+Size:0x18
+  - Name:_Z13someOtherFuncv
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x34
+Size:0x4
+  - Name:_Z13someOtherFuncd
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x38
+Size:0x10
+  - Name:_Z18ignoreThisFunctionv
+Type:STT_FUNC
+Section: .text
+Binding: STB_GLOBAL
+Value:   0x48
+Size:0x8
+...

diff  --git a/lldb/test/Shell/Commands/command-target-modules-lookup.test 
b/lldb/test/Shell/Commands/command-target-modules-lookup.test
new file mode 100644
index 0..5ac211b007946
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-target-modules-lookup.test
@@ -0,0 +1,12 @@
+# RUN: yaml2obj %S/Inputs/symbols.yaml -o %t
+
+# RUN: %lldb %t -b -o "target modules lookup -A -r -s some" | FileCheck %s 
-DMODULE=%basename_t.tmp --implicit-check-not ignoreThisFunction
+# CHECK:  4 symbols match the regular expression 'some' in 
{{.*}}[[MODULE]]:
+# CHECK-NEXT: Address: [[MODULE]][0x] ([[MODULE]]..text + 0)
+# CHECK-NEXT: Summary: [[MODULE]]`someFunc(int, int, int)
+# CHECK-NEXT: Address: [[MODULE]][0x001c] ([[MODULE]]..text + 28)
+# CHECK-NEXT: Summary: [[MODULE]]`someFunc(char, int)
+# CHECK-NEXT: Address: [[MODULE]][0x0034] ([[MODULE]]..text + 52)
+# CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc()
+# CHECK-NEXT: Address: [[MODULE]][0x0038] ([[MODULE]]..text + 56)
+# CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc(double)



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

[Lldb-commits] [lldb] f4991bf - [lldb] Improve display of absolute symbol lookup

[Martin Storsjö via lldb-commits]

Author: Alvin Wong
Date: 2022-09-27T13:09:59+03:00
New Revision: f4991bfa891eaf9f36aa3b79da6fed4228b69de9

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

LOG: [lldb] Improve display of absolute symbol lookup

When running `target module lookup` command, show the name of absolute
symbols. Also fix indentation issue after printing an absolute symbol.

Reviewed By: clayborg, DavidSpickett

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp
lldb/test/Shell/SymbolFile/absolute-symbol.test

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index deb7f84f70b70..59b7dfd74e852 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1549,12 +1549,16 @@ static uint32_t LookupSymbolInModule(CommandInterpreter 
,
   strm.EOL();
 } else {
   strm.IndentMore();
+  strm.Indent("Name: ");
+  strm.PutCString(symbol->GetDisplayName().GetStringRef());
+  strm.EOL();
   strm.Indent("Value: ");
   strm.Printf("0x%16.16" PRIx64 "\n", symbol->GetRawValue());
   if (symbol->GetByteSizeIsValid()) {
 strm.Indent("Size: ");
 strm.Printf("0x%16.16" PRIx64 "\n", symbol->GetByteSize());
   }
+  strm.IndentLess();
 }
   }
 }

diff  --git a/lldb/test/Shell/SymbolFile/absolute-symbol.test 
b/lldb/test/Shell/SymbolFile/absolute-symbol.test
index 11feb1f89c10a..edcda7119d567 100644
--- a/lldb/test/Shell/SymbolFile/absolute-symbol.test
+++ b/lldb/test/Shell/SymbolFile/absolute-symbol.test
@@ -1,6 +1,7 @@
 # RUN: yaml2obj %s -o %t.o
 # RUN: %lldb -b -o 'target modules lookup -s absolute_symbol' %t.o | FileCheck 
%s
 # CHECK: 1 symbols match 'absolute_symbol'
+# CHECK:   Name: absolute_symbol
 # CHECK:   Value: 0x12345678
 # Created from:
 #   .globl absolute_symbol



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

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Turns out you're not the only one 
https://github.com/llvm/llvm-project/issues/56268.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134037

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

[Lldb-commits] [PATCH] D132734: [lldb] Fix member access in GetExpressionPath

[Tonko Sabolčec via Phabricator via lldb-commits]

tonkosi updated this revision to Diff 463158.
tonkosi added a comment.

Forgot to propagate "epformat" when calling GetExpressionPath


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132734

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/python_api/expression_path/Makefile
  lldb/test/API/python_api/expression_path/TestExpressionPath.py
  lldb/test/API/python_api/expression_path/main.cpp

Index: lldb/test/API/python_api/expression_path/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/expression_path/main.cpp
@@ -0,0 +1,34 @@
+struct StructA {
+  int x;
+  int y;
+};
+
+struct StructB {
+  int x;
+  StructA _ref;
+  StructA *_ptr_ref;
+};
+
+struct StructC : public StructB {
+  int y;
+
+  StructC(int x, StructA _ref, StructA *_ref_ptr, int y)
+  : StructB{x, a_ref, a_ref_ptr}, y(y) {}
+};
+
+int main() {
+  StructA a{1, 2};
+  StructA *a_ptr = 
+
+  StructB b{3, a, a_ptr};
+  StructB *b_ptr = 
+  StructB _ref = b;
+  StructB *_ptr_ref = b_ptr;
+
+  StructC c(4, a, a_ptr, 5);
+  StructC *c_ptr = 
+  StructC _ref = c;
+  StructC *_ptr_ref = c_ptr;
+
+  return 0; // Set breakpoint here
+}
\ No newline at end of file
Index: lldb/test/API/python_api/expression_path/TestExpressionPath.py
===
--- /dev/null
+++ lldb/test/API/python_api/expression_path/TestExpressionPath.py
@@ -0,0 +1,119 @@
+"""Test that SBFrame::GetExpressionPath construct valid expressions"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class SBValueGetExpressionPathTest(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def path(self, value):
+"""Constructs the expression path given the SBValue"""
+if not value:
+return None
+stream = lldb.SBStream()
+if not value.GetExpressionPath(stream):
+return None
+return stream.GetData()
+
+def test_expression_path(self):
+"""Test that SBFrame::GetExpressionPath construct valid expressions"""
+self.build()
+self.setTearDownCleanup()
+
+exe = self.getBuildArtifact("a.out")
+
+# Create the target
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Set the breakpoints
+breakpoint = target.BreakpointCreateBySourceRegex(
+'Set breakpoint here', lldb.SBFileSpec("main.cpp"))
+self.assertTrue(breakpoint.GetNumLocations() > 0, VALID_BREAKPOINT)
+
+# Launch the process, and do not stop at the entry point.
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Frame #0 should be at our breakpoint.
+threads = lldbutil.get_threads_stopped_at_breakpoint(
+process, breakpoint)
+
+self.assertEquals(len(threads), 1)
+self.thread = threads[0]
+self.frame = self.thread.frames[0]
+self.assertTrue(self.frame, "Frame 0 is valid.")
+
+# Find "b" variables in frame
+b = self.frame.FindVariable("b")
+bp = self.frame.FindVariable("b_ptr")
+br = self.frame.FindVariable("b_ref")
+bpr = self.frame.FindVariable("b_ptr_ref")
+# Check expression paths
+self.assertEqual(self.path(b), "b")
+self.assertEqual(self.path(bp), "b_ptr")
+self.assertEqual(self.path(br), "b_ref")
+self.assertEqual(self.path(bpr), "b_ptr_ref")
+
+# Dereference "b" pointers
+bp_deref = bp.Dereference()
+bpr_deref = bpr.Dereference()  # a pointer
+bpr_deref2 = bpr_deref.Dereference()  # two Dereference() calls to get object
+# Check expression paths
+self.assertEqual(self.path(bp_deref), "*(b_ptr)")
+self.assertEqual(self.path(bpr_deref), "b_ptr_ref")
+self.assertEqual(self.path(bpr_deref2), "*(b_ptr_ref)")
+
+# Access "b" members and check expression paths
+self.assertEqual(self.path(b.GetChildMemberWithName("x")), "b.x")
+self.assertEqual(self.path(bp.GetChildMemberWithName("x")), "b_ptr->x")
+self.assertEqual(self.path(br.GetChildMemberWithName("x")), "b_ref.x")
+self.assertEqual(self.path(bp_deref.GetChildMemberWithName("x")), "(*(b_ptr)).x")
+self.assertEqual(self.path(bpr_deref.GetChildMemberWithName("x")), "b_ptr_ref->x")
+self.assertEqual(self.path(bpr_deref2.GetChildMemberWithName("x")), "(*(b_ptr_ref)).x")
+# TODO: Uncomment once accessing members on pointer references is supported.
+# self.assertEqual(self.path(bpr.GetChildMemberWithName("x")), "b_ptr_ref->x")
+
+# Try few expressions with 

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:53
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, 
m_cvr.cr));
+ m_layout.bit_size = m_cvr.cr.getSize() * 8;
 break;

Drive by comment: you have an extra space at the start of the added lines. 
(unless phabricator is just indenting it for effect)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134656

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

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added a comment.

In D134581#3815766 , @jasonmolenda 
wrote:

> In D134581#3813757 , @alvinhochun 
> wrote:
>
>> Thanks for the comment.
>>
>> In D134581#3813484 , @jasonmolenda 
>> wrote:
>>
>>> I probably misunderstand the situation DynamicLoaderWindows finds itself 
>>> in, but in DynamicLoaderDarwin we have similar behavior - you run 'lldb 
>>> binary' and lldb loads all the directly linked dynamic libraries into the 
>>> target it creates, pre-execution.  When execution starts, the dynamic 
>>> linker tells us where the binary is loaded in memory and we call 
>>> Target::SetExecutableModule with it.  Target::SetExecutableModule has a 
>>> side effect of clearing the Target's module list - you now have only one 
>>> binary in the Target, your executable module.  (this is done so the 0th 
>>> image in the image list is your executable module)
>>>
>>> Why aren't your pre-loaded DLLs unloaded when you call 
>>> Target::SetExecutableModule?
>>
>> In `ProcessWindows::OnDebuggerConnected`, it first checks 
>> `GetTarget().GetExecutableModule()`. Only when the returned module is null 
>> (i.e. the binary hasn't already been loaded) then it calls 
>> `GetTarget().SetExecutableModule(module, eLoadDependentsNo)`. If I 
>> understood you correctly, then the right thing to do there should be to call 
>> `GetTarget().SetExecutableModule(module, eLoadDependentsNo)` in all cases, 
>> without checking `GetExecutableModule`, right?
>>
>> It seems to make sense, but I may need some comments from other reviewers on 
>> this.
>
> Just my opinion, but I know how DynamicDarwinLoader works is that when it 
> starts the actual debug session, it clears the image list entirely (iirc or 
> maybe it just calls Target::SetExecutableModule - which is effectively the 
> same thing).  I don't know how Windows works, but on Darwin we pre-load the 
> binaries we THINK will be loaded, but when the process actually runs, 
> different binaries may end up getting loaded, and we need to use what the 
> dynamic linker tells us instead of our original logic.  
> `GetTarget().SetExecutableModule(module, eLoadDependentsNo)` would be one 
> option, or clear the list and start adding, effectively the same thing.

Sounds right. On Windows, events from the debug loop will tell us which DLLs 
are actually being loaded by the process and we add them to the module list.

> I think it would be more straightforward than adding this change to 
> Target::GetOrAddModule.

Here's the thing, even if we change the Windows debugging support to clear the 
module list when starting the process, the logic of `Target::GetOrAddModule` 
still appears to be flawed if it can result in duplicated modules in the target 
module list, so IMO it needs to be fixed regardless.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

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

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 463102.
clayborg added a comment.

Don't constify SBError::GetCString anymore. We can fix this in another patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134333

Files:
  lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2953,6 +2953,31 @@
 }
 
 num_children = top_scope->GetSize();
+if (num_children == 0 && variablesReference == VARREF_LOCALS) {
+  // Check for an error in the SBValueList that might explain why we don't
+  // have locals. If we have an error display it as the sole value in the
+  // the locals.
+
+  // "error" owns the error string so we must keep it alive as long as we
+  // want to use the returns "const char *"
+  SBError error = top_scope->GetError();
+  const char *var_err = error.GetCString();
+  if (var_err) {
+// Create a fake variable named "error" to explain why variables were
+// not available. This new error will help let users know when there was
+// a problem that kept variables from being available for display and
+// allow users to fix this issue instead of seeing no variables. The
+// errors are only set when there is a problem that the user could
+// fix, so no error will show up when you have no debug info, only when
+// we do have debug info and something that is fixable can be done.
+llvm::json::Object object;
+EmplaceSafeString(object, "name", "");
+EmplaceSafeString(object, "type", "const char *");
+EmplaceSafeString(object, "value", var_err);
+object.try_emplace("variablesReference", (int64_t)0);
+variables.emplace_back(std::move(object));
+  }
+}
 const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
 
 // We first find out which variable names are duplicated
Index: lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
===
--- lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -7,7 +7,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
-
+import os
 
 def make_buffer_verify_dict(start_idx, count, offset=0):
 verify_dict = {}
@@ -38,6 +38,19 @@
  ' "%s")') % (
 key, actual_value,
 verify_value))
+if 'contains' in verify_dict:
+verify = verify_dict['contains']
+for key in verify:
+contains_array = verify[key]
+actual_value = actual[key]
+self.assertTrue(isinstance(contains_array, list))
+for verify_value in contains_array:
+contains = verify_value in actual_value
+self.assertTrue(contains,
+('"%s" value "%s" doesn\'t contain'
+ ' "%s")') % (
+key, actual_value,
+verify_value))
 if 'missing' in verify_dict:
 missing = verify_dict['missing']
 for key in missing:
@@ -508,3 +521,46 @@
 self.assertTrue(value.startswith('0x'))
 self.assertTrue('a.out`main + ' in value)
 self.assertTrue('at main.cpp:' in value)
+
+@no_debug_info_test
+@skipUnlessDarwin
+def test_darwin_dwarf_missing_obj(self):
+'''
+Test that if we build a binary with DWARF in .o files and we remove
+the .o file for main.cpp, that we get a variable named ""
+whose value matches the appriopriate error. Errors when getting
+variables are returned in the LLDB API when the user should be
+notified of issues that can easily be solved by rebuilding or
+changing compiler options and are designed to give better feedback
+to the user.
+'''
+self.build(debug_info="dwarf")
+program = self.getBuildArtifact("a.out")
+main_obj = self.getBuildArtifact("main.o")
+self.assertTrue(os.path.exists(main_obj))
+# Delete the main.o file that contains the debug info so we force an
+# error when we run to main and try to get variables
+os.unlink(main_obj)
+
+self.create_debug_adaptor()
+self.assertTrue(os.path.exists(program), 'executable must exist')
+

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D134333#3812653 , @jingham wrote:

> In D134333#3812448 , @clayborg 
> wrote:
>
>> I just did a search through our test sources and we use 
>> GetError().GetCString() 34 times in our test suites python files. So the 
>> SBError::GetCString() is being misused a lot like this already and is 
>> probably making some tests flaky. I would highly recommend we fix 
>> SBError::GetCString() to make sure things work correctly. If people are 
>> already doing this, or if they can do this with our API, then we should make 
>> our API as stable as possible for users even if it costs a bit more memory 
>> in our string pools.
>
> When we return Python strings from the SWIG'ed version of 
> GetError().GetCString() does the Python string we make & return copy the 
> string data, or is it just holding onto the pointer?  In C/C++ if someone 
> returns you a char * unless explicitly specified you assume that string is 
> only going to live as long as the object that handed it out.  But a python 
> string is generally self-contained, so you wouldn't expect it to lose its 
> data when the generating object goes away.  I haven't checked yet, but if 
> this is how SWIG handles making python strings from C strings, then this 
> wouldn't be an error in the testsuite, only in C++ code.  If that's not how 
> it works, that might be a good way to finesse the issue.

The main issue is people are already using this API this way. We can fix this 
by adding documentation and saying "don't do that", but that will lead to bugs 
and our API appearing flaky. Many strings we return have no lifetime issues due 
to string constification. We can't really even stop people from doing this by 
making sure the string is emptied when the object destructs because another 
object could come and live in its place quickly on the stack and we could be 
using a bogus data as the error string.

All that said, I am fine going with what the majority of people want on this. I 
will remove this issue from this patch so it doesn't hold up this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134333

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