[Lldb-commits] [lldb] 92b475f - [lldb] silence -Wsometimes-uninitialized warnings

[Krasimir Georgiev via lldb-commits]

Author: Krasimir Georgiev
Date: 2021-09-27T09:35:58+02:00
New Revision: 92b475f0b079e125c588b121dc50116ea5d6d9f2

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

LOG: [lldb] silence -Wsometimes-uninitialized warnings

No functional changes intended.

Silence warnings from
https://github.com/llvm/llvm-project/commit/3a6ba3675177cb5e47dee325f300aced4cd864ed.

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp 
b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
index a3a9d963c2213..f6a2cdf309815 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -141,8 +141,8 @@ DynamicRegisterInfo::SetRegisterInfo(const 
StructuredData::Dictionary &dict,
   std::string reg_name_str = matches[1].str();
   std::string msbit_str = matches[2].str();
   std::string lsbit_str = matches[3].str();
-  uint32_t msbit;
-  uint32_t lsbit;
+  uint32_t msbit = 0;
+  uint32_t lsbit = 0;
   if (llvm::to_integer(msbit_str, msbit) &&
   llvm::to_integer(lsbit_str, lsbit)) {
 if (msbit > lsbit) {



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

[Lldb-commits] [PATCH] D110496: [llvm] [ADT] Add a range/iterator-based Split()

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 375162.
mgorny retitled this revision from "[llvm] [ADT] Add a range/iterator-based 
split()" to "[llvm] [ADT] Add a range/iterator-based Split()".
mgorny edited the summary of this revision.
mgorny added a comment.

Attempt to fix linter issues.


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

https://reviews.llvm.org/D110496

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  llvm/include/llvm/ADT/StringExtras.h
  llvm/unittests/ADT/StringExtrasTest.cpp

Index: llvm/unittests/ADT/StringExtrasTest.cpp
===
--- llvm/unittests/ADT/StringExtrasTest.cpp
+++ llvm/unittests/ADT/StringExtrasTest.cpp
@@ -274,3 +274,35 @@
   EXPECT_EQ(toString(APSInt(APInt(8, 255), isUnsigned), 10), "-1");
   EXPECT_EQ(toString(APSInt(APInt(8, 255), isUnsigned), 16), "-1");
 }
+
+TEST(StringExtrasTest, splitStringRef) {
+  auto Spl = Split("foo<=>bar<=><=>baz", "<=>");
+  auto It = Spl.begin();
+  auto End = Spl.end();
+
+  ASSERT_NE(It, End);
+  EXPECT_EQ(*It, StringRef("foo"));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef("bar"));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef(""));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef("baz"));
+  ASSERT_EQ(++It, End);
+}
+
+TEST(StringExtrasTest, splItChar) {
+  auto Spl = Split("foo,bar,,baz", ',');
+  auto It = Spl.begin();
+  auto End = Spl.end();
+
+  ASSERT_NE(It, End);
+  EXPECT_EQ(*It, StringRef("foo"));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef("bar"));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef(""));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef("baz"));
+  ASSERT_EQ(++It, End);
+}
Index: llvm/include/llvm/ADT/StringExtras.h
===
--- llvm/include/llvm/ADT/StringExtras.h
+++ llvm/include/llvm/ADT/StringExtras.h
@@ -501,6 +501,62 @@
   }
 };
 
+/// A forward iterator over partitions of string over a separator.
+class SplittingIterator
+: public iterator_facade_base {
+  StringRef Current;
+  StringRef Next;
+  StringRef Separator;
+
+public:
+  SplittingIterator(StringRef Str, StringRef Separator)
+  : Next(Str), Separator(Separator) {
+++*this;
+  }
+
+  bool operator==(const SplittingIterator &R) const {
+return Current == R.Current && Next == R.Next && Separator == R.Separator;
+  }
+
+  const StringRef &operator*() const { return Current; }
+
+  StringRef &operator*() { return Current; }
+
+  SplittingIterator &operator++() {
+std::pair Res = Next.split(Separator);
+Current = Res.first;
+Next = Res.second;
+return *this;
+  }
+};
+
+/// Split the specified string over a separator and return a range-compatible
+/// iterable over its partitions.  Used to permit conveniently iterating
+/// over separated strings like so:
+///
+/// \code
+///   for (StringRef x : llvm::Split("foo,bar,baz", ','))
+/// ...;
+/// \end
+///
+/// Note that the passed string must remain valid throuhgout lifetime
+/// of the iterators.
+class Split {
+  StringRef Str;
+  std::string SeparatorStr;
+
+public:
+  Split(StringRef NewStr, StringRef Separator)
+  : Str(NewStr), SeparatorStr(Separator) {}
+  Split(StringRef NewStr, char Separator)
+  : Str(NewStr), SeparatorStr(1, Separator) {}
+
+  SplittingIterator begin() { return SplittingIterator(Str, SeparatorStr); }
+
+  SplittingIterator end() { return SplittingIterator("", SeparatorStr); }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_ADT_STRINGEXTRAS_H
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
@@ -383,9 +383,7 @@
 const llvm::StringRef &comma_separated_register_numbers,
 std::vector ®nums, int base) {
   regnums.clear();
-  llvm::SmallVector split_string;
-  comma_separated_register_numbers.split(split_string, ',');
-  for (llvm::StringRef x : split_string) {
+  for (llvm::StringRef x : llvm::Split(comma_separated_register_numbers, ',')) {
 uint32_t reg;
 if (llvm::to_integer(x, reg, base))
   regnums.push_back(reg);
@@ -1457,9 +1455,7 @@
 size_t ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(
 llvm::StringRef value) {
   m_thread_pcs.clear();
-  llvm::SmallVector split_value;
-  value.split(split_value, ',');
-  for (llvm::StringRef x : split_value) {
+  for (llvm::StringRef x : llvm::Split(value, ',')) {
 lldb::addr_t pc;
 if (llvm::to_integer(x, pc, 16))
   m_thread_pcs.push_back(pc);
@@ -5111,9 +5107,7 @@
 std::string path;
 
 // process the response
-llvm::SmallVector reply_data;
-response.GetStringRef().split(reply_data, 

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Siger Young via Phabricator via lldb-commits]

siger-young added inline comments.



Comment at: lldb/test/API/lua_api/lua_lldb_test.lua:3
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+

tammela wrote:
> Could we not use an external dependency?
> For instance in my setup it fails because it couldn't find this library.
Does it make sense to directly copy "luaunit.lua" to the Lua test dir?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

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

[Lldb-commits] [lldb] f4b71e3 - [llvm] [ADT] Add a range/iterator-based Split()

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-27T10:43:09+02:00
New Revision: f4b71e3479bfaec71ba5f4bb56c6a34357a7f938

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

LOG: [llvm] [ADT] Add a range/iterator-based Split()

Add a llvm::Split() implementation that can be used via range-for loop,
e.g.:

for (StringRef x : llvm::Split("foo,bar,baz", ','))
  ...

The implementation uses an additional SplittingIterator class that
uses StringRef::split() internally.

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
llvm/include/llvm/ADT/StringExtras.h
llvm/unittests/ADT/StringExtrasTest.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 6d2a267f294c..805a836942d6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -355,10 +355,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
 // configuration of the transport before attaching/launching the process.
 m_qSupported_response = response.GetStringRef().str();
 
-llvm::SmallVector server_features;
-response.GetStringRef().split(server_features, ';');
-
-for (llvm::StringRef x : server_features) {
+for (llvm::StringRef x : llvm::Split(response.GetStringRef(), ';')) {
   if (x == "qXfer:auxv:read+")
 m_supports_qXfer_auxv_read = eLazyBoolYes;
   else if (x == "qXfer:libraries-svr4:read+")
@@ -1659,10 +1656,8 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo(
   error_extractor.GetHexByteString(error_string);
   error.SetErrorString(error_string.c_str());
 } else if (name.equals("dirty-pages")) {
-  llvm::SmallVector split_value;
   std::vector dirty_page_list;
-  value.split(split_value, ',');
-  for (llvm::StringRef x : split_value) {
+  for (llvm::StringRef x : llvm::Split(value, ',')) {
 addr_t page;
 x.consume_front("0x");
 if (llvm::to_integer(x, page, 16))

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 1ad838b51c26..cd8537812eff 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3656,10 +3656,7 @@ GDBRemoteCommunicationServerLLGS::Handle_qSaveCore(
   StringRef packet_str{packet.GetStringRef()};
   assert(packet_str.startswith("qSaveCore"));
   if (packet_str.consume_front("qSaveCore;")) {
-llvm::SmallVector fields;
-packet_str.split(fields, ';');
-
-for (auto x : fields) {
+for (auto x : llvm::Split(packet_str, ';')) {
   if (x.consume_front("path-hint:"))
 StringExtractor(x).GetHexByteString(path_hint);
   else

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 2c1b4fa319ff..fdedb9b83416 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -383,9 +383,7 @@ static size_t SplitCommaSeparatedRegisterNumberString(
 const llvm::StringRef &comma_separated_register_numbers,
 std::vector ®nums, int base) {
   regnums.clear();
-  llvm::SmallVector split_string;
-  comma_separated_register_numbers.split(split_string, ',');
-  for (llvm::StringRef x : split_string) {
+  for (llvm::StringRef x : llvm::Split(comma_separated_register_numbers, ',')) 
{
 uint32_t reg;
 if (llvm::to_integer(x, reg, base))
   regnums.push_back(reg);
@@ -1457,9 +1455,7 @@ size_t 
ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(
 size_t ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(
 llvm::StringRef value) {
   m_thread_pcs.clear();
-  llvm::SmallVector split_value;
-  value.split(split_value, ',');
-  for (llvm::StringRef x : split_value) {
+  for (llvm::StringRef x : llvm::Split(value, ',')) {
 lldb::addr_t pc;
 if (llvm::to_integer(x, pc, 16))
   m_thread_pcs.push_back(pc);
@@ -5111,9 +5107,7 @@ llvm::Expected 
ProcessGDBRemote::SaveCore(llvm::StringRef outfile) {
 std::string path;
 
 // process the response
-llvm::SmallVector reply_data;
-response.GetStringRef().split(reply_data,

[Lldb-commits] [PATCH] D110496: [llvm] [ADT] Add a range/iterator-based Split()

[Michał Górny 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 rGf4b71e3479bf: [llvm] [ADT] Add a range/iterator-based 
Split() (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110496

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  llvm/include/llvm/ADT/StringExtras.h
  llvm/unittests/ADT/StringExtrasTest.cpp

Index: llvm/unittests/ADT/StringExtrasTest.cpp
===
--- llvm/unittests/ADT/StringExtrasTest.cpp
+++ llvm/unittests/ADT/StringExtrasTest.cpp
@@ -274,3 +274,35 @@
   EXPECT_EQ(toString(APSInt(APInt(8, 255), isUnsigned), 10), "-1");
   EXPECT_EQ(toString(APSInt(APInt(8, 255), isUnsigned), 16), "-1");
 }
+
+TEST(StringExtrasTest, splitStringRef) {
+  auto Spl = Split("foo<=>bar<=><=>baz", "<=>");
+  auto It = Spl.begin();
+  auto End = Spl.end();
+
+  ASSERT_NE(It, End);
+  EXPECT_EQ(*It, StringRef("foo"));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef("bar"));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef(""));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef("baz"));
+  ASSERT_EQ(++It, End);
+}
+
+TEST(StringExtrasTest, splItChar) {
+  auto Spl = Split("foo,bar,,baz", ',');
+  auto It = Spl.begin();
+  auto End = Spl.end();
+
+  ASSERT_NE(It, End);
+  EXPECT_EQ(*It, StringRef("foo"));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef("bar"));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef(""));
+  ASSERT_NE(++It, End);
+  EXPECT_EQ(*It, StringRef("baz"));
+  ASSERT_EQ(++It, End);
+}
Index: llvm/include/llvm/ADT/StringExtras.h
===
--- llvm/include/llvm/ADT/StringExtras.h
+++ llvm/include/llvm/ADT/StringExtras.h
@@ -501,6 +501,62 @@
   }
 };
 
+/// A forward iterator over partitions of string over a separator.
+class SplittingIterator
+: public iterator_facade_base {
+  StringRef Current;
+  StringRef Next;
+  StringRef Separator;
+
+public:
+  SplittingIterator(StringRef Str, StringRef Separator)
+  : Next(Str), Separator(Separator) {
+++*this;
+  }
+
+  bool operator==(const SplittingIterator &R) const {
+return Current == R.Current && Next == R.Next && Separator == R.Separator;
+  }
+
+  const StringRef &operator*() const { return Current; }
+
+  StringRef &operator*() { return Current; }
+
+  SplittingIterator &operator++() {
+std::pair Res = Next.split(Separator);
+Current = Res.first;
+Next = Res.second;
+return *this;
+  }
+};
+
+/// Split the specified string over a separator and return a range-compatible
+/// iterable over its partitions.  Used to permit conveniently iterating
+/// over separated strings like so:
+///
+/// \code
+///   for (StringRef x : llvm::Split("foo,bar,baz", ','))
+/// ...;
+/// \end
+///
+/// Note that the passed string must remain valid throuhgout lifetime
+/// of the iterators.
+class Split {
+  StringRef Str;
+  std::string SeparatorStr;
+
+public:
+  Split(StringRef NewStr, StringRef Separator)
+  : Str(NewStr), SeparatorStr(Separator) {}
+  Split(StringRef NewStr, char Separator)
+  : Str(NewStr), SeparatorStr(1, Separator) {}
+
+  SplittingIterator begin() { return SplittingIterator(Str, SeparatorStr); }
+
+  SplittingIterator end() { return SplittingIterator("", SeparatorStr); }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_ADT_STRINGEXTRAS_H
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
@@ -383,9 +383,7 @@
 const llvm::StringRef &comma_separated_register_numbers,
 std::vector ®nums, int base) {
   regnums.clear();
-  llvm::SmallVector split_string;
-  comma_separated_register_numbers.split(split_string, ',');
-  for (llvm::StringRef x : split_string) {
+  for (llvm::StringRef x : llvm::Split(comma_separated_register_numbers, ',')) {
 uint32_t reg;
 if (llvm::to_integer(x, reg, base))
   regnums.push_back(reg);
@@ -1457,9 +1455,7 @@
 size_t ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(
 llvm::StringRef value) {
   m_thread_pcs.clear();
-  llvm::SmallVector split_value;
-  value.split(split_value, ',');
-  for (llvm::StringRef x : split_value) {
+  for (llvm::StringRef x : llvm::Split(value, ',')) {
 lldb::addr_t pc;
 if (llvm::to_integer(x, pc, 16))
   m_thread_pcs.push_back(pc);
@@ -5111,9 +5107,7 @@
 std::string path;
 
 // process the response
-llvm::SmallVector reply_data;
-response.GetStringRef().

[Lldb-commits] [PATCH] D110496: [llvm] [ADT] Add a range/iterator-based Split()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like it as well :)




Comment at: llvm/include/llvm/ADT/StringExtras.h:519
+  bool operator==(const SplittingIterator &R) const {
+return Current == R.Current && Next == R.Next && Separator == R.Separator;
+  }

This compares the contents of the StringRefs, while it should be comparing the 
pointers instead. I don't think it can cause correctness issues though I think 
it might be able to cause super-linear iteration complexity for some 
pathological inputs and more complex algorithms (a string consisting of many 
identical substrings, and an algorithm which compares two successive iterators).

Since comparing iterators from different sequences is UB, I think that 
comparing just `Current.data()` could actually suffice (the rest could be 
asserts) -- one has to be careful how he initializes the past-the-end iterator 
though.



Comment at: llvm/include/llvm/ADT/StringExtras.h:527-529
+std::pair Res = Next.split(Separator);
+Current = Res.first;
+Next = Res.second;

`std::tie(Current, Next) = ...`



Comment at: llvm/include/llvm/ADT/StringExtras.h:545-558
+class Split {
+  StringRef Str;
+  std::string SeparatorStr;
+
+public:
+  Split(StringRef NewStr, StringRef Separator)
+  : Str(NewStr), SeparatorStr(Separator) {}

Could this be a function that returns `iterator_range` ?
(I suppose lifetimes could be an issue, but it's not clear to me if that's the 
case here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110496

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

[Lldb-commits] [PATCH] D110510: [lldb] Remove "0x" prefix from hex values in dirty-pages

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: jasonmolenda.
labath added a comment.

Jason, I'm not sure how this came to be, but I think it's more common to not 
send the 0x for hex numbers.


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

https://reviews.llvm.org/D110510

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

[Lldb-commits] [PATCH] D110410: [lldb] [Host] Refactor XML converting getters

[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/source/Host/common/XML.cpp:299-304
+  std::string text;
+  if (GetElementText(text) && llvm::to_integer(text,value, base))
+return true;
+
+  value = fail_value;
+  return false;

/me wonders if it would be too weird to rely on the fact that to_integer does 
not modify the result variable on failure.

It would definitely streamline this code:
```
value = fail_value;
return GetElementText(text) && llvm::to_integer(text,value, base);
```


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

https://reviews.llvm.org/D110410

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

[Lldb-commits] [PATCH] D110410: [lldb] [Host] Refactor XML converting getters

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: lldb/source/Host/common/XML.cpp:299-304
+  std::string text;
+  if (GetElementText(text) && llvm::to_integer(text,value, base))
+return true;
+
+  value = fail_value;
+  return false;

labath wrote:
> /me wonders if it would be too weird to rely on the fact that to_integer does 
> not modify the result variable on failure.
> 
> It would definitely streamline this code:
> ```
> value = fail_value;
> return GetElementText(text) && llvm::to_integer(text,value, base);
> ```
Other code in LLDB already does that, so I suppose it's fine.


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

https://reviews.llvm.org/D110410

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

[Lldb-commits] [PATCH] D110023: [lldb] [DynamicRegisterInfo] Add a convenience method to add suppl. registers

[Pavel Labath via Phabricator via lldb-commits]

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

I think this is fine. The tricky thing will be deciding what to do with the x86 
and arm registers which start at non-zero offsets (`ah` et al.)

One thing I was considering was doing this addition while were still in the 
original "vector of strings" form instead of this C thingy. The tricky part 
there is that (since this in would be the ABI classes which do this 
manipulation) we would need to expose the gdb-remote struct to the outside 
world. I don't think that would be necessarily bad (we just wouldn't call it 
"RemoteRegisterInfo, but something else), but it's not clear to me whether its 
worth the churn. Still, I think it's worth keeping this in the back of your 
mind as you work on this.




Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:441
+
+  reg_to_regs_map to_add;
+  for (uint32_t value_reg : m_value_regs_map[reg_num]) {

Maybe call this new_invalidates? I've found it hard to track what to_add means, 
with all the mixing of value_regs and invalidates...



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:443-444
+  for (uint32_t value_reg : m_value_regs_map[reg_num]) {
+if (value_reg == LLDB_INVALID_REGNUM)
+  break;
+

Is this still needed?



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:457
+
+  for (reg_to_regs_map::value_type &x : to_add)
+llvm::append_range(m_invalidate_regs_map[x.first], x.second);

This would be a good use case for `(const) auto`, as `value_type` does not say 
much anyway.



Comment at: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp:184-190
+  struct RegisterInfo ah_reg {
+"ah", nullptr, 1, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, ah, 
ah},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(ah_reg, group);

Could we remove ah from this test, as its offset is going to be wrong?


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

https://reviews.llvm.org/D110023

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

[Lldb-commits] [lldb] d5629b5 - Fix rendezvous for rebase_exec=true case

[Pavel Labath via lldb-commits]

Author: Emre Kultursay
Date: 2021-09-27T13:27:27+02:00
New Revision: d5629b5d4d41ce71703105362f58dfcdbb6cc175

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

LOG: Fix rendezvous for rebase_exec=true case

When rebase_exec=true in DidAttach(), all modules are loaded
before the rendezvous breakpoint is set, which means the
LoadInterpreterModule() method is not called and m_interpreter_module
is not initialized.

This causes the very first rendezvous breakpoint hit with
m_initial_modules_added=false to accidentally unload the
module_sp that corresponds to the dynamic loader.

This bug (introduced in D92187) was causing the rendezvous
mechanism to not work in Android 28. The mechanism works
fine on older/newer versions of Android.

Test: Verified rendezvous on Android 28 and 29
Test: Added dlopen test

Reviewed By: labath

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

Added: 
lldb/test/API/functionalities/load_after_attach/Makefile
lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
lldb/test/API/functionalities/load_after_attach/b.cpp
lldb/test/API/functionalities/load_after_attach/main.cpp

Modified: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 0bb383ff0c00..7732f27d27ca 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -449,14 +449,18 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
 if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
 &m_process->GetTarget()) == m_interpreter_base &&
 module_sp != m_interpreter_module.lock()) {
-  // If this is a duplicate instance of ld.so, unload it.  We may end 
up
-  // with it if we load it via a 
diff erent path than before (symlink
-  // vs real path).
-  // TODO: remove this once we either fix library matching or avoid
-  // loading the interpreter when setting the rendezvous breakpoint.
-  UnloadSections(module_sp);
-  loaded_modules.Remove(module_sp);
-  continue;
+  if (m_interpreter_module.lock() == nullptr) {
+m_interpreter_module = module_sp;
+  } else {
+// If this is a duplicate instance of ld.so, unload it.  We may end
+// up with it if we load it via a 
diff erent path than before
+// (symlink vs real path).
+// TODO: remove this once we either fix library matching or avoid
+// loading the interpreter when setting the rendezvous breakpoint.
+UnloadSections(module_sp);
+loaded_modules.Remove(module_sp);
+continue;
+  }
 }
 
 loaded_modules.AppendIfNeeded(module_sp);
@@ -627,6 +631,7 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
   }
 
   m_process->GetTarget().ModulesDidLoad(module_list);
+  m_initial_modules_added = true;
 }
 
 addr_t DynamicLoaderPOSIXDYLD::ComputeLoadOffset() {

diff  --git a/lldb/test/API/functionalities/load_after_attach/Makefile 
b/lldb/test/API/functionalities/load_after_attach/Makefile
new file mode 100644
index ..0f3fb37bdadf
--- /dev/null
+++ b/lldb/test/API/functionalities/load_after_attach/Makefile
@@ -0,0 +1,9 @@
+CXX_SOURCES := main.cpp
+USE_LIBDL := 1
+
+lib_b:
+   $(MAKE) -f $(MAKEFILE_RULES) \
+   DYLIB_ONLY=YES DYLIB_CXX_SOURCES=b.cpp DYLIB_NAME=lib_b
+all: lib_b
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py 
b/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
new file mode 100644
index ..0e9b3c40ff2b
--- /dev/null
+++ b/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
@@ -0,0 +1,63 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfRemote
+def test_load_after_attach(self):
+self.build()
+
+ctx = self.platformContext
+lib_name = ctx.shlib_prefix + 'lib_b.' + ctx.shlib_extension
+
+exe = self.getBuildArtifact("a.out")
+lib = self.getBuildArtifact(lib_name)
+
+# Spawn a new process.
+# use realpath to workaround llvm.org/pr48376
+# Pass path to solib for dlopen to properly locate the library.
+popen = self.spawnSubprocess(os.path.realpath(exe), args = 
[os.path.realpath(

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd5629b5d4d41: Fix rendezvous for rebase_exec=true case 
(authored by emrekultursay, committed by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/functionalities/load_after_attach/Makefile
  lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
  lldb/test/API/functionalities/load_after_attach/b.cpp
  lldb/test/API/functionalities/load_after_attach/main.cpp

Index: lldb/test/API/functionalities/load_after_attach/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/load_after_attach/main.cpp
@@ -0,0 +1,45 @@
+#ifdef _WIN32
+#include 
+#else
+#include 
+#include 
+#endif
+
+#include 
+#include 
+#include 
+#include 
+
+// We do not use the dylib.h implementation, because
+// we need to pass full path to the dylib.
+void* dylib_open(const char* full_path) {
+#ifdef _WIN32
+  return LoadLibraryA(full_path);
+#else
+  return dlopen(full_path, RTLD_LAZY);
+#endif
+}
+
+int main(int argc, char* argv[]) {
+  assert(argc == 2 && "argv[1] must be the full path to lib_b library");
+  const char* dylib_full_path= argv[1];
+  printf("Using dylib at: %s\n", dylib_full_path);
+
+  // Wait until debugger is attached.
+  int main_thread_continue = 0;
+  int i = 0;
+  int timeout = 10;
+  for (i = 0; i < timeout; i++) {
+std::this_thread::sleep_for(std::chrono::seconds(1));  // break here
+if (main_thread_continue) {
+  break;
+}
+  }
+  assert(i != timeout && "timed out waiting for debugger");
+
+  // dlopen the 'liblib_b.so' shared library.
+  void* dylib_handle = dylib_open(dylib_full_path);
+  assert(dylib_handle && "dlopen failed");
+
+  return i; // break after dlopen
+}
Index: lldb/test/API/functionalities/load_after_attach/b.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/load_after_attach/b.cpp
@@ -0,0 +1 @@
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
Index: lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
===
--- /dev/null
+++ lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
@@ -0,0 +1,63 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfRemote
+def test_load_after_attach(self):
+self.build()
+
+ctx = self.platformContext
+lib_name = ctx.shlib_prefix + 'lib_b.' + ctx.shlib_extension
+
+exe = self.getBuildArtifact("a.out")
+lib = self.getBuildArtifact(lib_name)
+
+# Spawn a new process.
+# use realpath to workaround llvm.org/pr48376
+# Pass path to solib for dlopen to properly locate the library.
+popen = self.spawnSubprocess(os.path.realpath(exe), args = [os.path.realpath(lib)])
+pid = popen.pid
+
+# Attach to the spawned process.
+self.runCmd("process attach -p " + str(pid))
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Continue until first breakpoint.
+breakpoint1 = self.target().BreakpointCreateBySourceRegex(
+"// break here", lldb.SBFileSpec("main.cpp"))
+self.assertEqual(breakpoint1.GetNumResolvedLocations(), 1)
+stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint1)
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that image list does not contain liblib_b before dlopen.
+self.match(
+"image list",
+patterns = [lib_name],
+matching = False,
+msg = lib_name + " should not have been in image list")
+
+# Change a variable to escape the loop
+self.runCmd("expression main_thread_continue = 1")
+
+# Continue so that dlopen is called.
+breakpoint2 = self.target().BreakpointCreateBySourceRegex(
+"// break after dlopen", lldb.SBFileSpec("main.cpp"))
+self.assertEqual(breakpoint2.GetNumResolvedLocations(), 1)
+stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint2)
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that image list contains liblib_b after dlopen.
+self.match(
+"image list",
+patterns = [lib_name],
+matching = True,
+msg = lib_name + " missing in image list")
+
Index: lldb/test/API/functionalities/load_after_attach/Makefile
=

[Lldb-commits] [PATCH] D110535: [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, lattner.
Herald added a subscriber: dexonsmith.
mgorny requested review of this revision.
Herald added a project: LLVM.

Optimize the iterator comparison logic to compare Current.data()
pointers.  Use std::tie for assignments from std::pair.  Remove the char
Separator overload and replace the custom class with a function
returning iterator_range.


https://reviews.llvm.org/D110535

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  llvm/include/llvm/ADT/StringExtras.h
  llvm/unittests/ADT/StringExtrasTest.cpp

Index: llvm/unittests/ADT/StringExtrasTest.cpp
===
--- llvm/unittests/ADT/StringExtrasTest.cpp
+++ llvm/unittests/ADT/StringExtrasTest.cpp
@@ -275,7 +275,7 @@
   EXPECT_EQ(toString(APSInt(APInt(8, 255), isUnsigned), 16), "-1");
 }
 
-TEST(StringExtrasTest, splitStringRef) {
+TEST(StringExtrasTest, SplitStringRef) {
   auto Spl = Split("foo<=>bar<=><=>baz", "<=>");
   auto It = Spl.begin();
   auto End = Spl.end();
@@ -290,19 +290,3 @@
   EXPECT_EQ(*It, StringRef("baz"));
   ASSERT_EQ(++It, End);
 }
-
-TEST(StringExtrasTest, splItChar) {
-  auto Spl = Split("foo,bar,,baz", ',');
-  auto It = Spl.begin();
-  auto End = Spl.end();
-
-  ASSERT_NE(It, End);
-  EXPECT_EQ(*It, StringRef("foo"));
-  ASSERT_NE(++It, End);
-  EXPECT_EQ(*It, StringRef("bar"));
-  ASSERT_NE(++It, End);
-  EXPECT_EQ(*It, StringRef(""));
-  ASSERT_NE(++It, End);
-  EXPECT_EQ(*It, StringRef("baz"));
-  ASSERT_EQ(++It, End);
-}
Index: llvm/include/llvm/ADT/StringExtras.h
===
--- llvm/include/llvm/ADT/StringExtras.h
+++ llvm/include/llvm/ADT/StringExtras.h
@@ -516,7 +516,8 @@
   }
 
   bool operator==(const SplittingIterator &R) const {
-return Current == R.Current && Next == R.Next && Separator == R.Separator;
+assert(Separator == R.Separator);
+return Current.data() == R.Current.data();
   }
 
   const StringRef &operator*() const { return Current; }
@@ -524,9 +525,7 @@
   StringRef &operator*() { return Current; }
 
   SplittingIterator &operator++() {
-std::pair Res = Next.split(Separator);
-Current = Res.first;
-Next = Res.second;
+std::tie(Current, Next) = Next.split(Separator);
 return *this;
   }
 };
@@ -536,26 +535,16 @@
 /// over separated strings like so:
 ///
 /// \code
-///   for (StringRef x : llvm::Split("foo,bar,baz", ','))
+///   for (StringRef x : llvm::Split("foo,bar,baz", ","))
 /// ...;
 /// \end
 ///
 /// Note that the passed string must remain valid throuhgout lifetime
 /// of the iterators.
-class Split {
-  StringRef Str;
-  std::string SeparatorStr;
-
-public:
-  Split(StringRef NewStr, StringRef Separator)
-  : Str(NewStr), SeparatorStr(Separator) {}
-  Split(StringRef NewStr, char Separator)
-  : Str(NewStr), SeparatorStr(1, Separator) {}
-
-  SplittingIterator begin() { return SplittingIterator(Str, SeparatorStr); }
-
-  SplittingIterator end() { return SplittingIterator("", SeparatorStr); }
-};
+inline iterator_range Split(StringRef Str, StringRef Separator) {
+  return {SplittingIterator(Str, Separator),
+  SplittingIterator(StringRef(), Separator)};
+}
 
 } // end namespace llvm
 
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
@@ -383,7 +383,7 @@
 const llvm::StringRef &comma_separated_register_numbers,
 std::vector ®nums, int base) {
   regnums.clear();
-  for (llvm::StringRef x : llvm::Split(comma_separated_register_numbers, ',')) {
+  for (llvm::StringRef x : llvm::Split(comma_separated_register_numbers, ",")) {
 uint32_t reg;
 if (llvm::to_integer(x, reg, base))
   regnums.push_back(reg);
@@ -1455,7 +1455,7 @@
 size_t ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(
 llvm::StringRef value) {
   m_thread_pcs.clear();
-  for (llvm::StringRef x : llvm::Split(value, ',')) {
+  for (llvm::StringRef x : llvm::Split(value, ",")) {
 lldb::addr_t pc;
 if (llvm::to_integer(x, pc, 16))
   m_thread_pcs.push_back(pc);
@@ -5107,7 +5107,7 @@
 std::string path;
 
 // process the response
-for (auto x : llvm::Split(response.GetStringRef(), ';')) {
+for (auto x : llvm::Split(response.GetStringRef(), ";")) {
   if (x.consume_front("core-path:"))
 StringExtractor(x).GetHexByteString(path);
 }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicat

[Lldb-commits] [PATCH] D110496: [llvm] [ADT] Add a range/iterator-based Split()

[Michał Górny via Phabricator via lldb-commits]

mgorny marked 3 inline comments as done.
mgorny added a comment.

Filed D110535  for suggested changes.




Comment at: llvm/include/llvm/ADT/StringExtras.h:527-529
+std::pair Res = Next.split(Separator);
+Current = Res.first;
+Next = Res.second;

labath wrote:
> `std::tie(Current, Next) = ...`
Damnit, I swear it didn't work before! Complained about `this` pointer 
something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110496

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

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Pedro Tammela via Phabricator via lldb-commits]

tammela added inline comments.



Comment at: lldb/test/API/lua_api/lua_lldb_test.lua:3
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+

siger-young wrote:
> tammela wrote:
> > Could we not use an external dependency?
> > For instance in my setup it fails because it couldn't find this library.
> Does it make sense to directly copy "luaunit.lua" to the Lua test dir?
You don't seem to have a hard dependency on it.
Couldn't you just replicate what you are interested? Instead of bringing in a 
full blown unit testing framework...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

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

[Lldb-commits] [PATCH] D110535: [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: llvm/include/llvm/ADT/StringExtras.h:544
 /// of the iterators.
-class Split {
-  StringRef Str;
-  std::string SeparatorStr;
-
-public:
-  Split(StringRef NewStr, StringRef Separator)
-  : Str(NewStr), SeparatorStr(Separator) {}
-  Split(StringRef NewStr, char Separator)
-  : Str(NewStr), SeparatorStr(1, Separator) {}
-
-  SplittingIterator begin() { return SplittingIterator(Str, SeparatorStr); }
-
-  SplittingIterator end() { return SplittingIterator("", SeparatorStr); }
-};
+inline iterator_range Split(StringRef Str, StringRef 
Separator) {
+  return {SplittingIterator(Str, Separator),

this should also be a lower-case `split` now that it's a function.

(Believe it or not, this is the reason I was first drawn to this -- it's 
uncommon to see a class used like this because, even in the cases where you do 
have a separate class, you usually create a function wrapper for it. The 
original reason for this is pragmatic (use of template argument deduction), but 
the practice is so ubiquitous that a deviation from it stands out.)


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

https://reviews.llvm.org/D110535

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

[Lldb-commits] [lldb] 93b82f4 - [lldb] [Host] Refactor XML converting getters

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-27T14:26:33+02:00
New Revision: 93b82f45bc3e58a526d2486841ea6d96b407807e

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

LOG: [lldb] [Host] Refactor XML converting getters

Refactor the XML converting attribute and text getters to use LLVM API.
While at it, remove some redundant error and missing XML support
handling, as the called base functions do that anyway.  Add tests
for these methods.

Note that this patch changes the getter behavior to be IMHO more
correct.  In particular:

- negative and overflowing integers are now reported as failures to
  convert, rather than being wrapped over or capped

- digits followed by text are now reported as failures to convert
  to double, rather than their numeric part being converted

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

Added: 
lldb/unittests/Host/XMLTest.cpp

Modified: 
lldb/source/Host/common/XML.cpp
lldb/unittests/Host/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/Host/common/XML.cpp b/lldb/source/Host/common/XML.cpp
index c3225d3f4433..7f6f74396d7a 100644
--- a/lldb/source/Host/common/XML.cpp
+++ b/lldb/source/Host/common/XML.cpp
@@ -6,10 +6,7 @@
 //
 
//===--===//
 
-#include  /* atof */
-
 #include "lldb/Host/Config.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Host/XML.h"
 
 using namespace lldb;
@@ -153,14 +150,8 @@ llvm::StringRef XMLNode::GetAttributeValue(const char 
*name,
 
 bool XMLNode::GetAttributeValueAsUnsigned(const char *name, uint64_t &value,
   uint64_t fail_value, int base) const 
{
-#if LLDB_ENABLE_LIBXML2
-  llvm::StringRef str_value = GetAttributeValue(name, "");
-#else
-  llvm::StringRef str_value;
-#endif
-  bool success = false;
-  value = StringConvert::ToUInt64(str_value.data(), fail_value, base, 
&success);
-  return success;
+  value = fail_value;
+  return llvm::to_integer(GetAttributeValue(name, ""), value, base);
 }
 
 void XMLNode::ForEachChildNode(NodeCallback const &callback) const {
@@ -302,33 +293,17 @@ bool XMLNode::GetElementText(std::string &text) const {
 
 bool XMLNode::GetElementTextAsUnsigned(uint64_t &value, uint64_t fail_value,
int base) const {
-  bool success = false;
-#if LLDB_ENABLE_LIBXML2
-  if (IsValid()) {
-std::string text;
-if (GetElementText(text))
-  value = StringConvert::ToUInt64(text.c_str(), fail_value, base, 
&success);
-  }
-#endif
-  if (!success)
-value = fail_value;
-  return success;
+  std::string text;
+
+  value = fail_value;
+  return GetElementText(text) && llvm::to_integer(text, value, base);
 }
 
 bool XMLNode::GetElementTextAsFloat(double &value, double fail_value) const {
-  bool success = false;
-#if LLDB_ENABLE_LIBXML2
-  if (IsValid()) {
-std::string text;
-if (GetElementText(text)) {
-  value = atof(text.c_str());
-  success = true;
-}
-  }
-#endif
-  if (!success)
-value = fail_value;
-  return success;
+  std::string text;
+
+  value = fail_value;
+  return GetElementText(text) && llvm::to_float(text, value);
 }
 
 bool XMLNode::NameIs(const char *name) const {

diff  --git a/lldb/unittests/Host/CMakeLists.txt 
b/lldb/unittests/Host/CMakeLists.txt
index 1cc0cb081e49..14c4fe4d0b8a 100644
--- a/lldb/unittests/Host/CMakeLists.txt
+++ b/lldb/unittests/Host/CMakeLists.txt
@@ -12,6 +12,7 @@ set (FILES
   SocketAddressTest.cpp
   SocketTest.cpp
   SocketTestUtilities.cpp
+  XMLTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")

diff  --git a/lldb/unittests/Host/XMLTest.cpp b/lldb/unittests/Host/XMLTest.cpp
new file mode 100644
index ..a8456128cbfc
--- /dev/null
+++ b/lldb/unittests/Host/XMLTest.cpp
@@ -0,0 +1,119 @@
+//===-- XMLTest.cpp 
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/XML.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+#if LLDB_ENABLE_LIBXML2
+
+static void assertGetElement(XMLNode &root, const char *element_name,
+ bool expected_uint_success, uint64_t 
expected_uint,
+ bool expected_double_success,
+ double expected_double) {
+  XMLNode node = root.FindFirstChildElementWithName(element_name);
+  ASSERT_TRUE(node.IsValid());
+
+  uint64_t uint_val;
+  EXPECT_EQ(node.GetElementTextAsUnsigned(uint_val, 66, 0),
+expected_

[Lldb-commits] [PATCH] D110410: [lldb] [Host] Refactor XML converting getters

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG93b82f45bc3e: [lldb] [Host] Refactor XML converting getters 
(authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D110410?vs=374829&id=375212#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110410

Files:
  lldb/source/Host/common/XML.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/XMLTest.cpp

Index: lldb/unittests/Host/XMLTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/XMLTest.cpp
@@ -0,0 +1,119 @@
+//===-- XMLTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/XML.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+#if LLDB_ENABLE_LIBXML2
+
+static void assertGetElement(XMLNode &root, const char *element_name,
+ bool expected_uint_success, uint64_t expected_uint,
+ bool expected_double_success,
+ double expected_double) {
+  XMLNode node = root.FindFirstChildElementWithName(element_name);
+  ASSERT_TRUE(node.IsValid());
+
+  uint64_t uint_val;
+  EXPECT_EQ(node.GetElementTextAsUnsigned(uint_val, 66, 0),
+expected_uint_success);
+  EXPECT_EQ(uint_val, expected_uint);
+
+  double double_val;
+  EXPECT_EQ(node.GetElementTextAsFloat(double_val, 66.0),
+expected_double_success);
+  EXPECT_EQ(double_val, expected_double);
+
+  XMLNode attr_node = root.FindFirstChildElementWithName("attr");
+  ASSERT_TRUE(node.IsValid());
+
+  EXPECT_EQ(
+  attr_node.GetAttributeValueAsUnsigned(element_name, uint_val, 66, 0),
+  expected_uint_success);
+  EXPECT_EQ(uint_val, expected_uint);
+}
+
+#define ASSERT_GET(element_name, ...)  \
+  {\
+SCOPED_TRACE("at element/attribute " element_name);\
+assertGetElement(root, element_name, __VA_ARGS__); \
+  }
+
+TEST(XML, GetAs) {
+  std::string test_xml =
+  "\n"
+  "\n"
+  "  \n"
+  "  123foo\n"
+  "  11\n"
+  "  -11\n"
+  "  18446744073709551616\n"
+  "  -9223372036854775809\n"
+  "  0x1234\n"
+  "  12.5\n"
+  "  -12.5\n"
+  "  \n"
+  "\n";
+
+  XMLDocument doc;
+  ASSERT_TRUE(doc.ParseMemory(test_xml.data(), test_xml.size()));
+
+  XMLNode root = doc.GetRootElement();
+  ASSERT_TRUE(root.IsValid());
+
+  ASSERT_GET("empty", false, 66, false, 66.0);
+  ASSERT_GET("text", false, 66, false, 66.0);
+  ASSERT_GET("positive-int", true, 11, true, 11.0);
+  ASSERT_GET("negative-int", false, 66, true, -11.0);
+  ASSERT_GET("positive-overflow", false, 66, true, 18446744073709551616.0);
+  ASSERT_GET("negative-overflow", false, 66, true, -9223372036854775809.0);
+  ASSERT_GET("hex", true, 0x1234, true, 4660.0);
+  ASSERT_GET("positive-float", false, 66, true, 12.5);
+  ASSERT_GET("negative-float", false, 66, true, -12.5);
+}
+
+#else // !LLDB_ENABLE_LIBXML2
+
+TEST(XML, GracefulNoXML) {
+  std::string test_xml =
+  "\n"
+  "\n"
+  "  123\n"
+  "\n";
+
+  XMLDocument doc;
+  ASSERT_FALSE(doc.ParseMemory(test_xml.data(), test_xml.size()));
+
+  XMLNode root = doc.GetRootElement();
+  EXPECT_FALSE(root.IsValid());
+
+  XMLNode node = root.FindFirstChildElementWithName("text");
+  EXPECT_FALSE(node.IsValid());
+
+  uint64_t uint_val;
+  EXPECT_FALSE(node.GetElementTextAsUnsigned(uint_val, 66, 0));
+  EXPECT_EQ(uint_val, 66U);
+  EXPECT_FALSE(node.GetAttributeValueAsUnsigned("attribute", uint_val, 66, 0));
+  EXPECT_EQ(uint_val, 66U);
+
+  double double_val;
+  EXPECT_FALSE(node.GetElementTextAsFloat(double_val, 66.0));
+  EXPECT_EQ(double_val, 66.0);
+}
+
+#endif // LLDB_ENABLE_LIBXML2
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -12,6 +12,7 @@
   SocketAddressTest.cpp
   SocketTest.cpp
   SocketTestUtilities.cpp
+  XMLTest.cpp
 )
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
Index: lldb/source/Host/common/XML.cpp
===
--- lldb/source/Host/common/XML.cpp
+++ lldb/source/Host/common/XML.cpp
@@ -6,10 +6,7 @@
 //
 //===--===//
 
-#include  /* atof */
-
 #include "lldb/Host/Config.h"
-#include "lldb/Host/StringConvert.h"

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Siger Young via Phabricator via lldb-commits]

siger-young added inline comments.



Comment at: lldb/test/API/lua_api/lua_lldb_test.lua:3
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+

tammela wrote:
> siger-young wrote:
> > tammela wrote:
> > > Could we not use an external dependency?
> > > For instance in my setup it fails because it couldn't find this library.
> > Does it make sense to directly copy "luaunit.lua" to the Lua test dir?
> You don't seem to have a hard dependency on it.
> Couldn't you just replicate what you are interested? Instead of bringing in a 
> full blown unit testing framework...
Oh sorry, I didn't notice the comments here.

I will remove the dependency of "luaunit" soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

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

[Lldb-commits] [lldb] 9da2fa2 - [lldb] Move StringConvert inside debugserver

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-27T14:32:42+02:00
New Revision: 9da2fa277e818e94e45b836a3d416623c838e02a

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

LOG: [lldb] Move StringConvert inside debugserver

The StringConvert API is no longer used anywhere but in debugserver.
Since debugserver does not use LLVM API, we cannot replace it with
llvm::to_integer() and llvm::to_float() there.  Let's just move
the sources into debugserver.

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

Added: 
lldb/tools/debugserver/source/StringConvert.cpp
lldb/tools/debugserver/source/StringConvert.h

Modified: 
lldb/include/lldb/module.modulemap
lldb/source/Host/CMakeLists.txt
lldb/tools/debugserver/source/CMakeLists.txt
lldb/tools/debugserver/source/JSON.cpp

Removed: 
lldb/include/lldb/Host/StringConvert.h
lldb/source/Host/common/StringConvert.cpp



diff  --git a/lldb/include/lldb/module.modulemap 
b/lldb/include/lldb/module.modulemap
index 1ddaa1fb737d..c0d467a6505e 100644
--- a/lldb/include/lldb/module.modulemap
+++ b/lldb/include/lldb/module.modulemap
@@ -48,7 +48,6 @@ module lldb_Host {
   module SafeMachO { header "Host/SafeMachO.h" export * }
   module SocketAddress { header "Host/SocketAddress.h" export * }
   module Socket { header "Host/Socket.h" export * }
-  module StringConvert { textual header "Host/StringConvert.h" export * }
   module Terminal { header "Host/Terminal.h" export * }
   module ThreadLauncher { header "Host/ThreadLauncher.h" export * }
   module Time { header "Host/Time.h" export * }

diff  --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index c18e8ce004b0..4374abca0506 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -37,7 +37,6 @@ add_host_subdirectory(common
   common/PseudoTerminal.cpp
   common/SocketAddress.cpp
   common/Socket.cpp
-  common/StringConvert.cpp
   common/TCPSocket.cpp
   common/Terminal.cpp
   common/ThreadLauncher.cpp

diff  --git a/lldb/tools/debugserver/source/CMakeLists.txt 
b/lldb/tools/debugserver/source/CMakeLists.txt
index 98c79d9f7349..588df7f65355 100644
--- a/lldb/tools/debugserver/source/CMakeLists.txt
+++ b/lldb/tools/debugserver/source/CMakeLists.txt
@@ -206,8 +206,8 @@ set(lldbDebugserverCommonSources
   DNBThreadResumeActions.cpp
   JSON.cpp
   StdStringExtractor.cpp
+  StringConvert.cpp
   # JSON reader depends on the following LLDB-common files
-  ${LLDB_SOURCE_DIR}/source/Host/common/StringConvert.cpp
   ${LLDB_SOURCE_DIR}/source/Host/common/SocketAddress.cpp
   # end JSON reader dependencies
   libdebugserver.cpp

diff  --git a/lldb/tools/debugserver/source/JSON.cpp 
b/lldb/tools/debugserver/source/JSON.cpp
index 5eff683a080b..315c52aafc93 100644
--- a/lldb/tools/debugserver/source/JSON.cpp
+++ b/lldb/tools/debugserver/source/JSON.cpp
@@ -13,12 +13,10 @@
 #include 
 
 // C++ includes
-#include "lldb/Host/StringConvert.h"
+#include "StringConvert.h"
 #include 
 #include 
 
-using namespace lldb_private;
-
 std::string JSONString::json_string_quote_metachars(const std::string &s) {
   if (s.find('"') == std::string::npos)
 return s;

diff  --git a/lldb/source/Host/common/StringConvert.cpp 
b/lldb/tools/debugserver/source/StringConvert.cpp
similarity index 62%
rename from lldb/source/Host/common/StringConvert.cpp
rename to lldb/tools/debugserver/source/StringConvert.cpp
index b4eb92755367..fac7525c1980 100644
--- a/lldb/source/Host/common/StringConvert.cpp
+++ b/lldb/tools/debugserver/source/StringConvert.cpp
@@ -8,43 +8,10 @@
 
 #include 
 
-#include "lldb/Host/StringConvert.h"
+#include "StringConvert.h"
 
-namespace lldb_private {
 namespace StringConvert {
 
-int32_t ToSInt32(const char *s, int32_t fail_value, int base,
- bool *success_ptr) {
-  if (s && s[0]) {
-char *end = nullptr;
-const long sval = ::strtol(s, &end, base);
-if (*end == '\0') {
-  if (success_ptr)
-*success_ptr = ((sval <= INT32_MAX) && (sval >= INT32_MIN));
-  return (int32_t)sval; // All characters were used, return the result
-}
-  }
-  if (success_ptr)
-*success_ptr = false;
-  return fail_value;
-}
-
-uint32_t ToUInt32(const char *s, uint32_t fail_value, int base,
-  bool *success_ptr) {
-  if (s && s[0]) {
-char *end = nullptr;
-const unsigned long uval = ::strtoul(s, &end, base);
-if (*end == '\0') {
-  if (success_ptr)
-*success_ptr = (uval <= UINT32_MAX);
-  return (uint32_t)uval; // All characters were used, return the result
-}
-  }
-  if (success_ptr)
-*success_ptr = false;
-  return fail_value;
-}
-
 int64_t ToSInt64(const char *s, int64_t fail_value, int base,
  bool *success_ptr) {
   if (s && s[0]) {
@@ -91,5 +58,5 @@ double 

[Lldb-commits] [PATCH] D110478: [lldb] Move StringConvert inside debugserver

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9da2fa277e81: [lldb] Move StringConvert inside debugserver 
(authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110478

Files:
  lldb/include/lldb/Host/StringConvert.h
  lldb/include/lldb/module.modulemap
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/StringConvert.cpp
  lldb/tools/debugserver/source/CMakeLists.txt
  lldb/tools/debugserver/source/JSON.cpp
  lldb/tools/debugserver/source/StringConvert.cpp
  lldb/tools/debugserver/source/StringConvert.h

Index: lldb/tools/debugserver/source/StringConvert.h
===
--- lldb/tools/debugserver/source/StringConvert.h
+++ lldb/tools/debugserver/source/StringConvert.h
@@ -6,24 +6,13 @@
 //
 //===--===//
 
-#ifndef LLDB_HOST_STRINGCONVERT_H
-#define LLDB_HOST_STRINGCONVERT_H
+#ifndef LLDB_TOOLS_DEBUGSERVER_SOURCE_STRINGCONVERT_H
+#define LLDB_TOOLS_DEBUGSERVER_SOURCE_STRINGCONVERT_H
 
 #include 
 
-namespace lldb_private {
-
 namespace StringConvert {
 
-/// \namespace StringConvert StringConvert.h "lldb/Host/StringConvert.h"
-/// Utility classes for converting strings into Integers
-
-int32_t ToSInt32(const char *s, int32_t fail_value = 0, int base = 0,
- bool *success_ptr = nullptr);
-
-uint32_t ToUInt32(const char *s, uint32_t fail_value = 0, int base = 0,
-  bool *success_ptr = nullptr);
-
 int64_t ToSInt64(const char *s, int64_t fail_value = 0, int base = 0,
  bool *success_ptr = nullptr);
 
@@ -32,7 +21,7 @@
 
 double ToDouble(const char *s, double fail_value = 0.0,
 bool *success_ptr = nullptr);
+
 } // namespace StringConvert
-} // namespace lldb_private
 
 #endif
Index: lldb/tools/debugserver/source/StringConvert.cpp
===
--- lldb/tools/debugserver/source/StringConvert.cpp
+++ lldb/tools/debugserver/source/StringConvert.cpp
@@ -8,43 +8,10 @@
 
 #include 
 
-#include "lldb/Host/StringConvert.h"
+#include "StringConvert.h"
 
-namespace lldb_private {
 namespace StringConvert {
 
-int32_t ToSInt32(const char *s, int32_t fail_value, int base,
- bool *success_ptr) {
-  if (s && s[0]) {
-char *end = nullptr;
-const long sval = ::strtol(s, &end, base);
-if (*end == '\0') {
-  if (success_ptr)
-*success_ptr = ((sval <= INT32_MAX) && (sval >= INT32_MIN));
-  return (int32_t)sval; // All characters were used, return the result
-}
-  }
-  if (success_ptr)
-*success_ptr = false;
-  return fail_value;
-}
-
-uint32_t ToUInt32(const char *s, uint32_t fail_value, int base,
-  bool *success_ptr) {
-  if (s && s[0]) {
-char *end = nullptr;
-const unsigned long uval = ::strtoul(s, &end, base);
-if (*end == '\0') {
-  if (success_ptr)
-*success_ptr = (uval <= UINT32_MAX);
-  return (uint32_t)uval; // All characters were used, return the result
-}
-  }
-  if (success_ptr)
-*success_ptr = false;
-  return fail_value;
-}
-
 int64_t ToSInt64(const char *s, int64_t fail_value, int base,
  bool *success_ptr) {
   if (s && s[0]) {
@@ -91,5 +58,5 @@
 *success_ptr = false;
   return fail_value;
 }
-}
-}
+
+} // namespace StringConvert
Index: lldb/tools/debugserver/source/JSON.cpp
===
--- lldb/tools/debugserver/source/JSON.cpp
+++ lldb/tools/debugserver/source/JSON.cpp
@@ -13,12 +13,10 @@
 #include 
 
 // C++ includes
-#include "lldb/Host/StringConvert.h"
+#include "StringConvert.h"
 #include 
 #include 
 
-using namespace lldb_private;
-
 std::string JSONString::json_string_quote_metachars(const std::string &s) {
   if (s.find('"') == std::string::npos)
 return s;
Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -206,8 +206,8 @@
   DNBThreadResumeActions.cpp
   JSON.cpp
   StdStringExtractor.cpp
+  StringConvert.cpp
   # JSON reader depends on the following LLDB-common files
-  ${LLDB_SOURCE_DIR}/source/Host/common/StringConvert.cpp
   ${LLDB_SOURCE_DIR}/source/Host/common/SocketAddress.cpp
   # end JSON reader dependencies
   libdebugserver.cpp
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -37,7 +37,6 @@
   common/PseudoTerminal.cpp
   common/SocketAddress.cpp
   common/Socket.cpp
-  common/StringConvert.cpp
   common/TCPSocket.cpp
   common/Terminal.cpp
   common/ThreadLauncher.cpp
Index: lldb/include/lldb/m

[Lldb-commits] [PATCH] D110545: [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

[Andrew Turner via Phabricator via lldb-commits]

andrew created this revision.
Herald added subscribers: omjavaid, kristof.beyls, emaste.
andrew requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110545

Files:
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp


Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);


Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110535: [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions

[Michał Górny via Phabricator via lldb-commits]

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



Comment at: llvm/include/llvm/ADT/StringExtras.h:544
 /// of the iterators.
-class Split {
-  StringRef Str;
-  std::string SeparatorStr;
-
-public:
-  Split(StringRef NewStr, StringRef Separator)
-  : Str(NewStr), SeparatorStr(Separator) {}
-  Split(StringRef NewStr, char Separator)
-  : Str(NewStr), SeparatorStr(1, Separator) {}
-
-  SplittingIterator begin() { return SplittingIterator(Str, SeparatorStr); }
-
-  SplittingIterator end() { return SplittingIterator("", SeparatorStr); }
-};
+inline iterator_range Split(StringRef Str, StringRef 
Separator) {
+  return {SplittingIterator(Str, Separator),

labath wrote:
> this should also be a lower-case `split` now that it's a function.
> 
> (Believe it or not, this is the reason I was first drawn to this -- it's 
> uncommon to see a class used like this because, even in the cases where you 
> do have a separate class, you usually create a function wrapper for it. The 
> original reason for this is pragmatic (use of template argument deduction), 
> but the practice is so ubiquitous that a deviation from it stands out.)
Yeah, that makes sense. In fact, I originally named the class lowercase for 
this exact reason.


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

https://reviews.llvm.org/D110535

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

[Lldb-commits] [PATCH] D110535: [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 375238.
mgorny marked an inline comment as done.
mgorny added a comment.
Herald added a subscriber: hiraditya.

Make the function lowercase. I had to modify `IR/DataLayout.cpp` to call its 
own static `split()` via `::split()`.


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

https://reviews.llvm.org/D110535

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  llvm/include/llvm/ADT/StringExtras.h
  llvm/lib/IR/DataLayout.cpp
  llvm/unittests/ADT/StringExtrasTest.cpp

Index: llvm/unittests/ADT/StringExtrasTest.cpp
===
--- llvm/unittests/ADT/StringExtrasTest.cpp
+++ llvm/unittests/ADT/StringExtrasTest.cpp
@@ -276,23 +276,7 @@
 }
 
 TEST(StringExtrasTest, splitStringRef) {
-  auto Spl = Split("foo<=>bar<=><=>baz", "<=>");
-  auto It = Spl.begin();
-  auto End = Spl.end();
-
-  ASSERT_NE(It, End);
-  EXPECT_EQ(*It, StringRef("foo"));
-  ASSERT_NE(++It, End);
-  EXPECT_EQ(*It, StringRef("bar"));
-  ASSERT_NE(++It, End);
-  EXPECT_EQ(*It, StringRef(""));
-  ASSERT_NE(++It, End);
-  EXPECT_EQ(*It, StringRef("baz"));
-  ASSERT_EQ(++It, End);
-}
-
-TEST(StringExtrasTest, splItChar) {
-  auto Spl = Split("foo,bar,,baz", ',');
+  auto Spl = split("foo<=>bar<=><=>baz", "<=>");
   auto It = Spl.begin();
   auto End = Spl.end();
 
Index: llvm/lib/IR/DataLayout.cpp
===
--- llvm/lib/IR/DataLayout.cpp
+++ llvm/lib/IR/DataLayout.cpp
@@ -260,12 +260,12 @@
   while (!Desc.empty()) {
 // Split at '-'.
 std::pair Split;
-if (Error Err = split(Desc, '-', Split))
+if (Error Err = ::split(Desc, '-', Split))
   return Err;
 Desc = Split.second;
 
 // Split at ':'.
-if (Error Err = split(Split.first, ':', Split))
+if (Error Err = ::split(Split.first, ':', Split))
   return Err;
 
 // Aliases used below.
@@ -274,7 +274,7 @@
 
 if (Tok == "ni") {
   do {
-if (Error Err = split(Rest, ':', Split))
+if (Error Err = ::split(Rest, ':', Split))
   return Err;
 Rest = Split.second;
 unsigned AS;
@@ -315,7 +315,7 @@
   if (Rest.empty())
 return reportError(
 "Missing size specification for pointer in datalayout string");
-  if (Error Err = split(Rest, ':', Split))
+  if (Error Err = ::split(Rest, ':', Split))
 return Err;
   unsigned PointerMemSize;
   if (Error Err = getIntInBytes(Tok, PointerMemSize))
@@ -327,7 +327,7 @@
   if (Rest.empty())
 return reportError(
 "Missing alignment specification for pointer in datalayout string");
-  if (Error Err = split(Rest, ':', Split))
+  if (Error Err = ::split(Rest, ':', Split))
 return Err;
   unsigned PointerABIAlign;
   if (Error Err = getIntInBytes(Tok, PointerABIAlign))
@@ -342,7 +342,7 @@
   // Preferred alignment.
   unsigned PointerPrefAlign = PointerABIAlign;
   if (!Rest.empty()) {
-if (Error Err = split(Rest, ':', Split))
+if (Error Err = ::split(Rest, ':', Split))
   return Err;
 if (Error Err = getIntInBytes(Tok, PointerPrefAlign))
   return Err;
@@ -352,7 +352,7 @@
 
 // Now read the index. It is the second optional parameter here.
 if (!Rest.empty()) {
-  if (Error Err = split(Rest, ':', Split))
+  if (Error Err = ::split(Rest, ':', Split))
 return Err;
   if (Error Err = getIntInBytes(Tok, IndexSize))
 return Err;
@@ -393,7 +393,7 @@
   if (Rest.empty())
 return reportError(
 "Missing alignment specification in datalayout string");
-  if (Error Err = split(Rest, ':', Split))
+  if (Error Err = ::split(Rest, ':', Split))
 return Err;
   unsigned ABIAlign;
   if (Error Err = getIntInBytes(Tok, ABIAlign))
@@ -410,7 +410,7 @@
   // Preferred alignment.
   unsigned PrefAlign = ABIAlign;
   if (!Rest.empty()) {
-if (Error Err = split(Rest, ':', Split))
+if (Error Err = ::split(Rest, ':', Split))
   return Err;
 if (Error Err = getIntInBytes(Tok, PrefAlign))
   return Err;
@@ -439,7 +439,7 @@
 LegalIntWidths.push_back(Width);
 if (Rest.empty())
   break;
-if (Error Err = split(Rest, ':', Split))
+if (Error Err = ::split(Rest, ':', Split))
   return Err;
   }
   break;
Index: llvm/include/llvm/ADT/StringExtras.h
===
--- llvm/include/llvm/ADT/StringExtras.h
+++ llvm/include/llvm/ADT/StringExtras.h
@@ -516,7 +516,8 @@
   }
 
   bool operator==(const SplittingIterator &R) const {
-return Current == R.Current && Next == R.Next &&

[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module

[Siger Young via Phabricator via lldb-commits]

siger-young updated this revision to Diff 375240.
siger-young added a comment.

Add assertion functions and error status detection to remove "luaunit"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108090

Files:
  lldb/CMakeLists.txt
  lldb/bindings/lua/CMakeLists.txt
  lldb/bindings/lua/lua-typemaps.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/lua/lua.swig
  lldb/source/API/liblldb-private.exports
  lldb/source/API/liblldb.exports
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/API/lldbtest.py
  lldb/test/API/lua_api/Makefile
  lldb/test/API/lua_api/TestBreakpointAPI.lua
  lldb/test/API/lua_api/TestComprehensive.lua
  lldb/test/API/lua_api/TestFileHandle.lua
  lldb/test/API/lua_api/TestLuaAPI.py
  lldb/test/API/lua_api/TestProcessAPI.lua
  lldb/test/API/lua_api/lua_lldb_test.lua
  lldb/test/API/lua_api/main.c

Index: lldb/test/API/lua_api/main.c
===
--- /dev/null
+++ lldb/test/API/lua_api/main.c
@@ -0,0 +1,35 @@
+#include 
+
+void BFunction()
+{
+}
+
+void AFunction()
+{
+printf("I am a function.\n");
+}
+
+int main(int argc, const char *argv[])
+{
+int inited = 0xDEADBEEF;
+int sum = 0;
+if(argc > 1)
+{
+for(int i = 0; i < argc; i++)
+{
+puts(argv[i]);
+}
+if(argc > 2)
+{
+return argc;
+}
+}
+AFunction();
+for(int i = 1; i <= 100; i++)
+{
+BFunction();
+sum += i;
+}
+printf("sum = %d\n", sum);
+return 0;
+}
Index: lldb/test/API/lua_api/lua_lldb_test.lua
===
--- /dev/null
+++ lldb/test/API/lua_api/lua_lldb_test.lua
@@ -0,0 +1,155 @@
+-- Make lldb available in global
+lldb = require('lldb')
+
+-- Global assertion functions
+function assertTrue(x)
+if not x then error('assertTrue failure') end
+end
+
+function assertFalse(x)
+if x then error('assertNotNil failure') end
+end
+
+function assertNotNil(x)
+if x == nil then error('assertNotNil failure') end
+end
+
+function assertEquals(x, y)
+if type(x) == 'table' and type(y) == 'table' then
+for k, _ in pairs(x) do
+assertEquals(x[k], y[k])
+end
+elseif type(x) ~= type(y) then
+error('assertEquals failure')
+elseif x ~= y then
+error('assertEquals failure')
+end
+end
+
+function assertStrContains(x, y)
+if not string.find(x, y, 1, true) then
+error('assertStrContains failure')
+end
+end
+
+-- Global helper functions
+function read_file_non_empty_lines(f)
+local lines = {}
+while true do
+local line = f:read('*l')
+if not line then break end
+if line ~= '\n' then table.insert(lines, line) end
+end
+return lines
+end
+
+function split_lines(str)
+local lines = {}
+for line in str:gmatch("[^\r\n]+") do
+table.insert(lines, line)
+end
+return lines
+end
+
+function get_stopped_threads(process, reason)
+local threads = {}
+for i = 0, process:GetNumThreads() - 1 do
+local t = process:GetThreadAtIndex(i)
+if t:IsValid() and t:GetStopReason() == reason then
+table.insert(threads, t)
+end
+end
+return threads
+end
+
+function get_stopped_thread(process, reason)
+local threads = get_stopped_threads(process, reason)
+if #threads ~= 0 then return threads[1]
+else return nil end
+end
+
+-- Test helper
+
+local _M = {}
+local _m = {}
+
+local _mt = { __index = _m }
+
+function _M.create_test(name, exe, output, input)
+print('[lldb/lua] Create test ' .. name)
+exe = exe or os.getenv('TEST_EXE')
+output = output or os.getenv('TEST_OUTPUT')
+input = input or os.getenv('TEST_INPUT')
+lldb.SBDebugger.Initialize()
+local debugger = lldb.SBDebugger.Create()
+-- Ensure that debugger is created
+assertNotNil(debugger)
+assertTrue(debugger:IsValid())
+
+debugger:SetAsync(false)
+
+local lua_language = debugger:GetScriptingLanguage('lua')
+assertNotNil(lua_language)
+debugger:SetScriptLanguage(lua_language)
+
+local test = setmetatable({
+output = output,
+input = input,
+name = name,
+exe = exe,
+debugger = debugger
+}, _mt)
+_G[name] = test
+return test
+end
+
+function _m:create_target(exe)
+local target
+if not exe then exe = self.exe end
+target = self.debugger:CreateTarget(exe)
+-- Ensure that target is created
+assertNotNil(target)
+assertTrue(target:IsValid())
+return target
+end
+
+function _m:handle_command(command, collect)
+if collect == nil then collect = true end
+if collect then
+local ret = lldb.SBCommandReturnObject()
+local interpreter = self.debugger:GetCommandInterpreter()
+assertTrue(interpreter:IsValid())
+interpreter:Han

[Lldb-commits] [PATCH] D110023: [lldb] [DynamicRegisterInfo] Add a convenience method to add suppl. registers

[Michał Górny via Phabricator via lldb-commits]

mgorny marked 4 inline comments as done.
mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:441
+
+  reg_to_regs_map to_add;
+  for (uint32_t value_reg : m_value_regs_map[reg_num]) {

labath wrote:
> Maybe call this new_invalidates? I've found it hard to track what to_add 
> means, with all the mixing of value_regs and invalidates...
Good idea.



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:443-444
+  for (uint32_t value_reg : m_value_regs_map[reg_num]) {
+if (value_reg == LLDB_INVALID_REGNUM)
+  break;
+

labath wrote:
> Is this still needed?
Probably not indeed.



Comment at: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp:184-190
+  struct RegisterInfo ah_reg {
+"ah", nullptr, 1, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, ah, 
ah},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(ah_reg, group);

labath wrote:
> Could we remove ah from this test, as its offset is going to be wrong?
Sure.


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

https://reviews.llvm.org/D110023

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

[Lldb-commits] [lldb] 3303154 - [lldb] [DynamicRegisterInfo] Add a convenience method to add suppl. registers

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-27T16:01:30+02:00
New Revision: 33031545bf4d8264af98e3f0ca72dbe09bc57496

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

LOG: [lldb] [DynamicRegisterInfo] Add a convenience method to add suppl. 
registers

Add a convenience method to add supplementary registers that takes care
of adding invalidate_regs to all (potentially) overlapping registers.

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp 
b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
index f6a2cdf30981..6535df20de11 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -430,6 +430,29 @@ void DynamicRegisterInfo::AddRegister(RegisterInfo 
reg_info,
   m_set_reg_nums[set].push_back(reg_num);
 }
 
+void DynamicRegisterInfo::AddSupplementaryRegister(RegisterInfo new_reg_info,
+   ConstString &set_name) {
+  assert(new_reg_info.value_regs != nullptr);
+  const uint32_t reg_num = m_regs.size();
+  AddRegister(new_reg_info, set_name);
+
+  reg_to_regs_map new_invalidates;
+  for (uint32_t value_reg : m_value_regs_map[reg_num]) {
+// copy value_regs to invalidate_regs
+new_invalidates[reg_num].push_back(value_reg);
+
+// copy invalidate_regs from the parent register
+llvm::append_range(new_invalidates[reg_num], 
m_invalidate_regs_map[value_reg]);
+
+// add reverse invalidate entries
+for (uint32_t x : new_invalidates[reg_num])
+  new_invalidates[x].push_back(new_reg_info.kinds[eRegisterKindLLDB]);
+  }
+
+  for (const auto &x : new_invalidates)
+llvm::append_range(m_invalidate_regs_map[x.first], x.second);
+}
+
 void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
   if (m_finalized)
 return;

diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h 
b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
index a6495dcda608..44030c8132d8 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
@@ -38,6 +38,11 @@ class DynamicRegisterInfo {
   void AddRegister(lldb_private::RegisterInfo reg_info,
lldb_private::ConstString &set_name);
 
+  // Add a new register and cross-link it via invalidate_regs with other
+  // registers sharing its value_regs.
+  void AddSupplementaryRegister(lldb_private::RegisterInfo reg_info,
+lldb_private::ConstString &set_name);
+
   void Finalize(const lldb_private::ArchSpec &arch);
 
   size_t GetNumRegisters() const;

diff  --git a/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp 
b/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
index 5f2b278b9b75..53b53a0d5777 100644
--- a/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
+++ b/lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -124,3 +124,47 @@ TEST_F(DynamicRegisterInfoTest, no_finalize_regs) {
   ASSERT_REG(i1, LLDB_INVALID_INDEX32);
   ASSERT_REG(i2, LLDB_INVALID_INDEX32);
 }
+
+TEST_F(DynamicRegisterInfoTest, add_supplementary_register) {
+  // Add a base register
+  uint32_t rax = AddTestRegister("rax", 8);
+
+  // Register numbers
+  uint32_t eax = 1;
+  uint32_t ax = 2;
+  uint32_t al = 3;
+
+  ConstString group{"supplementary registers"};
+  uint32_t value_regs[2] = {rax, LLDB_INVALID_REGNUM};
+  struct RegisterInfo eax_reg {
+"eax", nullptr, 4, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, eax,
+ eax},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(eax_reg, group);
+
+  struct RegisterInfo ax_reg {
+"ax", nullptr, 2, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, ax, 
ax},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(ax_reg, group);
+
+  struct RegisterInfo al_reg {
+"al", nullptr, 1, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, al, 
al},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(al_reg, group);
+
+  info.Finalize(lldb_private::ArchSpec());
+
+  ASSERT_REG(rax, 0, {}, {eax, ax, al});
+  ASSERT_REG(eax, 0, 

[Lldb-commits] [PATCH] D110023: [lldb] [DynamicRegisterInfo] Add a convenience method to add suppl. registers

[Michał Górny via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mgorny marked 3 inline comments as done.
Closed by commit rG33031545bf4d: [lldb] [DynamicRegisterInfo] Add a convenience 
method to add suppl. registers (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D110023?vs=373515&id=375247#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110023

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp

Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -124,3 +124,47 @@
   ASSERT_REG(i1, LLDB_INVALID_INDEX32);
   ASSERT_REG(i2, LLDB_INVALID_INDEX32);
 }
+
+TEST_F(DynamicRegisterInfoTest, add_supplementary_register) {
+  // Add a base register
+  uint32_t rax = AddTestRegister("rax", 8);
+
+  // Register numbers
+  uint32_t eax = 1;
+  uint32_t ax = 2;
+  uint32_t al = 3;
+
+  ConstString group{"supplementary registers"};
+  uint32_t value_regs[2] = {rax, LLDB_INVALID_REGNUM};
+  struct RegisterInfo eax_reg {
+"eax", nullptr, 4, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, eax,
+ eax},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(eax_reg, group);
+
+  struct RegisterInfo ax_reg {
+"ax", nullptr, 2, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, ax, ax},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(ax_reg, group);
+
+  struct RegisterInfo al_reg {
+"al", nullptr, 1, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, al, al},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(al_reg, group);
+
+  info.Finalize(lldb_private::ArchSpec());
+
+  ASSERT_REG(rax, 0, {}, {eax, ax, al});
+  ASSERT_REG(eax, 0, {rax}, {rax, ax, al});
+  ASSERT_REG(ax, 0, {rax}, {rax, eax, al});
+  ASSERT_REG(al, 0, {rax}, {rax, eax, ax});
+}
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
@@ -38,6 +38,11 @@
   void AddRegister(lldb_private::RegisterInfo reg_info,
lldb_private::ConstString &set_name);
 
+  // Add a new register and cross-link it via invalidate_regs with other
+  // registers sharing its value_regs.
+  void AddSupplementaryRegister(lldb_private::RegisterInfo reg_info,
+lldb_private::ConstString &set_name);
+
   void Finalize(const lldb_private::ArchSpec &arch);
 
   size_t GetNumRegisters() const;
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -430,6 +430,29 @@
   m_set_reg_nums[set].push_back(reg_num);
 }
 
+void DynamicRegisterInfo::AddSupplementaryRegister(RegisterInfo new_reg_info,
+   ConstString &set_name) {
+  assert(new_reg_info.value_regs != nullptr);
+  const uint32_t reg_num = m_regs.size();
+  AddRegister(new_reg_info, set_name);
+
+  reg_to_regs_map new_invalidates;
+  for (uint32_t value_reg : m_value_regs_map[reg_num]) {
+// copy value_regs to invalidate_regs
+new_invalidates[reg_num].push_back(value_reg);
+
+// copy invalidate_regs from the parent register
+llvm::append_range(new_invalidates[reg_num], m_invalidate_regs_map[value_reg]);
+
+// add reverse invalidate entries
+for (uint32_t x : new_invalidates[reg_num])
+  new_invalidates[x].push_back(new_reg_info.kinds[eRegisterKindLLDB]);
+  }
+
+  for (const auto &x : new_invalidates)
+llvm::append_range(m_invalidate_regs_map[x.first], x.second);
+}
+
 void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
   if (m_finalized)
 return;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110545: [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp:328
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets;
   ArchSpec arch{"aarch64-unknown-freebsd"};

Maybe set it to `eRegsetMaskDefault` explicitly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110545

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

[Lldb-commits] [PATCH] D110553: [lldb] Remove non-stop mode code

[Pavel Labath via Phabricator via lldb-commits]

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

We added some support for this mode back in 2015, but the feature was
never productionized. It is completely untested, and there are known
major structural lldb issues that need to be resolved before this
feature can really be supported.

It also complicates making further changes to stop reply packet
handling, which is what I am about to do.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110553

Files:
  lldb/include/lldb/Target/Target.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
  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/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -163,9 +163,6 @@
   def DisplayRecognizedArguments: Property<"display-recognized-arguments", "Boolean">,
 DefaultFalse,
 Desc<"Show recognized arguments in variable listings by default.">;
-  def NonStopModeEnabled: Property<"non-stop-mode", "Boolean">,
-DefaultFalse,
-Desc<"Disable lock-step debugging, instead control threads independently.">;
   def RequireHardwareBreakpoints: Property<"require-hardware-breakpoint", "Boolean">,
 DefaultFalse,
 Desc<"Require all breakpoints to be hardware breakpoints.">;
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -4288,16 +4288,6 @@
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
 }
 
-bool TargetProperties::GetNonStopModeEnabled() const {
-  const uint32_t idx = ePropertyNonStopModeEnabled;
-  return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, false);
-}
-
-void TargetProperties::SetNonStopModeEnabled(bool b) {
-  const uint32_t idx = ePropertyNonStopModeEnabled;
-  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
-}
-
 const ProcessLaunchInfo &TargetProperties::GetProcessLaunchInfo() const {
   return m_launch_info;
 }
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -268,11 +268,10 @@
   GDBRemoteCommunicationClient m_gdb_comm;
   GDBRemoteCommunicationReplayServer m_gdb_replay_server;
   std::atomic m_debugserver_pid;
-  std::vector m_stop_packet_stack; // The stop packet
- // stack replaces
- // the last stop
- // packet variable
+
+  llvm::Optional m_last_stop_packet;
   std::recursive_mutex m_last_stop_packet_mutex;
+
   GDBRemoteDynamicRegisterInfoSP m_register_info_sp;
   Broadcaster m_async_broadcaster;
   lldb::ListenerSP m_async_listener_sp;
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
@@ -592,10 +592,6 @@
 if (m_gdb_comm.GetStopReply(response)) {
   SetLastStopPacket(response);
 
-  // '?' Packets must be handled differently in non-stop mode
-  if (GetTarget().GetNonStopModeEnabled())
-HandleStopReplySequence();
-
   Target &target = GetTarget();
   if (!target.GetArchitecture().IsValid()) {
 if (m_gdb_comm.GetProcessArchitecture().IsValid()) {
@@ -835,9 +831,6 @@
   StringExtractorGDBRemote response;
   if (m_gdb_comm.GetStopReply(response)) {
 SetLastStopPacket(response);
-// '?' Packets must be handled differently in non-stop mode
-if (GetTarget().GetNonStopModeEnabled())
-  HandleStopReplySequence();
 
 const ArchSpec &process_arch = m_gdb_comm.GetProcessArchitecture();
 
@@ -908,11 +901,6 @@
 return error;
   }
 
-  // Start the communications read thread so all incoming data can be parsed
-  // into packets and queued as they arrive.
-  if (GetTarget().GetNonStopModeEnabled())
-m_gdb_comm.StartReadThread();
-
   // We always seem to be able to open a connection to a local port so we need
   // to make sure we can then send data to it. If we can't then we 

[Lldb-commits] [lldb] be2a421 - [lldb] Fix SocketTest.DomainGetConnectURI on macOS by stripping more zeroes from getpeername result

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-09-27T17:34:45+02:00
New Revision: be2a4216fc5664ae465ec3dcc32fc3d40cecfdcd

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

LOG: [lldb] Fix SocketTest.DomainGetConnectURI on macOS by stripping more 
zeroes from getpeername result

Apparently macOS is padding the name result with several padding zeroes at
the end. Just strip them all to pretend it's a C-string.

Thanks to Pavel for suggesting this fix.

Added: 


Modified: 
lldb/source/Host/posix/DomainSocket.cpp

Removed: 




diff  --git a/lldb/source/Host/posix/DomainSocket.cpp 
b/lldb/source/Host/posix/DomainSocket.cpp
index 790458ee13d0..8138b6ff1dc4 100644
--- a/lldb/source/Host/posix/DomainSocket.cpp
+++ b/lldb/source/Host/posix/DomainSocket.cpp
@@ -143,8 +143,7 @@ std::string DomainSocket::GetSocketName() const {
   llvm::StringRef name(saddr_un.sun_path + GetNameOffset(),
sock_addr_len - offsetof(struct sockaddr_un, sun_path) -
GetNameOffset());
-  if (name.back() == '\0')
-name = name.drop_back();
+  name = name.drop_while([](char c) { return c == '\0'; });
 
   return name.str();
 }



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

[Lldb-commits] [PATCH] D110545: [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

[Andrew Turner via Phabricator via lldb-commits]

andrew updated this revision to Diff 375293.
andrew added a comment.

Set opt_regsets to eRegsetMaskDefault


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110545

Files:
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp


Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets = RegisterInfoPOSIX_arm64::eRegsetMaskDefault;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);


Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets = RegisterInfoPOSIX_arm64::eRegsetMaskDefault;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110545: [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

[Michał Górny via Phabricator via lldb-commits]

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

LGTM now, thanks. I don't have an arm64 cross env handy at the time, so I'm 
assuming you've tested it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110545

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

[Lldb-commits] [lldb] 3dbf27e - [lldb] A different fix for Domain Socket tests

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-09-27T18:00:27+02:00
New Revision: 3dbf27e762008757c0de7034c778d941bfeb0388

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

LOG: [lldb] A different fix for Domain Socket tests

we need to drop nuls from the end of the string.

Added: 


Modified: 
lldb/source/Host/posix/DomainSocket.cpp

Removed: 




diff  --git a/lldb/source/Host/posix/DomainSocket.cpp 
b/lldb/source/Host/posix/DomainSocket.cpp
index 8138b6ff1dc4..0a43d00e93ee 100644
--- a/lldb/source/Host/posix/DomainSocket.cpp
+++ b/lldb/source/Host/posix/DomainSocket.cpp
@@ -143,7 +143,7 @@ std::string DomainSocket::GetSocketName() const {
   llvm::StringRef name(saddr_un.sun_path + GetNameOffset(),
sock_addr_len - offsetof(struct sockaddr_un, sun_path) -
GetNameOffset());
-  name = name.drop_while([](char c) { return c == '\0'; });
+  name = name.rtrim('\0');
 
   return name.str();
 }



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

[Lldb-commits] [PATCH] D110545: [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

[Andrew Turner via Phabricator via lldb-commits]

andrew added a comment.

Yes, I found it by trying to build the tests on FreeBSD/arm64


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110545

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

[Lldb-commits] [PATCH] D110396: [nfc] [lldb] DWZ 01/17: Remove DWARFDIE from DeclContextToDIEMap m_decl_ctx_to_die

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110396

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

[Lldb-commits] [PATCH] D110397: [nfc] [lldb] DWZ 02/17: Refactor DIERef for a key in llvm::DenseMap

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110397

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

[Lldb-commits] [PATCH] D110399: [nfc] [lldb] DWZ 03/17: Use DIERef for DIEToDeclContextMap m_die_to_decl_ctx

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110399

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

[Lldb-commits] [PATCH] D110400: [nfc] [lldb] DWZ 04/17: Use DIERef for DIEToModuleMap m_die_to_module

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110400

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

[Lldb-commits] [PATCH] D110401: [nfc] [lldb] DWZ 05/17: Use DIERef for DIEToDeclMap m_die_to_decl

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110401

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

[Lldb-commits] [PATCH] D110402: [nfc] [lldb] DWZ 06/17: Use DIERef for DIEToTypePtr m_die_to_type and GetDIEToType()

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110402

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

[Lldb-commits] [PATCH] D110403: [nfc] [lldb] DWZ 07/17: Use DIERef for DIEToVariableSP m_die_to_variable_sp and GetDIEToVariable()

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110403

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

[Lldb-commits] [PATCH] D110404: [nfc] [lldb] DWZ 08/17: Use DIERef for DIEToClangType m_forward_decl_die_to_clang_type and GetForwardDeclDieToClangType()

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110404

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

[Lldb-commits] [PATCH] D96237: [lldb] DWZ 10/17: More support for .gnu_debugaltlink

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96237

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

[Lldb-commits] [PATCH] D96238: [nfc] [lldb] DWZ 11/17: Rename TypeUnitSupportFiles -> SharedUnitSupportFiles

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96238

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

[Lldb-commits] [PATCH] D96239: [lldb] DWZ 12/17: DIERef support

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96239

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

[Lldb-commits] [PATCH] D96240: [lldb] DWZ 13/17: Main functionality

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96240

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

[Lldb-commits] [PATCH] D98826: [lldb] DWZ 14/17: Workaround DWZ bug dropping DW_TAG_namespace::DW_AT_export_symbols

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98826

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

[Lldb-commits] [PATCH] D96241: [lldb] DWZ 15/17: New testsuite category 'dwz'

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96241

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

[Lldb-commits] [PATCH] D96242: [lldb] DWZ 16/17: New testcases

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96242

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

[Lldb-commits] [PATCH] D96243: [lldb] DWZ 17/17: Fix symlinked /usr/lib/debug/.build-id/**.debug files

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil abandoned this revision.
jankratochvil added a comment.

I will be no longer involved with this patchset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96243

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

[Lldb-commits] [PATCH] D110569: [lldb] [Process/FreeBSD] Rework arm64 register access

[Andrew Turner via Phabricator via lldb-commits]

andrew created this revision.
andrew added a reviewer: mgorny.
Herald added subscribers: omjavaid, kristof.beyls, krytarowski, arichardson, 
emaste.
andrew requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

To simplify future register access rework the FreeBSD arm64 register
read/write functions to be similar to on Linux.

This will simplify adding more register sets, e.g. for Pointer
Authentication.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110569

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h

Index: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
===
--- lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
+++ lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
@@ -63,14 +63,29 @@
   // LLDB's GPR/FPU structs.  However, all fields have matching offsets
   // and sizes, so we do not have to worry about these (and we have
   // a unittest to assert that).
-  std::array m_reg_data;
+  std::array m_reg_data;
+  std::array m_fpreg_data;
 #ifdef LLDB_HAS_FREEBSD_WATCHPOINT
   dbreg m_dbreg;
   bool m_read_dbreg;
 #endif
 
-  Status ReadRegisterSet(uint32_t set);
-  Status WriteRegisterSet(uint32_t set);
+  void *GetGPRBuffer() { return &m_reg_data; }
+  size_t GetGPRBufferSize() { return sizeof(m_reg_data); }
+
+  void *GetFPRBuffer() { return &m_fpreg_data; }
+  size_t GetFPRSize() { return sizeof(m_fpreg_data); }
+
+  bool IsGPR(unsigned reg) const;
+  bool IsFPR(unsigned reg) const;
+
+  Status ReadGPR();
+  Status WriteGPR();
+
+  Status ReadFPR();
+  Status WriteFPR();
+
+  uint32_t CalculateFprOffset(const RegisterInfo *reg_info) const;
 
   llvm::Error ReadHardwareDebugInfo() override;
   llvm::Error WriteHardwareDebugRegs(DREGType hwbType) override;
Index: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
@@ -24,6 +24,8 @@
 #include 
 // clang-format on
 
+#define REG_CONTEXT_SIZE (GetGPRSize() + GetFPRSize())
+
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_freebsd;
@@ -68,30 +70,43 @@
   return count;
 }
 
-Status NativeRegisterContextFreeBSD_arm64::ReadRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
-   m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(
-PT_GETFPREGS, m_thread.GetID(),
-m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::ReadRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsGPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
+  RegisterInfoPOSIX_arm64::GPRegSet)
+return true;
+  return false;
 }
 
-Status NativeRegisterContextFreeBSD_arm64::WriteRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(),
-   m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(
-PT_SETFPREGS, m_thread.GetID(),
-m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::WriteRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsFPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
+  RegisterInfoPOSIX_arm64::FPRegSet)
+return true;
+  return false;
+}
+
+Status NativeRegisterContextFreeBSD_arm64::ReadGPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_GETREGS, m_thread.GetID(), m_reg_data.data());
+}
+
+Status NativeRegisterContextFreeBSD_arm64::WriteGPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_SETREGS, m_thread.GetID(), m_reg_data.data());
+}
+
+Status NativeRegisterContextFreeBSD_arm64::ReadFPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_GETFPREGS, m_thread.GetID(), m_fpreg_data.data());
+}
+
+Status NativeRegisterContextFreeBSD_arm64::WriteFPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_SETFPREGS, m_thread.GetID(), m_fpreg_data.data());
+}
+
+uint32_t NativeRegisterContextFreeBSD_arm64::CalculateFprOffset(
+const RegisterInfo *reg_info) const {
+  return reg_inf

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin created this revision.
jarin added a reviewer: labath.
jarin added a project: LLDB.
Herald added a subscriber: JDevlieghere.
jarin requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.

This is in preparation for a fix of parsing omitted abstract
parameters of inlined functions (see the bug
https://bugs.llvm.org/show_bug.cgi?id=50076#c6 for more details). The
main idea is to move the recursive parsing of variables in a function
context into a separate method (ParseVariablesInFunctionContext).

We also move the setup of block variable list to the DIE node that
starts the block rather than setting up the list when processing
the first variable DIE in that block.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110570

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

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -380,11 +380,14 @@
 const DWARFDIE &die,
 const lldb::addr_t func_low_pc);
 
-  size_t ParseVariables(const lldb_private::SymbolContext &sc,
-const DWARFDIE &orig_die,
-const lldb::addr_t func_low_pc, bool parse_siblings,
-bool parse_children,
-lldb_private::VariableList *cc_variable_list = nullptr);
+  void
+  ParseAndAppendGlobalVariable(const lldb_private::SymbolContext &sc,
+   const DWARFDIE &die,
+   lldb_private::VariableList &cc_variable_list);
+
+  size_t ParseVariablesInFunctionContext(const lldb_private::SymbolContext &sc,
+ const DWARFDIE &die,
+ const lldb::addr_t func_low_pc);
 
   bool ClassOrStructIsVirtual(const DWARFDIE &die);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2135,7 +2135,7 @@
   }
 }
 
-ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+ParseAndAppendGlobalVariable(sc, die, variables);
 while (pruned_idx < variables.GetSize()) {
   VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
   if (name_is_mangled ||
@@ -2188,7 +2188,7 @@
   return true;
 sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
-ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+ParseAndAppendGlobalVariable(sc, die, variables);
 
 return variables.GetSize() - original_size < max_matches;
   });
@@ -3049,8 +3049,8 @@
   /*check_hi_lo_pc=*/true))
 func_lo_pc = ranges.GetMinRangeBase(0);
   if (func_lo_pc != LLDB_INVALID_ADDRESS) {
-const size_t num_variables = ParseVariables(
-sc, function_die.GetFirstChild(), func_lo_pc, true, true);
+const size_t num_variables =
+ParseVariablesInFunctionContext(sc, function_die, func_lo_pc);
 
 // Let all blocks know they have parse all their variables
 sc.function->GetBlock(false).SetDidParseVariables(true, true);
@@ -3481,118 +3481,134 @@
   return DWARFDIE();
 }
 
-size_t SymbolFileDWARF::ParseVariables(const SymbolContext &sc,
-   const DWARFDIE &orig_die,
-   const lldb::addr_t func_low_pc,
-   bool parse_siblings, bool parse_children,
-   VariableList *cc_variable_list) {
-  if (!orig_die)
-return 0;
+void SymbolFileDWARF::ParseAndAppendGlobalVariable(
+const SymbolContext &sc, const DWARFDIE &die,
+VariableList &cc_variable_list) {
+  if (!die)
+return;
 
-  VariableListSP variable_list_sp;
+  dw_tag_t tag = die.Tag();
+  if (tag != DW_TAG_variable && tag != DW_TAG_constant)
+return;
 
-  size_t vars_added = 0;
-  DWARFDIE die = orig_die;
-  while (die) {
-dw_tag_t tag = die.Tag();
+  // Check to see if we have already parsed this variable or constant?
+  VariableSP var_sp = GetDIEToVariable()[die.GetDIE()];
+  if (var_sp) {
+cc_variable_list.AddVariableIfUnique(var_sp);
+return;
+  }
 
-// Check to see if we have already parsed this variable or constant?
-VariableSP var_sp = GetDIEToVariable()[die.GetDIE()];
-if (var_sp) {
-  if (cc_variable_list)
-cc_variable_list->AddVariableIfUnique(var_sp);
+  // We haven't already parsed it, lets do that now.
+  VariableListSP variable_list_sp;
+  DWARFDIE sc_parent_d

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

friendly ping! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110115

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

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

Hi, could you possibly take a look at this change?

The main motivation for this patch is to move the setup of the variable list 
for parsing children to the DIE node that corresponds to the block containing 
the children. This will be particularly important for DW_TAG_inlined_subroutine 
because after a recent clang change (https://reviews.llvm.org/D95617), we can 
have inlined subroutines which contain no parameters (and thus no children) in 
the concrete DIE block, but still have some parameters in the abstract function 
(referenced by the inlined subroutine's DW_AT_abstract_origin attribute). If we 
started building the variable list when parsing the first variable in a 
function, we would never create the variable lists for the abstract parameters 
if they were all omitted.

Note that this change is just a refactoring for the actual change that will 
introduce parsing of the omitted abstract parameters. The actual change will be 
uploaded shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110570

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

[Lldb-commits] [PATCH] D110569: [lldb] [Process/FreeBSD] Rework arm64 register access

[Michał Górny via Phabricator via lldb-commits]

mgorny requested changes to this revision.
mgorny added inline comments.
This revision now requires changes to proceed.



Comment at: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:73
 
-Status NativeRegisterContextFreeBSD_arm64::ReadRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
-   m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(
-PT_GETFPREGS, m_thread.GetID(),
-m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::ReadRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsGPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==

These helpers don't seem very helpful. Isn't it better to use `switch` on the 
value returned by `GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg)`?



Comment at: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:133
+  if (IsGPR(reg)) {
+error = ReadGPR();
+if (error.Fail())

To be honest, it seems counterproductive to restore the duplication that I've 
removed before. Is there any real advantage to having two methods here, and 
calling them separately from inside the `if` instead of calling 
`ReadRegisterSet(set)` before the `if`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110569

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

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin created this revision.
jarin added a reviewer: labath.
jarin added a project: LLDB.
Herald added a subscriber: JDevlieghere.
jarin requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.

This change fixes a problem introduced by clang change
described by https://reviews.llvm.org/D95617 and described by
https://bugs.llvm.org/show_bug.cgi?id=50076#c6, where inlined
functions omit the unused parameters both in the stack trace
and in `frame var` command. With this change, the parameters
are listed correctly in the stack trace and in `frame var`
command (the included test tests `frame var`).

This change adds parsing of formal parameters from the abstract
version of inlined functions and use those formal parameters if
they are missing from the concrete version.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110571

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/unused-inlined-params.s
  lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test

Index: lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
@@ -0,0 +1,34 @@
+# UNSUPPORTED: system-darwin, system-windows
+
+# REQUIRES: lld
+
+# RUN: llvm-mc -filetype=obj %S/Inputs/unused-inlined-params.s \
+# RUN: -triple x86_64-pc-linux -o %t.o
+# RUN: ld.lld %t.o -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+breakpoint set -n f
+process launch
+
+# CHECK: Process {{.*}} stopped
+
+# Let us check that unused parameters (of an inlined function) are listed.
+frame variable
+# CHECK-LABEL: frame variable
+# CHECK: (void *) unused1 = <
+# CHECK: (int) used = 42
+# CHECK: (int) unused2 = <
+# CHECK: (int) partial = 1
+# CHECK: (int) unused3 = <
+
+# Step out of the live range of the 'partial' parameter.
+next
+next
+# Check the variable contents.
+frame variable
+# CHECK-LABEL: frame variable
+# CHECK: (void *) unused1 = <
+# CHECK: (int) used = 43
+# CHECK: (int) unused2 = <
+# CHECK: (int) partial = <
+# CHECK: (int) unused3 = <
Index: lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/unused-inlined-params.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/unused-inlined-params.s
@@ -0,0 +1,423 @@
+# Generated from the following program (with some pruning):
+#
+# 1   #include 
+# 2
+# 3   __attribute__((always_inline))
+# 4   void f(void* unused1, int used, int unused2, int partial, int unused3) {
+# 5 used += partial;
+# 6 printf("%i", partial);
+# 7 printf("%i", used);
+# 8   }
+# 9
+# 10  int main(int argc, char** argv) {
+# 11f(argv, 42, 1, argc, 2);
+# 12return 0;
+# 13  }
+	.text
+	.file	"unused-inlined-params.c"
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin1:
+	.file	1 "/example" "unused-inlined-params.c"
+	.loc	1 10 0  # unused-inlined-params.c:10:0
+	.cfi_startproc
+# %bb.0:
+	#DEBUG_VALUE: main:argc <- $edi
+	#DEBUG_VALUE: main:argv <- $rsi
+	#DEBUG_VALUE: f:partial <- $edi
+	pushq	%rbx
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbx, -16
+	movl	%edi, %esi
+.Lline5:
+	#DEBUG_VALUE: main:argv <- [DW_OP_LLVM_entry_value 1] $rsi
+	#DEBUG_VALUE: f:unused3 <- undef
+	#DEBUG_VALUE: f:unused2 <- undef
+	#DEBUG_VALUE: f:used <- 42
+	#DEBUG_VALUE: f:unused1 <- undef
+	#DEBUG_VALUE: f:partial <- $esi
+	#DEBUG_VALUE: main:argc <- $esi
+	.loc	1 5 8 prologue_end  # unused-inlined-params.c:5:8
+	leal	42(%rsi), %ebx
+.Lline6:
+	#DEBUG_VALUE: f:used <- $ebx
+	.loc	1 6 3   # unused-inlined-params.c:6:3
+	movl	$.L.str, %edi
+# kill: def $esi killed $esi killed $rsi
+	xorl	%eax, %eax
+	callq	printf
+.Lline7:
+	.loc	1 7 3   # unused-inlined-params.c:7:3
+	movl	$.L.str, %edi
+	movl	%ebx, %esi
+	xorl	%eax, %eax
+	callq	printf
+.Lline12:
+	.loc	1 12 3  # unused-inlined-params.c:12:3
+	xorl	%eax, %eax
+	popq	%rbx
+.Lreturn1:
+	.cfi_def_cfa_offset 8
+	retq
+.Lfunc_end1:
+	.size	main, .Lfunc_end1-main
+	.cfi_endproc
+# -- End function
+# Dummy printf implementation.
+printf:
+retq
+
+# Simple implementation of _start - set up argc, argv and tail-call main.
+	.globl	_start
+	.p2align	4, 0x90
+	.type	_start,@function
+_start:
+movl(%rsp), %edi
+leaq4(%rsp), %rsi
+jmp main
+
+	.type	.L.str,@object  # @.str
+	.section	.rodata.str1.1,"aMS",@progbits,1
+.L.str:
+	.asciz	"%i"
+	.size	.L.str, 3
+
+	.section	.debug_loc,"",@progbits
+.Ldebug

Re: [Lldb-commits] [lldb] 92b475f - [lldb] silence -Wsometimes-uninitialized warnings

[David Blaikie via lldb-commits]

Maybe this isn't the right fix - msbit and lsbit probably shouldn't be
printed if to_integer returns false in the diagnostic on line 195. If that
diagnostic didn't use the variables, then I don't think this'd warn?

On Mon, Sep 27, 2021 at 12:38 AM Krasimir Georgiev via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

>
> Author: Krasimir Georgiev
> Date: 2021-09-27T09:35:58+02:00
> New Revision: 92b475f0b079e125c588b121dc50116ea5d6d9f2
>
> URL:
> https://github.com/llvm/llvm-project/commit/92b475f0b079e125c588b121dc50116ea5d6d9f2
> DIFF:
> https://github.com/llvm/llvm-project/commit/92b475f0b079e125c588b121dc50116ea5d6d9f2.diff
>
> LOG: [lldb] silence -Wsometimes-uninitialized warnings
>
> No functional changes intended.
>
> Silence warnings from
>
> https://github.com/llvm/llvm-project/commit/3a6ba3675177cb5e47dee325f300aced4cd864ed
> .
>
> Added:
>
>
> Modified:
> lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
> b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
> index a3a9d963c2213..f6a2cdf309815 100644
> --- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
> +++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
> @@ -141,8 +141,8 @@ DynamicRegisterInfo::SetRegisterInfo(const
> StructuredData::Dictionary &dict,
>std::string reg_name_str = matches[1].str();
>std::string msbit_str = matches[2].str();
>std::string lsbit_str = matches[3].str();
> -  uint32_t msbit;
> -  uint32_t lsbit;
> +  uint32_t msbit = 0;
> +  uint32_t lsbit = 0;
>if (llvm::to_integer(msbit_str, msbit) &&
>llvm::to_integer(lsbit_str, lsbit)) {
>  if (msbit > lsbit) {
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110553: [lldb] Remove non-stop mode code

[Jim Ingham via Phabricator via lldb-commits]

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

I don't think this patch added all that much value, and pretty much only worked 
if the non-stop threads never stopped...  I think we'd have to start deeper in 
lldb to if we really want to support having threads that might stop while we 
think the process is stopped, and carrying this forward isn't going to help 
that effort when we get around to it.

So I have no problem with removing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110553

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

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I'm fine with this, but I'll defer to Jonas since he had the last significant 
comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110115

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

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

Hi, could you take a look at this change?

Some discussion points:

- In the ParseVariablesInFunctionContext method, we are using a lambda for the 
recursive parser. We could also use a function-local class or inner class of 
SymbolFileDWARF. Would any of these be preferable?
- The variables created by ParseVariableDIE on abstract formal parameters are 
fairly strange, especially if a function gets inlined into two different 
functions. If that happens, then the parsed variable will refer to a symbol 
context that does not contain the variable DIE and a block can contain a 
variable that is not in the DIE of tree of the block. Is that a big problem? 
(Quick testing of this situation did not reveal any strange stack traces or 
`frame var` anomalies.) Unfortunately, there is no good way to provide the 
correct block and the correct function because LLDB does not parse functions 
and blocks for the abstract functions (i.e., for the DW_TAG_subroutines that 
are referenced by DW_AT_abstract_origin of concrete functions).
- The provided test only tests the case of an inlined function where some 
parameters are unused/omitted. Would it make sense to also provide tests for 
other interesting cases or would that be too much bloat? The particularly 
interesting cases are:
  - Inlined function with all its parameters unused/omitted,
  - Inlined function that is called from different top-level functions.
  - Test correctness of the stack trace in the cases above.
- We could supply a test written in C, but it needs -O1 and is fairly sensitive 
to the meaning of -O1 (e.g., clang started inlining and omitting unsued inlined 
parameters only recently, so changes to -O1 could make a C test easily 
meaningless). Any concerns here?
- The provided test is a bit verbose, mostly because we wanted to mostly 
preserve the structure of the C compiler output. We could still cut the size of 
the test down by removing the main function in favour of _start and by removing 
all the file/line info. Would any of that make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110571

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

[Lldb-commits] [PATCH] D110553: [lldb] Remove non-stop mode code

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.

NetBSD and FreeBSD do not support non-stop in the kernel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110553

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

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Just a nitpick.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3564
+  // If we start a new block, compute a new block context and recurse.
+  Block *block = sc.function->GetBlock(true).FindBlockByID(die.GetID());
+  if (block == nullptr) {

`/*can_create=*/true`



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3573
+if (concrete_block_die)
+  block = sc.function->GetBlock(true).FindBlockByID(
+  concrete_block_die.GetID());

`/*can_create=*/true`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110570

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

[Lldb-commits] [PATCH] D110601: Add -relative-to-command-file to "command source"

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added a reviewer: JDevlieghere.
Herald added a subscriber: dang.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This option was recently added to "command script import" so that an 
"organizing" command file can find python script files relative to itself.  
It's natural to extend this to command files as well as script source files for 
much the same reason.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110601

Files:
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/Options.td
  lldb/test/API/commands/command/source/TestCommandSource.py
  lldb/test/API/commands/command/source/commands2.txt
  lldb/test/API/commands/command/source/subdir/subcmds.txt

Index: lldb/test/API/commands/command/source/subdir/subcmds.txt
===
--- /dev/null
+++ lldb/test/API/commands/command/source/subdir/subcmds.txt
@@ -0,0 +1 @@
+command source -C ../commands.txt
Index: lldb/test/API/commands/command/source/commands2.txt
===
--- /dev/null
+++ lldb/test/API/commands/command/source/commands2.txt
@@ -0,0 +1 @@
+command source -C subdir/subcmds.txt
Index: lldb/test/API/commands/command/source/TestCommandSource.py
===
--- lldb/test/API/commands/command/source/TestCommandSource.py
+++ lldb/test/API/commands/command/source/TestCommandSource.py
@@ -21,7 +21,18 @@
 # Sourcing .lldb in the current working directory, which in turn imports
 # the "my" package that defines the date() function.
 self.runCmd("command source .lldb")
+self.check_results()
+
+@no_debug_info_test
+def test_command_source_relative(self):
+"""Test that lldb command "command source" works correctly with relative paths."""
 
+# Sourcing .lldb in the current working directory, which in turn imports
+# the "my" package that defines the date() function.
+self.runCmd("command source commands2.txt")
+self.check_results()
+
+def check_results(self):
 # Python should evaluate "my.date()" successfully.
 command_interpreter = self.dbg.GetCommandInterpreter()
 self.assertTrue(command_interpreter, VALID_COMMAND_INTERPRETER)
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -536,6 +536,10 @@
 Desc<"If true, stop executing commands on continue.">;
   def source_silent_run : Option<"silent-run", "s">, Arg<"Boolean">,
 Desc<"If true don't echo commands while executing.">;
+  def cmd_relative_to_command_file : Option<"relative-to-command-file", "C">,
+Desc<"Resolve non-absolute paths relative to the location of the "
+"current command file. This argument can only be used when the command is "
+"being sourced from a file.">;
 }
 
 let Command = "alias" in {
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -77,7 +77,7 @@
   public:
 CommandOptions()
 : Options(), m_stop_on_error(true), m_silent_run(false),
-  m_stop_on_continue(true) {}
+  m_stop_on_continue(true), m_cmd_relative_to_command_file(false) {}
 
 ~CommandOptions() override = default;
 
@@ -95,6 +95,10 @@
 error = m_stop_on_continue.SetValueFromString(option_arg);
 break;
 
+  case 'C':
+m_cmd_relative_to_command_file = true;
+break;
+
   case 's':
 error = m_silent_run.SetValueFromString(option_arg);
 break;
@@ -110,6 +114,7 @@
   m_stop_on_error.Clear();
   m_silent_run.Clear();
   m_stop_on_continue.Clear();
+  m_cmd_relative_to_command_file.Clear();
 }
 
 llvm::ArrayRef GetDefinitions() override {
@@ -121,6 +126,7 @@
 OptionValueBoolean m_stop_on_error;
 OptionValueBoolean m_silent_run;
 OptionValueBoolean m_stop_on_continue;
+OptionValueBoolean m_cmd_relative_to_command_file;
   };
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
@@ -131,7 +137,26 @@
   return false;
 }
 
+FileSpec source_dir = {};
+if (m_options.m_cmd_relative_to_command_file) {
+  source_dir = GetDebugger().GetCommandInterpreter().GetCurrentSourceDir();
+  if (!source_dir) {
+result.AppendError("command script import -c can only be specified "
+   "from a command file");
+result.SetStatus(eReturnStatusFailed);
+return false;
+  }
+}
+
 FileSpec cmd_file(command[0].ref());
+if (source_dir) {
+  // Prepend the source_dir to the cmd_f

[Lldb-commits] [PATCH] D110601: Add -relative-to-command-file to "command source"

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

The one thing that makes me sad about this is that "command script import" 
chose "-c" as the short letter for this option, but "-c" was already taken for 
"command source", so I used -C for "command source".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110601

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

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I did a quick pass as I was reading through the patch to understand it. I'll do 
another one tomorrow.




Comment at: lldb/include/lldb/Interpreter/CommandCompletions.h:156-159
+  // This completer works for commands whose only arguments are a command path.
+  // It isn't tied to an argument type because it completes not on a single
+  // argument but on the sequence of arguments, so you have to invoke it by
+  // hand.





Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:238
+eCommandTypesAliases = 0x0008, // aliases such as "po"
+eCommandTypesHidden  = 0x0010,  // commands prefixed with an underscore
 eCommandTypesAllThem = 0x  // all commands

Really these should all be Doxygen comments (`//<`) but I know you're just 
adding values. 



Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:309-311
+  CommandObjectMultiword *VerifyUserMultiwordCmdPath(Args &path,
+ bool leaf_is_command,
+ Status &result);

Please clang-format before landing. 



Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:324
 
+  bool RemoveUserMultiword(llvm::StringRef alias_name);
+

Copy/paste? 



Comment at: lldb/include/lldb/Interpreter/CommandObject.h:198
+Status result;
+result.SetErrorString("can't add a subcommand to a plain command");
+return result;

Maybe say "built-in" command or "non user-defined" command? 



Comment at: lldb/source/Commands/CommandCompletions.cpp:811-813
+  sort(to_delete.begin(), to_delete.end(), std::greater());
+  for (size_t idx : to_delete)
+args.DeleteArgumentAtIndex(idx);

I'm surprised this works. Don't the indexes shift when one's deleted? I assumed 
that's why you're sorting them. 



Comment at: lldb/source/Commands/CommandObjectCommands.cpp:1543
+if (path_error.Fail()) {
+  result.AppendErrorWithFormat("Error in command path: %s",
+   path_error.AsCString());

This should probably be lowercase to be consistent with the other error 
strings. 



Comment at: lldb/source/Commands/CommandObjectCommands.cpp:1710
+!m_interpreter.HasUserMultiwordCommands()) {
+  result.AppendErrorWithFormat("Can only delete user defined commands, "
+   "but no user defined commands found");

Lowercase?



Comment at: lldb/source/Commands/CommandObjectCommands.cpp:1899
+if (num_args == 0) {
+  result.AppendError("No command was specified.");
+  return false;

All the previous errors started with a lowercase letter. I don't have a 
preference, but we should be consistent.



Comment at: lldb/source/Commands/CommandObjectMultiword.cpp:99
 
+Status CommandObjectMultiword::LoadUserSubcommand(
+llvm::StringRef name, const CommandObjectSP &cmd_obj, bool can_replace) {

In line with the direction of preferring `llvm::Error` over 
`lldb_private::Status`, I think these two functions would be good candidates to 
return `llvm::Error`s instead. We can always convert them to a Status up the 
chain. 



Comment at: lldb/source/Commands/CommandObjectMultiword.cpp:102-104
+  if (cmd_obj)
+assert((&GetCommandInterpreter() == &cmd_obj->GetCommandInterpreter()) &&
+   "tried to add a CommandObject from a different interpreter");

This is not going to compile in a non-assert build. You'll want to inline the 
if-check in the assert.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:900
 
+CommandObjectMultiword *CommandInterpreter::VerifyUserMultiwordCmdPath(
+Args &path, bool leaf_is_command, Status &result) {

Similar to my previous comment, this could be an expected instead of using a 
Status as an output argument. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110298

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

[Lldb-commits] [PATCH] D110298: Add "command multiword add" and the ability to add script commands into a user multiword hierarchy

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

The changes look fine from a quick read. See inline comments




Comment at: lldb/source/Commands/CommandObjectCommands.cpp:1817
+  : CommandObjectParsed(
+interpreter, "command multiword add",
+"Add a multiword command to lldb.  Adding to built-"

can't this just be "command add" instead of "command multiword add"? 



Comment at: lldb/source/Commands/CommandObjectCommands.cpp:1959
+  : CommandObjectParsed(
+interpreter, "command multiword delete",
+"Delete a multiword command previously added to "

Could we just modify the existing "command delete" instead of adding a new 
command here?



Comment at: lldb/source/Commands/CommandObjectCommands.cpp:2091-2092
  CommandObjectSP(new 
CommandObjectCommandsDelete(interpreter)));
+  LoadSubCommand("multiword", CommandObjectSP(new 
CommandObjectCommandMultiword(
+  interpreter)));
   LoadSubCommand(

"multiword" makes sense to us LLDB software folks, not sure it will resonate 
with general public. See comments above about maybe doing "command add" and 
then modifying "command delete" to also handle multiword duties


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110298

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

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

Thank you for taking a look, shafik@. I hope you don't mind if I address your 
comments later, once a full review arrives from Pavel (or Johannes).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110570

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