[Lldb-commits] [PATCH] D124113: [lldb] Adjust libc++ string formatter for changes in D123580

2022-04-21 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 424125.
labath marked 2 inline comments as done.
labath added a comment.

- address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124113

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -555,6 +555,7 @@
 
 /// Determine the size in bytes of \p valobj (a libc++ std::string object) and
 /// extract its data payload. Return the size + payload pair.
+// TODO: Support big-endian architectures.
 static llvm::Optional>
 ExtractLibcxxStringInfo(ValueObject &valobj) {
   ValueObjectSP D(valobj.GetChildAtIndexPath({0, 0, 0, 0}));
@@ -572,14 +573,27 @@
   ConstString g_size_name("__size_");
   bool short_mode = false; // this means the string is in short-mode and the
// data is stored inline
+  bool using_bitmasks = true; // Whether the class uses bitmasks for the mode
+  // flag (pre-D123580).
+  uint64_t size;
   LibcxxStringLayoutMode layout = (layout_decider->GetName() == g_data_name)
   ? eLibcxxStringLayoutModeDSC
   : eLibcxxStringLayoutModeCSD;
   uint64_t size_mode_value = 0;
 
-  if (layout == eLibcxxStringLayoutModeDSC) {
+  if (ValueObjectSP is_long = D->GetChildAtNamePath(
+  {ConstString("__s"), ConstString("__is_long_")})) {
+using_bitmasks = false;
+short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
+if (ValueObjectSP size_member =
+D->GetChildAtNamePath({ConstString("__s"), 
ConstString("__size_")}))
+  size = size_member->GetValueAsUnsigned(/*fail_value=*/0);
+else
+  return {};
+  } else if (layout == eLibcxxStringLayoutModeDSC) {
 llvm::SmallVector size_mode_locations[] = {
-{1, 2}, // Post-c3d0205ee771 layout
+{1, 2}, // Post-c3d0205ee771 layout. This was in use for only a brief
+// period, so we can delete it if it becomes a burden.
 {1, 1, 0},
 {1, 1, 1},
 };
@@ -610,9 +624,10 @@
   return {};
 ValueObjectSP location_sp = s->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
-const uint64_t size = (layout == eLibcxxStringLayoutModeDSC)
-  ? size_mode_value
-  : ((size_mode_value >> 1) % 256);
+if (using_bitmasks)
+  size = (layout == eLibcxxStringLayoutModeDSC)
+ ? size_mode_value
+ : ((size_mode_value >> 1) % 256);
 
 // When the small-string optimization takes place, the data must fit in the
 // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's
@@ -631,18 +646,18 @@
   if (!l)
 return {};
   // we can use the layout_decider object as the data pointer
-  ValueObjectSP location_sp = (layout == eLibcxxStringLayoutModeDSC)
-  ? layout_decider
-  : l->GetChildAtIndex(2, true);
-  ValueObjectSP size_vo(l->GetChildAtIndex(1, true));
-  const unsigned capacity_index =
-  (layout == eLibcxxStringLayoutModeDSC) ? 2 : 0;
-  ValueObjectSP capacity_vo(l->GetChildAtIndex(capacity_index, true));
+  ValueObjectSP location_sp =
+  l->GetChildMemberWithName(ConstString("__data_"), /*can_create=*/true);
+  ValueObjectSP size_vo =
+  l->GetChildMemberWithName(ConstString("__size_"), /*can_create=*/true);
+  ValueObjectSP capacity_vo =
+  l->GetChildMemberWithName(ConstString("__cap_"), /*can_create=*/true);
   if (!size_vo || !location_sp || !capacity_vo)
 return {};
-  const uint64_t size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
-  const uint64_t capacity =
-  capacity_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+  size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+  uint64_t capacity = capacity_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+  if (!using_bitmasks && layout == eLibcxxStringLayoutModeCSD)
+capacity *= 2;
   if (size == LLDB_INVALID_OFFSET || capacity == LLDB_INVALID_OFFSET ||
   capacity < size)
 return {};


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -555,6 +555,7 @@
 
 /// Determine the size in bytes of \p valobj (a libc++ std::string object) and
 /// extract its data payload. Return the size + payload pair.
+// TODO: Support big-endian architectures.
 static llvm::Optional>
 ExtractLibcxxStringInfo(ValueObject &valobj) {
   ValueObjectSP D(valobj.GetChildAtIndexPath({0, 0, 0, 0}));
@@ -572,14 +573,27 @@
  

[Lldb-commits] [PATCH] D124113: [lldb] Adjust libc++ string formatter for changes in D123580

2022-04-21 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:658
+  if (!using_bitmasks)
+capacity *= 2;
   if (size == LLDB_INVALID_OFFSET || capacity == LLDB_INVALID_OFFSET ||

philnik wrote:
> This should only be done if the string is in the normal layout and little 
> endian or in the alternate layout and big endian. Why do you care about the 
> capacity at all? Isn't that just another point of failure?
I've added the layout check and made a note that big-endian is not supported. 
The capacity check comes from , but I'm not 
sure of the overall purpose. I guess it was trying to screen out 
corrupt/uninitialized string objects, but I'm not convinced that we're doing 
anyone a favour by refusing to format those -- after all, the application 
itself might access such objects without checking the capacity, and operate on 
the data that we've decided to ignore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124113

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


[Lldb-commits] [PATCH] D124155: [lldb] Add tests which simulate the various std::string layouts

2022-04-21 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: shafik, philnik.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

Checking whether a formatter change does not break some of the supported
string layouts is difficult because it requires tracking down and/or
building different versions and build configurations of the library.

The purpose of this patch is to avoid that by providing an in-tree
simulation of the string class. It is a reduced version of the real
string class, obtained by elimitating all non-trivial code, leaving
just the internal data structures used by the data formatter. Different
versions of the class can be simulated through preprocessor defines.

The test (ab)uses the fact that our formatters kick in for any
double-underscore sub-namespace of `std`, so it avoids colliding with
the real string class by declaring the test class in the std::__lldb
namespace.

I do not consider this to be a replacement for the existing data
formatter tests, as producing this kind of a test is not trivial, and it
is easy to make a mistake in the process. However, it's also not
realistic to expect that every person changing the data formatter will
test it against all versions of the real class, so I think it can be
useful as a first line of defence.

Adding support for new layouts can become particularly unwieldy, but
this complexity will also be reflected in the actual code, so if we find
ourselves needing to support too many variants, we may need to start
dropping support for old ones, or come up with a completely different
strategy.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124155

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/TestDataFormatterLibcxxStringSimulator.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/a.out
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp
@@ -0,0 +1,217 @@
+#include 
+#include 
+#include 
+
+namespace std {
+namespace __lldb {
+
+template ::value && !std::is_final<_Tp>::value>
+struct __compressed_pair_elem {
+  explicit __compressed_pair_elem(_Tp __t) : __value_(__t) {}
+
+  _Tp &__get() { return __value_; }
+
+private:
+  _Tp __value_;
+};
+
+template 
+struct __compressed_pair_elem<_Tp, _Idx, true> : private _Tp {
+  explicit __compressed_pair_elem(_Tp __t) : _Tp(__t) {}
+
+  _Tp &__get() { return *this; }
+};
+
+template 
+class __compressed_pair : private __compressed_pair_elem<_T1, 0>,
+  private __compressed_pair_elem<_T2, 1> {
+public:
+  using _Base1 = __compressed_pair_elem<_T1, 0>;
+  using _Base2 = __compressed_pair_elem<_T2, 1>;
+
+  explicit __compressed_pair(_T1 __t1, _T2 __t2) : _Base1(__t1), _Base2(__t2) {}
+
+  _T1 &first() { return static_cast<_Base1 &>(*this).__get(); }
+};
+
+#if defined(ALTERNATE_LAYOUT) && defined(SUBCLASS_PADDING)
+template  struct __padding {
+  unsigned char __xx[sizeof(_CharT) - 1];
+};
+
+template  struct __padding<_CharT, 1> {};
+#endif
+
+template  class basic_string {
+public:
+  typedef _CharT value_type;
+  typedef _Allocator allocator_type;
+  typedef allocator_traits __alloc_traits;
+  typedef typename __alloc_traits::size_type size_type;
+  typedef typename __alloc_traits::pointer pointer;
+
+#ifdef ALTERNATE_LAYOUT
+
+  struct __long {
+pointer __data_;
+size_type __size_;
+#ifdef BITMASKS
+size_type __cap_;
+#else
+size_type __cap_ : sizeof(size_type) * CHAR_BIT - 1;
+size_type __is_long_ : 1;
+#endif
+  };
+
+  enum {
+__min_cap = (sizeof(__long) - 1) / sizeof(value_type) > 2
+? (sizeof(__long) - 1) / sizeof(value_type)
+: 2
+  };
+
+  struct __short {
+value_type __data_[__min_cap];
+#ifdef BITMASKS
+#ifdef SUBCLASS_PADDING
+struct : __padding {
+  unsigned char __size_;
+};
+#else
+unsigned char __padding[sizeof(value_type) - 1];
+unsigned char __size_;
+#endif
+#else // !BITMASKS
+unsigned char __size_ : 7;
+unsigned char __is_long_ : 1;
+#endif
+  };
+
+#ifdef BITMASKS
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+  static const size_type __short_shift = 1;
+  static const size_type __long_mask = 0x1ul;
+#else
+  static const size_type __short_shift = 0;
+  static const size_type __long_mask = ~(size_type(~0) >> 1);
+#endif
+#else // !BITMASKS
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+  static const size_type __endian_factor = 2;
+#else
+  static const size_type __endian_factor = 1;
+#endif
+#en

[Lldb-commits] [PATCH] D124155: [lldb] Add tests which simulate the various std::string layouts

2022-04-21 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 424131.
labath added a comment.
Herald added a subscriber: JDevlieghere.

remove the stray binary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124155

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/TestDataFormatterLibcxxStringSimulator.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/simulator/main.cpp
@@ -0,0 +1,217 @@
+#include 
+#include 
+#include 
+
+namespace std {
+namespace __lldb {
+
+template ::value && !std::is_final<_Tp>::value>
+struct __compressed_pair_elem {
+  explicit __compressed_pair_elem(_Tp __t) : __value_(__t) {}
+
+  _Tp &__get() { return __value_; }
+
+private:
+  _Tp __value_;
+};
+
+template 
+struct __compressed_pair_elem<_Tp, _Idx, true> : private _Tp {
+  explicit __compressed_pair_elem(_Tp __t) : _Tp(__t) {}
+
+  _Tp &__get() { return *this; }
+};
+
+template 
+class __compressed_pair : private __compressed_pair_elem<_T1, 0>,
+  private __compressed_pair_elem<_T2, 1> {
+public:
+  using _Base1 = __compressed_pair_elem<_T1, 0>;
+  using _Base2 = __compressed_pair_elem<_T2, 1>;
+
+  explicit __compressed_pair(_T1 __t1, _T2 __t2) : _Base1(__t1), _Base2(__t2) {}
+
+  _T1 &first() { return static_cast<_Base1 &>(*this).__get(); }
+};
+
+#if defined(ALTERNATE_LAYOUT) && defined(SUBCLASS_PADDING)
+template  struct __padding {
+  unsigned char __xx[sizeof(_CharT) - 1];
+};
+
+template  struct __padding<_CharT, 1> {};
+#endif
+
+template  class basic_string {
+public:
+  typedef _CharT value_type;
+  typedef _Allocator allocator_type;
+  typedef allocator_traits __alloc_traits;
+  typedef typename __alloc_traits::size_type size_type;
+  typedef typename __alloc_traits::pointer pointer;
+
+#ifdef ALTERNATE_LAYOUT
+
+  struct __long {
+pointer __data_;
+size_type __size_;
+#ifdef BITMASKS
+size_type __cap_;
+#else
+size_type __cap_ : sizeof(size_type) * CHAR_BIT - 1;
+size_type __is_long_ : 1;
+#endif
+  };
+
+  enum {
+__min_cap = (sizeof(__long) - 1) / sizeof(value_type) > 2
+? (sizeof(__long) - 1) / sizeof(value_type)
+: 2
+  };
+
+  struct __short {
+value_type __data_[__min_cap];
+#ifdef BITMASKS
+#ifdef SUBCLASS_PADDING
+struct : __padding {
+  unsigned char __size_;
+};
+#else
+unsigned char __padding[sizeof(value_type) - 1];
+unsigned char __size_;
+#endif
+#else // !BITMASKS
+unsigned char __size_ : 7;
+unsigned char __is_long_ : 1;
+#endif
+  };
+
+#ifdef BITMASKS
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+  static const size_type __short_shift = 1;
+  static const size_type __long_mask = 0x1ul;
+#else
+  static const size_type __short_shift = 0;
+  static const size_type __long_mask = ~(size_type(~0) >> 1);
+#endif
+#else // !BITMASKS
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+  static const size_type __endian_factor = 2;
+#else
+  static const size_type __endian_factor = 1;
+#endif
+#endif // BITMASKS
+
+#else // !ALTERNATE_LAYOUT
+
+  struct __long {
+#ifdef BITMASKS
+size_type __cap_;
+#else
+size_type __is_long_ : 1;
+size_type __cap_ : sizeof(size_type) * CHAR_BIT - 1;
+#endif
+size_type __size_;
+pointer __data_;
+  };
+
+  enum {
+__min_cap = (sizeof(__long) - 1) / sizeof(value_type) > 2
+? (sizeof(__long) - 1) / sizeof(value_type)
+: 2
+  };
+
+  struct __short {
+union {
+#ifdef BITMASKS
+  unsigned char __size_;
+#else
+  struct {
+unsigned char __is_long_ : 1;
+unsigned char __size_ : 7;
+  };
+#endif
+  value_type __lx;
+};
+value_type __data_[__min_cap];
+  };
+
+#ifdef BITMASKS
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+  static const size_type __short_shift = 0;
+  static const size_type __long_mask = ~(size_type(~0) >> 1);
+#else
+  static const size_type __short_shift = 1;
+  static const size_type __long_mask = 0x1ul;
+#endif
+#else // !BITMASKS
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+  static const size_type __endian_factor = 1;
+#else
+  static const size_type __endian_factor = 2;
+#endif
+#endif
+
+#endif // ALTERNATE_LAYOUT
+
+  union __ulx {
+__long __lx;
+__short __lxx;
+  };
+
+  enum { __n_words = sizeof(__ulx) / sizeof(size_type) };
+
+  struct __raw {
+size_type __words[__n_words];
+  };
+
+  struct __rep {
+union {
+  __long __l;
+  __short __s;
+  __raw __r;
+};
+  };
+
+  __compressed_pair<__

[Lldb-commits] [PATCH] D124110: Refactor protected virtual functions from SymbolFile into new SymbolFileActual class.

2022-04-21 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D124110#3463380 , @clayborg wrote:

> I might suggest we rename things like suggested in my inline comment, and 
> then have the "SymbolFile.h" class just include the extra needed header file? 
> Many of the changes in this diff would just go away.

Those would be consistent with some existing libraries (e.g. those which use 
the `I` prefix to denote pure interfaces), but if the goal is to reduce the 
number of changes, then I don't think it will help, as pretty much everything 
should still refer to the objects through the interface name. The name 
SymbolFileActual should pretty much only be used in the class declaration. but 
there are many more places that one would need to change from SymbolFile to 
SymbolFileInterface.

Instead of the name `Actual`, I might go for Common -- I'm thinking of the 
class as something that contains implementations "common" to many (but not 
necessarily all) symbol file classes.

> I am not a fan of the SymbolFileActual class nor having to switch to 
> SymbolFileActual.h. I would almost prefer both SymbolFileActual and 
> SymbolFile to stay in SymbolFile.h.

Having SymbolFile.h include the other header would be strange, as the include 
would have to go to the bottom of the file. I did something like that recently, 
but only as a temporary transition aid.

However, I don't have a problem with having both classes be defined in the same 
header file. I don't know if there's any direct reference from 
SymbolFileOnDemand to SymbolFileActual. Ideally there wouldn't be, but if there 
is, then that class would also have to be defined in the same file -- also not 
a tragedy if the file does not get too big.




Comment at: lldb/include/lldb/Symbol/SymbolFile.h:133-134
   // Compile Unit function calls
   // Approach 1 - iterator
-  uint32_t GetNumCompileUnits();
-  lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx);
+  // Make virtual so that SymbolFileOnDemand can override.
+  virtual uint32_t GetNumCompileUnits() = 0;

The comment probably not necessary. In this setup, there's no way that a 
non-virtual method can make sense, as there is nothing one could use to 
implement it.



Comment at: lldb/include/lldb/Symbol/SymbolFile.h:371-379
   lldb::ObjectFileSP m_objfile_sp; // Keep a reference to the object file in
// case it isn't the same as the module
// object file (debug symbols in a separate
// file)
-  llvm::Optional> m_compile_units;
-  TypeList m_type_list;
   Symtab *m_symtab = nullptr;
   uint32_t m_abilities = 0;
   bool m_calculated_abilities = false;

What's up with the rest of the members. Can we move these too?
Any member that stays here runs the risk of going out of sync when we have two 
instances floating around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124110

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


[Lldb-commits] [PATCH] D124155: [lldb] Add tests which simulate the various std::string layouts

2022-04-21 Thread Nikolas Klauser via Phabricator via lldb-commits
philnik accepted this revision.
philnik added a comment.
This revision is now accepted and ready to land.

LGTM, but you probably want to wait for someone from the LLDB team to approve 
too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124155

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


[Lldb-commits] [PATCH] D123502: [lldb][NFC] Refactor printing of short options in help

2022-04-21 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123502

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


[Lldb-commits] [PATCH] D123500: [lldb][NFC] Add more tests for GenerateOptionsUsage

2022-04-21 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123500

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


[Lldb-commits] [PATCH] D124113: [lldb] Adjust libc++ string formatter for changes in D123580

2022-04-21 Thread Nikolas Klauser via Phabricator via lldb-commits
philnik accepted this revision.
philnik added a comment.
This revision is now accepted and ready to land.

LGTM assuming you checked that it actually works with my patch applied. Do you 
want to land your patch first, or should we do it the other way around?




Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:658
+  if (!using_bitmasks)
+capacity *= 2;
   if (size == LLDB_INVALID_OFFSET || capacity == LLDB_INVALID_OFFSET ||

labath wrote:
> philnik wrote:
> > This should only be done if the string is in the normal layout and little 
> > endian or in the alternate layout and big endian. Why do you care about the 
> > capacity at all? Isn't that just another point of failure?
> I've added the layout check and made a note that big-endian is not supported. 
> The capacity check comes from , but I'm not 
> sure of the overall purpose. I guess it was trying to screen out 
> corrupt/uninitialized string objects, but I'm not convinced that we're doing 
> anyone a favour by refusing to format those -- after all, the application 
> itself might access such objects without checking the capacity, and operate 
> on the data that we've decided to ignore.
Maybe it would make sense to say that the string is uninitialized or corrupted 
when the `__invariants()` fail? That would probably catch more bugs, but I 
don't know if you can just run a member function in the pretty printer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124113

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


[Lldb-commits] [PATCH] D124113: [lldb] Adjust libc++ string formatter for changes in D123580

2022-04-21 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D124113#3464287 , @philnik wrote:

> LGTM assuming you checked that it actually works with my patch applied. Do 
> you want to land your patch first, or should we do it the other way around?

I've confirmed this with the real patch and with my magic string simulator. For 
me, it would be better if this patch went in first (so I guess I'll land it 
now).




Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:658
+  if (!using_bitmasks)
+capacity *= 2;
   if (size == LLDB_INVALID_OFFSET || capacity == LLDB_INVALID_OFFSET ||

philnik wrote:
> labath wrote:
> > philnik wrote:
> > > This should only be done if the string is in the normal layout and little 
> > > endian or in the alternate layout and big endian. Why do you care about 
> > > the capacity at all? Isn't that just another point of failure?
> > I've added the layout check and made a note that big-endian is not 
> > supported. The capacity check comes from , 
> > but I'm not sure of the overall purpose. I guess it was trying to screen 
> > out corrupt/uninitialized string objects, but I'm not convinced that we're 
> > doing anyone a favour by refusing to format those -- after all, the 
> > application itself might access such objects without checking the capacity, 
> > and operate on the data that we've decided to ignore.
> Maybe it would make sense to say that the string is uninitialized or 
> corrupted when the `__invariants()` fail? That would probably catch more 
> bugs, but I don't know if you can just run a member function in the pretty 
> printer.
Technically, we can run functions from pretty-printers, but we try hard to 
avoid that. If calling functions was ok, we could just call `data()` and get 
rid of this mess. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124113

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


[Lldb-commits] [lldb] 1056c56 - [lldb] Adjust libc++ string formatter for changes in D123580

2022-04-21 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2022-04-21T14:07:56+02:00
New Revision: 1056c56786c10866ffd7e878f8c75ad1f0914c07

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

LOG: [lldb] Adjust libc++ string formatter for changes in D123580

The code needs more TLC, but for now I've tried making only the changes
that are necessary to get the tests passing -- postponing the more
invasive changes after I create a more comprehensive test.

In a couple of places I have changed the index-based element accesses to
name-based ones (as these are less sensitive to code perturbations). I'm
not sure why the code was using indexes in the first place, but I've
(manually) tested the change with various libc++ versions, and found no
issues with this approach.

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

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 2fd69452217c7..be2e4da80881a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -555,6 +555,7 @@ enum LibcxxStringLayoutMode {
 
 /// Determine the size in bytes of \p valobj (a libc++ std::string object) and
 /// extract its data payload. Return the size + payload pair.
+// TODO: Support big-endian architectures.
 static llvm::Optional>
 ExtractLibcxxStringInfo(ValueObject &valobj) {
   ValueObjectSP D(valobj.GetChildAtIndexPath({0, 0, 0, 0}));
@@ -572,14 +573,27 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
   ConstString g_size_name("__size_");
   bool short_mode = false; // this means the string is in short-mode and the
// data is stored inline
+  bool using_bitmasks = true; // Whether the class uses bitmasks for the mode
+  // flag (pre-D123580).
+  uint64_t size;
   LibcxxStringLayoutMode layout = (layout_decider->GetName() == g_data_name)
   ? eLibcxxStringLayoutModeDSC
   : eLibcxxStringLayoutModeCSD;
   uint64_t size_mode_value = 0;
 
-  if (layout == eLibcxxStringLayoutModeDSC) {
+  if (ValueObjectSP is_long = D->GetChildAtNamePath(
+  {ConstString("__s"), ConstString("__is_long_")})) {
+using_bitmasks = false;
+short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
+if (ValueObjectSP size_member =
+D->GetChildAtNamePath({ConstString("__s"), 
ConstString("__size_")}))
+  size = size_member->GetValueAsUnsigned(/*fail_value=*/0);
+else
+  return {};
+  } else if (layout == eLibcxxStringLayoutModeDSC) {
 llvm::SmallVector size_mode_locations[] = {
-{1, 2}, // Post-c3d0205ee771 layout
+{1, 2}, // Post-c3d0205ee771 layout. This was in use for only a brief
+// period, so we can delete it if it becomes a burden.
 {1, 1, 0},
 {1, 1, 1},
 };
@@ -610,9 +624,10 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
   return {};
 ValueObjectSP location_sp = s->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
-const uint64_t size = (layout == eLibcxxStringLayoutModeDSC)
-  ? size_mode_value
-  : ((size_mode_value >> 1) % 256);
+if (using_bitmasks)
+  size = (layout == eLibcxxStringLayoutModeDSC)
+ ? size_mode_value
+ : ((size_mode_value >> 1) % 256);
 
 // When the small-string optimization takes place, the data must fit in the
 // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's
@@ -631,18 +646,18 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
   if (!l)
 return {};
   // we can use the layout_decider object as the data pointer
-  ValueObjectSP location_sp = (layout == eLibcxxStringLayoutModeDSC)
-  ? layout_decider
-  : l->GetChildAtIndex(2, true);
-  ValueObjectSP size_vo(l->GetChildAtIndex(1, true));
-  const unsigned capacity_index =
-  (layout == eLibcxxStringLayoutModeDSC) ? 2 : 0;
-  ValueObjectSP capacity_vo(l->GetChildAtIndex(capacity_index, true));
+  ValueObjectSP location_sp =
+  l->GetChildMemberWithName(ConstString("__data_"), /*can_create=*/true);
+  ValueObjectSP size_vo =
+  l->GetChildMemberWithName(ConstString("__size_"), /*can_create=*/true);
+  ValueObjectSP capacity_vo =
+  l->GetChildMemberWithName(ConstString("__cap_"), /*can_create=*/true);
   if (!size_vo || !location_sp || !capacity_vo)
 return {};
-  const uint64_t size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
-  const uint64_t capacity =
-  capaci

[Lldb-commits] [PATCH] D124113: [lldb] Adjust libc++ string formatter for changes in D123580

2022-04-21 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1056c56786c1: [lldb] Adjust libc++ string formatter for 
changes in D123580 (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124113

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -555,6 +555,7 @@
 
 /// Determine the size in bytes of \p valobj (a libc++ std::string object) and
 /// extract its data payload. Return the size + payload pair.
+// TODO: Support big-endian architectures.
 static llvm::Optional>
 ExtractLibcxxStringInfo(ValueObject &valobj) {
   ValueObjectSP D(valobj.GetChildAtIndexPath({0, 0, 0, 0}));
@@ -572,14 +573,27 @@
   ConstString g_size_name("__size_");
   bool short_mode = false; // this means the string is in short-mode and the
// data is stored inline
+  bool using_bitmasks = true; // Whether the class uses bitmasks for the mode
+  // flag (pre-D123580).
+  uint64_t size;
   LibcxxStringLayoutMode layout = (layout_decider->GetName() == g_data_name)
   ? eLibcxxStringLayoutModeDSC
   : eLibcxxStringLayoutModeCSD;
   uint64_t size_mode_value = 0;
 
-  if (layout == eLibcxxStringLayoutModeDSC) {
+  if (ValueObjectSP is_long = D->GetChildAtNamePath(
+  {ConstString("__s"), ConstString("__is_long_")})) {
+using_bitmasks = false;
+short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
+if (ValueObjectSP size_member =
+D->GetChildAtNamePath({ConstString("__s"), 
ConstString("__size_")}))
+  size = size_member->GetValueAsUnsigned(/*fail_value=*/0);
+else
+  return {};
+  } else if (layout == eLibcxxStringLayoutModeDSC) {
 llvm::SmallVector size_mode_locations[] = {
-{1, 2}, // Post-c3d0205ee771 layout
+{1, 2}, // Post-c3d0205ee771 layout. This was in use for only a brief
+// period, so we can delete it if it becomes a burden.
 {1, 1, 0},
 {1, 1, 1},
 };
@@ -610,9 +624,10 @@
   return {};
 ValueObjectSP location_sp = s->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
-const uint64_t size = (layout == eLibcxxStringLayoutModeDSC)
-  ? size_mode_value
-  : ((size_mode_value >> 1) % 256);
+if (using_bitmasks)
+  size = (layout == eLibcxxStringLayoutModeDSC)
+ ? size_mode_value
+ : ((size_mode_value >> 1) % 256);
 
 // When the small-string optimization takes place, the data must fit in the
 // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's
@@ -631,18 +646,18 @@
   if (!l)
 return {};
   // we can use the layout_decider object as the data pointer
-  ValueObjectSP location_sp = (layout == eLibcxxStringLayoutModeDSC)
-  ? layout_decider
-  : l->GetChildAtIndex(2, true);
-  ValueObjectSP size_vo(l->GetChildAtIndex(1, true));
-  const unsigned capacity_index =
-  (layout == eLibcxxStringLayoutModeDSC) ? 2 : 0;
-  ValueObjectSP capacity_vo(l->GetChildAtIndex(capacity_index, true));
+  ValueObjectSP location_sp =
+  l->GetChildMemberWithName(ConstString("__data_"), /*can_create=*/true);
+  ValueObjectSP size_vo =
+  l->GetChildMemberWithName(ConstString("__size_"), /*can_create=*/true);
+  ValueObjectSP capacity_vo =
+  l->GetChildMemberWithName(ConstString("__cap_"), /*can_create=*/true);
   if (!size_vo || !location_sp || !capacity_vo)
 return {};
-  const uint64_t size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
-  const uint64_t capacity =
-  capacity_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+  size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+  uint64_t capacity = capacity_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+  if (!using_bitmasks && layout == eLibcxxStringLayoutModeCSD)
+capacity *= 2;
   if (size == LLDB_INVALID_OFFSET || capacity == LLDB_INVALID_OFFSET ||
   capacity < size)
 return {};


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -555,6 +555,7 @@
 
 /// Determine the size in bytes of \p valobj (a libc++ std::string object) and
 /// extract its data payload. Return the size + payload pair.
+// TODO: Support big-endian architectures.
 static llvm::Optional>
 ExtractLibcxxStringInfo(ValueObject &valobj) {
   ValueObjectSP D(valobj.GetChil

[Lldb-commits] [PATCH] D124110: Refactor protected virtual functions from SymbolFile into new SymbolFileActual class.

2022-04-21 Thread jeffrey tan via Phabricator via lldb-commits
yinghuitan added a comment.

Sounds good.

Here are the changes I plan to do:

1. Rename SymbolFileActual => SymbolFileCommon.
2. Inline SymbolFileCommon.h content into SymbolFile.h,
3. Move all remaining member fields of SymbolFile class into SymbolFileCommon.




Comment at: lldb/include/lldb/Symbol/SymbolFile.h:133-134
   // Compile Unit function calls
   // Approach 1 - iterator
-  uint32_t GetNumCompileUnits();
-  lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx);
+  // Make virtual so that SymbolFileOnDemand can override.
+  virtual uint32_t GetNumCompileUnits() = 0;

labath wrote:
> The comment probably not necessary. In this setup, there's no way that a 
> non-virtual method can make sense, as there is nothing one could use to 
> implement it.
Sounds good.



Comment at: lldb/include/lldb/Symbol/SymbolFile.h:371-379
   lldb::ObjectFileSP m_objfile_sp; // Keep a reference to the object file in
// case it isn't the same as the module
// object file (debug symbols in a separate
// file)
-  llvm::Optional> m_compile_units;
-  TypeList m_type_list;
   Symtab *m_symtab = nullptr;
   uint32_t m_abilities = 0;
   bool m_calculated_abilities = false;

labath wrote:
> What's up with the rest of the members. Can we move these too?
> Any member that stays here runs the risk of going out of sync when we have 
> two instances floating around.
Sure, I can move them.
I did not do in this patch initially because I want to avoid change too much 
code and get further feedback. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124110

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


[Lldb-commits] [PATCH] D124198: [LLDB][Unwind] Add stack scanning as fallback unwind plan if no symbol file is available.

2022-04-21 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu created this revision.
zequanwu added reviewers: labath, clayborg, jasonmolenda.
Herald added subscribers: atanasyan, jrtc27, kbarton, nemanjai, sdardis.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This adds stack scanning as fallback unwind plan to find a value that looks like
return address. If the return address is not valid, continue scanning from next
locatoin on stack when trying fallback unwind and the active row is `raSearch`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124198

Files:
  lldb/include/lldb/Symbol/UnwindPlan.h
  lldb/include/lldb/Target/ABI.h
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
  lldb/source/Plugins/ABI/ARC/ABISysV_arc.h
  lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.h
  lldb/source/Plugins/ABI/ARM/ABISysV_arm.h
  lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.h
  lldb/source/Plugins/ABI/Mips/ABISysV_mips.h
  lldb/source/Plugins/ABI/Mips/ABISysV_mips64.h
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.h
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.h
  lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.h
  lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.h
  lldb/source/Plugins/ABI/X86/ABISysV_i386.h
  lldb/source/Plugins/ABI/X86/ABISysV_x86_64.h
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Symbol/UnwindPlan.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test

Index: lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
===
--- lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
+++ lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
@@ -16,13 +16,13 @@
 # CHECK: This UnwindPlan is valid at all instruction locations: no.
 # CHECK: This UnwindPlan is for a trap handler function: no.
 # CHECK: Address range of this UnwindPlan: [unwind-via-stack-win.exe..module_image + 4112-0x107d)
-# CHECK: row[0]:0: CFA=RaSearch@SP+0 => esp=DW_OP_pick 0x0, DW_OP_consts +4, DW_OP_plus  eip=DW_OP_pick 0x0, DW_OP_deref
+# CHECK: row[0]:0: CFA=RaSearch@SP+0, offset=+0 => esp=DW_OP_pick 0x0, DW_OP_consts +4, DW_OP_plus  eip=DW_OP_pick 0x0, DW_OP_deref
 
 image show-unwind -n nonzero_frame_size
 # CHECK-LABEL: image show-unwind -n nonzero_frame_size
 # CHECK: UNWIND PLANS for unwind-via-stack-win.exe`nonzero_frame_size
 # CHECK: Symbol file UnwindPlan:
-# CHECK: row[0]:0: CFA=RaSearch@SP+12 => esp=DW_OP_pick 0x0, DW_OP_consts +4, DW_OP_plus  eip=DW_OP_pick 0x0, DW_OP_deref
+# CHECK: row[0]:0: CFA=RaSearch@SP+12, offset=+0 => esp=DW_OP_pick 0x0, DW_OP_consts +4, DW_OP_plus  eip=DW_OP_pick 0x0, DW_OP_deref
 
 # Then, some invalid rules.
 image show-unwind -n complex_rasearch
Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -285,7 +285,18 @@
 cfa_status = true;
 }
 if (!cfa_status) {
-  UnwindLogMsg("could not read CFA value for first frame.");
+  UnwindLogMsg("could not read CFA value for first frame. Trying stack "
+   "walking unwind plan.");
+  if (abi) {
+UnwindPlanSP stack_walk_unwind_plan =
+std::make_shared(lldb::eRegisterKindGeneric);
+if (abi->CreateStackWalkingUnwindPlan(*stack_walk_unwind_plan)) {
+  m_fallback_unwind_plan_sp = stack_walk_unwind_plan;
+  cfa_status = TryFallbackUnwindPlan();
+}
+  }
+}
+if (!cfa_status) {
   m_frame_type = eNotAValidFrame;
   return;
 }
@@ -384,7 +395,7 @@
   // symbol/function information - just stick in some reasonable defaults and
   // hope we can unwind past this frame.  If we're above a trap handler,
   // we may be at a bogus address because we jumped through a bogus function
-  // pointer and trapped, so don't force the arch default unwind plan in that 
+  // pointer and trapped, so don't force the arch default unwind plan in that
   // case.
   ModuleSP pc_module_sp(m_current_pc.GetModule());
   if ((!m_current_pc.IsValid() || !pc_module_sp) &&
@@ -660,9 +671,20 @@
   }
 
   if (!ReadFrameAddress(row_register_kind, active_row->GetCFAValue(), m_cfa)) {
-UnwindLogMsg("failed to get cfa");
-m_frame_type = eNotAValidFrame;
-return;
+UnwindLogMsg("failed to get cfa. Trying stack walking unwind plan.");
+bool cfa_status = false;
+if (abi) {
+  UnwindPlanSP stack_walk_unwind_plan =
+  std::make_shared(lldb::eRegisterKindGeneric);
+  if (abi->Cre

[Lldb-commits] [PATCH] D124110: Refactor protected virtual functions from SymbolFile into new SymbolFileActual class.

2022-04-21 Thread jeffrey tan via Phabricator via lldb-commits
yinghuitan updated this revision to Diff 424317.
yinghuitan added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124110

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  lldb/source/Symbol/SymbolFile.cpp

Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -25,6 +25,7 @@
 using namespace lldb;
 
 char SymbolFile::ID;
+char SymbolFileCommon::ID;
 
 void SymbolFile::PreloadSymbols() {
   // No-op for most implementations.
@@ -33,9 +34,6 @@
 std::recursive_mutex &SymbolFile::GetModuleMutex() const {
   return GetObjectFile()->GetModule()->GetMutex();
 }
-ObjectFile *SymbolFile::GetMainObjectFile() {
-  return m_objfile_sp->GetModule()->GetObjectFile();
-}
 
 SymbolFile *SymbolFile::FindPlugin(ObjectFileSP objfile_sp) {
   std::unique_ptr best_symfile_up;
@@ -87,16 +85,6 @@
   return best_symfile_up.release();
 }
 
-llvm::Expected
-SymbolFile::GetTypeSystemForLanguage(lldb::LanguageType language) {
-  auto type_system_or_err =
-  m_objfile_sp->GetModule()->GetTypeSystemForLanguage(language);
-  if (type_system_or_err) {
-type_system_or_err->SetSymbolFile(this);
-  }
-  return type_system_or_err;
-}
-
 uint32_t
 SymbolFile::ResolveSymbolContext(const SourceLocationSpec &src_location_spec,
  lldb::SymbolContextItem resolve_scope,
@@ -154,7 +142,37 @@
 #endif
 }
 
-uint32_t SymbolFile::GetNumCompileUnits() {
+SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
+
+Symtab *SymbolFileCommon::GetSymtab() {
+  std::lock_guard guard(GetModuleMutex());
+  if (m_symtab)
+return m_symtab;
+
+  // Fetch the symtab from the main object file.
+  m_symtab = GetMainObjectFile()->GetSymtab();
+
+  // Then add our symbols to it.
+  if (m_symtab)
+AddSymbols(*m_symtab);
+
+  return m_symtab;
+}
+
+ObjectFile *SymbolFileCommon::GetMainObjectFile() {
+  return m_objfile_sp->GetModule()->GetObjectFile();
+}
+
+void SymbolFileCommon::SectionFileAddressesChanged() {
+  ObjectFile *module_objfile = GetMainObjectFile();
+  ObjectFile *symfile_objfile = GetObjectFile();
+  if (symfile_objfile != module_objfile)
+symfile_objfile->SectionFileAddressesChanged();
+  if (m_symtab)
+m_symtab->SectionFileAddressesChanged();
+}
+
+uint32_t SymbolFileCommon::GetNumCompileUnits() {
   std::lock_guard guard(GetModuleMutex());
   if (!m_compile_units) {
 // Create an array of compile unit shared pointers -- which will each
@@ -164,7 +182,7 @@
   return m_compile_units->size();
 }
 
-CompUnitSP SymbolFile::GetCompileUnitAtIndex(uint32_t idx) {
+CompUnitSP SymbolFileCommon::GetCompileUnitAtIndex(uint32_t idx) {
   std::lock_guard guard(GetModuleMutex());
   uint32_t num = GetNumCompileUnits();
   if (idx >= num)
@@ -175,7 +193,8 @@
   return cu_sp;
 }
 
-void SymbolFile::SetCompileUnitAtIndex(uint32_t idx, const CompUnitSP &cu_sp) {
+void SymbolFileCommon::SetCompileUnitAtIndex(uint32_t idx,
+ const CompUnitSP &cu_sp) {
   std::lock_guard guard(GetModuleMutex());
   const size_t num_compile_units = GetNumCompileUnits();
   assert(idx < num_compile_units);
@@ -190,31 +209,29 @@
   (*m_compile_units)[idx] = cu_sp;
 }
 
-Symtab *SymbolFile::GetSymtab() {
-  std::lock_guard guard(GetModuleMutex());
-  if (m_symtab)
-return m_symtab;
-
-  // Fetch the symtab from the main object file.
-  m_symtab = GetMainObjectFile()->GetSymtab();
-
-  // Then add our symbols to it.
-  if (m_symtab)
-AddSymbols(*m_symtab);
-
-  return m_symtab;
+llvm::Expected
+SymbolFileCommon::GetTypeSystemForLanguage(lldb::LanguageType language) {
+  auto type_system_or_err =
+  m_objfile_sp->GetModule()->GetTypeSystemForLanguage(language);
+  if (type_system_or_err) {
+type_system_or_err->SetSymbolFile(this);
+  }
+  return type_system_or_err;
 }
 
-void SymbolFile::SectionFileAddressesChanged() {
-  ObjectFile *module_objfile = GetMainObjectFile();
-  ObjectFile *symfile_objfile = GetObjectFile();
-  if (symfile_objfile != module_objfile)
-symfile_objfile->SectionFileAddressesChanged();
-  if (m_symtab)
-m_symtab->SectionFileAdd

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

2022-04-21 Thread jeffrey tan via Phabricator via lldb-commits
yinghuitan updated this revision to Diff 424318.
yinghuitan added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

Files:
  lldb/docs/use/ondemand.rst
  lldb/include/lldb/Core/ModuleList.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/include/lldb/Target/Statistics.h
  lldb/include/lldb/Utility/LLDBLog.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/source/Symbol/CMakeLists.txt
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/SymbolFile.cpp
  lldb/source/Symbol/SymbolFileOnDemand.cpp
  lldb/source/Target/Statistics.cpp
  lldb/source/Utility/LLDBLog.cpp
  lldb/test/API/symbol_ondemand/breakpoint_language/Makefile
  
lldb/test/API/symbol_ondemand/breakpoint_language/TestBreakpointLanguageOnDemand.py
  lldb/test/API/symbol_ondemand/breakpoint_language/c_lang.c
  lldb/test/API/symbol_ondemand/breakpoint_language/cpp_lang.cpp
  lldb/test/API/symbol_ondemand/breakpoint_language/main.cpp
  lldb/test/API/symbol_ondemand/breakpoint_source_regex/Makefile
  
lldb/test/API/symbol_ondemand/breakpoint_source_regex/TestSourceTextRegexBreakpoint.py
  lldb/test/API/symbol_ondemand/breakpoint_source_regex/main.cpp
  lldb/test/API/symbol_ondemand/shared_library/Makefile
  lldb/test/API/symbol_ondemand/shared_library/TestSharedLib.py
  lldb/test/API/symbol_ondemand/shared_library/foo.c
  lldb/test/API/symbol_ondemand/shared_library/foo.h
  lldb/test/API/symbol_ondemand/shared_library/shared.c
  lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
  lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
  lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test

Index: lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test
@@ -0,0 +1,24 @@
+
+# Test shows that symbolic function breakpoint works with LLDB on demand symbol loading.
+
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: %build %p/Inputs/basic.cpp -o basic.out
+# RUN: %lldb -b -O "settings set symbols.load-on-demand true" -s %s basic.out | FileCheck %s
+
+breakpoint list
+# CHECK: No breakpoints currently set
+
+b bar
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+breakpoint list
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+run
+# CHECK: stop reason = breakpoint
+
+bt
+# CHECK: {{.*}}`bar(x=33, y=78) at basic.cpp:1
+# CHECK: {{.*}}`foo(x=33, y=78) at basic.cpp:3
+# CHECK: {{.*}}`main(argc=1, argv={{.*}}) at basic.cpp:5
Index: lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
@@ -0,0 +1,23 @@
+# Test shows that source line breakpoint works with LLDB on demand symbol loading.
+
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: %build %p/Inputs/basic.cpp -o basic.out
+# RUN: %lldb -b -O "settings set symbols.load-on-demand true" -s %s basic.out | FileCheck %s
+
+breakpoint list
+# CHECK: No breakpoints currently set
+
+breakpoint set -f basic.cpp -l 1
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+breakpoint list
+# CHECK: file = 'basic.cpp'
+
+run
+# CHECK: stop reason = breakpoint
+
+bt
+# CHECK: {{.*}}`bar(x=33, y=78) at basic.cpp:1
+# CHECK: {{.*}}`foo(x=33, y=78) at basic.cpp:3
+# CHECK: {{.*}}`main(argc=1, argv={{.*}}) at basic.cpp:5
Index: lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
@@ -0,0 +1,5 @@
+int bar(int x, int y) { return x + y + 5; }
+
+int foo(int x, int y) { return bar(x, y) + 12; }
+
+int main(int argc, char **argv) { return foo(33, 78); }
Index: lldb/test/API/symbol_ondemand/shared_library/shared.c
===
--- /dev/null
+++ lldb/test/API/symbol_ondemand/shared_library/shared.c
@@ -0,0 +1,9 @@
+#include "foo.h"
+#include 
+
+int global_shared = 897;
+int main(void) {
+  puts("This is a shared library test...");
+  foo(); // Set breakpoint 0 here.
+  return 0;
+}
Index: lldb/test/API/symbol_ondemand/shared_library/foo.h
===
--- /dev/null
+++ lldb/test/API/symbol_ondemand/shared_library/foo.h
@@ -0,0 +1,6 @@
+#ifndef foo_h__
+#define foo_h__
+
+extern void foo(void);
+
+#endif // foo_h__
Index: lldb/test/API/symbol_ondemand/shared_library/foo.c
=

[Lldb-commits] [PATCH] D124198: [LLDB][Unwind] Add stack scanning as fallback unwind plan if no symbol file is available.

2022-04-21 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

It is unclear how this patch works. Was there support for searching for the 
return address on the stack already? And this patch is just adding the a few 
register unwind rules for when/if the return address _is_ found for x86?

This will need tests.




Comment at: lldb/include/lldb/Symbol/UnwindPlan.h:225
+  // Not the first search.
+  m_value.ra_search.search_offset |= 1;
+}

What is this magic number/bit here? Is it ok to clobber bit zero here?



Comment at: lldb/include/lldb/Symbol/UnwindPlan.h:266
+  int32_t GetRaSearchOffset() const {
+return m_type == isRaSearch ? m_value.ra_search.search_offset & ~1 : 0;
+  }

Are we assuming "search_offset" must be aligned to at least a 4 bit boundary so 
that we can put something in bit zero?



Comment at: lldb/include/lldb/Symbol/UnwindPlan.h:270
+  bool IsFirstRaSearch() const {
+return m_value.ra_search.search_offset % 2 == 0;
+  }

You are using & and | in other locations, and now we are using modulo? I would 
just keep with the & operators



Comment at: lldb/include/lldb/Target/ABI.h:99
 public:
+  virtual bool CreateStackWalkingUnwindPlan(UnwindPlan &unwind_plan) = 0;
+

Add a default implementation that returns false so you don't have to change all 
of the classes that don't implement this.



Comment at: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h:30-33
+  bool
+  CreateStackWalkingUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override 
{
+return false;
+  }

Don't make the CreateStackWalkingUnwindPlan pure virtual if most of the 
implementations are just "return false;". I would make a default implementation 
in the base class that just returns false. 



Comment at: lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp:734
 
+bool ABIWindows_x86_64::CreateStackWalkingUnwindPlan(UnwindPlan &unwind_plan) {
+  unwind_plan.Clear();

What code actually does the search for a return address? Is that already 
available somewhere in the unwind plans? Reading through this plan I see that 
it sets the CFA to be RA search but I fail to see any searching going on.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1258
   std::vector params;
-  while (begin != end && param_count > 0) {
+  for (uint32_t i = 0; i < param_count && begin != end;) {
 uint32_t record_offset = begin.offset();

all changes in this file don't seem related to this patch? Revert?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124198

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


[Lldb-commits] [PATCH] D124198: [LLDB][Unwind] Add stack scanning as fallback unwind plan if no symbol file is available.

2022-04-21 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

If we do find what looks like a return address, is there any validation done on 
the instruction before the return address to see if it is a function call 
instruction? That would be the best way to validate that something on the stack 
just doesn't look like a return address and is actually just a random function 
pointer. I would like to see validation on anything that looks like a return 
address like:

- make sure it doesn't point to the first address of a function since if it 
actually is a return address is should be in the middle of a function
- make sure the previous instruction was a function call to the function the 
current function from which we are unwinding if we have function bounds for the 
current frame
- if we don't have function bounds for the current function, we can benefit 
from knowing the function start address in the current frame if we can find the 
call instruction since that will help us parse the correct function prologue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124198

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


[Lldb-commits] [PATCH] D124198: [LLDB][Unwind] Add stack scanning as fallback unwind plan if no symbol file is available.

2022-04-21 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

This is obviously building on Pavel's work of https://reviews.llvm.org/D66638 
for Win32 systems.  What problem with that existing patchset is this 
addressing?  Pavel's original patch assumes that we can retrieve a function's 
expected stack frame size, and the size of the caller's arguments on the stack, 
plus a bit of searching around that location, as he describes.  It seems like 
in ABIWindows_x86_64::CreateStackWalkingUnwindPlan you're adding a scheme that 
tries to look at the area directly around $sp for a return pc?  This seems 
quite fragile by itself.

The idea of a stack walk based purely on looking at stack contents, outside of 
the Windows ABI, is interesting, but as Pavel mentions in his descriptions, you 
have to be careful to not get tricked by function pointers on the stack (for 
return-pc's) and pointers to stack memory (for spilled fp or sp values).  If we 
have an ABI that always writes fp+return-pc to stack at the start of the stack 
frame for non-leaf frames, we could maybe do something as a last resort, I 
haven't thought about it.

Greg's feedback is all solid.  I'm more curious about what higher level fix is 
being pursued with this patch.  Pavel is clearly the person who knows the most 
about this part of the unwinder.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124198

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


[Lldb-commits] [PATCH] D124198: [LLDB][Unwind] Add stack scanning as fallback unwind plan if no symbol file is available.

2022-04-21 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

I guess the other comment I would make is whether we should have a new ivar, 
instead of using the unused 0th bit of search_offset.

fwiw I wasn't real clear in my previous comment.  As you say in the title of 
this, you're trying to backtrace through a case where you have no symbol file, 
and I'm guessing the Windows ABI doesn't use a frame pointer register (rbp) 
that is is spilled to stack at the start of a stack frame by convention.  The 
ArchitecturalDefaultUnwindPlan's that we use on ABIs where you see saved-pc 
followed by saved-fp can do a stack walk following the chain of those two 
spilled registers pretty reliably, once they're off of the leaf frame.  This 
isn't the case on Windows right. You have nothing but a saved return address, 
and the stack pointer is not spilled unless there's some kind of variable 
length stack frame I'm guessing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124198

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


[Lldb-commits] [PATCH] D124198: [LLDB][Unwind] Add stack scanning as fallback unwind plan if no symbol file is available.

2022-04-21 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu planned changes to this revision.
zequanwu added a comment.

Thanks for the feedback. As you pointed out that stack scanning without symbol 
file info is fragile due to function pointers in stack and we don't know the 
parameter size pushed into the stack.

Originally, I want lldb to do continue unwinding when it encounters some frames 
are in modules that don't have symbol info and no rbp value, since we have some 
crash reports with such problem. But later I found that we actually do have the 
symbol files for those modules. It seems not necessary anymore...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124198

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


[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

2022-04-21 Thread jeffrey tan via Phabricator via lldb-commits
yinghuitan updated this revision to Diff 424360.
yinghuitan added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

Files:
  lldb/docs/use/ondemand.rst
  lldb/include/lldb/Core/ModuleList.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/include/lldb/Target/Statistics.h
  lldb/include/lldb/Utility/LLDBLog.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  lldb/source/Symbol/CMakeLists.txt
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/SymbolFile.cpp
  lldb/source/Symbol/SymbolFileOnDemand.cpp
  lldb/source/Target/Statistics.cpp
  lldb/source/Utility/LLDBLog.cpp
  lldb/test/API/symbol_ondemand/breakpoint_language/Makefile
  
lldb/test/API/symbol_ondemand/breakpoint_language/TestBreakpointLanguageOnDemand.py
  lldb/test/API/symbol_ondemand/breakpoint_language/c_lang.c
  lldb/test/API/symbol_ondemand/breakpoint_language/cpp_lang.cpp
  lldb/test/API/symbol_ondemand/breakpoint_language/main.cpp
  lldb/test/API/symbol_ondemand/breakpoint_source_regex/Makefile
  
lldb/test/API/symbol_ondemand/breakpoint_source_regex/TestSourceTextRegexBreakpoint.py
  lldb/test/API/symbol_ondemand/breakpoint_source_regex/main.cpp
  lldb/test/API/symbol_ondemand/shared_library/Makefile
  lldb/test/API/symbol_ondemand/shared_library/TestSharedLib.py
  lldb/test/API/symbol_ondemand/shared_library/foo.c
  lldb/test/API/symbol_ondemand/shared_library/foo.h
  lldb/test/API/symbol_ondemand/shared_library/shared.c
  lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
  lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
  lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test

Index: lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/symbolic-breakpoint.test
@@ -0,0 +1,24 @@
+
+# Test shows that symbolic function breakpoint works with LLDB on demand symbol loading.
+
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: %build %p/Inputs/basic.cpp -o basic.out
+# RUN: %lldb -b -O "settings set symbols.load-on-demand true" -s %s basic.out | FileCheck %s
+
+breakpoint list
+# CHECK: No breakpoints currently set
+
+b bar
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+breakpoint list
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+run
+# CHECK: stop reason = breakpoint
+
+bt
+# CHECK: {{.*}}`bar(x=33, y=78) at basic.cpp:1
+# CHECK: {{.*}}`foo(x=33, y=78) at basic.cpp:3
+# CHECK: {{.*}}`main(argc=1, argv={{.*}}) at basic.cpp:5
Index: lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/source-breakpoint.test
@@ -0,0 +1,23 @@
+# Test shows that source line breakpoint works with LLDB on demand symbol loading.
+
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: %build %p/Inputs/basic.cpp -o basic.out
+# RUN: %lldb -b -O "settings set symbols.load-on-demand true" -s %s basic.out | FileCheck %s
+
+breakpoint list
+# CHECK: No breakpoints currently set
+
+breakpoint set -f basic.cpp -l 1
+# CHECK: where = {{.*}}`bar(int, int) + {{.*}} at basic.cpp:1
+
+breakpoint list
+# CHECK: file = 'basic.cpp'
+
+run
+# CHECK: stop reason = breakpoint
+
+bt
+# CHECK: {{.*}}`bar(x=33, y=78) at basic.cpp:1
+# CHECK: {{.*}}`foo(x=33, y=78) at basic.cpp:3
+# CHECK: {{.*}}`main(argc=1, argv={{.*}}) at basic.cpp:5
Index: lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/OnDemand/Inputs/basic.cpp
@@ -0,0 +1,5 @@
+int bar(int x, int y) { return x + y + 5; }
+
+int foo(int x, int y) { return bar(x, y) + 12; }
+
+int main(int argc, char **argv) { return foo(33, 78); }
Index: lldb/test/API/symbol_ondemand/shared_library/shared.c
===
--- /dev/null
+++ lldb/test/API/symbol_ondemand/shared_library/shared.c
@@ -0,0 +1,9 @@
+#include "foo.h"
+#include 
+
+int global_shared = 897;
+int main(void) {
+  puts("This is a shared library test...");
+  foo(); // Set breakpoint 0 here.
+  return 0;
+}
Index: lldb/test/API/symbol_ondemand/shared_library/foo.h
===
--- /dev/null
+++ lldb/test/API/symbol_ondemand/shared_library/foo.h
@@ -0,0 +1,6 @@
+#ifndef foo_h__
+#define foo_h__
+
+extern void foo(void);
+
+#endif // foo_h__
Index: lldb/test/API/symbol_ondemand/shared_library/foo.c
=