[Lldb-commits] [PATCH] D116707: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

2022-01-07 Thread Lirong Yuan via Phabricator via lldb-commits 
yuanzi updated this revision to Diff 398193.
yuanzi added a comment.

free->xmlFree, remove conversions from std::string to llvm::StringRef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116707

Files:
  lldb/include/lldb/Host/XML.h
  lldb/source/Host/common/XML.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

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
@@ -4351,9 +4351,9 @@
 } else if (name == "osabi") {
   node.GetElementText(target_info.osabi);
 } else if (name == "xi:include" || name == "include") {
-  llvm::StringRef href = node.GetAttributeValue("href");
+  std::string href = node.GetAttributeValue("href");
   if (!href.empty())
-target_info.includes.push_back(href.str());
+target_info.includes.push_back(href);
 } else if (name == "feature") {
   feature_nodes.push_back(node);
 } else if (name == "groups") {
@@ -4392,9 +4392,9 @@
 const XMLNode &node) -> bool {
   llvm::StringRef name = node.GetName();
   if (name == "xi:include" || name == "include") {
-llvm::StringRef href = node.GetAttributeValue("href");
+std::string href = node.GetAttributeValue("href");
 if (!href.empty())
-  target_info.includes.push_back(href.str());
+  target_info.includes.push_back(href);
 }
 return true;
   });
@@ -4530,7 +4530,7 @@
   "Error finding library-list-svr4 xml element");
 
 // main link map structure
-llvm::StringRef main_lm = root_element.GetAttributeValue("main-lm");
+std::string main_lm = root_element.GetAttributeValue("main-lm");
 // FIXME: we're silently ignoring invalid data here
 if (!main_lm.empty())
   llvm::to_integer(main_lm, list.m_link_map);
@@ -4618,15 +4618,15 @@
 "library", [log, &list](const XMLNode &library) -> bool {
   LoadedModuleInfoList::LoadedModuleInfo module;
 
-  llvm::StringRef name = library.GetAttributeValue("name");
-  module.set_name(name.str());
+  std::string name = library.GetAttributeValue("name");
+  module.set_name(name);
 
   // The base address of a given library will be the address of its
   // first section. Most remotes send only one section for Windows
   // targets for example.
   const XMLNode §ion =
   library.FindFirstChildElementWithName("section");
-  llvm::StringRef address = section.GetAttributeValue("address");
+  std::string address = section.GetAttributeValue("address");
   uint64_t address_value = LLDB_INVALID_ADDRESS;
   llvm::to_integer(address, address_value);
   module.set_base(address_value);
Index: lldb/source/Host/common/XML.cpp
===
--- lldb/source/Host/common/XML.cpp
+++ lldb/source/Host/common/XML.cpp
@@ -130,22 +130,25 @@
 #endif
 }
 
-llvm::StringRef XMLNode::GetAttributeValue(const char *name,
-   const char *fail_value) const {
-  const char *attr_value = nullptr;
+std::string XMLNode::GetAttributeValue(const char *name,
+   const char *fail_value) const {
+  std::string attr_value;
 #if LLDB_ENABLE_LIBXML2
-
-  if (IsValid())
-attr_value = (const char *)xmlGetProp(m_node, (const xmlChar *)name);
-  else
-attr_value = fail_value;
+  if (IsValid()) {
+xmlChar *value = xmlGetProp(m_node, (const xmlChar *)name);
+if (value) {
+  attr_value = (const char *)value;
+  xmlFree(value);
+}
+  } else {
+if (fail_value)
+  attr_value = fail_value;
+  }
 #else
-  attr_value = fail_value;
+  if (fail_value)
+attr_value = fail_value;
 #endif
-  if (attr_value)
-return llvm::StringRef(attr_value);
-  else
-return llvm::StringRef();
+  return attr_value;
 }
 
 bool XMLNode::GetAttributeValueAsUnsigned(const char *name, uint64_t &value,
Index: lldb/include/lldb/Host/XML.h
===
--- lldb/include/lldb/Host/XML.h
+++ lldb/include/lldb/Host/XML.h
@@ -76,8 +76,8 @@
 
   XMLNode GetChild() const;
 
-  llvm::StringRef GetAttributeValue(const char *name,
-const char *fail_value = nullptr) const;
+  std::string GetAttributeValue(const char *name,
+const char *fail_value = nullptr) const;
 
   bool GetAttributeValueAsUnsigned(const char *name, uint64_t &value,
uint64_t fail_value = 0, int base = 0) const

[Lldb-commits] [PATCH] D116788: [lldb] Set result error state in 'frame variable'

2022-01-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Commands/CommandObjectFrame.cpp:560-563
   } else if (num_matches == 0) {
-result.GetErrorStream().Printf("error: no variables matched "
-   "the regular expression 
'%s'.\n",
-   entry.c_str());
+result.AppendErrorWithFormat(
+"no variables matched the regular expression '%s'.",
+entry.c_str());

kastiglione wrote:
> kastiglione wrote:
> > this regex error is a weird edge case. For example, considering running:
> > 
> > ```
> > frame var --regex matchesSomeVars doesntMatchAnyVars
> > ```
> > 
> > if the `doesntMatchAnyVars` pattern has no matches, then the command prints 
> > an error, and the result would be marked as an error. But if the 
> > `matchesSomeVars` does have matches, then we have a partial success / 
> > partial failure. In such a case, should the result be marked success, or 
> > failure? I don't know, but I would lean to success since it does entirely 
> > fail. Maybe a user could expect some patterns to match and some to not 
> > match. For example: a user alias that prints any variables based on a set 
> > of patterns they're interested in.
> I think this could be changed to a warning. @jingham what do you think?
Sounds reasonable to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116788

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

[Lldb-commits] [PATCH] D116707: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

2022-01-07 Thread Lirong Yuan via Phabricator via lldb-commits 
yuanzi marked 3 inline comments as done.
yuanzi added inline comments.



Comment at: lldb/source/Host/common/XML.cpp:157-159
+  std::string attr_str = GetAttributeValue(name, "");
+  llvm::StringRef attr(attr_str);
+  return llvm::to_integer(attr, value, base);

labath wrote:
> I don't see why we need these temporary variables. std::string is implicitly 
> convertible to a StringRef, and you don't need the value afterwards, so you 
> should be able to pass the GetAttributeValue straight into the to_integer 
> function. Did that not work for some reason?
I did that mainly to ensure the new code behaves exactly the same as the old. 
Since no complex functions are called, they should work interchangeably! 
Reverted to previous version.



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4534
+std::string main_lm_str = root_element.GetAttributeValue("main-lm");
+llvm::StringRef main_lm(main_lm_str);
 // FIXME: we're silently ignoring invalid data here

labath wrote:
> Same here. `.empty()` and `to_integer` should work on std::string as well. 
> Constructing a StringRef might make sense if we needed to perform some more 
> complex operations (ones which std::string does not support) but I don't see 
> anything like that here.
Right! Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116707

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

[Lldb-commits] [PATCH] D116707: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

2022-01-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision as: JDevlieghere.
JDevlieghere added a comment.

Thanks! LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116707

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

[Lldb-commits] [PATCH] D116539: [lldb/platform-gdb] Clear cached protocol state upon disconnection

2022-01-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: 
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp:84
 bool PlatformAndroidRemoteGDBServer::LaunchGDBServer(lldb::pid_t &pid,
  std::string &connect_url) 
{
   uint16_t remote_port = 0;

Should this have `assert(IsConnected());` like 
`PlatformRemoteGDBServer::LaunchGDBServer` does?



Comment at: 
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp:102
 bool PlatformAndroidRemoteGDBServer::KillSpawnedProcess(lldb::pid_t pid) {
   DeleteForwardPort(pid);
+  return m_gdb_client_up->KillSpawnedProcess(pid);

Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116539

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

[Lldb-commits] [lldb] 7244e9c - [LLDB] libcxx summary formatters for std::string_view

2022-01-07 Thread Jonas Devlieghere via lldb-commits 

Author: Ben Jackson
Date: 2022-01-07T11:41:16-08:00
New Revision: 7244e9c2f5f3ea02cc0d7103fa35782f050aacf0

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

LOG: [LLDB] libcxx summary formatters for std::string_view

When printing a std::string_view, print the referenced string as the
summary. Support string_view, u32string_view, u16string_view and
wstring_view, as we do for std::string and friends.

This is based on the existing fomratter for std::string, and just
extracts the data and length members, pushing them through the existing
string formatter.

In testing this, a "FIXME" was corrected for printing of non-ASCII empty
values. Previously, the "u", 'U" etc. prefixes were not printed for
basic_string<> types that were not char. This is trivial to resolve by
printing the prefix before the "".

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

Added: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/Makefile

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp

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

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index df61cc3853ebc..0fb65f5a317d5 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -579,6 +579,51 @@ static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP 
cpp_category_sp) {
 "std::__[[:alnum:]]+::allocator >$"),
 stl_summary_flags, true);
 
+  AddCXXSummary(cpp_category_sp,
+lldb_private::formatters::LibcxxStringViewSummaryProviderASCII,
+"std::string_view summary provider",
+ConstString("^std::__[[:alnum:]]+::string_view$"),
+stl_summary_flags, true);
+  AddCXXSummary(cpp_category_sp,
+lldb_private::formatters::LibcxxStringViewSummaryProviderASCII,
+"std::string_view summary provider",
+ConstString("^std::__[[:alnum:]]+::basic_string_view >$"),
+stl_summary_flags, true);
+  AddCXXSummary(
+  cpp_category_sp,
+  lldb_private::formatters::LibcxxStringViewSummaryProviderASCII,
+  "std::string_view summary provider",
+  ConstString("^std::__[[:alnum:]]+::basic_string_view >$"),
+  stl_summary_flags, true);
+
+  AddCXXSummary(cpp_category_sp,
+lldb_private::formatters::LibcxxStringViewSummaryProviderUTF16,
+"std::u16string_view summary provider",
+ConstString("^std::__[[:alnum:]]+::basic_string_view >$"),
+stl_summary_flags, true);
+
+  AddCXXSummary(cpp_category_sp,
+lldb_private::formatters::LibcxxStringViewSummaryProviderUTF32,
+"std::u32string_view summary provider",
+ConstString("^std::__[[:alnum:]]+::basic_string_view >$"),
+stl_summary_flags, true);
+
+  AddCXXSummary(cpp_category_sp,
+lldb_private::formatters::LibcxxWStringViewSummaryProvider,
+"std::wstring_view summary provider",
+ConstString("^std::__[[:alnum:]]+::wstring_view$"),
+stl_summary_flags, true);
+  AddCXXSummary(cpp_category_sp,
+lldb_private::formatters::LibcxxWStringViewSummaryProvider,
+"std::wstring_view summary provider",
+ConstString("^std::__[[:alnum:]]+::basic_string_view >$"),
+stl_summary_flags, true);
+
   SyntheticChildren::Flags stl_synth_flags;
   stl_synth_flags.SetCascades(true).SetSkipPointers(false).SetSkipReferences(
   false);

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index b9aef0ae7d9eb..21196393371ea 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -19,6 +19,7 @@
 #include "lldb/Target/ProcessStructReader.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Status.h"
@@ -26,6 +27,7 @@
 
 #include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+#include 
 
 using namespace lldb;
 us

[Lldb-commits] [PATCH] D112222: [LLDB] libcxx summary formatters for std::string_view

2022-01-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7244e9c2f5f3: [LLDB] libcxx summary formatters for 
std::string_view (authored by puremourning, committed by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D11

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
@@ -0,0 +1,108 @@
+#include 
+#include 
+#include 
+
+static size_t touch_string(std::string_view &in_str_view) {
+  return in_str_view.size(); // Break here to look at bad string view.
+}
+
+int main() {
+  std::wstring_view wempty(L"");
+  std::wstring_view s(L"hello world! מזל טוב!");
+  std::wstring_view S(L"");
+  std::string_view empty("");
+  std::string q_source = "hello world";
+  std::string_view q(q_source);
+  std::string_view Q("quite a long std::strin with lots of info inside it");
+  std::string_view TheVeryLongOne(
+  "123456789012345678901234567890123456789012345678901234567890123456789012"
+  "345678901234567890123456789012345678901234567890123456789012345678901234"
+  "567890123456789012345678901234567890123456789012345678901234567890123456"
+  "789012345678901234567890123456789012345678901234567890123456789012345678"
+  "901234567890123456789012345678901234567890123456789012345678901234567890"
+  "123456789012345678901234567890123456789012345678901234567890123456789012"
+  "345678901234567890123456789012345678901234567890123456789012345678901234"
+  "567890123456789012345678901234567890123456789012345678901234567890123456"
+  "789012345678901234567890123456789012345678901234567890123456789012345678"
+  "901234567890123456789012345678901234567890123456789012345678901234567890"
+  "123456789012345678901234567890123456789012345678901234567890123456789012"
+  "345678901234567890123456789012345678901234567890123456789012345678901234"
+  "567890123456789012345678901234567890123456789012345678901234567890123456"
+  "789012345678901234567890123456789012345678901234567890123456789012345678"
+  "901234567890123456789012345678901234567890123456789012345678901234567890"
+  "123456789012345678901234567890123456789012345678901234567890123456789012"
+  "345678901234567890123456789012345678901234567890123456789012345678901234"
+  "567890123456789012345678901234567890123456789012345678901234567890123456"
+  "789012345678901234567890123456789012345678901234567890123456789012345678"
+  "901234567890123456789012345678901234567890123456789012345678901234567890"
+  "123456789012345678901234567890123456789012345678901234567890123456789012"
+  "345678901234567890123456789012345678901234567890123456789012345678901234"
+  "567890123456789012345678901234567890123456789012345678901234567890123456"
+  "789012345678901234567890123456789012345678901234567890123456789012345678"
+  "901234567890123456789012345678901234567890123456789012345678901234567890"
+  "123456789012345678901234567890123456789012345678901234567890123456789012"
+  "345678901234567890123456789012345678901234567890123456789012345678901234"
+  "567890123456789012345678901234567890123456789012345678901234567890123456"
+  "789012345678901234567890123456789012345678901234567890123456789012345678"
+  "901234567890123456789012345678901234567890123456789012345678901234567890"
+  "123456789012345678901234567890123456789012345678901234567890123456789012"
+  "345678901234567890123456789012345678901234567890123456789012345678901234"
+  "567890123456789012345678901234567890123456789012345678901234567890123456"
+  "789012345678901234567890123456789012345678901234567890123456789012345678"
+  "9012345678901234567890123456789012345678901234567890someText123456789012"
+  "345678901234567890123456789012345678901234567890123456789012345678901234"
+  "567890123456789012345678901234567890123456789012345678901234567890123456"
+  "789012345678901234567890123456789012345678901234567890123456789012345678"
+  "901234567890123456789012345678901234567890123456789012345678901234567890"
+  "123456789012345678901234567890123456789012345678901234567890123456789012"
+  "3456789012345678901234567890123456789012345

[Lldb-commits] [lldb] 4f6d3a3 - [LLDB] Fix setting of success in Socket::Close()

2022-01-07 Thread Shafik Yaghmour via lldb-commits 

Author: Shafik Yaghmour
Date: 2022-01-07T12:42:58-08:00
New Revision: 4f6d3a376c9faba93bbdf105966cea7585b0b8e9

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

LOG: [LLDB] Fix setting of success in Socket::Close()

Both close and closesocket should return 0 on success so using !! looks 
incorrect. I replaced this will a more readable == 0 check.

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

Added: 


Modified: 
lldb/source/Host/common/Socket.cpp

Removed: 




diff  --git a/lldb/source/Host/common/Socket.cpp 
b/lldb/source/Host/common/Socket.cpp
index cc06597975300..1c74a8fb59029 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -281,9 +281,9 @@ Status Socket::Close() {
 static_cast(this), static_cast(m_socket));
 
 #if defined(_WIN32)
-  bool success = !!closesocket(m_socket);
+  bool success = closesocket(m_socket) == 0;
 #else
-  bool success = !!::close(m_socket);
+  bool success = ::close(m_socket) == 0;
 #endif
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;



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

[Lldb-commits] [PATCH] D116768: Fix setting of success in Socket::Close()

2022-01-07 Thread Shafik Yaghmour via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f6d3a376c9f: [LLDB] Fix setting of success in 
Socket::Close() (authored by shafik).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116768

Files:
  lldb/source/Host/common/Socket.cpp


Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -281,9 +281,9 @@
 static_cast(this), static_cast(m_socket));
 
 #if defined(_WIN32)
-  bool success = !!closesocket(m_socket);
+  bool success = closesocket(m_socket) == 0;
 #else
-  bool success = !!::close(m_socket);
+  bool success = ::close(m_socket) == 0;
 #endif
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;


Index: lldb/source/Host/common/Socket.cpp
===
--- lldb/source/Host/common/Socket.cpp
+++ lldb/source/Host/common/Socket.cpp
@@ -281,9 +281,9 @@
 static_cast(this), static_cast(m_socket));
 
 #if defined(_WIN32)
-  bool success = !!closesocket(m_socket);
+  bool success = closesocket(m_socket) == 0;
 #else
-  bool success = !!::close(m_socket);
+  bool success = ::close(m_socket) == 0;
 #endif
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116461: [lldb] Remove ProcessStructReader from NSStringSummaryProvider (NFC)

2022-01-07 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

This doesn't look any less readable than the code it replaces and is definitely 
faster.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116461

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

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

2022-01-07 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py:76
+
+self.runCmd("run", RUN_SUCCEEDED)
+

Doesn't `lldbutil.run_break_set_by_symbol()` do the "file" and "run" already 
and this run command runs it a second time?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116419

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

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

2022-01-07 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: lldb/source/Commands/CommandObjectMemory.cpp:1695
+  section_sp = section_sp->GetParent();
+section_name = section_sp->GetName();
+  }

Why did `section_sp->GetName();` return the Segment name in the original code? 
Is that a bug?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116419

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

[Lldb-commits] [PATCH] D112709: [lldb] Fix matchers for char array formatters

2022-01-07 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added a comment.

> it can vary on some platforms it is signed char but on others it is bool.

For those interested, Objective-C `BOOL` is a signed char on macOS and 32-bit 
iOS and `bool` on 64-bit iOS and derived platforms (watchOS & tvOS).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112709

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

[Lldb-commits] [lldb] ab76189 - [lldb] Use lit_config.note to print module cache message

2022-01-07 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-01-07T13:35:18-08:00
New Revision: ab7618914dec6384f24b30ad2d45b82a51647a33

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

LOG: [lldb] Use lit_config.note to print module cache message

Added: 


Modified: 
lldb/test/API/lit.cfg.py
lldb/test/Shell/lit.cfg.py

Removed: 




diff  --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index a7a9e4554b177..8120c4f716052 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -98,7 +98,7 @@ def delete_module_cache(path):
   This is necessary in an incremental build whenever clang changes underneath,
   so doing it once per lit.py invocation is close enough. """
   if os.path.isdir(path):
-print("Deleting module cache at %s." % path)
+lit_config.note("Deleting module cache at %s." % path)
 shutil.rmtree(path)
 
 if is_configured('llvm_use_sanitizer'):

diff  --git a/lldb/test/Shell/lit.cfg.py b/lldb/test/Shell/lit.cfg.py
index aecd9d62c2144..ccff49194319b 100644
--- a/lldb/test/Shell/lit.cfg.py
+++ b/lldb/test/Shell/lit.cfg.py
@@ -85,7 +85,7 @@ def calculate_arch_features(arch_string):
 # lit.py invocation is close enough.
 for cachedir in [config.clang_module_cache, config.lldb_module_cache]:
   if os.path.isdir(cachedir):
- print("Deleting module cache at %s."%cachedir)
+ lit_config.note("Deleting module cache at %s."%cachedir)
  shutil.rmtree(cachedir)
 
 # Set a default per-test timeout of 10 minutes. Setting a timeout per test



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

[Lldb-commits] [lldb] d13da5f - [lldb] Remove lldbconfig module

2022-01-07 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-01-07T15:58:43-08:00
New Revision: d13da5f0da1ca39d2130f4ae602184350c3ec968

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

LOG: [lldb] Remove lldbconfig module

The lldbconfig module was necessary to run the LLDB test suite against a
reproducer. Since this functionality has been removed, the module is no
longer necessary.

Added: 


Modified: 
lldb/bindings/python/python.swig

Removed: 
lldb/packages/Python/lldbconfig/__init__.py



diff  --git a/lldb/bindings/python/python.swig 
b/lldb/bindings/python/python.swig
index 5dcbd68d85447..cb80e1be61a82 100644
--- a/lldb/bindings/python/python.swig
+++ b/lldb/bindings/python/python.swig
@@ -133,15 +133,8 @@ using namespace lldb;
 %include "python-wrapper.swig"
 
 %pythoncode%{
-_initialize = True
-try:
-   import lldbconfig
-   _initialize = lldbconfig.INITIALIZE
-except ImportError:
-   pass
 debugger_unique_id = 0
-if _initialize:
-   SBDebugger.Initialize()
+SBDebugger.Initialize()
 debugger = None
 target = None
 process = None

diff  --git a/lldb/packages/Python/lldbconfig/__init__.py 
b/lldb/packages/Python/lldbconfig/__init__.py
deleted file mode 100644
index 6c43d709df71b..0
--- a/lldb/packages/Python/lldbconfig/__init__.py
+++ /dev/null
@@ -1 +0,0 @@
-INITIALIZE = True



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

[Lldb-commits] [lldb] 69c8e64 - [formatters] Improve documentation

2022-01-07 Thread Walter Erquinigo via lldb-commits 

Author: Walter Erquinigo
Date: 2022-01-07T16:27:03-08:00
New Revision: 69c8e64ba6be090d25657ca0fc2bc53797907837

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

LOG: [formatters] Improve documentation

This adds some important remarks to the data formatter documentation.

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

Added: 


Modified: 
lldb/docs/use/variable.rst

Removed: 




diff  --git a/lldb/docs/use/variable.rst b/lldb/docs/use/variable.rst
index 0250de5600ffe..e1adb96d2ad88 100644
--- a/lldb/docs/use/variable.rst
+++ b/lldb/docs/use/variable.rst
@@ -7,7 +7,7 @@ Variable Formatting
 LLDB has a data formatters subsystem that allows users to define custom display
 options for their variables.
 
-Usually, when you type frame variable or run some expression LLDB will
+Usually, when you type ``frame variable`` or run some expression LLDB will
 automatically choose the way to display your results on a per-type basis, as in
 the following example:
 
@@ -17,7 +17,11 @@ the following example:
(uint8_t) x = 'a'
(intptr_t) y = 124752287
 
-However, in certain cases, you may want to associate a 
diff erent style to the display for certain datatypes. To do so, you need to 
give hints to the debugger
+Note: ``frame variable`` without additional arguments prints the list of
+variables of the current frame.
+
+However, in certain cases, you may want to associate a 
diff erent style to the
+display for certain datatypes. To do so, you need to give hints to the debugger
 as to how variables should be displayed. The LLDB type command allows you to do
 just that.
 
@@ -29,6 +33,63 @@ Using it you can change your visualization to look like this:
(uint8_t) x = chr='a' dec=65 hex=0x41
(intptr_t) y = 0x76f919f
 
+In addition, some data structures can encode their data in a way that is not
+easily readable to the user, in which case a data formatter can be used to
+show the data in a human readable way. For example, without a formatter,
+printing a ``std::deque`` with the elements ``{2, 3, 4, 5, 6}`` would
+result in something like:
+
+::
+
+   (lldb) frame variable a_deque
+   (std::deque >) $0 = {
+  std::_Deque_base > = {
+ _M_impl = {
+_M_map = 0x0062ceb0
+_M_map_size = 8
+_M_start = {
+   _M_cur = 0x0062cf00
+   _M_first = 0x0062cf00
+   _M_last = 0x0062d2f4
+   _M_node = 0x0062cec8
+}
+_M_finish = {
+   _M_cur = 0x0062d300
+   _M_first = 0x0062d300
+   _M_last = 0x0062d6f4
+   _M_node = 0x0062ced0
+}
+ }
+  }
+   }
+
+which is very hard to make sense of.
+
+Note: ``frame variable `` prints out the variable  in the current
+frame.
+
+On the other hand, a proper formatter is able to produce the following output:
+
+::
+
+   (lldb) frame variable a_deque
+   (std::deque >) $0 = size=5 {
+  [0] = 2
+  [1] = 3
+  [2] = 4
+  [3] = 5
+  [4] = 6
+   }
+
+which is what the user would expect from a good debugger.
+
+Note: you can also use ``v `` instead of ``frame variable ``.
+
+It's worth mentioning that the ``size=5`` string is produced by a summary
+provider and the list of children is produced by a synthetic child provider.
+More information about these providers is available later in this document.
+
+
 There are several features related to data visualization: formats, summaries,
 filters, synthetic children.
 
@@ -871,18 +932,26 @@ be implemented by the Python class):
  this call should return a new LLDB SBValue object representing the 
child at the index given as argument
   def update(self):
  this call should be used to update the internal state of this Python 
object whenever the state of the variables in LLDB changes.[1]
+ Also, this method is invoked before any other method in the interface.
   def has_children(self):
  this call should return True if this object might have children, and 
False if this object can be guaranteed not to have children.[2]
   def get_value(self):
  this call can return an SBValue to be presented as the value of the 
synthetic value under consideration.[3]
 
-[1] This method is optional. Also, it may optionally choose to return a value
-(starting with SVN rev153061/LLDB-134). If it returns a value, and that value
-is True, LLDB will be allowed to cache the children and the children count it
-previously obtained, and will not return to the provider class to ask. If
-nothing, None, or anything other than True is returned, LLDB will discard the
-cached information and ask. Regardless, whenever necessa

[Lldb-commits] [PATCH] D115974: [formatters] Improve documentation

2022-01-07 Thread Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG69c8e64ba6be: [formatters] Improve documentation (authored 
by Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115974

Files:
  lldb/docs/use/variable.rst

Index: lldb/docs/use/variable.rst
===
--- lldb/docs/use/variable.rst
+++ lldb/docs/use/variable.rst
@@ -7,7 +7,7 @@
 LLDB has a data formatters subsystem that allows users to define custom display
 options for their variables.
 
-Usually, when you type frame variable or run some expression LLDB will
+Usually, when you type ``frame variable`` or run some expression LLDB will
 automatically choose the way to display your results on a per-type basis, as in
 the following example:
 
@@ -17,7 +17,11 @@
(uint8_t) x = 'a'
(intptr_t) y = 124752287
 
-However, in certain cases, you may want to associate a different style to the display for certain datatypes. To do so, you need to give hints to the debugger
+Note: ``frame variable`` without additional arguments prints the list of
+variables of the current frame.
+
+However, in certain cases, you may want to associate a different style to the
+display for certain datatypes. To do so, you need to give hints to the debugger
 as to how variables should be displayed. The LLDB type command allows you to do
 just that.
 
@@ -29,6 +33,63 @@
(uint8_t) x = chr='a' dec=65 hex=0x41
(intptr_t) y = 0x76f919f
 
+In addition, some data structures can encode their data in a way that is not
+easily readable to the user, in which case a data formatter can be used to
+show the data in a human readable way. For example, without a formatter,
+printing a ``std::deque`` with the elements ``{2, 3, 4, 5, 6}`` would
+result in something like:
+
+::
+
+   (lldb) frame variable a_deque
+   (std::deque >) $0 = {
+  std::_Deque_base > = {
+ _M_impl = {
+_M_map = 0x0062ceb0
+_M_map_size = 8
+_M_start = {
+   _M_cur = 0x0062cf00
+   _M_first = 0x0062cf00
+   _M_last = 0x0062d2f4
+   _M_node = 0x0062cec8
+}
+_M_finish = {
+   _M_cur = 0x0062d300
+   _M_first = 0x0062d300
+   _M_last = 0x0062d6f4
+   _M_node = 0x0062ced0
+}
+ }
+  }
+   }
+
+which is very hard to make sense of.
+
+Note: ``frame variable `` prints out the variable  in the current
+frame.
+
+On the other hand, a proper formatter is able to produce the following output:
+
+::
+
+   (lldb) frame variable a_deque
+   (std::deque >) $0 = size=5 {
+  [0] = 2
+  [1] = 3
+  [2] = 4
+  [3] = 5
+  [4] = 6
+   }
+
+which is what the user would expect from a good debugger.
+
+Note: you can also use ``v `` instead of ``frame variable ``.
+
+It's worth mentioning that the ``size=5`` string is produced by a summary
+provider and the list of children is produced by a synthetic child provider.
+More information about these providers is available later in this document.
+
+
 There are several features related to data visualization: formats, summaries,
 filters, synthetic children.
 
@@ -871,18 +932,26 @@
  this call should return a new LLDB SBValue object representing the child at the index given as argument
   def update(self):
  this call should be used to update the internal state of this Python object whenever the state of the variables in LLDB changes.[1]
+ Also, this method is invoked before any other method in the interface.
   def has_children(self):
  this call should return True if this object might have children, and False if this object can be guaranteed not to have children.[2]
   def get_value(self):
  this call can return an SBValue to be presented as the value of the synthetic value under consideration.[3]
 
-[1] This method is optional. Also, it may optionally choose to return a value
-(starting with SVN rev153061/LLDB-134). If it returns a value, and that value
-is True, LLDB will be allowed to cache the children and the children count it
-previously obtained, and will not return to the provider class to ask. If
-nothing, None, or anything other than True is returned, LLDB will discard the
-cached information and ask. Regardless, whenever necessary LLDB will call
-update.
+As a warning, exceptions that are thrown by python formatters are caught
+silently by LLDB and should be handled appropriately by the formatter itself.
+Being more specific, in case of exceptions, LLDB might assume that the given
+object has no children or it might skip printing some children, as they are
+printed one by one.
+
+[1] This method is optional. Also, a boolean value must be retu

[Lldb-commits] [PATCH] D116845: [LLDB][NativePDB] Add support for inlined functions

2022-01-07 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu created this revision.
zequanwu added reviewers: labath, rnk, akhuang.
Herald added a subscriber: arphaman.
zequanwu requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.
Herald added a project: LLDB.

This adds inline function support to NativePDB by parsing S_INLINESITE records
to retrieve inlinee line info and add them into line table at `ParseLineTable`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116845

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/inline_sites.s

Index: lldb/test/Shell/SymbolFile/NativePDB/inline_sites.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/inline_sites.s
@@ -0,0 +1,667 @@
+# clang-format off
+# REQUIRES: lld, x86
+
+# RUN: llvm-mc -triple=x86_64-windows-msvc --filetype=obj %s > %t.obj
+# RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj  -out:%t.exe
+# RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+# RUN: %p/Inputs/inline_sites.lldbinit 2>&1 | FileCheck %s
+
+# Compiled from the following files, but replaced the call to abort with nop.
+# a.cpp:
+# #include "stdlib.h"
+# #include "a.h"
+# int main(int argc, char** argv) {
+#   Namespace1::foo(2);
+#   return 0;
+# }
+# a.h:
+# #include "b.h"
+# namespace Namespace1 {
+# inline void foo(int x) {
+#   static volatile int gv_foo;
+#   ++gv_foo;
+#   if (!gv_foo)
+# abort();
+#   Class1::bar(x + 1);
+# }
+# }
+# b.h:
+# #include "c.h"
+# class Class1 {
+# public:
+#   inline static void bar(int x) {
+# static volatile int gv_bar;
+# ++gv_bar;
+# Namespace2::Class2::func(x + 1);
+#   }
+# };
+# c.h:
+# namespace Namespace2{
+#   class Class2{
+# public:
+# inline static void func(int x) {
+#   static volatile int gv_func;
+#   gv_func += x;
+# }
+#   };
+# }
+
+# CHECK:  (lldb) image dump line-table a.cpp -v
+# CHECK-NEXT: Line table for /tmp/a.cpp in
+# CHECK-NEXT: 0x000140001000: /tmp/a.cpp:3
+# CHECK-NEXT: 0x000140001004: /tmp/a.h:5, is_start_of_statement = TRUE, is_prologue_end = TRUE
+# CHECK-NEXT: 0x00014000100a: /tmp/a.h:6
+# CHECK-NEXT: 0x000140001014: /tmp/b.h:6, is_start_of_statement = TRUE, is_prologue_end = TRUE
+# CHECK-NEXT: 0x00014000101a: /tmp/c.h:6, is_start_of_statement = TRUE, is_prologue_end = TRUE
+# CHECK-NEXT: 0x000140001021: /tmp/a.cpp:5
+# CHECK-NEXT: 0x000140001028: /tmp/a.h:7, is_start_of_statement = TRUE
+# CHECK-NEXT: 0x00014000102a: /tmp/a.cpp:5, is_terminal_entry = TRUE
+
+# CEHCK: (lldb) b a.cpp:5
+# CHECK: Breakpoint 1: where = {{.*}}`main + 33 at a.cpp:5, address = 0x000140001021
+# CEHCK: (lldb) b a.h:5
+# CHECK: Breakpoint 2: where = {{.*}}`main + 4 [inlined] Namespace1::foo at a.h:5, address = 0x000140001004
+# CEHCK: (lldb) b a.h:6
+# CHECK: Breakpoint 3: where = {{.*}}`main + 10 [inlined] Namespace1::foo + 6 at a.h:6, address = 0x00014000100a
+# CEHCK: (lldb) b a.h:7
+# CHECK: Breakpoint 4: where = {{.*}}`main + 40 [inlined] Namespace1::foo at a.h:7, address = 0x000140001028
+# CEHCK: (lldb) b b.h:6
+# CHECK: Breakpoint 5: where = {{.*}}`main + 20 [inlined] Class1::bar at b.h:6, address = 0x000140001014
+# CEHCK: (lldb) b c.h:6
+# CHECK: Breakpoint 6: where = {{.*}}`main + 26 [inlined] Namespace2::Class2::func at c.h:6, address = 0x00014000101a
+
+# CEHCK-LABEL: (lldb) image lookup -a 0x140001003 -v
+# CHECK:  Summary: {{.*}}`main + 3 at a.cpp:3
+# CHECK: Function: id = {{.*}}, name = "main", range = [0x000140001000-0x00014000102a)
+# CHECK:   Blocks: id = {{.*}}, range = [0x140001000-0x14000102a)
+# CHECK:LineEntry: [0x000140001000-0x000140001004): /tmp/a.cpp:3
+
+# CEHCK-LABEL: (lldb) image lookup -a 0x140001004 -v
+# CHECK:  Summary: {{.*}}`main + 4 [inlined] Namespace1::foo at a.h:5
+# CHECK-NEXT:  {{.*}}`main + 4 at a.cpp:4
+# CHECK: Function: id = {{.*}}, name = "main", range = [0x000140001000-0x00014000102a)
+# CHECK:   Blocks: id = {{.*}}, range = [0x140001000-0x14000102a)
+# CHECK-NEXT:  id = {{.*}}, ranges = [0x140001004-0x140001021)[0x140001028-0x14000102a), name = "Namespace1::foo", decl = a.h:3
+# CHECK:LineEntry: [0x000140001004-0x00014000100a): /tmp/a.h:5
+
+# CEHCK-LABEL: (lldb) image lookup -a 0x140001014 -v
+# CHECK:  Summary: {{.*}}`main + 20 [inlined] Class1::bar at b.h:6
+# CHECK-NEXT:  {{.*}}`main + 20 [inlined] Namespace1::foo + 16 at a.h:8
+# CHECK-NEXT:  {{.*}}`main + 4 at a.cpp:4
+# CHECK: Function: id = {{.*}}, name = "main", range = [0x00014000

[Lldb-commits] [PATCH] D116847: [lldb] Remove most of the reproducer instrumentation

2022-01-07 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

With regret, I accept this patch. Too bad couldn't make it work!


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

https://reviews.llvm.org/D116847

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

[Lldb-commits] [PATCH] D116467: Deprecate `LLVM_LIBRARY_DIRS`

2022-01-07 Thread John Ericson via Phabricator via lldb-commits 
Ericson2314 updated this revision to Diff 398286.
Ericson2314 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116467

Files:
  lld/CMakeLists.txt
  lldb/cmake/modules/LLDBConfig.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in

Index: llvm/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/cmake/modules/LLVMConfig.cmake.in
+++ llvm/cmake/modules/LLVMConfig.cmake.in
@@ -103,7 +103,6 @@
 set(LLVM_LIBDIR_SUFFIX @LLVM_LIBDIR_SUFFIX@)
 
 set(LLVM_INCLUDE_DIRS "@LLVM_CONFIG_INCLUDE_DIRS@")
-set(LLVM_LIBRARY_DIRS "@LLVM_CONFIG_LIBRARY_DIRS@")
 
 set(LLVM_APPEND_VC_REV "@LLVM_APPEND_VC_REV@")
 
@@ -112,9 +111,8 @@
 # and generated include directories while the following variables have
 # them split.
 
-# These are the "main" dirs
+# This is the "main" dir
 set(LLVM_MAIN_INCLUDE_DIR "@LLVM_CONFIG_MAIN_INCLUDE_DIR@")
-set(LLVM_LIBRARY_DIR "@LLVM_CONFIG_LIBRARY_DIR@")
 
 # This is a secondary one for generated files
 set(LLVM_INCLUDE_DIR "@LLVM_CONFIG_INCLUDE_DIR@")
@@ -124,10 +122,15 @@
 set(LLVM_CMAKE_DIR "@LLVM_CONFIG_CMAKE_DIR@")
 set(LLVM_TOOLS_BINARY_DIR "@LLVM_CONFIG_TOOLS_BINARY_DIR@")
 set(LLVM_TOOLS_INSTALL_DIR "@LLVM_TOOLS_INSTALL_DIR@")
+set(LLVM_LIBRARY_DIR "@LLVM_CONFIG_LIBRARY_DIR@")
 set(LLVM_HAVE_OPT_VIEWER_MODULES @LLVM_HAVE_OPT_VIEWER_MODULES@)
 set(LLVM_CONFIGURATION_TYPES @CMAKE_CONFIGURATION_TYPES@)
 set(LLVM_ENABLE_SHARED_LIBS @BUILD_SHARED_LIBS@)
 
+# This is deprecated and just here for backwards compatibility. It will be
+# removed in the future.
+set(LLVM_LIBRARY_DIRS "${LLVM_LIBRARY_DIR}")
+
 set(LLVM_DEFAULT_EXTERNAL_LIT "@LLVM_CONFIG_DEFAULT_EXTERNAL_LIT@")
 set(LLVM_LIT_ARGS "@LLVM_LIT_ARGS@")
 
Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -56,16 +56,10 @@
   )
 list(REMOVE_DUPLICATES LLVM_CONFIG_INCLUDE_DIRS)
 
-set(LLVM_CONFIG_LIBRARY_DIR "${LLVM_LIBRARY_DIR}")
-set(LLVM_CONFIG_LIBRARY_DIRS
-  "${LLVM_CONFIG_LIBRARY_DIR}"
-  # FIXME: Should there be other entries here?
-  )
-list(REMOVE_DUPLICATES LLVM_CONFIG_LIBRARY_DIRS)
-
 set(LLVM_CONFIG_BINARY_DIR "${LLVM_BINARY_DIR}")
 set(LLVM_CONFIG_CMAKE_DIR "${CMAKE_CURRENT_SOURCE_DIR}")
 set(LLVM_CONFIG_TOOLS_BINARY_DIR "${LLVM_TOOLS_BINARY_DIR}")
+set(LLVM_CONFIG_LIBRARY_DIR "${LLVM_LIBRARY_DIR}")
 
 # Generate a default location for lit
 if (LLVM_BUILD_UTILS)
@@ -122,16 +116,10 @@
   )
 list(REMOVE_DUPLICATES LLVM_CONFIG_INCLUDE_DIRS)
 
-extend_path(LLVM_CONFIG_LIBRARY_DIR "\${LLVM_INSTALL_PREFIX}" "lib\${LLVM_LIBDIR_SUFFIX}")
-set(LLVM_CONFIG_LIBRARY_DIRS
-  "${LLVM_CONFIG_LIBRARY_DIR}"
-  # FIXME: Should there be other entries here?
-  )
-list(REMOVE_DUPLICATES LLVM_CONFIG_LIBRARY_DIRS)
-
 set(LLVM_CONFIG_BINARY_DIR "\${LLVM_INSTALL_PREFIX}")
 extend_path(LLVM_CONFIG_CMAKE_DIR "\${LLVM_INSTALL_PREFIX}" "${LLVM_INSTALL_PACKAGE_DIR}")
 extend_path(LLVM_CONFIG_TOOLS_BINARY_DIR "\${LLVM_INSTALL_PREFIX}" "${LLVM_TOOLS_INSTALL_DIR}")
+extend_path(LLVM_CONFIG_LIBRARY_DIR "\${LLVM_INSTALL_PREFIX}" "lib\${LLVM_LIBDIR_SUFFIX}")
 
 # Generate a default location for lit
 if (LLVM_INSTALL_UTILS AND LLVM_BUILD_UTILS)
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -262,7 +262,7 @@
   set(LLDB_CLANG_RESOURCE_DIR_NAME "${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}")
   # Iterate over the possible places where the external resource directory
   # could be and pick the first that exists.
-  foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"
+  foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIR}"
 "${LLVM_BUILD_LIBRARY_DIR}"
 "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
 # Build the resource directory path by appending 'clang/'.
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -82,7 +82,7 @@
   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
 
   include_directories("${LLVM_BINARY_DIR}/include" ${LLVM_INCLUDE_DIRS})
-  link_directories(${LLVM_LIBRARY_DIRS})
+  link_directories("${LLVM_LIBRARY_DIR}")
 
   if(LLVM_INCLUDE_TESTS)
 find_package(Python3 ${LLVM_MINIMUM_PYTHON_VERSION} REQUIRED
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116467: Deprecate `LLVM_LIBRARY_DIRS`

2022-01-07 Thread John Ericson via Phabricator via lldb-commits 
Ericson2314 updated this revision to Diff 398287.
Ericson2314 added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116467

Files:
  lld/CMakeLists.txt
  lldb/cmake/modules/LLDBConfig.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in

Index: llvm/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/cmake/modules/LLVMConfig.cmake.in
+++ llvm/cmake/modules/LLVMConfig.cmake.in
@@ -103,7 +103,6 @@
 set(LLVM_LIBDIR_SUFFIX @LLVM_LIBDIR_SUFFIX@)
 
 set(LLVM_INCLUDE_DIRS "@LLVM_CONFIG_INCLUDE_DIRS@")
-set(LLVM_LIBRARY_DIRS "@LLVM_CONFIG_LIBRARY_DIRS@")
 
 set(LLVM_APPEND_VC_REV "@LLVM_APPEND_VC_REV@")
 
@@ -112,9 +111,8 @@
 # and generated include directories while the following variables have
 # them split.
 
-# These are the "main" dirs
+# This is the "main" dir
 set(LLVM_MAIN_INCLUDE_DIR "@LLVM_CONFIG_MAIN_INCLUDE_DIR@")
-set(LLVM_LIBRARY_DIR "@LLVM_CONFIG_LIBRARY_DIR@")
 
 # This is a secondary one for generated files
 set(LLVM_INCLUDE_DIR "@LLVM_CONFIG_INCLUDE_DIR@")
@@ -124,10 +122,15 @@
 set(LLVM_CMAKE_DIR "@LLVM_CONFIG_CMAKE_DIR@")
 set(LLVM_TOOLS_BINARY_DIR "@LLVM_CONFIG_TOOLS_BINARY_DIR@")
 set(LLVM_TOOLS_INSTALL_DIR "@LLVM_TOOLS_INSTALL_DIR@")
+set(LLVM_LIBRARY_DIR "@LLVM_CONFIG_LIBRARY_DIR@")
 set(LLVM_HAVE_OPT_VIEWER_MODULES @LLVM_HAVE_OPT_VIEWER_MODULES@)
 set(LLVM_CONFIGURATION_TYPES @CMAKE_CONFIGURATION_TYPES@)
 set(LLVM_ENABLE_SHARED_LIBS @BUILD_SHARED_LIBS@)
 
+# This is deprecated and just here for backwards compatibility. It will be
+# removed in the future.
+set(LLVM_LIBRARY_DIRS "${LLVM_LIBRARY_DIR}")
+
 set(LLVM_DEFAULT_EXTERNAL_LIT "@LLVM_CONFIG_DEFAULT_EXTERNAL_LIT@")
 set(LLVM_LIT_ARGS "@LLVM_LIT_ARGS@")
 
Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -56,16 +56,10 @@
   )
 list(REMOVE_DUPLICATES LLVM_CONFIG_INCLUDE_DIRS)
 
-set(LLVM_CONFIG_LIBRARY_DIR "${LLVM_LIBRARY_DIR}")
-set(LLVM_CONFIG_LIBRARY_DIRS
-  "${LLVM_CONFIG_LIBRARY_DIR}"
-  # FIXME: Should there be other entries here?
-  )
-list(REMOVE_DUPLICATES LLVM_CONFIG_LIBRARY_DIRS)
-
 set(LLVM_CONFIG_BINARY_DIR "${LLVM_BINARY_DIR}")
 set(LLVM_CONFIG_CMAKE_DIR "${CMAKE_CURRENT_SOURCE_DIR}")
 set(LLVM_CONFIG_TOOLS_BINARY_DIR "${LLVM_TOOLS_BINARY_DIR}")
+set(LLVM_CONFIG_LIBRARY_DIR "${LLVM_LIBRARY_DIR}")
 
 # Generate a default location for lit
 if (LLVM_BUILD_UTILS)
@@ -122,16 +116,10 @@
   )
 list(REMOVE_DUPLICATES LLVM_CONFIG_INCLUDE_DIRS)
 
-extend_path(LLVM_CONFIG_LIBRARY_DIR "\${LLVM_INSTALL_PREFIX}" "lib\${LLVM_LIBDIR_SUFFIX}")
-set(LLVM_CONFIG_LIBRARY_DIRS
-  "${LLVM_CONFIG_LIBRARY_DIR}"
-  # FIXME: Should there be other entries here?
-  )
-list(REMOVE_DUPLICATES LLVM_CONFIG_LIBRARY_DIRS)
-
 set(LLVM_CONFIG_BINARY_DIR "\${LLVM_INSTALL_PREFIX}")
 extend_path(LLVM_CONFIG_CMAKE_DIR "\${LLVM_INSTALL_PREFIX}" "${LLVM_INSTALL_PACKAGE_DIR}")
 extend_path(LLVM_CONFIG_TOOLS_BINARY_DIR "\${LLVM_INSTALL_PREFIX}" "${LLVM_TOOLS_INSTALL_DIR}")
+extend_path(LLVM_CONFIG_LIBRARY_DIR "\${LLVM_INSTALL_PREFIX}" "lib\${LLVM_LIBDIR_SUFFIX}")
 
 # Generate a default location for lit
 if (LLVM_INSTALL_UTILS AND LLVM_BUILD_UTILS)
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -262,7 +262,7 @@
   set(LLDB_CLANG_RESOURCE_DIR_NAME "${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}")
   # Iterate over the possible places where the external resource directory
   # could be and pick the first that exists.
-  foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"
+  foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIR}"
 "${LLVM_BUILD_LIBRARY_DIR}"
 "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
 # Build the resource directory path by appending 'clang/'.
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -82,7 +82,7 @@
   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
 
   include_directories("${LLVM_BINARY_DIR}/include" ${LLVM_INCLUDE_DIRS})
-  link_directories(${LLVM_LIBRARY_DIRS})
+  link_directories("${LLVM_LIBRARY_DIR}")
 
   if(LLVM_INCLUDE_TESTS)
 find_package(Python3 ${LLVM_MINIMUM_PYTHON_VERSION} REQUIRED
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116853: [CMake][LLDB] Resolve install conflict when `LLDB_BUILD_FRAMEWORK=ON`

2022-01-07 Thread LJC via Phabricator via lldb-commits 
paperchalice created this revision.
paperchalice added a reviewer: mgorny.
paperchalice added a project: LLDB.
Herald added a subscriber: JDevlieghere.
paperchalice requested review of this revision.
Herald added a subscriber: lldb-commits.

Try to fix https://github.com/llvm/llvm-project/issues/108

- skip install `lldb-python-scripts` when `LLDB_BUILD_FRAMEWORK=ON`
- add `@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}` to `rpath` when 
`LLDB_BUILD_FRAMEWORK=ON`

`LLDB.framework/Resources/Python` is installed by target `lldb-python-scripts` 
and `liblldb`, so
`cmake --install build` will fail with `CMake Error: failed to create symbolic 
link '~/buildspace/llvm-inst/Library/Frameworks/LLDB.framework/Resources': file 
already exists`

The rpath seems incorrect after `ninja install`, so I add 
`@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}` to the rpath list.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116853

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/tools/driver/CMakeLists.txt
  lldb/tools/lldb-vscode/CMakeLists.txt


Index: lldb/tools/lldb-vscode/CMakeLists.txt
===
--- lldb/tools/lldb-vscode/CMakeLists.txt
+++ lldb/tools/lldb-vscode/CMakeLists.txt
@@ -57,5 +57,6 @@
   "@loader_path/../../../SharedFrameworks"
   "@loader_path/../../System/Library/PrivateFrameworks"
   "@loader_path/../../Library/PrivateFrameworks"
+  "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}"
   )
 endif()
Index: lldb/tools/driver/CMakeLists.txt
===
--- lldb/tools/driver/CMakeLists.txt
+++ lldb/tools/driver/CMakeLists.txt
@@ -44,5 +44,6 @@
   "@loader_path/../../../SharedFrameworks"
   "@loader_path/../../System/Library/PrivateFrameworks"
   "@loader_path/../../Library/PrivateFrameworks"
+  "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}"
   )
 endif()
Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -170,9 +170,11 @@
   set(python_scripts_install_target "install-${python_scripts_target}")
   add_custom_target(${python_scripts_target})
   add_dependencies(${python_scripts_target} ${swig_target})
-  install(DIRECTORY ${lldb_python_target_dir}/../
-  DESTINATION ${LLDB_PYTHON_INSTALL_PATH}
-  COMPONENT ${python_scripts_target})
+  if(NOT LLDB_BUILD_FRAMEWORK)
+install(DIRECTORY ${lldb_python_target_dir}/../
+DESTINATION ${LLDB_PYTHON_INSTALL_PATH}
+COMPONENT ${python_scripts_target})
+  endif()
   if (NOT LLVM_ENABLE_IDE)
 add_llvm_install_targets(${python_scripts_install_target}
  COMPONENT ${python_scripts_target}


Index: lldb/tools/lldb-vscode/CMakeLists.txt
===
--- lldb/tools/lldb-vscode/CMakeLists.txt
+++ lldb/tools/lldb-vscode/CMakeLists.txt
@@ -57,5 +57,6 @@
   "@loader_path/../../../SharedFrameworks"
   "@loader_path/../../System/Library/PrivateFrameworks"
   "@loader_path/../../Library/PrivateFrameworks"
+  "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}"
   )
 endif()
Index: lldb/tools/driver/CMakeLists.txt
===
--- lldb/tools/driver/CMakeLists.txt
+++ lldb/tools/driver/CMakeLists.txt
@@ -44,5 +44,6 @@
   "@loader_path/../../../SharedFrameworks"
   "@loader_path/../../System/Library/PrivateFrameworks"
   "@loader_path/../../Library/PrivateFrameworks"
+  "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}"
   )
 endif()
Index: lldb/bindings/python/CMakeLists.txt
===
--- lldb/bindings/python/CMakeLists.txt
+++ lldb/bindings/python/CMakeLists.txt
@@ -170,9 +170,11 @@
   set(python_scripts_install_target "install-${python_scripts_target}")
   add_custom_target(${python_scripts_target})
   add_dependencies(${python_scripts_target} ${swig_target})
-  install(DIRECTORY ${lldb_python_target_dir}/../
-  DESTINATION ${LLDB_PYTHON_INSTALL_PATH}
-  COMPONENT ${python_scripts_target})
+  if(NOT LLDB_BUILD_FRAMEWORK)
+install(DIRECTORY ${lldb_python_target_dir}/../
+DESTINATION ${LLDB_PYTHON_INSTALL_PATH}
+COMPONENT ${python_scripts_target})
+  endif()
   if (NOT LLVM_ENABLE_IDE)
 add_llvm_install_targets(${python_scripts_install_target}
  COMPONENT ${python_scripts_target}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits