[Lldb-commits] [PATCH] D120718: Qualify DataExtractor with lldb_private

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
emrekultursay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

There are two DataExtractors in scope: one from the llvm namespace
and one from the lldb_private namespace. Some Microsoft Visual C++
compilers (I tested with MSVC 14.23 specifically) cannot handle this
situation, and generate ambiguous symbol errors. This change fixes
this compile error.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120718

Files:
  lldb/include/lldb/Core/DataFileCache.h
  lldb/source/Core/DataFileCache.cpp


Index: lldb/source/Core/DataFileCache.cpp
===
--- lldb/source/Core/DataFileCache.cpp
+++ lldb/source/Core/DataFileCache.cpp
@@ -219,7 +219,7 @@
   return true;
 }
 
-bool CacheSignature::Decode(const DataExtractor &data,
+bool CacheSignature::Decode(const lldb_private::DataExtractor &data,
 lldb::offset_t *offset_ptr) {
   Clear();
   while (uint8_t sig_encoding = data.GetU8(offset_ptr)) {
@@ -284,7 +284,7 @@
   return true;
 }
 
-bool StringTableReader::Decode(const DataExtractor &data,
+bool StringTableReader::Decode(const lldb_private::DataExtractor &data,
lldb::offset_t *offset_ptr) {
   llvm::StringRef identifier((const char *)data.GetData(offset_ptr, 4), 4);
   if (identifier != kStringTableIdentifier)
Index: lldb/include/lldb/Core/DataFileCache.h
===
--- lldb/include/lldb/Core/DataFileCache.h
+++ lldb/include/lldb/Core/DataFileCache.h
@@ -161,7 +161,8 @@
   ///
   /// \return
   ///   True if the signature was successfully decoded, false otherwise.
-  bool Decode(const DataExtractor &data, lldb::offset_t *offset_ptr);
+  bool Decode(const lldb_private::DataExtractor &data,
+  lldb::offset_t *offset_ptr);
 };
 
 /// Many cache files require string tables to store data efficiently. This
@@ -204,7 +205,8 @@
 
   llvm::StringRef Get(uint32_t offset) const;
 
-  bool Decode(const DataExtractor &data, lldb::offset_t *offset_ptr);
+  bool Decode(const lldb_private::DataExtractor &data,
+  lldb::offset_t *offset_ptr);
 
 protected:
   /// All of the strings in the string table are contained in m_data.


Index: lldb/source/Core/DataFileCache.cpp
===
--- lldb/source/Core/DataFileCache.cpp
+++ lldb/source/Core/DataFileCache.cpp
@@ -219,7 +219,7 @@
   return true;
 }
 
-bool CacheSignature::Decode(const DataExtractor &data,
+bool CacheSignature::Decode(const lldb_private::DataExtractor &data,
 lldb::offset_t *offset_ptr) {
   Clear();
   while (uint8_t sig_encoding = data.GetU8(offset_ptr)) {
@@ -284,7 +284,7 @@
   return true;
 }
 
-bool StringTableReader::Decode(const DataExtractor &data,
+bool StringTableReader::Decode(const lldb_private::DataExtractor &data,
lldb::offset_t *offset_ptr) {
   llvm::StringRef identifier((const char *)data.GetData(offset_ptr, 4), 4);
   if (identifier != kStringTableIdentifier)
Index: lldb/include/lldb/Core/DataFileCache.h
===
--- lldb/include/lldb/Core/DataFileCache.h
+++ lldb/include/lldb/Core/DataFileCache.h
@@ -161,7 +161,8 @@
   ///
   /// \return
   ///   True if the signature was successfully decoded, false otherwise.
-  bool Decode(const DataExtractor &data, lldb::offset_t *offset_ptr);
+  bool Decode(const lldb_private::DataExtractor &data,
+  lldb::offset_t *offset_ptr);
 };
 
 /// Many cache files require string tables to store data efficiently. This
@@ -204,7 +205,8 @@
 
   llvm::StringRef Get(uint32_t offset) const;
 
-  bool Decode(const DataExtractor &data, lldb::offset_t *offset_ptr);
+  bool Decode(const lldb_private::DataExtractor &data,
+  lldb::offset_t *offset_ptr);
 
 protected:
   /// All of the strings in the string table are contained in m_data.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D120718: Qualify DataExtractor with lldb_private

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

Hi shafik, can you also submit it? I don't have submit permission. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120718

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

[Lldb-commits] [PATCH] D120718: Qualify DataExtractor with lldb_private

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 412361.
emrekultursay added a comment.

Removed "using namespace llvm" from the implementation file
and qualified all associated references with "llvm::".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120718

Files:
  lldb/include/lldb/Core/DataFileCache.h
  lldb/source/Core/DataFileCache.cpp

Index: lldb/source/Core/DataFileCache.cpp
===
--- lldb/source/Core/DataFileCache.cpp
+++ lldb/source/Core/DataFileCache.cpp
@@ -17,17 +17,16 @@
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/MemoryBuffer.h"
 
-using namespace llvm;
 using namespace lldb_private;
 
-DataFileCache::DataFileCache(StringRef path) {
+DataFileCache::DataFileCache(llvm::StringRef path) {
   m_cache_dir.SetPath(path);
 
   // Prune the cache based off of the LLDB settings each time we create a cache
   // object.
   ModuleListProperties &properties =
   ModuleList::GetGlobalModuleListProperties();
-  CachePruningPolicy policy;
+  llvm::CachePruningPolicy policy;
   // Only scan once an hour. If we have lots of debug sessions we don't want
   // to scan this directory too often. A timestamp file is written to the
   // directory to ensure different processes don't scan the directory too often.
@@ -52,7 +51,7 @@
 if (m_take_ownership)
   m_mem_buff_up = std::move(m);
   };
-  Expected cache_or_err =
+  llvm::Expected cache_or_err =
   llvm::localCache("LLDBModuleCache", "lldb-module", path, add_buffer);
   if (cache_or_err)
 m_cache_callback = std::move(*cache_or_err);
@@ -64,7 +63,7 @@
 }
 
 std::unique_ptr
-DataFileCache::GetCachedData(StringRef key) {
+DataFileCache::GetCachedData(llvm::StringRef key) {
   std::lock_guard guard(m_mutex);
 
   const unsigned task = 1;
@@ -73,13 +72,13 @@
   // call the "add_buffer" lambda function from the constructor which will in
   // turn take ownership of the member buffer that is passed to the callback and
   // put it into a member variable.
-  Expected add_stream_or_err = m_cache_callback(task, key);
+  llvm::Expected add_stream_or_err = m_cache_callback(task, key);
   m_take_ownership = false;
   // At this point we either already called the "add_buffer" lambda with
   // the data or we haven't. We can tell if we got the cached data by checking
   // the add_stream function pointer value below.
   if (add_stream_or_err) {
-AddStreamFn &add_stream = *add_stream_or_err;
+llvm::AddStreamFn &add_stream = *add_stream_or_err;
 // If the "add_stream" is nullptr, then the data was cached and we already
 // called the "add_buffer" lambda. If it is valid, then if we were to call
 // the add_stream function it would cause a cache file to get generated
@@ -97,18 +96,18 @@
   return std::unique_ptr();
 }
 
-bool DataFileCache::SetCachedData(StringRef key, llvm::ArrayRef data) {
+bool DataFileCache::SetCachedData(llvm::StringRef key, llvm::ArrayRef data) {
   std::lock_guard guard(m_mutex);
   const unsigned task = 2;
   // If we call this function and the data is cached, it will call the
   // add_buffer lambda function from the constructor which will ignore the
   // data.
-  Expected add_stream_or_err = m_cache_callback(task, key);
+  llvm::Expected add_stream_or_err = m_cache_callback(task, key);
   // If we reach this code then we either already called the callback with
   // the data or we haven't. We can tell if we had the cached data by checking
   // the CacheAddStream function pointer value below.
   if (add_stream_or_err) {
-AddStreamFn &add_stream = *add_stream_or_err;
+llvm::AddStreamFn &add_stream = *add_stream_or_err;
 // If the "add_stream" is nullptr, then the data was cached. If it is
 // valid, then if we call the add_stream function with a task it will
 // cause the file to get generated, but we only want to check if the data
@@ -117,10 +116,10 @@
 // provided, but we won't take ownership of the memory buffer as we just
 // want to write the data.
 if (add_stream) {
-  Expected> file_or_err =
+  llvm::Expected> file_or_err =
   add_stream(task);
   if (file_or_err) {
-CachedFileStream *cfs = file_or_err->get();
+llvm::CachedFileStream *cfs = file_or_err->get();
 cfs->OS->write((const char *)data.data(), data.size());
 return true;
   } else {
@@ -219,7 +218,7 @@
   return true;
 }
 
-bool CacheSignature::Decode(const DataExtractor &data,
+bool CacheSignature::Decode(const lldb_private::DataExtractor &data,
 lldb::offset_t *offset_ptr) {
   Clear();
   while (uint8_t sig_encoding = data.GetU8(offset_ptr)) {
@@ -284,7 +283,7 @@
   return true;
 }
 
-bool StringTableReader::Decode(const DataExtractor &data,
+bool StringTableReader::Decode(const lldb_private::DataExtractor &data,
lldb::offset_t *offset_p

[Lldb-commits] [PATCH] D120718: Qualify DataExtractor with lldb_private

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

In D120718#3352450 , @JDevlieghere 
wrote:

> I think the better solution here is to get rid of the `using namespace llvm;` 
> in the implementation file instead.

Done. PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120718

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

[Lldb-commits] [PATCH] D120718: Fix DataExtractor symbol conflict

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 412621.
emrekultursay added a comment.

Reverted the changes to the header file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120718

Files:
  lldb/source/Core/DataFileCache.cpp

Index: lldb/source/Core/DataFileCache.cpp
===
--- lldb/source/Core/DataFileCache.cpp
+++ lldb/source/Core/DataFileCache.cpp
@@ -17,17 +17,16 @@
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/MemoryBuffer.h"
 
-using namespace llvm;
 using namespace lldb_private;
 
-DataFileCache::DataFileCache(StringRef path) {
+DataFileCache::DataFileCache(llvm::StringRef path) {
   m_cache_dir.SetPath(path);
 
   // Prune the cache based off of the LLDB settings each time we create a cache
   // object.
   ModuleListProperties &properties =
   ModuleList::GetGlobalModuleListProperties();
-  CachePruningPolicy policy;
+  llvm::CachePruningPolicy policy;
   // Only scan once an hour. If we have lots of debug sessions we don't want
   // to scan this directory too often. A timestamp file is written to the
   // directory to ensure different processes don't scan the directory too often.
@@ -52,7 +51,7 @@
 if (m_take_ownership)
   m_mem_buff_up = std::move(m);
   };
-  Expected cache_or_err =
+  llvm::Expected cache_or_err =
   llvm::localCache("LLDBModuleCache", "lldb-module", path, add_buffer);
   if (cache_or_err)
 m_cache_callback = std::move(*cache_or_err);
@@ -64,7 +63,7 @@
 }
 
 std::unique_ptr
-DataFileCache::GetCachedData(StringRef key) {
+DataFileCache::GetCachedData(llvm::StringRef key) {
   std::lock_guard guard(m_mutex);
 
   const unsigned task = 1;
@@ -73,13 +72,13 @@
   // call the "add_buffer" lambda function from the constructor which will in
   // turn take ownership of the member buffer that is passed to the callback and
   // put it into a member variable.
-  Expected add_stream_or_err = m_cache_callback(task, key);
+  llvm::Expected add_stream_or_err = m_cache_callback(task, key);
   m_take_ownership = false;
   // At this point we either already called the "add_buffer" lambda with
   // the data or we haven't. We can tell if we got the cached data by checking
   // the add_stream function pointer value below.
   if (add_stream_or_err) {
-AddStreamFn &add_stream = *add_stream_or_err;
+llvm::AddStreamFn &add_stream = *add_stream_or_err;
 // If the "add_stream" is nullptr, then the data was cached and we already
 // called the "add_buffer" lambda. If it is valid, then if we were to call
 // the add_stream function it would cause a cache file to get generated
@@ -97,18 +96,18 @@
   return std::unique_ptr();
 }
 
-bool DataFileCache::SetCachedData(StringRef key, llvm::ArrayRef data) {
+bool DataFileCache::SetCachedData(llvm::StringRef key, llvm::ArrayRef data) {
   std::lock_guard guard(m_mutex);
   const unsigned task = 2;
   // If we call this function and the data is cached, it will call the
   // add_buffer lambda function from the constructor which will ignore the
   // data.
-  Expected add_stream_or_err = m_cache_callback(task, key);
+  llvm::Expected add_stream_or_err = m_cache_callback(task, key);
   // If we reach this code then we either already called the callback with
   // the data or we haven't. We can tell if we had the cached data by checking
   // the CacheAddStream function pointer value below.
   if (add_stream_or_err) {
-AddStreamFn &add_stream = *add_stream_or_err;
+llvm::AddStreamFn &add_stream = *add_stream_or_err;
 // If the "add_stream" is nullptr, then the data was cached. If it is
 // valid, then if we call the add_stream function with a task it will
 // cause the file to get generated, but we only want to check if the data
@@ -117,10 +116,10 @@
 // provided, but we won't take ownership of the memory buffer as we just
 // want to write the data.
 if (add_stream) {
-  Expected> file_or_err =
+  llvm::Expected> file_or_err =
   add_stream(task);
   if (file_or_err) {
-CachedFileStream *cfs = file_or_err->get();
+llvm::CachedFileStream *cfs = file_or_err->get();
 cfs->OS->write((const char *)data.data(), data.size());
 return true;
   } else {
@@ -219,7 +218,7 @@
   return true;
 }
 
-bool CacheSignature::Decode(const DataExtractor &data,
+bool CacheSignature::Decode(const lldb_private::DataExtractor &data,
 lldb::offset_t *offset_ptr) {
   Clear();
   while (uint8_t sig_encoding = data.GetU8(offset_ptr)) {
@@ -284,7 +283,7 @@
   return true;
 }
 
-bool StringTableReader::Decode(const DataExtractor &data,
+bool StringTableReader::Decode(const lldb_private::DataExtractor &data,
lldb::offset_t *offset_ptr) {
   llvm::StringRef identifier((const char *)data.GetData(offset_ptr, 4), 4);
   if (identifier != kStringTabl

[Lldb-commits] [PATCH] D120718: Fix DataExtractor symbol conflict

[Emre Kultursay via Phabricator via lldb-commits]

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

Thanks. PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120718

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

[Lldb-commits] [PATCH] D128832: [lldb-server] Skip shared regions for memory allocation

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a project: All.
emrekultursay 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/D128832

Files:
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp
  lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
  lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -378,15 +378,15 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x0, 0x1}, no, no, no, no, ConstString(),
+  MemoryRegionInfo({0x0, 0x1}, no, no, no, unknown, no, ConstString(),
unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x1, 0x21000}, yes, yes, no, yes,
+  MemoryRegionInfo({0x1, 0x21000}, yes, yes, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x4, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x4, 0x1000}, yes, no, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x7ffe, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x7ffe, 0x1000}, yes, no, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, yes,
+  MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown)),
   true));
 }
@@ -412,9 +412,9 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown)),
   false));
 }
@@ -428,9 +428,9 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown)),
   false));
 }
@@ -460,17 +460,17 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x400d9000, 0x2000}, yes, no, yes, yes,
+  MemoryRegionInfo({0x400d9000, 0x2000}, yes, no, yes, no, yes,
app_process, unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400db000, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x400db000, 0x1000}, yes, no, no, no, yes,
app_process, unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400dc000, 0x1000}, yes, yes, no, yes,
+  MemoryRegionInfo({0x400dc000, 0x1000}, yes, yes, no, no, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400ec000, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x400ec000, 0x1000}, yes, no, no, no, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400ee000, 0x1000}, yes, yes, no, yes, linker,
+  MemoryRegionInfo({0x400ee000, 0x1000}, yes, yes, no, no, yes, linker,
unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400fc000, 0x1000}, yes, yes, yes, yes, liblog,
+  MemoryRegionInfo({0x400fc000, 0x1000}, yes, yes, yes, no, yes, liblog,
unknown, 0, unknown, unknown)),
   true));
 }
@@ -491,7 +491,7 @@
   EXPECT_THAT(
   parser->BuildMemoryRegions(),
   testing::Pair(testing::ElementsAre(MemoryRegionInfo(

[Lldb-commits] [PATCH] D128832: [lldb-server] Skip shared regions for memory allocation

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 441058.
emrekultursay added a comment.

Added test that covers shared (s) input


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128832

Files:
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp
  lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
  lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -378,15 +378,15 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x0, 0x1}, no, no, no, no, ConstString(),
+  MemoryRegionInfo({0x0, 0x1}, no, no, no, unknown, no, ConstString(),
unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x1, 0x21000}, yes, yes, no, yes,
+  MemoryRegionInfo({0x1, 0x21000}, yes, yes, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x4, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x4, 0x1000}, yes, no, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x7ffe, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x7ffe, 0x1000}, yes, no, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, yes,
+  MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown)),
   true));
 }
@@ -412,9 +412,9 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown)),
   false));
 }
@@ -428,9 +428,9 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown)),
   false));
 }
@@ -460,17 +460,17 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x400d9000, 0x2000}, yes, no, yes, yes,
+  MemoryRegionInfo({0x400d9000, 0x2000}, yes, no, yes, no, yes,
app_process, unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400db000, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x400db000, 0x1000}, yes, no, no, no, yes,
app_process, unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400dc000, 0x1000}, yes, yes, no, yes,
+  MemoryRegionInfo({0x400dc000, 0x1000}, yes, yes, no, no, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400ec000, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x400ec000, 0x1000}, yes, no, no, no, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400ee000, 0x1000}, yes, yes, no, yes, linker,
+  MemoryRegionInfo({0x400ee000, 0x1000}, yes, yes, no, no, yes, linker,
unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400fc000, 0x1000}, yes, yes, yes, yes, liblog,
+  MemoryRegionInfo({0x400fc000, 0x1000}, yes, yes, yes, no, yes, liblog,
unknown, 0, unknown, unknown)),
   true));
 }
@@ -491,7 +491,7 @@
   EXPECT_THAT(
   parser->BuildMemoryRegions(),
   testing::Pair(testing::ElementsAre(MemoryRegio

[Lldb-commits] [PATCH] D128832: [lldb-server] Skip shared regions for memory allocation

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 441060.
emrekultursay added a comment.

Removed redundant comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128832

Files:
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp
  lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
  lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
  lldb/unittests/Process/minidump/MinidumpParserTest.cpp

Index: lldb/unittests/Process/minidump/MinidumpParserTest.cpp
===
--- lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -378,15 +378,15 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x0, 0x1}, no, no, no, no, ConstString(),
+  MemoryRegionInfo({0x0, 0x1}, no, no, no, unknown, no, ConstString(),
unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x1, 0x21000}, yes, yes, no, yes,
+  MemoryRegionInfo({0x1, 0x21000}, yes, yes, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x4, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x4, 0x1000}, yes, no, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x7ffe, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x7ffe, 0x1000}, yes, no, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, yes,
+  MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, unknown, yes,
ConstString(), unknown, 0, unknown, unknown)),
   true));
 }
@@ -412,9 +412,9 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown)),
   false));
 }
@@ -428,9 +428,9 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, yes,
+  MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown, unknown, yes,
ConstString(), unknown, 0, unknown, unknown)),
   false));
 }
@@ -460,17 +460,17 @@
   parser->BuildMemoryRegions(),
   testing::Pair(
   testing::ElementsAre(
-  MemoryRegionInfo({0x400d9000, 0x2000}, yes, no, yes, yes,
+  MemoryRegionInfo({0x400d9000, 0x2000}, yes, no, yes, no, yes,
app_process, unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400db000, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x400db000, 0x1000}, yes, no, no, no, yes,
app_process, unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400dc000, 0x1000}, yes, yes, no, yes,
+  MemoryRegionInfo({0x400dc000, 0x1000}, yes, yes, no, no, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400ec000, 0x1000}, yes, no, no, yes,
+  MemoryRegionInfo({0x400ec000, 0x1000}, yes, no, no, no, yes,
ConstString(), unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400ee000, 0x1000}, yes, yes, no, yes, linker,
+  MemoryRegionInfo({0x400ee000, 0x1000}, yes, yes, no, no, yes, linker,
unknown, 0, unknown, unknown),
-  MemoryRegionInfo({0x400fc000, 0x1000}, yes, yes, yes, yes, liblog,
+  MemoryRegionInfo({0x400fc000, 0x1000}, yes, yes, yes, no, yes, liblog,
unknown, 0, unknown, unknown)),
   true));
 }
@@ -491,7 +491,7 @@
   EXPECT_THAT(
   parser->BuildMemoryRegions(),
   testing::Pair(testing::ElementsAre(MemoryRegionInfo(
-  

[Lldb-commits] [PATCH] D128832: [lldb-server] Skip shared regions for memory allocation

[Emre Kultursay via Phabricator via lldb-commits]

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

PTAL.




Comment at: lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp:278
   check_regions(params);
 }

DavidSpickett wrote:
> I don't see a test for a mapping with "s" for the sharing char, did I miss it?
I added tests to cover shared (s). Thanks for noticing the missing test.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128832

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

[Lldb-commits] [PATCH] D128832: [lldb-server] Skip shared regions for memory allocation

[Emre Kultursay via Phabricator via lldb-commits]

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

Thanks. Please commit on my behalf.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128832

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

[Lldb-commits] [PATCH] D131821: Add test that shows the problem with aed965d5

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a project: All.
emrekultursay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The test is identical to API/lang/c/stepping/ except:

1. It uses C++ and thus needs demangling support
2. It uses dynamic library, and tries to step into a function that is defined 
in the library.

Test fails with:

File 
"...llvm-project/lldb/test/API/lang/cpp/stepping/TestStepAndBreakpointsCpp.py", 
line 252, in test_and_python_api
  self.assertEqual(thread.GetFrameAtIndex(0).GetFunctionName(), "b(int)")
  AssertionError: 'main' != 'b(int)'

...because it fails to step into the `b(int)` method and hits the next
breakpoint inside the `main` method.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131821

Files:
  lldb/test/API/lang/cpp/stepping/Makefile
  lldb/test/API/lang/cpp/stepping/TestStepAndBreakpointsCpp.py
  lldb/test/API/lang/cpp/stepping/helper.cpp
  lldb/test/API/lang/cpp/stepping/main.cpp

Index: lldb/test/API/lang/cpp/stepping/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/stepping/main.cpp
@@ -0,0 +1,24 @@
+int a(int);
+int b(int);
+int c(int);
+const char *print_string = "aa\n";
+int complex (int first, int second, int third);
+
+int main (int argc, char const *argv[])
+{
+int A1 = a(1); // frame select 2, thread step-out while stopped at "c(1)"
+
+int B2 = b(2); // assignment to B2
+
+int A3 = a(3); // frame select 1, thread step-out while stopped at "c(3)"
+
+int A4 = complex (a(1), b(2), c(3)); // Stop here to try step in targeting b.
+
+int A5 = complex (a(2), b(3), c(4)); // Stop here to try step in targeting complex.
+
+int A6 = complex (a(4), b(5), c(6)); // Stop here to step targeting b and hitting breakpoint.
+
+int A7 = complex (a(5), b(6), c(7)); // Stop here to make sure bogus target steps over.
+
+return A1 + B2 + A3 + A4 + A5 + A6 + A7 + *print_string;
+}
Index: lldb/test/API/lang/cpp/stepping/helper.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/stepping/helper.cpp
@@ -0,0 +1,35 @@
+int a(int);
+int b(int);
+int c(int);
+
+int a(int val)
+{
+int return_value = val;  // basic break at the start of b
+
+if (val <= 1)
+{
+return_value =  b(val); // break here to stop in a before calling b
+}
+else if (val >= 3)
+{
+return_value = c(val);
+}
+
+return return_value;
+}
+
+int b(int val)
+{
+int rc = c(val); // thread step-out while stopped at "c(2)"
+return rc;
+}
+
+int c(int val)
+{
+return val + 3; // Find the line number of function "c" here.
+}
+
+int complex (int first, int second, int third)
+{
+return first + second + third;  // Step in targeting complex should stop here
+}
Index: lldb/test/API/lang/cpp/stepping/TestStepAndBreakpointsCpp.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/stepping/TestStepAndBreakpointsCpp.py
@@ -0,0 +1,302 @@
+"""Test stepping over vrs. hitting breakpoints & subsequent stepping in various forms."""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCppStepping(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line numbers that we will step to in main:
+self.main_source = "main.cpp"
+self.helper_source = "helper.cpp"
+
+@add_test_categories(['pyapi', 'basic_process'])
+@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr17932')
+@expectedFailureAll(oslist=["linux"], archs=no_match(["i386", "x86_64"]))
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24777")
+@expectedFailureNetBSD
+@skipIfWindows
+@skipIfDarwinEmbedded
+def test_and_python_api(self):
+"""Test stepping over vrs. hitting breakpoints & subsequent stepping in various forms."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+environment = self.registerSharedLibrariesWithTarget(target, ["helper"])
+
+self.main_source_spec = lldb.SBFileSpec(self.main_source)
+self.helper_source_spec = lldb.SBFileSpec(self.helper_source)
+
+breakpoints_to_disable = []
+
+break_1_in_main = target.BreakpointCreateBySourceRegex(
+'// frame select 2, thread step-out while stopped at .c.1..',
+self.main_source_spec)
+self.assertTrue(break_1_in_main, VALID_BREAKPOINT)
+breakpoints_to_disable.append(break_1_in_main)
+
+break_in_a = target.BreakpointCreateBySourceRegex(
+'// break here to stop i

[Lldb-commits] [PATCH] D131821: Add test that shows the problem with D118814

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

In D131821#3723474 , @JDevlieghere 
wrote:

> I applied your patch and ran the test and for me it's passing:

Thanks Jonas, I believe https://reviews.llvm.org/D127999 (and follow-ups) fixed 
the bug I was observing. I was sync'ed to a version older than that patch, 
that's why I didn't notice it was fixed.  I synced to head and I don't 
reproduce it either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131821

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

[Lldb-commits] [PATCH] D128126: Support expressions in the context of a reference

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a project: All.
emrekultursay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

...type variable by dereferencing the variable before
evaluating the expression.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128126

Files:
  lldb/source/Expression/UserExpression.cpp
  lldb/test/API/commands/expression/context-object/TestContextObject.py
  lldb/test/API/commands/expression/context-object/main.cpp


Index: lldb/test/API/commands/expression/context-object/main.cpp
===
--- lldb/test/API/commands/expression/context-object/main.cpp
+++ lldb/test/API/commands/expression/context-object/main.cpp
@@ -31,6 +31,9 @@
   cpp_namespace::CppStruct cpp_struct = cpp_namespace::GetCppStruct();
   cpp_struct.function();
 
+  cpp_namespace::CppStruct& cpp_struct_ref = cpp_struct;
+  cpp_struct_ref.function();
+
   int field = ;
 
   cpp_namespace::CppUnion cpp_union;
Index: lldb/test/API/commands/expression/context-object/TestContextObject.py
===
--- lldb/test/API/commands/expression/context-object/TestContextObject.py
+++ lldb/test/API/commands/expression/context-object/TestContextObject.py
@@ -47,6 +47,36 @@
 self.assertSuccess(value.GetError())
 self.assertEqual(value.GetValueAsSigned(), )
 
+#
+# Test C++ reference-to-struct variable
+#
+
+obj_val = frame.FindVariable("cpp_struct_ref")
+self.assertTrue(obj_val.IsValid())
+
+# Test an empty expression evaluation
+value = obj_val.EvaluateExpression("")
+self.assertFalse(value.IsValid())
+self.assertFalse(value.GetError().Success())
+
+# Test retrieveing of a field (not a local with the same name)
+value = obj_val.EvaluateExpression("field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test functions evaluation
+value = obj_val.EvaluateExpression("function()")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test that we retrieve the right global
+value = obj_val.EvaluateExpression("global.field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
 #
 # Test C++ union variable
 #
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -145,8 +145,9 @@
   Log *log(GetLog(LLDBLog::Expressions | LLDBLog::Step));
 
   if (ctx_obj) {
-static unsigned const ctx_type_mask =
-lldb::TypeFlags::eTypeIsClass | lldb::TypeFlags::eTypeIsStructUnion;
+static unsigned const ctx_type_mask = lldb::TypeFlags::eTypeIsClass |
+  lldb::TypeFlags::eTypeIsStructUnion |
+  lldb::TypeFlags::eTypeIsReference;
 if (!(ctx_obj->GetTypeInfo() & ctx_type_mask)) {
   LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
 "an invalid type, can't run expressions.");
@@ -155,6 +156,19 @@
 }
   }
 
+  if (ctx_obj && ctx_obj->GetTypeInfo() & lldb::TypeFlags::eTypeIsReference) {
+Status error;
+lldb::ValueObjectSP deref_ctx_sp = ctx_obj->Dereference(error);
+if (!error.Success()) {
+  LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
+"a reference type that can't be dereferenced, can't run 
expressions.");
+  error.SetErrorString("passed context object of an reference type cannot 
be deferenced");
+  return lldb::eExpressionSetupError;
+}
+
+ctx_obj = deref_ctx_sp.get();
+  }
+
   lldb_private::ExecutionPolicy execution_policy = 
options.GetExecutionPolicy();
   lldb::LanguageType language = options.GetLanguage();
   const ResultType desired_type = options.DoesCoerceToId()


Index: lldb/test/API/commands/expression/context-object/main.cpp
===
--- lldb/test/API/commands/expression/context-object/main.cpp
+++ lldb/test/API/commands/expression/context-object/main.cpp
@@ -31,6 +31,9 @@
   cpp_namespace::CppStruct cpp_struct = cpp_namespace::GetCppStruct();
   cpp_struct.function();
 
+  cpp_namespace::CppStruct& cpp_struct_ref = cpp_struct;
+  cpp_struct_ref.function();
+
   int field = ;
 
   cpp_namespace::CppUnion cpp_union;
Index: lldb/test/API/commands/expression/context-object/TestContextObject.py
==

[Lldb-commits] [PATCH] D128126: Support expressions in the context of a reference

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 438156.
emrekultursay added a comment.

Rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128126

Files:
  lldb/source/Expression/UserExpression.cpp
  lldb/test/API/commands/expression/context-object/TestContextObject.py
  lldb/test/API/commands/expression/context-object/main.cpp


Index: lldb/test/API/commands/expression/context-object/main.cpp
===
--- lldb/test/API/commands/expression/context-object/main.cpp
+++ lldb/test/API/commands/expression/context-object/main.cpp
@@ -31,6 +31,9 @@
   cpp_namespace::CppStruct cpp_struct = cpp_namespace::GetCppStruct();
   cpp_struct.function();
 
+  cpp_namespace::CppStruct& cpp_struct_ref = cpp_struct;
+  cpp_struct_ref.function();
+
   int field = ;
 
   cpp_namespace::CppUnion cpp_union;
Index: lldb/test/API/commands/expression/context-object/TestContextObject.py
===
--- lldb/test/API/commands/expression/context-object/TestContextObject.py
+++ lldb/test/API/commands/expression/context-object/TestContextObject.py
@@ -45,6 +45,36 @@
 self.assertSuccess(value.GetError())
 self.assertEqual(value.GetValueAsSigned(), )
 
+#
+# Test C++ reference-to-struct variable
+#
+
+obj_val = frame.FindVariable("cpp_struct_ref")
+self.assertTrue(obj_val.IsValid())
+
+# Test an empty expression evaluation
+value = obj_val.EvaluateExpression("")
+self.assertFalse(value.IsValid())
+self.assertFalse(value.GetError().Success())
+
+# Test retrieveing of a field (not a local with the same name)
+value = obj_val.EvaluateExpression("field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test functions evaluation
+value = obj_val.EvaluateExpression("function()")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test that we retrieve the right global
+value = obj_val.EvaluateExpression("global.field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
 #
 # Test C++ union variable
 #
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -145,8 +145,9 @@
   Log *log(GetLog(LLDBLog::Expressions | LLDBLog::Step));
 
   if (ctx_obj) {
-static unsigned const ctx_type_mask =
-lldb::TypeFlags::eTypeIsClass | lldb::TypeFlags::eTypeIsStructUnion;
+static unsigned const ctx_type_mask = lldb::TypeFlags::eTypeIsClass |
+  lldb::TypeFlags::eTypeIsStructUnion |
+  lldb::TypeFlags::eTypeIsReference;
 if (!(ctx_obj->GetTypeInfo() & ctx_type_mask)) {
   LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
 "an invalid type, can't run expressions.");
@@ -155,6 +156,19 @@
 }
   }
 
+  if (ctx_obj && ctx_obj->GetTypeInfo() & lldb::TypeFlags::eTypeIsReference) {
+Status error;
+lldb::ValueObjectSP deref_ctx_sp = ctx_obj->Dereference(error);
+if (!error.Success()) {
+  LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
+"a reference type that can't be dereferenced, can't run 
expressions.");
+  error.SetErrorString("passed context object of an reference type cannot 
be deferenced");
+  return lldb::eExpressionSetupError;
+}
+
+ctx_obj = deref_ctx_sp.get();
+  }
+
   lldb_private::ExecutionPolicy execution_policy = 
options.GetExecutionPolicy();
   lldb::LanguageType language = options.GetLanguage();
   const ResultType desired_type = options.DoesCoerceToId()


Index: lldb/test/API/commands/expression/context-object/main.cpp
===
--- lldb/test/API/commands/expression/context-object/main.cpp
+++ lldb/test/API/commands/expression/context-object/main.cpp
@@ -31,6 +31,9 @@
   cpp_namespace::CppStruct cpp_struct = cpp_namespace::GetCppStruct();
   cpp_struct.function();
 
+  cpp_namespace::CppStruct& cpp_struct_ref = cpp_struct;
+  cpp_struct_ref.function();
+
   int field = ;
 
   cpp_namespace::CppUnion cpp_union;
Index: lldb/test/API/commands/expression/context-object/TestContextObject.py
===
--- lldb/test/API/commands/expression/context-object/TestContextObject.py
+++ lldb/test/API/commands/ex

[Lldb-commits] [PATCH] D128126: Support expressions in the context of a reference

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 438157.
emrekultursay added a comment.

re-rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128126

Files:
  lldb/source/Expression/UserExpression.cpp
  lldb/test/API/commands/expression/context-object/TestContextObject.py
  lldb/test/API/commands/expression/context-object/main.cpp


Index: lldb/test/API/commands/expression/context-object/main.cpp
===
--- lldb/test/API/commands/expression/context-object/main.cpp
+++ lldb/test/API/commands/expression/context-object/main.cpp
@@ -31,6 +31,9 @@
   cpp_namespace::CppStruct cpp_struct = cpp_namespace::GetCppStruct();
   cpp_struct.function();
 
+  cpp_namespace::CppStruct& cpp_struct_ref = cpp_struct;
+  cpp_struct_ref.function();
+
   int field = ;
 
   cpp_namespace::CppUnion cpp_union;
Index: lldb/test/API/commands/expression/context-object/TestContextObject.py
===
--- lldb/test/API/commands/expression/context-object/TestContextObject.py
+++ lldb/test/API/commands/expression/context-object/TestContextObject.py
@@ -45,6 +45,36 @@
 self.assertSuccess(value.GetError())
 self.assertEqual(value.GetValueAsSigned(), )
 
+#
+# Test C++ reference-to-struct variable
+#
+
+obj_val = frame.FindVariable("cpp_struct_ref")
+self.assertTrue(obj_val.IsValid())
+
+# Test an empty expression evaluation
+value = obj_val.EvaluateExpression("")
+self.assertFalse(value.IsValid())
+self.assertFalse(value.GetError().Success())
+
+# Test retrieveing of a field (not a local with the same name)
+value = obj_val.EvaluateExpression("field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test functions evaluation
+value = obj_val.EvaluateExpression("function()")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test that we retrieve the right global
+value = obj_val.EvaluateExpression("global.field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
 #
 # Test C++ union variable
 #
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -145,8 +145,9 @@
   Log *log(GetLog(LLDBLog::Expressions | LLDBLog::Step));
 
   if (ctx_obj) {
-static unsigned const ctx_type_mask =
-lldb::TypeFlags::eTypeIsClass | lldb::TypeFlags::eTypeIsStructUnion;
+static unsigned const ctx_type_mask = lldb::TypeFlags::eTypeIsClass |
+  lldb::TypeFlags::eTypeIsStructUnion |
+  lldb::TypeFlags::eTypeIsReference;
 if (!(ctx_obj->GetTypeInfo() & ctx_type_mask)) {
   LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
 "an invalid type, can't run expressions.");
@@ -155,6 +156,19 @@
 }
   }
 
+  if (ctx_obj && ctx_obj->GetTypeInfo() & lldb::TypeFlags::eTypeIsReference) {
+Status error;
+lldb::ValueObjectSP deref_ctx_sp = ctx_obj->Dereference(error);
+if (!error.Success()) {
+  LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
+"a reference type that can't be dereferenced, can't run 
expressions.");
+  error.SetErrorString("passed context object of an reference type cannot 
be deferenced");
+  return lldb::eExpressionSetupError;
+}
+
+ctx_obj = deref_ctx_sp.get();
+  }
+
   lldb_private::ExecutionPolicy execution_policy = 
options.GetExecutionPolicy();
   lldb::LanguageType language = options.GetLanguage();
   const ResultType desired_type = options.DoesCoerceToId()


Index: lldb/test/API/commands/expression/context-object/main.cpp
===
--- lldb/test/API/commands/expression/context-object/main.cpp
+++ lldb/test/API/commands/expression/context-object/main.cpp
@@ -31,6 +31,9 @@
   cpp_namespace::CppStruct cpp_struct = cpp_namespace::GetCppStruct();
   cpp_struct.function();
 
+  cpp_namespace::CppStruct& cpp_struct_ref = cpp_struct;
+  cpp_struct_ref.function();
+
   int field = ;
 
   cpp_namespace::CppUnion cpp_union;
Index: lldb/test/API/commands/expression/context-object/TestContextObject.py
===
--- lldb/test/API/commands/expression/context-object/TestContextObject.py
+++ lldb/test/API/commands/

[Lldb-commits] [PATCH] D128126: Support expressions in the context of a reference

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 438454.
emrekultursay added a comment.

Use loop instead of code duplication in test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128126

Files:
  lldb/source/Expression/UserExpression.cpp
  lldb/test/API/commands/expression/context-object/TestContextObject.py
  lldb/test/API/commands/expression/context-object/main.cpp

Index: lldb/test/API/commands/expression/context-object/main.cpp
===
--- lldb/test/API/commands/expression/context-object/main.cpp
+++ lldb/test/API/commands/expression/context-object/main.cpp
@@ -31,6 +31,9 @@
   cpp_namespace::CppStruct cpp_struct = cpp_namespace::GetCppStruct();
   cpp_struct.function();
 
+  cpp_namespace::CppStruct& cpp_struct_ref = cpp_struct;
+  cpp_struct_ref.function();
+
   int field = ;
 
   cpp_namespace::CppUnion cpp_union;
Index: lldb/test/API/commands/expression/context-object/TestContextObject.py
===
--- lldb/test/API/commands/expression/context-object/TestContextObject.py
+++ lldb/test/API/commands/expression/context-object/TestContextObject.py
@@ -16,34 +16,34 @@
 frame = thread.GetFrameAtIndex(0)
 
 #
-# Test C++ struct variable
+# Test C++ struct variable and reference-to-struct variable
 #
-
-obj_val = frame.FindVariable("cpp_struct")
-self.assertTrue(obj_val.IsValid())
-
-# Test an empty expression evaluation
-value = obj_val.EvaluateExpression("")
-self.assertFalse(value.IsValid())
-self.assertFalse(value.GetError().Success())
-
-# Test retrieveing of a field (not a local with the same name)
-value = obj_val.EvaluateExpression("field")
-self.assertTrue(value.IsValid())
-self.assertSuccess(value.GetError())
-self.assertEqual(value.GetValueAsSigned(), )
-
-# Test functions evaluation
-value = obj_val.EvaluateExpression("function()")
-self.assertTrue(value.IsValid())
-self.assertSuccess(value.GetError())
-self.assertEqual(value.GetValueAsSigned(), )
-
-# Test that we retrieve the right global
-value = obj_val.EvaluateExpression("global.field")
-self.assertTrue(value.IsValid())
-self.assertSuccess(value.GetError())
-self.assertEqual(value.GetValueAsSigned(), )
+for obj in "cpp_struct", "cpp_struct_ref":
+obj_val = frame.FindVariable(obj)
+self.assertTrue(obj_val.IsValid())
+
+# Test an empty expression evaluation
+value = obj_val.EvaluateExpression("")
+self.assertFalse(value.IsValid())
+self.assertFalse(value.GetError().Success())
+
+# Test retrieveing of a field (not a local with the same name)
+value = obj_val.EvaluateExpression("field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test functions evaluation
+value = obj_val.EvaluateExpression("function()")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test that we retrieve the right global
+value = obj_val.EvaluateExpression("global.field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
 
 #
 # Test C++ union variable
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -145,8 +145,9 @@
   Log *log(GetLog(LLDBLog::Expressions | LLDBLog::Step));
 
   if (ctx_obj) {
-static unsigned const ctx_type_mask =
-lldb::TypeFlags::eTypeIsClass | lldb::TypeFlags::eTypeIsStructUnion;
+static unsigned const ctx_type_mask = lldb::TypeFlags::eTypeIsClass |
+  lldb::TypeFlags::eTypeIsStructUnion |
+  lldb::TypeFlags::eTypeIsReference;
 if (!(ctx_obj->GetTypeInfo() & ctx_type_mask)) {
   LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
 "an invalid type, can't run expressions.");
@@ -155,6 +156,19 @@
 }
   }
 
+  if (ctx_obj && ctx_obj->GetTypeInfo() & lldb::TypeFlags::eTypeIsReference) {
+Status error;
+lldb::ValueObjectSP deref_ctx_sp = ctx_obj->Dereference(error);
+if (!error.Success()) {
+  LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
+"a reference type that can't be

[Lldb-commits] [PATCH] D128126: Support expressions in the context of a reference

[Emre Kultursay via Phabricator via lldb-commits]

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

Thanks. Please submit the change on my behalf.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128126

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

[Lldb-commits] [PATCH] D128264: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

> Mainly added you in case you want to try this out on android.

Thanks. I tested this on a few Android API levels, and it looks good to me.




Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:441
+  &m_process->GetTarget()) == m_interpreter_base) {
+ModuleSP interpreter_sp = m_interpreter_module.lock();
+if (m_interpreter_module.lock() == nullptr) {

Use this variable below, instead of calling lock() again?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128264

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

[Lldb-commits] [PATCH] D144240: Clear read_fd_set if EINTR received

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a project: All.
emrekultursay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Leaving bits uncleared set causes callbacks to be triggered even
though there are no events to process. Starting with D131160 

we have a callback that makes blocking read calls over pipe which
was causing the lldb-server main loop to become unresponsive / blocked
on Android.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144240

Files:
  lldb/source/Host/posix/MainLoopPosix.cpp


Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -156,9 +156,12 @@
 size_t sigset_len;
   } extra_data = {&kernel_sigset, sizeof(kernel_sigset)};
   if (syscall(__NR_pselect6, nfds, &read_fd_set, nullptr, nullptr, nullptr,
-  &extra_data) == -1 &&
-  errno != EINTR)
-return Status(errno, eErrorTypePOSIX);
+  &extra_data) == -1) {
+  if (errno != EINTR)
+return Status(errno, eErrorTypePOSIX);
+  else
+FD_ZERO(&read_fd_set);
+  }
 
   return Status();
 }


Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -156,9 +156,12 @@
 size_t sigset_len;
   } extra_data = {&kernel_sigset, sizeof(kernel_sigset)};
   if (syscall(__NR_pselect6, nfds, &read_fd_set, nullptr, nullptr, nullptr,
-  &extra_data) == -1 &&
-  errno != EINTR)
-return Status(errno, eErrorTypePOSIX);
+  &extra_data) == -1) {
+  if (errno != EINTR)
+return Status(errno, eErrorTypePOSIX);
+  else
+FD_ZERO(&read_fd_set);
+  }
 
   return Status();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D144240: Clear read_fd_set if EINTR received

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 498257.
emrekultursay added a comment.
Herald added a subscriber: JDevlieghere.

Fix indentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144240

Files:
  lldb/source/Host/posix/MainLoopPosix.cpp


Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -156,9 +156,12 @@
 size_t sigset_len;
   } extra_data = {&kernel_sigset, sizeof(kernel_sigset)};
   if (syscall(__NR_pselect6, nfds, &read_fd_set, nullptr, nullptr, nullptr,
-  &extra_data) == -1 &&
-  errno != EINTR)
-return Status(errno, eErrorTypePOSIX);
+  &extra_data) == -1) {
+if (errno != EINTR)
+  return Status(errno, eErrorTypePOSIX);
+else
+  FD_ZERO(&read_fd_set);
+  }
 
   return Status();
 }


Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -156,9 +156,12 @@
 size_t sigset_len;
   } extra_data = {&kernel_sigset, sizeof(kernel_sigset)};
   if (syscall(__NR_pselect6, nfds, &read_fd_set, nullptr, nullptr, nullptr,
-  &extra_data) == -1 &&
-  errno != EINTR)
-return Status(errno, eErrorTypePOSIX);
+  &extra_data) == -1) {
+if (errno != EINTR)
+  return Status(errno, eErrorTypePOSIX);
+else
+  FD_ZERO(&read_fd_set);
+  }
 
   return Status();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D144240: Clear read_fd_set if EINTR received

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

> As discussed internally, it would be great if we could remove this 
> android-specific code path (by removing support for OS versions which 
> necessitated it).

For now, Android Studio still requires the ability to debug API levels 16-19. 
The next time that min API level is bumped up, we can revisit this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144240

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

[Lldb-commits] [PATCH] D144240: Clear read_fd_set if EINTR received

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

Can you submit the change for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144240

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

[Lldb-commits] [PATCH] D144240: Clear read_fd_set if EINTR received

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

Sure, you are right that it should be pulled back into release/16.x, and I 
expect it to apply cleanly. However, I don't know what mechanism should bve 
used to create cherry-pick changes into release branches on GitHub: (1) this 
was not a GitHub issue/PR, so I guess I cannot use that mechanism; (2) I can 
run `git cherry-pick` myself, and then run `arc diff` to create a new 
Phabricator review (with a new D number?) but I don't know if that's the right 
way, (3) maybe there's some special "arc" commands to create a cherry-pick? (4) 
any other way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144240

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

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
emrekultursay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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 was causing the rendezvous mechanism to not work
in Android 28, which this CL fixes.

Test: Verified rendezvous on Android 28 and 29


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109797

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


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 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 different 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 different 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);


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 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 different 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 different 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);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 372711.
emrekultursay added a comment.

Applied clang-format


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


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 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 different 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 different 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);


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 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 different 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 different 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);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 372982.
emrekultursay added a comment.

Added m_initial_modules_added=true into LoadAllCurrentModules.


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


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 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 different 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 different 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);
@@ -620,6 +624,7 @@
   }
 
   m_process->GetTarget().ModulesDidLoad(module_list);
+  m_initial_modules_added = true;
 }
 
 addr_t DynamicLoaderPOSIXDYLD::ComputeLoadOffset() {


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 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 different 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 different 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);
@@ -620,6 +624,7 @@
   }
 
   m_process->GetTarget().ModulesDidLoad(module_list);
+  m_initial_modules_added = true;
 }
 
 addr_t DynamicLoaderPOSIXDYLD::ComputeLoadOffset() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

In D109797#3003265 , @labath wrote:

> Since the meaning of `m_initial_modules_added` is basically "should I do a 
> full scan through the linked list" and LoadAllCurrentModules (called from 
> DidAttach) does a full scan, it seems to me the real bug is that this 
> function does not set `m_initial_modules_added = true`. Would that fix your 
> issue?

Yes, it does fix my bug, thanks for the suggestion. Would you like me to remove 
the lines 445-447 that I added, or keep them as a secondary protection (since 
`module_sp != m_interpreter_module.lock()` condition doesn't seem to be strong 
enough).

In D109797#3003265 , @labath wrote:

> Also, what makes Android 28 special? Is it the presence/absence of a dynamic 
> linker symlink? I'm wondering if we could reproduce (test) this more 
> generally by ensuring we launch a binary through a symlink (or not).

On Android, `GetExecutableModule()` returns `GetModuleAtIndex(0)`. For Android 
27/29, this returns `app_process64` (correct), but for Android 28 it returns 
`[vdso]`, which is incorrect. This causes `ResolveExecutableModule()` to fall 
through due to `module_spec` (app_process64) not matching `module_sp` ([vdso]). 
This causes `target.SetExecutableModule` to be called, which triggers 
`ClearModules`. Since all modules are cleared, we cannot get the load address 
of the `.dynamic` section when we check whether we should rebase (i.e., 
`addr.GetLoadAddress(&target)` returns invalid address), thus 
`rebase_exec=true`.  The rest is in CL description.

To test this, we would need a binary that (1) doesn't have `eTypeExecutable` in 
any of its modules (2) has a module other than the executable module at 
location 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

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

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 373559.
emrekultursay added a comment.

Add new dlopen test


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/dlopen/Makefile
  lldb/test/API/functionalities/dlopen/TestDlopen.py
  lldb/test/API/functionalities/dlopen/b.cpp
  lldb/test/API/functionalities/dlopen/main.cpp

Index: lldb/test/API/functionalities/dlopen/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/main.cpp
@@ -0,0 +1,30 @@
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char* argv[]) {
+  const char* solib = "./liblib_b.so";
+  if (argc == 2) {
+solib = argv[1];
+  }
+  printf("Using solib at: %s\n", solib);
+
+  int main_thread_continue = 0;
+  // Wait until debugger is attached.
+  int i = 0;
+  for (i = 0; i < 100; i++) {
+usleep(1000*1000); // break here
+if (main_thread_continue) {
+  break;
+}
+  }
+  assert(i != 100 && "timed out waiting for debugger");
+
+  // dlopen the 'lib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  if (h == nullptr) {
+assert(h && "dlopen failed?"); // break on error
+  }
+  return i; // break after dlopen
+}
Index: lldb/test/API/functionalities/dlopen/b.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/b.cpp
@@ -0,0 +1,4 @@
+
+
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
+
Index: lldb/test/API/functionalities/dlopen/TestDlopen.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/TestDlopen.py
@@ -0,0 +1,60 @@
+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
+@skipIfWindows
+def test_dlopen_after_attach(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+so = self.getBuildArtifact("liblib_b.so")
+
+# 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(so)])
+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 lib_b.so before dlopen.
+self.match(
+"image list",
+patterns = ["liblib_b.so"],
+matching = False,
+msg = "liblib_b.so 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)
+process.Continue()
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that image list contains lib_b.so after dlopen.
+self.match(
+"image list",
+patterns = ["liblib_b.so"],
+matching = True,
+msg = "Missing liblib_b.so in image list")
+
Index: lldb/test/API/functionalities/dlopen/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/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
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
 &m_process->GetTarget()) == m_interpreter

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 373560.
emrekultursay added a comment.

Minor touch-ups to the newly added test.


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/dlopen/Makefile
  lldb/test/API/functionalities/dlopen/TestDlopen.py
  lldb/test/API/functionalities/dlopen/b.cpp
  lldb/test/API/functionalities/dlopen/main.cpp

Index: lldb/test/API/functionalities/dlopen/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/main.cpp
@@ -0,0 +1,29 @@
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char* argv[]) {
+  const char* solib = "./liblib_b.so";
+  if (argc == 2) {
+solib = argv[1];
+  }
+  printf("Using solib at: %s\n", solib);
+
+  int main_thread_continue = 0;
+  // Wait until debugger is attached.
+  int i = 0;
+  int timeout = 10;
+  for (i = 0; i < timeout; i++) {
+usleep(1000*1000); // break here
+if (main_thread_continue) {
+  break;
+}
+  }
+  assert(i != timeout && "timed out waiting for debugger");
+
+  // dlopen the 'liblib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  assert(h && "dlopen failed?");
+  return i; // break after dlopen
+}
Index: lldb/test/API/functionalities/dlopen/b.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/b.cpp
@@ -0,0 +1,4 @@
+
+
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
+
Index: lldb/test/API/functionalities/dlopen/TestDlopen.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/TestDlopen.py
@@ -0,0 +1,60 @@
+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
+@skipIfWindows
+def test_dlopen_after_attach(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+so = self.getBuildArtifact("liblib_b.so")
+
+# 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(so)])
+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 lib_b.so before dlopen.
+self.match(
+"image list",
+patterns = ["liblib_b.so"],
+matching = False,
+msg = "liblib_b.so 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)
+process.Continue()
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that image list contains lib_b.so after dlopen.
+self.match(
+"image list",
+patterns = ["liblib_b.so"],
+matching = True,
+msg = "Missing liblib_b.so in image list")
+
Index: lldb/test/API/functionalities/dlopen/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/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
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
 &m_process->GetTarget()) == m_interp

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

In D109797#3005885 , @labath wrote:

> I'm not sure all of this is relevant. I've just tried to debug a very simple 
> (linux) executable that does a dlopens a library. If I attach to the 
> executable before the dlopen call, lldb does not notice the new library. This 
> patch fixes the issue. So I guess the test could be just that. You could do 
> something similar to TestLoadUnload -- either add a new test case which 
> attaches to that binary, or use it as inspiration to write a new test.

PTAL. I added a new `dlopen` test for this. I can only test on Linux at the 
moment, so LMK if there's something I need to do to support more platforms or 
remove them from testing matrix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

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

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 374026.
emrekultursay added a comment.

Update test to pass on MacOS


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/dlopen/Makefile
  lldb/test/API/functionalities/dlopen/TestDlopen.py
  lldb/test/API/functionalities/dlopen/b.cpp
  lldb/test/API/functionalities/dlopen/main.cpp

Index: lldb/test/API/functionalities/dlopen/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/main.cpp
@@ -0,0 +1,29 @@
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char* argv[]) {
+  const char* solib = "./liblib_b.so";
+  if (argc == 2) {
+solib = argv[1];
+  }
+  printf("Using solib at: %s\n", solib);
+
+  int main_thread_continue = 0;
+  // Wait until debugger is attached.
+  int i = 0;
+  int timeout = 10;
+  for (i = 0; i < timeout; i++) {
+usleep(1000*1000); // break here
+if (main_thread_continue) {
+  break;
+}
+  }
+  assert(i != timeout && "timed out waiting for debugger");
+
+  // dlopen the 'liblib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  assert(h && "dlopen failed?");
+  return i; // break after dlopen
+}
Index: lldb/test/API/functionalities/dlopen/b.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/b.cpp
@@ -0,0 +1,4 @@
+
+
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
+
Index: lldb/test/API/functionalities/dlopen/TestDlopen.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/TestDlopen.py
@@ -0,0 +1,64 @@
+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
+@skipIfWindows
+def test_dlopen_after_attach(self):
+self.build()
+lib_name = 'liblib_b.so'
+if self.platformIsDarwin():
+  lib_name = 'liblib_b.dylib'
+
+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)
+process.Continue()
+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/dlopen/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/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
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 if (module_sp->GetObjectFile()-

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 374027.
emrekultursay added a comment.

Minor fix in test.


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/dlopen/Makefile
  lldb/test/API/functionalities/dlopen/TestDlopen.py
  lldb/test/API/functionalities/dlopen/b.cpp
  lldb/test/API/functionalities/dlopen/main.cpp

Index: lldb/test/API/functionalities/dlopen/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/main.cpp
@@ -0,0 +1,29 @@
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char* argv[]) {
+  const char* solib = "./liblib_b.so";
+  if (argc == 2) {
+solib = argv[1];
+  }
+  printf("Using solib at: %s\n", solib);
+
+  int main_thread_continue = 0;
+  // Wait until debugger is attached.
+  int i = 0;
+  int timeout = 10;
+  for (i = 0; i < timeout; i++) {
+usleep(1000*1000); // break here
+if (main_thread_continue) {
+  break;
+}
+  }
+  assert(i != timeout && "timed out waiting for debugger");
+
+  // dlopen the 'liblib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  assert(h && "dlopen failed?");
+  return i; // break after dlopen
+}
Index: lldb/test/API/functionalities/dlopen/b.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/b.cpp
@@ -0,0 +1,4 @@
+
+
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
+
Index: lldb/test/API/functionalities/dlopen/TestDlopen.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/TestDlopen.py
@@ -0,0 +1,64 @@
+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
+@skipIfWindows
+def test_dlopen_after_attach(self):
+self.build()
+lib_name = 'liblib_b.so'
+if self.platformIsDarwin():
+  lib_name = 'liblib_b.dylib'
+
+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/dlopen/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/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
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

plabath/mgorny: This change is ready for another round of review.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

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

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 374237.
emrekultursay edited the summary of this revision.
emrekultursay added a comment.

Make test also run on Windows.


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/dlopen/Makefile
  lldb/test/API/functionalities/dlopen/TestDlopen.py
  lldb/test/API/functionalities/dlopen/b.cpp
  lldb/test/API/functionalities/dlopen/main.cpp

Index: lldb/test/API/functionalities/dlopen/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/main.cpp
@@ -0,0 +1,51 @@
+#ifdef _WIN32
+#include 
+#else
+#include 
+#include 
+#endif
+
+#include 
+#include 
+
+void sleepFor(int milliseconds) {
+ #if _WIN32
+Sleep(milliseconds);
+#else
+usleep(milliseconds*1000);
+#endif
+}
+
+// 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++) {
+sleepFor(1000);  // 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/dlopen/b.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/b.cpp
@@ -0,0 +1,4 @@
+
+
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
+
Index: lldb/test/API/functionalities/dlopen/TestDlopen.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dlopen/TestDlopen.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_dlopen_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/dlopen/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/d

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

[Emre Kultursay via Phabricator via lldb-commits]

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

PTAL. The test now passes on Linux, MacOS, and Windows.




Comment at: lldb/test/API/functionalities/dlopen/main.cpp:26
+  // dlopen the 'liblib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  assert(h && "dlopen failed?");

labath wrote:
> see dylib.h and the functions within (the inferior of TestLoadUnload uses 
> them) for a windows-compatible way to load shared libraries.
Since we are attaching to an already running process, which cannot find the 
dynamic library unless I pass the full path to `dlopen()`. That's why I 
couldn't use dylib.h (which doesn't add full path), but created my own version 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

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

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

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 374536.
emrekultursay marked 4 inline comments as done.
emrekultursay added a comment.

Renamed test to load_after_attach.


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] D109797: Fix rendezvous for rebase_exec=true case

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

Done. Can you also submit it please?




Comment at: lldb/test/API/functionalities/dlopen/main.cpp:26
+  // dlopen the 'liblib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  assert(h && "dlopen failed?");

labath wrote:
> emrekultursay wrote:
> > labath wrote:
> > > see dylib.h and the functions within (the inferior of TestLoadUnload uses 
> > > them) for a windows-compatible way to load shared libraries.
> > Since we are attaching to an already running process, which cannot find the 
> > dynamic library unless I pass the full path to `dlopen()`. That's why I 
> > couldn't use dylib.h (which doesn't add full path), but created my own 
> > version here.
> Ok, what I think you're saying is that when we run a process for attaching, 
> we skip the code paths which set ((DY)LD_LIBRARY_)PATH, which is what makes 
> the relative imports work. It shouldn't be too hard to extend the launch 
> infrastructure to do that. Let's commit this in this form, and I'll do that 
> as a follow-up.
Yes. That SGTM. Thanks. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

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

[Lldb-commits] [PATCH] D76736: [LLDB] Fix parsing of IPv6 host:port inside brackets

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
emrekultursay added a reviewer: LLDB.
Herald added a project: LLDB.

When using IPv6 host:port pairs, typically the host is put inside
brackets, such as [2601:1234:...:0213]:, and the UriParser
can handle this format.

However, the Android infrastructure in LLDB assumes an additional
brackets around the host:port pair, such that the entire host:port
string can be treated as the host (which is used as an Android Serial
Number), and UriParser cannot handle multiple brackets. Parsing
inputs with such extra backets requires searching the closing bracket
from the right.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76736

Files:
  lldb/source/Utility/UriParser.cpp


Index: lldb/source/Utility/UriParser.cpp
===
--- lldb/source/Utility/UriParser.cpp
+++ lldb/source/Utility/UriParser.cpp
@@ -42,7 +42,7 @@
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
 // hostname is enclosed with square brackets.
-pos = host_port.find(']');
+pos = host_port.rfind(']');
 if (pos == std::string::npos)
   return false;
 


Index: lldb/source/Utility/UriParser.cpp
===
--- lldb/source/Utility/UriParser.cpp
+++ lldb/source/Utility/UriParser.cpp
@@ -42,7 +42,7 @@
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
 // hostname is enclosed with square brackets.
-pos = host_port.find(']');
+pos = host_port.rfind(']');
 if (pos == std::string::npos)
   return false;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76736: [LLDB] Fix parsing of IPv6 host:port inside brackets

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252499.
emrekultursay edited the summary of this revision.
emrekultursay added a comment.

Added unit tests.


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

https://reviews.llvm.org/D76736

Files:
  lldb/source/Utility/UriParser.cpp
  lldb/unittests/Utility/UriParserTest.cpp


Index: lldb/unittests/Utility/UriParserTest.cpp
===
--- lldb/unittests/Utility/UriParserTest.cpp
+++ lldb/unittests/Utility/UriParserTest.cpp
@@ -74,12 +74,18 @@
   VALIDATE
 }
 
-TEST(UriParserTest, TypicalPortPath) {
+TEST(UriParserTest, TypicalPortPathIPv4) {
   const UriTestCase testCase("connect://192.168.100.132:5432/", "connect",
  "192.168.100.132", 5432, "/");
   VALIDATE;
 }
 
+TEST(UriParserTest, TypicalPortPathIPv6) {
+  const UriTestCase 
testCase("connect://[2601:600:107f:db64:a42b:4faa:284:3082]:5432/", "connect",
+ "2601:600:107f:db64:a42b:4faa:284:3082", 5432, 
"/");
+  VALIDATE;
+}
+
 TEST(UriParserTest, BracketedHostnamePort) {
   const UriTestCase testCase("connect://[192.168.100.132]:5432/", "connect",
  "192.168.100.132", 5432, "/");
@@ -102,6 +108,18 @@
   VALIDATE
 }
 
+TEST(UriParserTest, BracketedHostnameWithPortIPv4) {
+  const UriTestCase testCase("connect://[192.168.100.132:1234]", "connect",
+ "192.168.100.132:1234", -1, "/");
+  VALIDATE
+}
+
+TEST(UriParserTest, BracketedHostnameWithPortIPv6) {
+  const UriTestCase 
testCase("connect://[[2601:600:107f:db64:a42b:4faa:284]:1234]", "connect",
+ "[2601:600:107f:db64:a42b:4faa:284]:1234", -1, 
"/");
+  VALIDATE
+}
+
 TEST(UriParserTest, BracketedHostnameWithColon) {
   const UriTestCase testCase("connect://[192.168.100.132:]:1234", 
"connect",
  "192.168.100.132:", 1234, "/");
Index: lldb/source/Utility/UriParser.cpp
===
--- lldb/source/Utility/UriParser.cpp
+++ lldb/source/Utility/UriParser.cpp
@@ -42,7 +42,7 @@
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
 // hostname is enclosed with square brackets.
-pos = host_port.find(']');
+pos = host_port.rfind(']');
 if (pos == std::string::npos)
   return false;
 


Index: lldb/unittests/Utility/UriParserTest.cpp
===
--- lldb/unittests/Utility/UriParserTest.cpp
+++ lldb/unittests/Utility/UriParserTest.cpp
@@ -74,12 +74,18 @@
   VALIDATE
 }
 
-TEST(UriParserTest, TypicalPortPath) {
+TEST(UriParserTest, TypicalPortPathIPv4) {
   const UriTestCase testCase("connect://192.168.100.132:5432/", "connect",
  "192.168.100.132", 5432, "/");
   VALIDATE;
 }
 
+TEST(UriParserTest, TypicalPortPathIPv6) {
+  const UriTestCase testCase("connect://[2601:600:107f:db64:a42b:4faa:284:3082]:5432/", "connect",
+ "2601:600:107f:db64:a42b:4faa:284:3082", 5432, "/");
+  VALIDATE;
+}
+
 TEST(UriParserTest, BracketedHostnamePort) {
   const UriTestCase testCase("connect://[192.168.100.132]:5432/", "connect",
  "192.168.100.132", 5432, "/");
@@ -102,6 +108,18 @@
   VALIDATE
 }
 
+TEST(UriParserTest, BracketedHostnameWithPortIPv4) {
+  const UriTestCase testCase("connect://[192.168.100.132:1234]", "connect",
+ "192.168.100.132:1234", -1, "/");
+  VALIDATE
+}
+
+TEST(UriParserTest, BracketedHostnameWithPortIPv6) {
+  const UriTestCase testCase("connect://[[2601:600:107f:db64:a42b:4faa:284]:1234]", "connect",
+ "[2601:600:107f:db64:a42b:4faa:284]:1234", -1, "/");
+  VALIDATE
+}
+
 TEST(UriParserTest, BracketedHostnameWithColon) {
   const UriTestCase testCase("connect://[192.168.100.132:]:1234", "connect",
  "192.168.100.132:", 1234, "/");
Index: lldb/source/Utility/UriParser.cpp
===
--- lldb/source/Utility/UriParser.cpp
+++ lldb/source/Utility/UriParser.cpp
@@ -42,7 +42,7 @@
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
 // hostname is enclosed with square brackets.
-pos = host_port.find(']');
+pos = host_port.rfind(']');
 if (pos == std::string::npos)
   return false;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76736: [LLDB] Fix parsing of IPv6 host:port inside brackets

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252622.
emrekultursay added a comment.

- Added comments to Android-specific test cases, as suggested by labath@.
- Reformatted lines that exceeded 80 chars.




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

https://reviews.llvm.org/D76736

Files:
  lldb/source/Utility/UriParser.cpp
  lldb/unittests/Utility/UriParserTest.cpp


Index: lldb/unittests/Utility/UriParserTest.cpp
===
--- lldb/unittests/Utility/UriParserTest.cpp
+++ lldb/unittests/Utility/UriParserTest.cpp
@@ -74,12 +74,19 @@
   VALIDATE
 }
 
-TEST(UriParserTest, TypicalPortPath) {
+TEST(UriParserTest, TypicalPortPathIPv4) {
   const UriTestCase testCase("connect://192.168.100.132:5432/", "connect",
  "192.168.100.132", 5432, "/");
   VALIDATE;
 }
 
+TEST(UriParserTest, TypicalPortPathIPv6) {
+  const UriTestCase testCase(
+  "connect://[2601:600:107f:db64:a42b:4faa:284:3082]:5432/", "connect",
+  "2601:600:107f:db64:a42b:4faa:284:3082", 5432, "/");
+  VALIDATE;
+}
+
 TEST(UriParserTest, BracketedHostnamePort) {
   const UriTestCase testCase("connect://[192.168.100.132]:5432/", "connect",
  "192.168.100.132", 5432, "/");
@@ -102,6 +109,21 @@
   VALIDATE
 }
 
+TEST(UriParserTest, BracketedHostnameWithPortIPv4) {
+  // Android device over IPv4: port is a part of the hostname.
+  const UriTestCase testCase("connect://[192.168.100.132:1234]", "connect",
+ "192.168.100.132:1234", -1, "/");
+  VALIDATE
+}
+
+TEST(UriParserTest, BracketedHostnameWithPortIPv6) {
+  // Android device over IPv6: port is a part of the hostname.
+  const UriTestCase testCase(
+  "connect://[[2601:600:107f:db64:a42b:4faa:284]:1234]", "connect",
+  "[2601:600:107f:db64:a42b:4faa:284]:1234", -1, "/");
+  VALIDATE
+}
+
 TEST(UriParserTest, BracketedHostnameWithColon) {
   const UriTestCase testCase("connect://[192.168.100.132:]:1234", 
"connect",
  "192.168.100.132:", 1234, "/");
Index: lldb/source/Utility/UriParser.cpp
===
--- lldb/source/Utility/UriParser.cpp
+++ lldb/source/Utility/UriParser.cpp
@@ -42,7 +42,7 @@
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
 // hostname is enclosed with square brackets.
-pos = host_port.find(']');
+pos = host_port.rfind(']');
 if (pos == std::string::npos)
   return false;
 


Index: lldb/unittests/Utility/UriParserTest.cpp
===
--- lldb/unittests/Utility/UriParserTest.cpp
+++ lldb/unittests/Utility/UriParserTest.cpp
@@ -74,12 +74,19 @@
   VALIDATE
 }
 
-TEST(UriParserTest, TypicalPortPath) {
+TEST(UriParserTest, TypicalPortPathIPv4) {
   const UriTestCase testCase("connect://192.168.100.132:5432/", "connect",
  "192.168.100.132", 5432, "/");
   VALIDATE;
 }
 
+TEST(UriParserTest, TypicalPortPathIPv6) {
+  const UriTestCase testCase(
+  "connect://[2601:600:107f:db64:a42b:4faa:284:3082]:5432/", "connect",
+  "2601:600:107f:db64:a42b:4faa:284:3082", 5432, "/");
+  VALIDATE;
+}
+
 TEST(UriParserTest, BracketedHostnamePort) {
   const UriTestCase testCase("connect://[192.168.100.132]:5432/", "connect",
  "192.168.100.132", 5432, "/");
@@ -102,6 +109,21 @@
   VALIDATE
 }
 
+TEST(UriParserTest, BracketedHostnameWithPortIPv4) {
+  // Android device over IPv4: port is a part of the hostname.
+  const UriTestCase testCase("connect://[192.168.100.132:1234]", "connect",
+ "192.168.100.132:1234", -1, "/");
+  VALIDATE
+}
+
+TEST(UriParserTest, BracketedHostnameWithPortIPv6) {
+  // Android device over IPv6: port is a part of the hostname.
+  const UriTestCase testCase(
+  "connect://[[2601:600:107f:db64:a42b:4faa:284]:1234]", "connect",
+  "[2601:600:107f:db64:a42b:4faa:284]:1234", -1, "/");
+  VALIDATE
+}
+
 TEST(UriParserTest, BracketedHostnameWithColon) {
   const UriTestCase testCase("connect://[192.168.100.132:]:1234", "connect",
  "192.168.100.132:", 1234, "/");
Index: lldb/source/Utility/UriParser.cpp
===
--- lldb/source/Utility/UriParser.cpp
+++ lldb/source/Utility/UriParser.cpp
@@ -42,7 +42,7 @@
   // Extract hostname
   if (!host_port.empty() && host_port[0] == '[') {
 // hostname is enclosed with square brackets.
-pos = host_port.find(']');
+pos = host_port.rfind(']');
 if (pos == std::string::npos)
   return false;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76736: [LLDB] Fix parsing of IPv6 host:port inside brackets

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

I don't have commit access.


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

https://reviews.llvm.org/D76736



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

[Lldb-commits] [PATCH] D76803: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
emrekultursay updated this revision to Diff 252675.
emrekultursay added a comment.
emrekultursay updated this revision to Diff 252677.

No changes


emrekultursay added a comment.

Adding 3rd commit to diff


...and replace it with m_last_file_spec instead.

When Source Cache is enabled, the value stored in m_last_file_sp is
already in the Source Cache, and caching it again in SourceManager
brings no extra benefit. All we need is to "remember" the last used
file, and FileSpec can serve the same purpose.

When Source Cache is disabled, the user explicitly requested no caching
of source files, and therefore, m_last_file_sp should NOT be used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76803

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/SourceManager.cpp

Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -52,27 +52,21 @@
 
 // SourceManager constructor
 SourceManager::SourceManager(const TargetSP &target_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(target_sp),
   m_debugger_wp(target_sp->GetDebugger().shared_from_this()) {}
 
 SourceManager::SourceManager(const DebuggerSP &debugger_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(), m_debugger_wp(debugger_sp) {}
 
 // Destructor
 SourceManager::~SourceManager() {}
 
 SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
-  bool same_as_previous =
-  m_last_file_sp &&
-  FileSpec::Match(file_spec, m_last_file_sp->GetFileSpec());
-
   DebuggerSP debugger_sp(m_debugger_wp.lock());
   FileSP file_sp;
-  if (same_as_previous)
-file_sp = m_last_file_sp;
-  else if (debugger_sp)
+  if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -95,7 +89,7 @@
 else
   file_sp = std::make_shared(file_spec, debugger_sp);
 
-if (debugger_sp)
+if (debugger_sp && debugger_sp->GetUseSourceCache())
   debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
   }
   return file_sp;
@@ -178,10 +172,11 @@
   m_last_line = start_line;
   m_last_count = count;
 
-  if (m_last_file_sp.get()) {
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp.get()) {
 const uint32_t end_line = start_line + count - 1;
 for (uint32_t line = start_line; line <= end_line; ++line) {
-  if (!m_last_file_sp->LineIsValid(line)) {
+  if (!last_file_sp->LineIsValid(line)) {
 m_last_line = UINT32_MAX;
 break;
   }
@@ -219,12 +214,12 @@
 columnToHighlight = column - 1;
 
   size_t this_line_size =
-  m_last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
+  last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
   if (column != 0 && line == curr_line &&
   should_show_stop_column_with_caret(debugger_sp)) {
 // Display caret cursor.
 std::string src_line;
-m_last_file_sp->GetLine(line, src_line);
+last_file_sp->GetLine(line, src_line);
 s->Printf("\t");
 // Insert a space for every non-tab character in the source line.
 for (size_t i = 0; i + 1 < column && i < src_line.length(); ++i)
@@ -255,10 +250,11 @@
   else
 start_line = 1;
 
-  if (m_last_file_sp.get() != file_sp.get()) {
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp.get() != file_sp.get()) {
 if (line == 0)
   m_last_line = 0;
-m_last_file_sp = file_sp;
+m_last_file_spec = file_spec;
   }
   return DisplaySourceLinesWithLineNumbersUsingLastFile(
   start_line, count, line, column, current_line_cstr, s, bp_locs);
@@ -268,14 +264,15 @@
 Stream *s, uint32_t count, bool reverse, const SymbolContextList *bp_locs) {
   // If we get called before anybody has set a default file and line, then try
   // to figure it out here.
-  const bool have_default_file_line = m_last_file_sp && m_last_line > 0;
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  const bool have_default_file_line = last_file_sp && m_last_line > 0;
   if (!m_default_set) {
 FileSpec tmp_spec;
 uint32_t tmp_line;
 GetDefaultFileAndLine(tmp_spec, tmp_line);
   }
 
-  if (m_last_file_sp) {
+  if (last_file_sp) {
 if (m_last_line == UINT32_MAX)
   return 0;
 
@@ -310,22 +307,22 @@
 
 bool SourceManager::SetDefaultFileAndLine(const FileSpec &file_sp

[Lldb-commits] [PATCH] D76803: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252677.
emrekultursay added a comment.

Adding 3rd commit to diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76803

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/SourceManager.cpp

Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -52,27 +52,21 @@
 
 // SourceManager constructor
 SourceManager::SourceManager(const TargetSP &target_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(target_sp),
   m_debugger_wp(target_sp->GetDebugger().shared_from_this()) {}
 
 SourceManager::SourceManager(const DebuggerSP &debugger_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(), m_debugger_wp(debugger_sp) {}
 
 // Destructor
 SourceManager::~SourceManager() {}
 
 SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
-  bool same_as_previous =
-  m_last_file_sp &&
-  FileSpec::Match(file_spec, m_last_file_sp->GetFileSpec());
-
   DebuggerSP debugger_sp(m_debugger_wp.lock());
   FileSP file_sp;
-  if (same_as_previous)
-file_sp = m_last_file_sp;
-  else if (debugger_sp)
+  if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -95,7 +89,7 @@
 else
   file_sp = std::make_shared(file_spec, debugger_sp);
 
-if (debugger_sp)
+if (debugger_sp && debugger_sp->GetUseSourceCache())
   debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
   }
   return file_sp;
@@ -178,10 +172,11 @@
   m_last_line = start_line;
   m_last_count = count;
 
-  if (m_last_file_sp.get()) {
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp.get()) {
 const uint32_t end_line = start_line + count - 1;
 for (uint32_t line = start_line; line <= end_line; ++line) {
-  if (!m_last_file_sp->LineIsValid(line)) {
+  if (!last_file_sp->LineIsValid(line)) {
 m_last_line = UINT32_MAX;
 break;
   }
@@ -219,12 +214,12 @@
 columnToHighlight = column - 1;
 
   size_t this_line_size =
-  m_last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
+  last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
   if (column != 0 && line == curr_line &&
   should_show_stop_column_with_caret(debugger_sp)) {
 // Display caret cursor.
 std::string src_line;
-m_last_file_sp->GetLine(line, src_line);
+last_file_sp->GetLine(line, src_line);
 s->Printf("\t");
 // Insert a space for every non-tab character in the source line.
 for (size_t i = 0; i + 1 < column && i < src_line.length(); ++i)
@@ -255,10 +250,11 @@
   else
 start_line = 1;
 
-  if (m_last_file_sp.get() != file_sp.get()) {
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp.get() != file_sp.get()) {
 if (line == 0)
   m_last_line = 0;
-m_last_file_sp = file_sp;
+m_last_file_spec = file_spec;
   }
   return DisplaySourceLinesWithLineNumbersUsingLastFile(
   start_line, count, line, column, current_line_cstr, s, bp_locs);
@@ -268,14 +264,15 @@
 Stream *s, uint32_t count, bool reverse, const SymbolContextList *bp_locs) {
   // If we get called before anybody has set a default file and line, then try
   // to figure it out here.
-  const bool have_default_file_line = m_last_file_sp && m_last_line > 0;
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  const bool have_default_file_line = last_file_sp && m_last_line > 0;
   if (!m_default_set) {
 FileSpec tmp_spec;
 uint32_t tmp_line;
 GetDefaultFileAndLine(tmp_spec, tmp_line);
   }
 
-  if (m_last_file_sp) {
+  if (last_file_sp) {
 if (m_last_line == UINT32_MAX)
   return 0;
 
@@ -310,22 +307,22 @@
 
 bool SourceManager::SetDefaultFileAndLine(const FileSpec &file_spec,
   uint32_t line) {
-  FileSP old_file_sp = m_last_file_sp;
-  m_last_file_sp = GetFile(file_spec);
-
   m_default_set = true;
-  if (m_last_file_sp) {
+  FileSP file_sp(GetFile(file_spec));
+
+  if (file_sp) {
 m_last_line = line;
+m_last_file_spec = file_spec;
 return true;
   } else {
-m_last_file_sp = old_file_sp;
 return false;
   }
 }
 
 bool SourceManager::GetDefaultFileAndLine(FileSpec &file_spec, uint32_t &line) {
-  if (m_last_file_sp) {
-file_spec = m_last_file_sp->GetFileSpec();
+  FileSP last_fi

[Lldb-commits] [PATCH] D76803: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252675.
emrekultursay added a comment.

No changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76803

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp

Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -52,27 +52,21 @@
 
 // SourceManager constructor
 SourceManager::SourceManager(const TargetSP &target_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(target_sp),
   m_debugger_wp(target_sp->GetDebugger().shared_from_this()) {}
 
 SourceManager::SourceManager(const DebuggerSP &debugger_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(), m_debugger_wp(debugger_sp) {}
 
 // Destructor
 SourceManager::~SourceManager() {}
 
 SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
-  bool same_as_previous =
-  m_last_file_sp &&
-  FileSpec::Match(file_spec, m_last_file_sp->GetFileSpec());
-
   DebuggerSP debugger_sp(m_debugger_wp.lock());
   FileSP file_sp;
-  if (same_as_previous)
-file_sp = m_last_file_sp;
-  else if (debugger_sp && debugger_sp->GetUseSourceCache())
+  if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -178,10 +172,11 @@
   m_last_line = start_line;
   m_last_count = count;
 
-  if (m_last_file_sp.get()) {
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp.get()) {
 const uint32_t end_line = start_line + count - 1;
 for (uint32_t line = start_line; line <= end_line; ++line) {
-  if (!m_last_file_sp->LineIsValid(line)) {
+  if (!last_file_sp->LineIsValid(line)) {
 m_last_line = UINT32_MAX;
 break;
   }
@@ -219,12 +214,12 @@
 columnToHighlight = column - 1;
 
   size_t this_line_size =
-  m_last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
+  last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
   if (column != 0 && line == curr_line &&
   should_show_stop_column_with_caret(debugger_sp)) {
 // Display caret cursor.
 std::string src_line;
-m_last_file_sp->GetLine(line, src_line);
+last_file_sp->GetLine(line, src_line);
 s->Printf("\t");
 // Insert a space for every non-tab character in the source line.
 for (size_t i = 0; i + 1 < column && i < src_line.length(); ++i)
@@ -255,10 +250,11 @@
   else
 start_line = 1;
 
-  if (m_last_file_sp.get() != file_sp.get()) {
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp.get() != file_sp.get()) {
 if (line == 0)
   m_last_line = 0;
-m_last_file_sp = file_sp;
+m_last_file_spec = file_spec;
   }
   return DisplaySourceLinesWithLineNumbersUsingLastFile(
   start_line, count, line, column, current_line_cstr, s, bp_locs);
@@ -268,14 +264,15 @@
 Stream *s, uint32_t count, bool reverse, const SymbolContextList *bp_locs) {
   // If we get called before anybody has set a default file and line, then try
   // to figure it out here.
-  const bool have_default_file_line = m_last_file_sp && m_last_line > 0;
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  const bool have_default_file_line = last_file_sp && m_last_line > 0;
   if (!m_default_set) {
 FileSpec tmp_spec;
 uint32_t tmp_line;
 GetDefaultFileAndLine(tmp_spec, tmp_line);
   }
 
-  if (m_last_file_sp) {
+  if (last_file_sp) {
 if (m_last_line == UINT32_MAX)
   return 0;
 
@@ -310,22 +307,22 @@
 
 bool SourceManager::SetDefaultFileAndLine(const FileSpec &file_spec,
   uint32_t line) {
-  FileSP old_file_sp = m_last_file_sp;
-  m_last_file_sp = GetFile(file_spec);
-
   m_default_set = true;
-  if (m_last_file_sp) {
+  FileSP file_sp(GetFile(file_spec));
+
+  if (file_sp) {
 m_last_line = line;
+m_last_file_spec = file_spec;
 return true;
   } else {
-m_last_file_sp = old_file_sp;
 return false;
   }
 }
 
 bool SourceManager::GetDefaultFileAndLine(FileSpec &file_spec, uint32_t &line) {
-  if (m_last_file_sp) {
-file_spec = m_last_file_sp->GetFileSpec();
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp) {
+file_spec = last_file_sp->GetFileSpec();
 line = m_last_line;
 return true;
   } else if (!m_default_set) {
@@ -355,7 +352,7 @@
 .GetBaseAddress()
 .CalculateSymbolContextLineEntry(line_entry)) {
   SetDefaultFileAndLine

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
emrekultursay added a child revision: D76806: Remove m_last_file_sp from 
SourceManager.

Lookup and subsequent insert was done using uninitialized
FileSpec object, which caused the cache to be a no-op.

Depends on D76804 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76805

Files:
  lldb/source/Core/SourceManager.cpp


Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();;
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;


Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();;
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76804: Add new LLDB setting: use-source-cache

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
emrekultursay added a child revision: D76805: Fix 
SourceManager::SourceFileCache insertion.

LLDB memory-maps large source files, and at the same time, caches
all source files in the Source Cache.

On Windows, memory-mapped source files are not writeable, causing
bad user experience in IDEs (such as errors when saving edited files).
IDEs should have the ability to disable the Source Cache at LLDB
startup, so that users can edit source files while debugging.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76804

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/SourceManager.cpp


Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -72,7 +72,7 @@
   FileSP file_sp;
   if (same_as_previous)
 file_sp = m_last_file_sp;
-  else if (debugger_sp)
+  else if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -95,7 +95,7 @@
 else
   file_sp = std::make_shared(file_spec, debugger_sp);
 
-if (debugger_sp)
+if (debugger_sp && debugger_sp->GetUseSourceCache())
   debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
   }
   return file_sp;
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -338,6 +338,18 @@
   return ret;
 }
 
+bool Debugger::GetUseSourceCache() const {
+  const uint32_t idx = ePropertyUseSourceCache;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
+}
+
+bool Debugger::SetUseSourceCache(bool b) {
+  const uint32_t idx = ePropertyUseSourceCache;
+  bool ret = m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
+  SetPrompt(GetPrompt());
+  return ret;
+}
 bool Debugger::GetHighlightSource() const {
   const uint32_t idx = ePropertyHighlightSource;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
Index: lldb/source/Core/CoreProperties.td
===
--- lldb/source/Core/CoreProperties.td
+++ lldb/source/Core/CoreProperties.td
@@ -103,6 +103,10 @@
 Global,
 DefaultTrue,
 Desc<"Whether to use Ansi color codes or not.">;
+  def UseSourceCache: Property<"use-source-cache", "Boolean">,
+Global,
+DefaultTrue,
+Desc<"Whether to cache source files in memory or not.">;
   def AutoOneLineSummaries: Property<"auto-one-line-summaries", "Boolean">,
 Global,
 DefaultTrue,
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -273,6 +273,10 @@
 
   bool SetUseColor(bool use_color);
 
+  bool GetUseSourceCache() const;
+
+  bool SetUseSourceCache(bool use_source_cache);
+
   bool GetHighlightSource() const;
 
   lldb::StopShowColumn GetStopShowColumn() const;


Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -72,7 +72,7 @@
   FileSP file_sp;
   if (same_as_previous)
 file_sp = m_last_file_sp;
-  else if (debugger_sp)
+  else if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -95,7 +95,7 @@
 else
   file_sp = std::make_shared(file_spec, debugger_sp);
 
-if (debugger_sp)
+if (debugger_sp && debugger_sp->GetUseSourceCache())
   debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
   }
   return file_sp;
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -338,6 +338,18 @@
   return ret;
 }
 
+bool Debugger::GetUseSourceCache() const {
+  const uint32_t idx = ePropertyUseSourceCache;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
+}
+
+bool Debugger::SetUseSourceCache(bool b) {
+  const uint32_t idx = ePropertyUseSourceCache;
+  bool ret = m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
+  SetPrompt(GetPrompt());
+  return ret;
+}
 bool Debugger::GetHighlightSource() const {
   const uint32_t idx = ePropertyHighlightSource;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
Index: lldb/source/Core/CoreProperties.td
===

[Lldb-commits] [PATCH] D76803: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252679.
emrekultursay added a comment.

- Remove m_last_file_sp from SourceManager


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76803

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/SourceManager.cpp

Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -52,27 +52,21 @@
 
 // SourceManager constructor
 SourceManager::SourceManager(const TargetSP &target_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(target_sp),
   m_debugger_wp(target_sp->GetDebugger().shared_from_this()) {}
 
 SourceManager::SourceManager(const DebuggerSP &debugger_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(), m_debugger_wp(debugger_sp) {}
 
 // Destructor
 SourceManager::~SourceManager() {}
 
 SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
-  bool same_as_previous =
-  m_last_file_sp &&
-  FileSpec::Match(file_spec, m_last_file_sp->GetFileSpec());
-
   DebuggerSP debugger_sp(m_debugger_wp.lock());
   FileSP file_sp;
-  if (same_as_previous)
-file_sp = m_last_file_sp;
-  else if (debugger_sp)
+  if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -95,7 +89,7 @@
 else
   file_sp = std::make_shared(file_spec, debugger_sp);
 
-if (debugger_sp)
+if (debugger_sp && debugger_sp->GetUseSourceCache())
   debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
   }
   return file_sp;
@@ -178,10 +172,11 @@
   m_last_line = start_line;
   m_last_count = count;
 
-  if (m_last_file_sp.get()) {
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp.get()) {
 const uint32_t end_line = start_line + count - 1;
 for (uint32_t line = start_line; line <= end_line; ++line) {
-  if (!m_last_file_sp->LineIsValid(line)) {
+  if (!last_file_sp->LineIsValid(line)) {
 m_last_line = UINT32_MAX;
 break;
   }
@@ -219,12 +214,12 @@
 columnToHighlight = column - 1;
 
   size_t this_line_size =
-  m_last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
+  last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
   if (column != 0 && line == curr_line &&
   should_show_stop_column_with_caret(debugger_sp)) {
 // Display caret cursor.
 std::string src_line;
-m_last_file_sp->GetLine(line, src_line);
+last_file_sp->GetLine(line, src_line);
 s->Printf("\t");
 // Insert a space for every non-tab character in the source line.
 for (size_t i = 0; i + 1 < column && i < src_line.length(); ++i)
@@ -255,10 +250,11 @@
   else
 start_line = 1;
 
-  if (m_last_file_sp.get() != file_sp.get()) {
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp.get() != file_sp.get()) {
 if (line == 0)
   m_last_line = 0;
-m_last_file_sp = file_sp;
+m_last_file_spec = file_spec;
   }
   return DisplaySourceLinesWithLineNumbersUsingLastFile(
   start_line, count, line, column, current_line_cstr, s, bp_locs);
@@ -268,14 +264,15 @@
 Stream *s, uint32_t count, bool reverse, const SymbolContextList *bp_locs) {
   // If we get called before anybody has set a default file and line, then try
   // to figure it out here.
-  const bool have_default_file_line = m_last_file_sp && m_last_line > 0;
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  const bool have_default_file_line = last_file_sp && m_last_line > 0;
   if (!m_default_set) {
 FileSpec tmp_spec;
 uint32_t tmp_line;
 GetDefaultFileAndLine(tmp_spec, tmp_line);
   }
 
-  if (m_last_file_sp) {
+  if (last_file_sp) {
 if (m_last_line == UINT32_MAX)
   return 0;
 
@@ -310,22 +307,22 @@
 
 bool SourceManager::SetDefaultFileAndLine(const FileSpec &file_spec,
   uint32_t line) {
-  FileSP old_file_sp = m_last_file_sp;
-  m_last_file_sp = GetFile(file_spec);
-
   m_default_set = true;
-  if (m_last_file_sp) {
+  FileSP file_sp(GetFile(file_spec));
+
+  if (file_sp) {
 m_last_line = line;
+m_last_file_spec = file_spec;
 return true;
   } else {
-m_last_file_sp = old_file_sp;
 return false;
   }
 }
 
 bool SourceManager::GetDefaultFileAndLine(FileSpec &file_spec, uint32_t &line) {
-  if (m_last_file_sp) {
-file_spec = m_last_file_sp->GetFileSpec();

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

...and replace it with m_last_file_spec instead.

When Source Cache is enabled, the value stored in m_last_file_sp is
already in the Source Cache, and caching it again in SourceManager
brings no extra benefit. All we need is to "remember" the last used
file, and FileSpec can serve the same purpose.

When Source Cache is disabled, the user explicitly requested no caching
of source files, and therefore, m_last_file_sp should NOT be used.

Depends on D76805 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp

Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -52,27 +52,21 @@
 
 // SourceManager constructor
 SourceManager::SourceManager(const TargetSP &target_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(target_sp),
   m_debugger_wp(target_sp->GetDebugger().shared_from_this()) {}
 
 SourceManager::SourceManager(const DebuggerSP &debugger_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(), m_debugger_wp(debugger_sp) {}
 
 // Destructor
 SourceManager::~SourceManager() {}
 
 SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
-  bool same_as_previous =
-  m_last_file_sp &&
-  FileSpec::Match(file_spec, m_last_file_sp->GetFileSpec());
-
   DebuggerSP debugger_sp(m_debugger_wp.lock());
   FileSP file_sp;
-  if (same_as_previous)
-file_sp = m_last_file_sp;
-  else if (debugger_sp && debugger_sp->GetUseSourceCache())
+  if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -178,10 +172,11 @@
   m_last_line = start_line;
   m_last_count = count;
 
-  if (m_last_file_sp.get()) {
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp.get()) {
 const uint32_t end_line = start_line + count - 1;
 for (uint32_t line = start_line; line <= end_line; ++line) {
-  if (!m_last_file_sp->LineIsValid(line)) {
+  if (!last_file_sp->LineIsValid(line)) {
 m_last_line = UINT32_MAX;
 break;
   }
@@ -219,12 +214,12 @@
 columnToHighlight = column - 1;
 
   size_t this_line_size =
-  m_last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
+  last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
   if (column != 0 && line == curr_line &&
   should_show_stop_column_with_caret(debugger_sp)) {
 // Display caret cursor.
 std::string src_line;
-m_last_file_sp->GetLine(line, src_line);
+last_file_sp->GetLine(line, src_line);
 s->Printf("\t");
 // Insert a space for every non-tab character in the source line.
 for (size_t i = 0; i + 1 < column && i < src_line.length(); ++i)
@@ -255,10 +250,11 @@
   else
 start_line = 1;
 
-  if (m_last_file_sp.get() != file_sp.get()) {
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  if (last_file_sp.get() != file_sp.get()) {
 if (line == 0)
   m_last_line = 0;
-m_last_file_sp = file_sp;
+m_last_file_spec = file_spec;
   }
   return DisplaySourceLinesWithLineNumbersUsingLastFile(
   start_line, count, line, column, current_line_cstr, s, bp_locs);
@@ -268,14 +264,15 @@
 Stream *s, uint32_t count, bool reverse, const SymbolContextList *bp_locs) {
   // If we get called before anybody has set a default file and line, then try
   // to figure it out here.
-  const bool have_default_file_line = m_last_file_sp && m_last_line > 0;
+  FileSP last_file_sp(GetFile(m_last_file_spec));
+  const bool have_default_file_line = last_file_sp && m_last_line > 0;
   if (!m_default_set) {
 FileSpec tmp_spec;
 uint32_t tmp_line;
 GetDefaultFileAndLine(tmp_spec, tmp_line);
   }
 
-  if (m_last_file_sp) {
+  if (last_file_sp) {
 if (m_last_line == UINT32_MAX)
   return 0;
 
@@ -310,22 +307,22 @@
 
 bool SourceManager::SetDefaultFileAndLine(const FileSpec &file_spec,
   uint32_t line) {
-  FileSP old_file_sp = m_last_file_sp;
-  m_last_file_sp = GetFile(file_spec);
-
   m_default_set = true;
-  if (m_last_file_sp) {
+  FileSP file_sp(GetFile(file_spec));
+
+  if (file_sp) {
 m_last_line = line;
+m_last_file_spec = file_spec;
 return true;
   } else {
-m_last_file_sp = old_file_sp;
 return false;
   }
 }
 
 bool SourceManager::GetDefaultFileAndLine(FileSpec &file_spec, uint32_t

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252702.
emrekultursay added a comment.

fix link warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76805

Files:
  lldb/source/Core/SourceManager.cpp


Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;


Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76804: Add new LLDB setting: use-source-cache

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

I did not see any existing tests that validate setting/reading of properties. 
Can you point to me if you know any that exists?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76804



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

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252734.
emrekultursay added a comment.
Herald added a subscriber: mgorny.

Add unit tests for SourceManager::SourceFileCache


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76805

Files:
  lldb/source/Core/SourceManager.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/SourceManagerTest.cpp


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,64 @@
+//===-- SourceManagerTest.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/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/RegularExpression.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, AddSourceFile) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec file_spec("foo");
+  auto file_sp = std::make_shared(file_spec, nullptr);
+  cache.AddSourceFile(file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;  
+  
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp = std::make_shared(foo_file_spec, 
nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo
+  FileSpec another_foo_file_spec("foo");
+  SourceManager::FileSP result = cache.FindSourceFile(another_foo_file_spec);
+
+  // Expect found.
+  ASSERT_EQ(result, foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+   
+  // Query: bar
+  FileSpec bar_file_spec("bar");
+  SourceManager::FileSP result = cache.FindSourceFile(bar_file_spec);
+
+  // Expect not found.
+  ASSERT_EQ(result, nullptr);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(LLDBCoreTests
   MangledTest.cpp
   RichManglingContextTest.cpp
+  SourceManagerTest.cpp
   StreamCallbackTest.cpp
   UniqueCStringMapTest.cpp
 
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,64 @@
+//===-- SourceManagerTest.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/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/RegularExpression.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, AddSourceFile) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec file_spec("foo");
+  auto file_sp = std::make_shared(file_spec, nullptr);
+  cache.AddSourceFile(file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;  
+  
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp = std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo
+  FileSpec another_foo_file_spec("foo");
+  SourceManager::FileSP result = cache.FindSourceFile(another_foo_file_spec);
+
+  // Expect found.
+  ASSERT_EQ(result, foo_file_sp);

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

> Does this actually depend on the other patch? It looks like an independent 
> fix we could commit separately.

This bug seems to have existed forever. Fixing it means we will have source 
file cache enabled for the first time. If it causes any unforeseen issues, I'd 
like users to have the ability to disable the cache, which is why I made this 
change depend on the other change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76805



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

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252881.
emrekultursay marked 3 inline comments as done.
emrekultursay added a comment.

Applied labath@'s suggestions on test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76805

Files:
  lldb/source/Core/SourceManager.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/SourceManagerTest.cpp


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,48 @@
+//===-- SourceManagerTest.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/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;  
+  
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp = std::make_shared(foo_file_spec, 
nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo, expect found.
+  FileSpec another_foo_file_spec("foo");
+  ASSERT_EQ(cache.FindSourceFile(another_foo_file_spec), foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+   
+  // Query: bar, expect not found.
+  FileSpec bar_file_spec("bar");
+  ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(LLDBCoreTests
   MangledTest.cpp
   RichManglingContextTest.cpp
+  SourceManagerTest.cpp
   StreamCallbackTest.cpp
   UniqueCStringMapTest.cpp
 
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,48 @@
+//===-- SourceManagerTest.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/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;  
+  
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp = std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo, expect found.
+  FileSpec another_foo_file_spec("foo");
+  ASSERT_EQ(cache.FindSourceFile(another_foo_file_spec), foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+   
+  // Query: bar, expect not found.
+  FileSpec bar_file_spec("bar");
+  ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(LLDBCoreTests
   MangledTest.cpp
   RichManglingContextTest.cpp
+  Sou

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

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

In D76806#1943062 , @labath wrote:

> It's not clear to me what is the user-visible effect of this. It sounds like 
> there should be one, but I don't know what it is...


If LLDB will cache FileSP shared pointers, there should be only one place for 
caching, and it's the Source File Cache, which can be enabled/disabled by the 
user. 
User-facing effect of this change can be found in bug: llvm.org/PR45310


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806



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

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252888.
emrekultursay added a comment.

Applied suggestions from labath@


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp

Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -52,27 +52,21 @@
 
 // SourceManager constructor
 SourceManager::SourceManager(const TargetSP &target_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
+: m_last_line(0), m_last_count(0), m_default_set(false),
   m_target_wp(target_sp),
   m_debugger_wp(target_sp->GetDebugger().shared_from_this()) {}
 
 SourceManager::SourceManager(const DebuggerSP &debugger_sp)
-: m_last_file_sp(), m_last_line(0), m_last_count(0), m_default_set(false),
-  m_target_wp(), m_debugger_wp(debugger_sp) {}
+: m_last_line(0), m_last_count(0), m_default_set(false), m_target_wp(),
+  m_debugger_wp(debugger_sp) {}
 
 // Destructor
 SourceManager::~SourceManager() {}
 
 SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
-  bool same_as_previous =
-  m_last_file_sp &&
-  FileSpec::Match(file_spec, m_last_file_sp->GetFileSpec());
-
   DebuggerSP debugger_sp(m_debugger_wp.lock());
   FileSP file_sp;
-  if (same_as_previous)
-file_sp = m_last_file_sp;
-  else if (debugger_sp && debugger_sp->GetUseSourceCache())
+  if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -178,10 +172,10 @@
   m_last_line = start_line;
   m_last_count = count;
 
-  if (m_last_file_sp.get()) {
+  if (FileSP last_file_sp = GetFile(m_last_file_spec)) {
 const uint32_t end_line = start_line + count - 1;
 for (uint32_t line = start_line; line <= end_line; ++line) {
-  if (!m_last_file_sp->LineIsValid(line)) {
+  if (!last_file_sp->LineIsValid(line)) {
 m_last_line = UINT32_MAX;
 break;
   }
@@ -219,12 +213,12 @@
 columnToHighlight = column - 1;
 
   size_t this_line_size =
-  m_last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
+  last_file_sp->DisplaySourceLines(line, columnToHighlight, 0, 0, s);
   if (column != 0 && line == curr_line &&
   should_show_stop_column_with_caret(debugger_sp)) {
 // Display caret cursor.
 std::string src_line;
-m_last_file_sp->GetLine(line, src_line);
+last_file_sp->GetLine(line, src_line);
 s->Printf("\t");
 // Insert a space for every non-tab character in the source line.
 for (size_t i = 0; i + 1 < column && i < src_line.length(); ++i)
@@ -255,10 +249,11 @@
   else
 start_line = 1;
 
-  if (m_last_file_sp.get() != file_sp.get()) {
+  FileSP last_file_sp(GetLastFile());
+  if (last_file_sp.get() != file_sp.get()) {
 if (line == 0)
   m_last_line = 0;
-m_last_file_sp = file_sp;
+m_last_file_spec = file_spec;
   }
   return DisplaySourceLinesWithLineNumbersUsingLastFile(
   start_line, count, line, column, current_line_cstr, s, bp_locs);
@@ -268,14 +263,15 @@
 Stream *s, uint32_t count, bool reverse, const SymbolContextList *bp_locs) {
   // If we get called before anybody has set a default file and line, then try
   // to figure it out here.
-  const bool have_default_file_line = m_last_file_sp && m_last_line > 0;
+  FileSP last_file_sp(GetLastFile());
+  const bool have_default_file_line = last_file_sp && m_last_line > 0;
   if (!m_default_set) {
 FileSpec tmp_spec;
 uint32_t tmp_line;
 GetDefaultFileAndLine(tmp_spec, tmp_line);
   }
 
-  if (m_last_file_sp) {
+  if (last_file_sp) {
 if (m_last_line == UINT32_MAX)
   return 0;
 
@@ -310,22 +306,21 @@
 
 bool SourceManager::SetDefaultFileAndLine(const FileSpec &file_spec,
   uint32_t line) {
-  FileSP old_file_sp = m_last_file_sp;
-  m_last_file_sp = GetFile(file_spec);
-
   m_default_set = true;
-  if (m_last_file_sp) {
+  FileSP file_sp(GetFile(file_spec));
+
+  if (file_sp) {
 m_last_line = line;
+m_last_file_spec = file_spec;
 return true;
   } else {
-m_last_file_sp = old_file_sp;
 return false;
   }
 }
 
 bool SourceManager::GetDefaultFileAndLine(FileSpec &file_spec, uint32_t &line) {
-  if (m_last_file_sp) {
-file_spec = m_last_file_sp->GetFileSpec();
+  if (FileSP last_file_sp = GetLastFile()) {
+file_spec = last_file_sp->GetFileSpec();
 line = m_last_line;
 return true;
   } else if (!m_default_set) {
@@ -355,7 +350,7 @@
 .GetBaseAddress()
 .CalculateSymbolContextLineEntry(line_entry)) {
   SetDefaultFileAndLine

[Lldb-commits] [PATCH] D76804: Add new LLDB setting: use-source-cache

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 253013.
emrekultursay added a comment.

- Apply code review comments from labath@
- Wipe source cache when user sets the property to false
- Also expose set/get methods for use-source-cache in SBDebugger


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76804

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/SourceManager.cpp

Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -72,7 +72,7 @@
   FileSP file_sp;
   if (same_as_previous)
 file_sp = m_last_file_sp;
-  else if (debugger_sp)
+  else if (debugger_sp && debugger_sp->GetUseSourceCache())
 file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
 
   TargetSP target_sp(m_target_wp.lock());
@@ -95,7 +95,7 @@
 else
   file_sp = std::make_shared(file_spec, debugger_sp);
 
-if (debugger_sp)
+if (debugger_sp && debugger_sp->GetUseSourceCache())
   debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
   }
   return file_sp;
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -212,6 +212,11 @@
   // use-color changed. Ping the prompt so it can reset the ansi terminal
   // codes.
   SetPrompt(GetPrompt());
+} else if (property_path == g_debugger_properties[ePropertyUseSourceCache].name) {
+  // use-source-cache changed. Wipe out the cache contents if it was disabled.
+  if (!GetUseSourceCache()) {
+m_source_file_cache.Clear();
+  }
 } else if (is_load_script && target_sp &&
load_script_old_value == eLoadScriptFromSymFileWarn) {
   if (target_sp->TargetProperties::GetLoadScriptFromSymbolFile() ==
@@ -338,6 +343,20 @@
   return ret;
 }
 
+bool Debugger::GetUseSourceCache() const {
+  const uint32_t idx = ePropertyUseSourceCache;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
+}
+
+bool Debugger::SetUseSourceCache(bool b) {
+  const uint32_t idx = ePropertyUseSourceCache;
+  bool ret = m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
+  if (!ret) {
+m_source_file_cache.Clear();
+  }
+  return ret;
+}
 bool Debugger::GetHighlightSource() const {
   const uint32_t idx = ePropertyHighlightSource;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
Index: lldb/source/Core/CoreProperties.td
===
--- lldb/source/Core/CoreProperties.td
+++ lldb/source/Core/CoreProperties.td
@@ -103,6 +103,10 @@
 Global,
 DefaultTrue,
 Desc<"Whether to use Ansi color codes or not.">;
+  def UseSourceCache: Property<"use-source-cache", "Boolean">,
+Global,
+DefaultTrue,
+Desc<"Whether to cache source files in memory or not.">;
   def AutoOneLineSummaries: Property<"auto-one-line-summaries", "Boolean">,
 Global,
 DefaultTrue,
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -1374,6 +1374,18 @@
   return (m_opaque_sp ? m_opaque_sp->GetUseColor() : false);
 }
 
+bool SBDebugger::SetUseSourceCache(bool value) {
+  LLDB_RECORD_METHOD(bool, SBDebugger, SetUseSourceCache, (bool), value);
+
+  return (m_opaque_sp ? m_opaque_sp->SetUseSourceCache(value) : false);
+}
+
+bool SBDebugger::GetUseSourceCache() const {
+  LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBDebugger, GetUseSourceCache);
+
+  return (m_opaque_sp ? m_opaque_sp->GetUseSourceCache() : false);
+}
+
 bool SBDebugger::GetDescription(SBStream &description) {
   LLDB_RECORD_METHOD(bool, SBDebugger, GetDescription, (lldb::SBStream &),
  description);
Index: lldb/include/lldb/Core/SourceManager.h
===
--- lldb/include/lldb/Core/SourceManager.h
+++ lldb/include/lldb/Core/SourceManager.h
@@ -101,6 +101,9 @@
 void AddSourceFile(const FileSP &file_sp);
 FileSP FindSourceFile(const FileSpec &file_spec) const;
 
+// Removes all elements from the cache.
+void Clear() { m_file_cache.clear(); }
+
   protected:
 typedef std::map FileCache;
 FileCache m_file_cache;
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -273,6 +273,10 @@
 
   bool SetUseColor(bool use_color);
 
+  bool GetUseSourceCache() const;
+

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 253254.
emrekultursay added a comment.

Added test that verifies end-to-end impact of
"setting set use_source_cache false" on Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/commands/settings/use_source_cache/Makefile
  lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
  lldb/test/API/commands/settings/use_source_cache/header.h
  lldb/test/API/commands/settings/use_source_cache/main.cpp

Index: lldb/test/API/commands/settings/use_source_cache/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/main.cpp
@@ -0,0 +1,17 @@
+#include 
+#include 
+
+#include "header.h"
+
+int
+main(int argc, char const *argv[])
+{
+  //for (int j=0;;j++) {
+//Sleep(1000);
+int j = 0;
+fprintf(stderr, "Hello, world (%d) => %d\n", j, calc(j, j+1, j+2));
+  //}
+
+  return 0;
+}
+
Index: lldb/test/API/commands/settings/use_source_cache/header.h
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/header.h
@@ -0,0 +1,1006 @@
+// This file should be large enough that LLDB decides to load it using memory mapping.
+// See: ⁠llvm/lib/Support/MemoryBuffer.cp:shouldUseMmap()
+
+// The name of this function is used in tests to set breakpoints by name.
+int calc(int x0, int x1, int x2) {
+  int x3 = x2 * x1 + x0;
+  int x4 = x3 * x2 + x1;
+  int x5 = x4 * x3 + x2;
+  int x6 = x5 * x4 + x3;
+  int x7 = x6 * x5 + x4;
+  int x8 = x7 * x6 + x5;
+  int x9 = x8 * x7 + x6;
+  int x10 = x9 * x8 + x7;
+  int x11 = x10 * x9 + x8;
+  int x12 = x11 * x10 + x9;
+  int x13 = x12 * x11 + x10;
+  int x14 = x13 * x12 + x11;
+  int x15 = x14 * x13 + x12;
+  int x16 = x15 * x14 + x13;
+  int x17 = x16 * x15 + x14;
+  int x18 = x17 * x16 + x15;
+  int x19 = x18 * x17 + x16;
+  int x20 = x19 * x18 + x17;  
+  int x21 = x20 * x19 + x18;
+  int x22 = x21 * x20 + x19;
+  int x23 = x22 * x21 + x20;
+  int x24 = x23 * x22 + x21;
+  int x25 = x24 * x23 + x22;
+  int x26 = x25 * x24 + x23;
+  int x27 = x26 * x25 + x24;
+  int x28 = x27 * x26 + x25;
+  int x29 = x28 * x27 + x26;
+  int x30 = x29 * x28 + x27;
+  int x31 = x30 * x29 + x28;
+  int x32 = x31 * x30 + x29;
+  int x33 = x32 * x31 + x30;
+  int x34 = x33 * x32 + x31;
+  int x35 = x34 * x33 + x32;
+  int x36 = x35 * x34 + x33;
+  int x37 = x36 * x35 + x34;
+  int x38 = x37 * x36 + x35;
+  int x39 = x38 * x37 + x36;
+  int x40 = x39 * x38 + x37;
+  int x41 = x40 * x39 + x38;
+  int x42 = x41 * x40 + x39;
+  int x43 = x42 * x41 + x40;
+  int x44 = x43 * x42 + x41;
+  int x45 = x44 * x43 + x42;
+  int x46 = x45 * x44 + x43;
+  int x47 = x46 * x45 + x44;
+  int x48 = x47 * x46 + x45;
+  int x49 = x48 * x47 + x46;
+  int x50 = x49 * x48 + x47;
+  int x51 = x50 * x49 + x48;
+  int x52 = x51 * x50 + x49;
+  int x53 = x52 * x51 + x50;
+  int x54 = x53 * x52 + x51;
+  int x55 = x54 * x53 + x52;
+  int x56 = x55 * x54 + x53;
+  int x57 = x56 * x55 + x54;
+  int x58 = x57 * x56 + x55;
+  int x59 = x58 * x57 + x56;
+  int x60 = x59 * x58 + x57;
+  int x61 = x60 * x59 + x58;
+  int x62 = x61 * x60 + x59;
+  int x63 = x62 * x61 + x60;
+  int x64 = x63 * x62 + x61;
+  int x65 = x64 * x63 + x62;
+  int x66 = x65 * x64 + x63;
+  int x67 = x66 * x65 + x64;
+  int x68 = x67 * x66 + x65;
+  int x69 = x68 * x67 + x66;
+  int x70 = x69 * x68 + x67;
+  int x71 = x70 * x69 + x68;
+  int x72 = x71 * x70 + x69;
+  int x73 = x72 * x71 + x70;
+  int x74 = x73 * x72 + x71;
+  int x75 = x74 * x73 + x72;
+  int x76 = x75 * x74 + x73;
+  int x77 = x76 * x75 + x74;
+  int x78 = x77 * x76 + x75;
+  int x79 = x78 * x77 + x76;
+  int x80 = x79 * x78 + x77;
+  int x81 = x80 * x79 + x78;
+  int x82 = x81 * x80 + x79;
+  int x83 = x82 * x81 + x80;
+  int x84 = x83 * x82 + x81;
+  int x85 = x84 * x83 + x82;
+  int x86 = x85 * x84 + x83;
+  int x87 = x86 * x85 + x84;
+  int x88 = x87 * x86 + x85;
+  int x89 = x88 * x87 + x86;
+  int x90 = x89 * x88 + x87;
+  int x91 = x90 * x89 + x88;
+  int x92 = x91 * x90 + x89;
+  int x93 = x92 * x91 + x90;
+  int x94 = x93 * x92 + x91;
+  int x95 = x94 * x93 + x92;
+  int x96 = x95 * x94 + x93;
+  int x97 = x96 * x95 + x94;
+  int x98 = x97 * x96 + x95;
+  int x99 = x98 * x97 + x96;
+  int x100 = x99 * x98 + x97;
+  int x101 = x100 * x99 + x98;
+  int x102 = x101 * x100 + x99;
+  int x103 = x102 * x101 + x100;
+  int x104 = x103 * x102 + x101;
+  int x105 = x104 * x103 + x102;
+  int x106 = x105 * x104 + x103;
+  int x107 = x106 * x105 + x104;
+  int x108 = x107 * x106 + x105;
+  int x109 = x108 * x107 + x106;
+  int x110 = x109 * x108 + x107;
+  int x111 = x110 * x109 + x108;
+  int x112 = x111 * x110 + x109;
+  int x113 = x112 * x111 + x110;
+  int x114 = x113 * x112 

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806



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

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 253260.
emrekultursay added a comment.

Cleanup the Python test to make it a bit more readable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/commands/settings/use_source_cache/Makefile
  lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
  lldb/test/API/commands/settings/use_source_cache/header.h
  lldb/test/API/commands/settings/use_source_cache/main.cpp

Index: lldb/test/API/commands/settings/use_source_cache/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/main.cpp
@@ -0,0 +1,11 @@
+#include 
+
+#include "header.h"
+
+int
+main(int argc, char const *argv[])
+{
+  fprintf(stderr, "Hello, world => %d\n", calc(0, 1, 2));
+  return 0;
+}
+
Index: lldb/test/API/commands/settings/use_source_cache/header.h
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/header.h
@@ -0,0 +1,607 @@
+// This file should be large enough that LLDB decides to load it
+// using memory mapping. See:
+//   ⁠llvm/lib/Support/MemoryBuffer.cp:shouldUseMmap()
+
+// The name of this function is used in tests to set breakpoints by name.
+int calc(int x0, int x1, int x2) {
+  int x3 = x2 * x1 + x0;
+  int x4 = x3 * x2 + x1;
+  int x5 = x4 * x3 + x2;
+  int x6 = x5 * x4 + x3;
+  int x7 = x6 * x5 + x4;
+  int x8 = x7 * x6 + x5;
+  int x9 = x8 * x7 + x6;
+  int x10 = x9 * x8 + x7;
+  int x11 = x10 * x9 + x8;
+  int x12 = x11 * x10 + x9;
+  int x13 = x12 * x11 + x10;
+  int x14 = x13 * x12 + x11;
+  int x15 = x14 * x13 + x12;
+  int x16 = x15 * x14 + x13;
+  int x17 = x16 * x15 + x14;
+  int x18 = x17 * x16 + x15;
+  int x19 = x18 * x17 + x16;
+  int x20 = x19 * x18 + x17;  
+  int x21 = x20 * x19 + x18;
+  int x22 = x21 * x20 + x19;
+  int x23 = x22 * x21 + x20;
+  int x24 = x23 * x22 + x21;
+  int x25 = x24 * x23 + x22;
+  int x26 = x25 * x24 + x23;
+  int x27 = x26 * x25 + x24;
+  int x28 = x27 * x26 + x25;
+  int x29 = x28 * x27 + x26;
+  int x30 = x29 * x28 + x27;
+  int x31 = x30 * x29 + x28;
+  int x32 = x31 * x30 + x29;
+  int x33 = x32 * x31 + x30;
+  int x34 = x33 * x32 + x31;
+  int x35 = x34 * x33 + x32;
+  int x36 = x35 * x34 + x33;
+  int x37 = x36 * x35 + x34;
+  int x38 = x37 * x36 + x35;
+  int x39 = x38 * x37 + x36;
+  int x40 = x39 * x38 + x37;
+  int x41 = x40 * x39 + x38;
+  int x42 = x41 * x40 + x39;
+  int x43 = x42 * x41 + x40;
+  int x44 = x43 * x42 + x41;
+  int x45 = x44 * x43 + x42;
+  int x46 = x45 * x44 + x43;
+  int x47 = x46 * x45 + x44;
+  int x48 = x47 * x46 + x45;
+  int x49 = x48 * x47 + x46;
+  int x50 = x49 * x48 + x47;
+  int x51 = x50 * x49 + x48;
+  int x52 = x51 * x50 + x49;
+  int x53 = x52 * x51 + x50;
+  int x54 = x53 * x52 + x51;
+  int x55 = x54 * x53 + x52;
+  int x56 = x55 * x54 + x53;
+  int x57 = x56 * x55 + x54;
+  int x58 = x57 * x56 + x55;
+  int x59 = x58 * x57 + x56;
+  int x60 = x59 * x58 + x57;
+  int x61 = x60 * x59 + x58;
+  int x62 = x61 * x60 + x59;
+  int x63 = x62 * x61 + x60;
+  int x64 = x63 * x62 + x61;
+  int x65 = x64 * x63 + x62;
+  int x66 = x65 * x64 + x63;
+  int x67 = x66 * x65 + x64;
+  int x68 = x67 * x66 + x65;
+  int x69 = x68 * x67 + x66;
+  int x70 = x69 * x68 + x67;
+  int x71 = x70 * x69 + x68;
+  int x72 = x71 * x70 + x69;
+  int x73 = x72 * x71 + x70;
+  int x74 = x73 * x72 + x71;
+  int x75 = x74 * x73 + x72;
+  int x76 = x75 * x74 + x73;
+  int x77 = x76 * x75 + x74;
+  int x78 = x77 * x76 + x75;
+  int x79 = x78 * x77 + x76;
+  int x80 = x79 * x78 + x77;
+  int x81 = x80 * x79 + x78;
+  int x82 = x81 * x80 + x79;
+  int x83 = x82 * x81 + x80;
+  int x84 = x83 * x82 + x81;
+  int x85 = x84 * x83 + x82;
+  int x86 = x85 * x84 + x83;
+  int x87 = x86 * x85 + x84;
+  int x88 = x87 * x86 + x85;
+  int x89 = x88 * x87 + x86;
+  int x90 = x89 * x88 + x87;
+  int x91 = x90 * x89 + x88;
+  int x92 = x91 * x90 + x89;
+  int x93 = x92 * x91 + x90;
+  int x94 = x93 * x92 + x91;
+  int x95 = x94 * x93 + x92;
+  int x96 = x95 * x94 + x93;
+  int x97 = x96 * x95 + x94;
+  int x98 = x97 * x96 + x95;
+  int x99 = x98 * x97 + x96;
+  int x100 = x99 * x98 + x97;
+  int x101 = x100 * x99 + x98;
+  int x102 = x101 * x100 + x99;
+  int x103 = x102 * x101 + x100;
+  int x104 = x103 * x102 + x101;
+  int x105 = x104 * x103 + x102;
+  int x106 = x105 * x104 + x103;
+  int x107 = x106 * x105 + x104;
+  int x108 = x107 * x106 + x105;
+  int x109 = x108 * x107 + x106;
+  int x110 = x109 * x108 + x107;
+  int x111 = x110 * x109 + x108;
+  int x112 = x111 * x110 + x109;
+  int x113 = x112 * x111 + x110;
+  int x114 = x113 * x112 + x111;
+  int x115 = x114 * x113 + x112;
+  int x116 = x115 * x114 + x113;
+  int x117 = x116 * x115 + x114;
+  int x118 = x117 * 

[Lldb-commits] [PATCH] D76804: Add new LLDB setting: use-source-cache

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

I added an end-to-end test to the entire use-source-cache feature in 
https://reviews.llvm.org/D76806


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76804



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

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

In D76805#1949642 , @labath wrote:

> Ok, that makes kind of sense, though I am left wondering if we really need 
> this feature, given that we have survived so long without noticing it is 
> missing...
>
> Am I understanding it correctly that without this patch, we would only cache 
> the most recently accessed file (via `m_last_file_sp` member), and would 
> always reload when switching to a new file?


Yes, without this patch, only the most recently accessed file is cached inside 
that member, and switching to a new file replaces that entry. I guess this was 
designed for optimizing the case where the user hits "next line" while staying 
in the same file. Yet, if a breakpoint on a different file is hit during that 
"next line" command, we trigger a reload (twice).

This patch will increase memory usage, as mapped source files will stay in the 
cache, and thus, never be unmapped. The increase will be and proportional to 
the size of source files loaded by LLDB on-demand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76805



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

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 253648.
emrekultursay marked 6 inline comments as done.
emrekultursay added a comment.

PTAL. Applied suggested changes, thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/commands/settings/use_source_cache/Makefile
  lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
  lldb/test/API/commands/settings/use_source_cache/main.cpp

Index: lldb/test/API/commands/settings/use_source_cache/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/main.cpp
@@ -0,0 +1,619 @@
+// This file should be large enough that LLDB decides to load it
+// using memory mapping. See:
+//   ⁠llvm/lib/Support/MemoryBuffer.cp:shouldUseMmap()
+
+#include 
+
+int calc(int x0, int x1, int x2);
+
+int
+main(int argc, char const *argv[])
+{
+  fprintf(stderr, "Hello, world => %d\n", calc(0, 1, 2));
+  return 0;
+}
+
+
+// The name of this function is used in tests to set breakpoints by name.
+int calc(int x0, int x1, int x2) {
+  int x3 = x2 * x1 + x0;
+  int x4 = x3 * x2 + x1;
+  int x5 = x4 * x3 + x2;
+  int x6 = x5 * x4 + x3;
+  int x7 = x6 * x5 + x4;
+  int x8 = x7 * x6 + x5;
+  int x9 = x8 * x7 + x6;
+  int x10 = x9 * x8 + x7;
+  int x11 = x10 * x9 + x8;
+  int x12 = x11 * x10 + x9;
+  int x13 = x12 * x11 + x10;
+  int x14 = x13 * x12 + x11;
+  int x15 = x14 * x13 + x12;
+  int x16 = x15 * x14 + x13;
+  int x17 = x16 * x15 + x14;
+  int x18 = x17 * x16 + x15;
+  int x19 = x18 * x17 + x16;
+  int x20 = x19 * x18 + x17;  
+  int x21 = x20 * x19 + x18;
+  int x22 = x21 * x20 + x19;
+  int x23 = x22 * x21 + x20;
+  int x24 = x23 * x22 + x21;
+  int x25 = x24 * x23 + x22;
+  int x26 = x25 * x24 + x23;
+  int x27 = x26 * x25 + x24;
+  int x28 = x27 * x26 + x25;
+  int x29 = x28 * x27 + x26;
+  int x30 = x29 * x28 + x27;
+  int x31 = x30 * x29 + x28;
+  int x32 = x31 * x30 + x29;
+  int x33 = x32 * x31 + x30;
+  int x34 = x33 * x32 + x31;
+  int x35 = x34 * x33 + x32;
+  int x36 = x35 * x34 + x33;
+  int x37 = x36 * x35 + x34;
+  int x38 = x37 * x36 + x35;
+  int x39 = x38 * x37 + x36;
+  int x40 = x39 * x38 + x37;
+  int x41 = x40 * x39 + x38;
+  int x42 = x41 * x40 + x39;
+  int x43 = x42 * x41 + x40;
+  int x44 = x43 * x42 + x41;
+  int x45 = x44 * x43 + x42;
+  int x46 = x45 * x44 + x43;
+  int x47 = x46 * x45 + x44;
+  int x48 = x47 * x46 + x45;
+  int x49 = x48 * x47 + x46;
+  int x50 = x49 * x48 + x47;
+  int x51 = x50 * x49 + x48;
+  int x52 = x51 * x50 + x49;
+  int x53 = x52 * x51 + x50;
+  int x54 = x53 * x52 + x51;
+  int x55 = x54 * x53 + x52;
+  int x56 = x55 * x54 + x53;
+  int x57 = x56 * x55 + x54;
+  int x58 = x57 * x56 + x55;
+  int x59 = x58 * x57 + x56;
+  int x60 = x59 * x58 + x57;
+  int x61 = x60 * x59 + x58;
+  int x62 = x61 * x60 + x59;
+  int x63 = x62 * x61 + x60;
+  int x64 = x63 * x62 + x61;
+  int x65 = x64 * x63 + x62;
+  int x66 = x65 * x64 + x63;
+  int x67 = x66 * x65 + x64;
+  int x68 = x67 * x66 + x65;
+  int x69 = x68 * x67 + x66;
+  int x70 = x69 * x68 + x67;
+  int x71 = x70 * x69 + x68;
+  int x72 = x71 * x70 + x69;
+  int x73 = x72 * x71 + x70;
+  int x74 = x73 * x72 + x71;
+  int x75 = x74 * x73 + x72;
+  int x76 = x75 * x74 + x73;
+  int x77 = x76 * x75 + x74;
+  int x78 = x77 * x76 + x75;
+  int x79 = x78 * x77 + x76;
+  int x80 = x79 * x78 + x77;
+  int x81 = x80 * x79 + x78;
+  int x82 = x81 * x80 + x79;
+  int x83 = x82 * x81 + x80;
+  int x84 = x83 * x82 + x81;
+  int x85 = x84 * x83 + x82;
+  int x86 = x85 * x84 + x83;
+  int x87 = x86 * x85 + x84;
+  int x88 = x87 * x86 + x85;
+  int x89 = x88 * x87 + x86;
+  int x90 = x89 * x88 + x87;
+  int x91 = x90 * x89 + x88;
+  int x92 = x91 * x90 + x89;
+  int x93 = x92 * x91 + x90;
+  int x94 = x93 * x92 + x91;
+  int x95 = x94 * x93 + x92;
+  int x96 = x95 * x94 + x93;
+  int x97 = x96 * x95 + x94;
+  int x98 = x97 * x96 + x95;
+  int x99 = x98 * x97 + x96;
+  int x100 = x99 * x98 + x97;
+  int x101 = x100 * x99 + x98;
+  int x102 = x101 * x100 + x99;
+  int x103 = x102 * x101 + x100;
+  int x104 = x103 * x102 + x101;
+  int x105 = x104 * x103 + x102;
+  int x106 = x105 * x104 + x103;
+  int x107 = x106 * x105 + x104;
+  int x108 = x107 * x106 + x105;
+  int x109 = x108 * x107 + x106;
+  int x110 = x109 * x108 + x107;
+  int x111 = x110 * x109 + x108;
+  int x112 = x111 * x110 + x109;
+  int x113 = x112 * x111 + x110;
+  int x114 = x113 * x112 + x111;
+  int x115 = x114 * x113 + x112;
+  int x116 = x115 * x114 + x113;
+  int x117 = x116 * x115 + x114;
+  int x118 = x117 * x116 + x115;
+  int x119 = x118 * x117 + x116;
+  int x120 = x119 * x118 + x117;
+  int x121 = x120 * x119 + x118;
+  int x122 = x121 * x120 + x119;
+  int x123 = x122 * x121 + x120;
+  int x124 = x123 * x122 + x121;
+  i

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 253651.
emrekultursay added a comment.

Fix formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/commands/settings/use_source_cache/Makefile
  lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
  lldb/test/API/commands/settings/use_source_cache/main.cpp

Index: lldb/test/API/commands/settings/use_source_cache/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/main.cpp
@@ -0,0 +1,619 @@
+// This file should be large enough that LLDB decides to load it
+// using memory mapping. See:
+//   ⁠llvm/lib/Support/MemoryBuffer.cp:shouldUseMmap()
+
+#include 
+
+int calc(int x0, int x1, int x2);
+
+int
+main(int argc, char const *argv[])
+{
+  fprintf(stderr, "Hello, world => %d\n", calc(0, 1, 2));
+  return 0;
+}
+
+
+// The name of this function is used in tests to set breakpoints by name.
+int calc(int x0, int x1, int x2) {
+  int x3 = x2 * x1 + x0;
+  int x4 = x3 * x2 + x1;
+  int x5 = x4 * x3 + x2;
+  int x6 = x5 * x4 + x3;
+  int x7 = x6 * x5 + x4;
+  int x8 = x7 * x6 + x5;
+  int x9 = x8 * x7 + x6;
+  int x10 = x9 * x8 + x7;
+  int x11 = x10 * x9 + x8;
+  int x12 = x11 * x10 + x9;
+  int x13 = x12 * x11 + x10;
+  int x14 = x13 * x12 + x11;
+  int x15 = x14 * x13 + x12;
+  int x16 = x15 * x14 + x13;
+  int x17 = x16 * x15 + x14;
+  int x18 = x17 * x16 + x15;
+  int x19 = x18 * x17 + x16;
+  int x20 = x19 * x18 + x17;  
+  int x21 = x20 * x19 + x18;
+  int x22 = x21 * x20 + x19;
+  int x23 = x22 * x21 + x20;
+  int x24 = x23 * x22 + x21;
+  int x25 = x24 * x23 + x22;
+  int x26 = x25 * x24 + x23;
+  int x27 = x26 * x25 + x24;
+  int x28 = x27 * x26 + x25;
+  int x29 = x28 * x27 + x26;
+  int x30 = x29 * x28 + x27;
+  int x31 = x30 * x29 + x28;
+  int x32 = x31 * x30 + x29;
+  int x33 = x32 * x31 + x30;
+  int x34 = x33 * x32 + x31;
+  int x35 = x34 * x33 + x32;
+  int x36 = x35 * x34 + x33;
+  int x37 = x36 * x35 + x34;
+  int x38 = x37 * x36 + x35;
+  int x39 = x38 * x37 + x36;
+  int x40 = x39 * x38 + x37;
+  int x41 = x40 * x39 + x38;
+  int x42 = x41 * x40 + x39;
+  int x43 = x42 * x41 + x40;
+  int x44 = x43 * x42 + x41;
+  int x45 = x44 * x43 + x42;
+  int x46 = x45 * x44 + x43;
+  int x47 = x46 * x45 + x44;
+  int x48 = x47 * x46 + x45;
+  int x49 = x48 * x47 + x46;
+  int x50 = x49 * x48 + x47;
+  int x51 = x50 * x49 + x48;
+  int x52 = x51 * x50 + x49;
+  int x53 = x52 * x51 + x50;
+  int x54 = x53 * x52 + x51;
+  int x55 = x54 * x53 + x52;
+  int x56 = x55 * x54 + x53;
+  int x57 = x56 * x55 + x54;
+  int x58 = x57 * x56 + x55;
+  int x59 = x58 * x57 + x56;
+  int x60 = x59 * x58 + x57;
+  int x61 = x60 * x59 + x58;
+  int x62 = x61 * x60 + x59;
+  int x63 = x62 * x61 + x60;
+  int x64 = x63 * x62 + x61;
+  int x65 = x64 * x63 + x62;
+  int x66 = x65 * x64 + x63;
+  int x67 = x66 * x65 + x64;
+  int x68 = x67 * x66 + x65;
+  int x69 = x68 * x67 + x66;
+  int x70 = x69 * x68 + x67;
+  int x71 = x70 * x69 + x68;
+  int x72 = x71 * x70 + x69;
+  int x73 = x72 * x71 + x70;
+  int x74 = x73 * x72 + x71;
+  int x75 = x74 * x73 + x72;
+  int x76 = x75 * x74 + x73;
+  int x77 = x76 * x75 + x74;
+  int x78 = x77 * x76 + x75;
+  int x79 = x78 * x77 + x76;
+  int x80 = x79 * x78 + x77;
+  int x81 = x80 * x79 + x78;
+  int x82 = x81 * x80 + x79;
+  int x83 = x82 * x81 + x80;
+  int x84 = x83 * x82 + x81;
+  int x85 = x84 * x83 + x82;
+  int x86 = x85 * x84 + x83;
+  int x87 = x86 * x85 + x84;
+  int x88 = x87 * x86 + x85;
+  int x89 = x88 * x87 + x86;
+  int x90 = x89 * x88 + x87;
+  int x91 = x90 * x89 + x88;
+  int x92 = x91 * x90 + x89;
+  int x93 = x92 * x91 + x90;
+  int x94 = x93 * x92 + x91;
+  int x95 = x94 * x93 + x92;
+  int x96 = x95 * x94 + x93;
+  int x97 = x96 * x95 + x94;
+  int x98 = x97 * x96 + x95;
+  int x99 = x98 * x97 + x96;
+  int x100 = x99 * x98 + x97;
+  int x101 = x100 * x99 + x98;
+  int x102 = x101 * x100 + x99;
+  int x103 = x102 * x101 + x100;
+  int x104 = x103 * x102 + x101;
+  int x105 = x104 * x103 + x102;
+  int x106 = x105 * x104 + x103;
+  int x107 = x106 * x105 + x104;
+  int x108 = x107 * x106 + x105;
+  int x109 = x108 * x107 + x106;
+  int x110 = x109 * x108 + x107;
+  int x111 = x110 * x109 + x108;
+  int x112 = x111 * x110 + x109;
+  int x113 = x112 * x111 + x110;
+  int x114 = x113 * x112 + x111;
+  int x115 = x114 * x113 + x112;
+  int x116 = x115 * x114 + x113;
+  int x117 = x116 * x115 + x114;
+  int x118 = x117 * x116 + x115;
+  int x119 = x118 * x117 + x116;
+  int x120 = x119 * x118 + x117;
+  int x121 = x120 * x119 + x118;
+  int x122 = x121 * x120 + x119;
+  int x123 = x122 * x121 + x120;
+  int x124 = x123 * x122 + x121;
+  int x125 = x124 * x123 + x122;
+  int x126 = x125 * x124 + x123;
+  int x127 = x126 * x12

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 254241.
emrekultursay marked an inline comment as done.
emrekultursay added a comment.

use assertGreater instead of assertTrue(x>y)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/commands/settings/use_source_cache/Makefile
  lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
  lldb/test/API/commands/settings/use_source_cache/main.cpp

Index: lldb/test/API/commands/settings/use_source_cache/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/main.cpp
@@ -0,0 +1,619 @@
+// This file should be large enough that LLDB decides to load it
+// using memory mapping. See:
+//   ⁠llvm/lib/Support/MemoryBuffer.cp:shouldUseMmap()
+
+#include 
+
+int calc(int x0, int x1, int x2);
+
+int
+main(int argc, char const *argv[])
+{
+  fprintf(stderr, "Hello, world => %d\n", calc(0, 1, 2));
+  return 0;
+}
+
+
+// The name of this function is used in tests to set breakpoints by name.
+int calc(int x0, int x1, int x2) {
+  int x3 = x2 * x1 + x0;
+  int x4 = x3 * x2 + x1;
+  int x5 = x4 * x3 + x2;
+  int x6 = x5 * x4 + x3;
+  int x7 = x6 * x5 + x4;
+  int x8 = x7 * x6 + x5;
+  int x9 = x8 * x7 + x6;
+  int x10 = x9 * x8 + x7;
+  int x11 = x10 * x9 + x8;
+  int x12 = x11 * x10 + x9;
+  int x13 = x12 * x11 + x10;
+  int x14 = x13 * x12 + x11;
+  int x15 = x14 * x13 + x12;
+  int x16 = x15 * x14 + x13;
+  int x17 = x16 * x15 + x14;
+  int x18 = x17 * x16 + x15;
+  int x19 = x18 * x17 + x16;
+  int x20 = x19 * x18 + x17;  
+  int x21 = x20 * x19 + x18;
+  int x22 = x21 * x20 + x19;
+  int x23 = x22 * x21 + x20;
+  int x24 = x23 * x22 + x21;
+  int x25 = x24 * x23 + x22;
+  int x26 = x25 * x24 + x23;
+  int x27 = x26 * x25 + x24;
+  int x28 = x27 * x26 + x25;
+  int x29 = x28 * x27 + x26;
+  int x30 = x29 * x28 + x27;
+  int x31 = x30 * x29 + x28;
+  int x32 = x31 * x30 + x29;
+  int x33 = x32 * x31 + x30;
+  int x34 = x33 * x32 + x31;
+  int x35 = x34 * x33 + x32;
+  int x36 = x35 * x34 + x33;
+  int x37 = x36 * x35 + x34;
+  int x38 = x37 * x36 + x35;
+  int x39 = x38 * x37 + x36;
+  int x40 = x39 * x38 + x37;
+  int x41 = x40 * x39 + x38;
+  int x42 = x41 * x40 + x39;
+  int x43 = x42 * x41 + x40;
+  int x44 = x43 * x42 + x41;
+  int x45 = x44 * x43 + x42;
+  int x46 = x45 * x44 + x43;
+  int x47 = x46 * x45 + x44;
+  int x48 = x47 * x46 + x45;
+  int x49 = x48 * x47 + x46;
+  int x50 = x49 * x48 + x47;
+  int x51 = x50 * x49 + x48;
+  int x52 = x51 * x50 + x49;
+  int x53 = x52 * x51 + x50;
+  int x54 = x53 * x52 + x51;
+  int x55 = x54 * x53 + x52;
+  int x56 = x55 * x54 + x53;
+  int x57 = x56 * x55 + x54;
+  int x58 = x57 * x56 + x55;
+  int x59 = x58 * x57 + x56;
+  int x60 = x59 * x58 + x57;
+  int x61 = x60 * x59 + x58;
+  int x62 = x61 * x60 + x59;
+  int x63 = x62 * x61 + x60;
+  int x64 = x63 * x62 + x61;
+  int x65 = x64 * x63 + x62;
+  int x66 = x65 * x64 + x63;
+  int x67 = x66 * x65 + x64;
+  int x68 = x67 * x66 + x65;
+  int x69 = x68 * x67 + x66;
+  int x70 = x69 * x68 + x67;
+  int x71 = x70 * x69 + x68;
+  int x72 = x71 * x70 + x69;
+  int x73 = x72 * x71 + x70;
+  int x74 = x73 * x72 + x71;
+  int x75 = x74 * x73 + x72;
+  int x76 = x75 * x74 + x73;
+  int x77 = x76 * x75 + x74;
+  int x78 = x77 * x76 + x75;
+  int x79 = x78 * x77 + x76;
+  int x80 = x79 * x78 + x77;
+  int x81 = x80 * x79 + x78;
+  int x82 = x81 * x80 + x79;
+  int x83 = x82 * x81 + x80;
+  int x84 = x83 * x82 + x81;
+  int x85 = x84 * x83 + x82;
+  int x86 = x85 * x84 + x83;
+  int x87 = x86 * x85 + x84;
+  int x88 = x87 * x86 + x85;
+  int x89 = x88 * x87 + x86;
+  int x90 = x89 * x88 + x87;
+  int x91 = x90 * x89 + x88;
+  int x92 = x91 * x90 + x89;
+  int x93 = x92 * x91 + x90;
+  int x94 = x93 * x92 + x91;
+  int x95 = x94 * x93 + x92;
+  int x96 = x95 * x94 + x93;
+  int x97 = x96 * x95 + x94;
+  int x98 = x97 * x96 + x95;
+  int x99 = x98 * x97 + x96;
+  int x100 = x99 * x98 + x97;
+  int x101 = x100 * x99 + x98;
+  int x102 = x101 * x100 + x99;
+  int x103 = x102 * x101 + x100;
+  int x104 = x103 * x102 + x101;
+  int x105 = x104 * x103 + x102;
+  int x106 = x105 * x104 + x103;
+  int x107 = x106 * x105 + x104;
+  int x108 = x107 * x106 + x105;
+  int x109 = x108 * x107 + x106;
+  int x110 = x109 * x108 + x107;
+  int x111 = x110 * x109 + x108;
+  int x112 = x111 * x110 + x109;
+  int x113 = x112 * x111 + x110;
+  int x114 = x113 * x112 + x111;
+  int x115 = x114 * x113 + x112;
+  int x116 = x115 * x114 + x113;
+  int x117 = x116 * x115 + x114;
+  int x118 = x117 * x116 + x115;
+  int x119 = x118 * x117 + x116;
+  int x120 = x119 * x118 + x117;
+  int x121 = x120 * x119 + x118;
+  int x122 = x121 * x120 + x119;
+  int x123 = x122 * x121 + x120;
+  int x124 = x123 * x122 + x121;
+  int x125 = x

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 254292.
emrekultursay added a comment.

Fix missing use of GetLastFile


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/commands/settings/use_source_cache/Makefile
  lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
  lldb/test/API/commands/settings/use_source_cache/main.cpp

Index: lldb/test/API/commands/settings/use_source_cache/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/main.cpp
@@ -0,0 +1,619 @@
+// This file should be large enough that LLDB decides to load it
+// using memory mapping. See:
+//   ⁠llvm/lib/Support/MemoryBuffer.cp:shouldUseMmap()
+
+#include 
+
+int calc(int x0, int x1, int x2);
+
+int
+main(int argc, char const *argv[])
+{
+  fprintf(stderr, "Hello, world => %d\n", calc(0, 1, 2));
+  return 0;
+}
+
+
+// The name of this function is used in tests to set breakpoints by name.
+int calc(int x0, int x1, int x2) {
+  int x3 = x2 * x1 + x0;
+  int x4 = x3 * x2 + x1;
+  int x5 = x4 * x3 + x2;
+  int x6 = x5 * x4 + x3;
+  int x7 = x6 * x5 + x4;
+  int x8 = x7 * x6 + x5;
+  int x9 = x8 * x7 + x6;
+  int x10 = x9 * x8 + x7;
+  int x11 = x10 * x9 + x8;
+  int x12 = x11 * x10 + x9;
+  int x13 = x12 * x11 + x10;
+  int x14 = x13 * x12 + x11;
+  int x15 = x14 * x13 + x12;
+  int x16 = x15 * x14 + x13;
+  int x17 = x16 * x15 + x14;
+  int x18 = x17 * x16 + x15;
+  int x19 = x18 * x17 + x16;
+  int x20 = x19 * x18 + x17;  
+  int x21 = x20 * x19 + x18;
+  int x22 = x21 * x20 + x19;
+  int x23 = x22 * x21 + x20;
+  int x24 = x23 * x22 + x21;
+  int x25 = x24 * x23 + x22;
+  int x26 = x25 * x24 + x23;
+  int x27 = x26 * x25 + x24;
+  int x28 = x27 * x26 + x25;
+  int x29 = x28 * x27 + x26;
+  int x30 = x29 * x28 + x27;
+  int x31 = x30 * x29 + x28;
+  int x32 = x31 * x30 + x29;
+  int x33 = x32 * x31 + x30;
+  int x34 = x33 * x32 + x31;
+  int x35 = x34 * x33 + x32;
+  int x36 = x35 * x34 + x33;
+  int x37 = x36 * x35 + x34;
+  int x38 = x37 * x36 + x35;
+  int x39 = x38 * x37 + x36;
+  int x40 = x39 * x38 + x37;
+  int x41 = x40 * x39 + x38;
+  int x42 = x41 * x40 + x39;
+  int x43 = x42 * x41 + x40;
+  int x44 = x43 * x42 + x41;
+  int x45 = x44 * x43 + x42;
+  int x46 = x45 * x44 + x43;
+  int x47 = x46 * x45 + x44;
+  int x48 = x47 * x46 + x45;
+  int x49 = x48 * x47 + x46;
+  int x50 = x49 * x48 + x47;
+  int x51 = x50 * x49 + x48;
+  int x52 = x51 * x50 + x49;
+  int x53 = x52 * x51 + x50;
+  int x54 = x53 * x52 + x51;
+  int x55 = x54 * x53 + x52;
+  int x56 = x55 * x54 + x53;
+  int x57 = x56 * x55 + x54;
+  int x58 = x57 * x56 + x55;
+  int x59 = x58 * x57 + x56;
+  int x60 = x59 * x58 + x57;
+  int x61 = x60 * x59 + x58;
+  int x62 = x61 * x60 + x59;
+  int x63 = x62 * x61 + x60;
+  int x64 = x63 * x62 + x61;
+  int x65 = x64 * x63 + x62;
+  int x66 = x65 * x64 + x63;
+  int x67 = x66 * x65 + x64;
+  int x68 = x67 * x66 + x65;
+  int x69 = x68 * x67 + x66;
+  int x70 = x69 * x68 + x67;
+  int x71 = x70 * x69 + x68;
+  int x72 = x71 * x70 + x69;
+  int x73 = x72 * x71 + x70;
+  int x74 = x73 * x72 + x71;
+  int x75 = x74 * x73 + x72;
+  int x76 = x75 * x74 + x73;
+  int x77 = x76 * x75 + x74;
+  int x78 = x77 * x76 + x75;
+  int x79 = x78 * x77 + x76;
+  int x80 = x79 * x78 + x77;
+  int x81 = x80 * x79 + x78;
+  int x82 = x81 * x80 + x79;
+  int x83 = x82 * x81 + x80;
+  int x84 = x83 * x82 + x81;
+  int x85 = x84 * x83 + x82;
+  int x86 = x85 * x84 + x83;
+  int x87 = x86 * x85 + x84;
+  int x88 = x87 * x86 + x85;
+  int x89 = x88 * x87 + x86;
+  int x90 = x89 * x88 + x87;
+  int x91 = x90 * x89 + x88;
+  int x92 = x91 * x90 + x89;
+  int x93 = x92 * x91 + x90;
+  int x94 = x93 * x92 + x91;
+  int x95 = x94 * x93 + x92;
+  int x96 = x95 * x94 + x93;
+  int x97 = x96 * x95 + x94;
+  int x98 = x97 * x96 + x95;
+  int x99 = x98 * x97 + x96;
+  int x100 = x99 * x98 + x97;
+  int x101 = x100 * x99 + x98;
+  int x102 = x101 * x100 + x99;
+  int x103 = x102 * x101 + x100;
+  int x104 = x103 * x102 + x101;
+  int x105 = x104 * x103 + x102;
+  int x106 = x105 * x104 + x103;
+  int x107 = x106 * x105 + x104;
+  int x108 = x107 * x106 + x105;
+  int x109 = x108 * x107 + x106;
+  int x110 = x109 * x108 + x107;
+  int x111 = x110 * x109 + x108;
+  int x112 = x111 * x110 + x109;
+  int x113 = x112 * x111 + x110;
+  int x114 = x113 * x112 + x111;
+  int x115 = x114 * x113 + x112;
+  int x116 = x115 * x114 + x113;
+  int x117 = x116 * x115 + x114;
+  int x118 = x117 * x116 + x115;
+  int x119 = x118 * x117 + x116;
+  int x120 = x119 * x118 + x117;
+  int x121 = x120 * x119 + x118;
+  int x122 = x121 * x120 + x119;
+  int x123 = x122 * x121 + x120;
+  int x124 = x123 * x122 + x121;
+  int x125 = x124 * x123 + x122;
+  int x126 = x125 * x124 + x123;
+  int x1

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

I don't have commit access, can someone with access commit all three approved 
CLs in this stack?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806



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

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 256416.
emrekultursay added a comment.

Fix broken tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/commands/settings/use_source_cache/Makefile
  lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
  lldb/test/API/commands/settings/use_source_cache/main.cpp

Index: lldb/test/API/commands/settings/use_source_cache/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/settings/use_source_cache/main.cpp
@@ -0,0 +1,619 @@
+// This file should be large enough that LLDB decides to load it
+// using memory mapping. See:
+//   ⁠llvm/lib/Support/MemoryBuffer.cp:shouldUseMmap()
+
+#include 
+
+int calc(int x0, int x1, int x2);
+
+int
+main(int argc, char const *argv[])
+{
+  fprintf(stderr, "Hello, world => %d\n", calc(0, 1, 2));
+  return 0;
+}
+
+
+// The name of this function is used in tests to set breakpoints by name.
+int calc(int x0, int x1, int x2) {
+  int x3 = x2 * x1 + x0;
+  int x4 = x3 * x2 + x1;
+  int x5 = x4 * x3 + x2;
+  int x6 = x5 * x4 + x3;
+  int x7 = x6 * x5 + x4;
+  int x8 = x7 * x6 + x5;
+  int x9 = x8 * x7 + x6;
+  int x10 = x9 * x8 + x7;
+  int x11 = x10 * x9 + x8;
+  int x12 = x11 * x10 + x9;
+  int x13 = x12 * x11 + x10;
+  int x14 = x13 * x12 + x11;
+  int x15 = x14 * x13 + x12;
+  int x16 = x15 * x14 + x13;
+  int x17 = x16 * x15 + x14;
+  int x18 = x17 * x16 + x15;
+  int x19 = x18 * x17 + x16;
+  int x20 = x19 * x18 + x17;  
+  int x21 = x20 * x19 + x18;
+  int x22 = x21 * x20 + x19;
+  int x23 = x22 * x21 + x20;
+  int x24 = x23 * x22 + x21;
+  int x25 = x24 * x23 + x22;
+  int x26 = x25 * x24 + x23;
+  int x27 = x26 * x25 + x24;
+  int x28 = x27 * x26 + x25;
+  int x29 = x28 * x27 + x26;
+  int x30 = x29 * x28 + x27;
+  int x31 = x30 * x29 + x28;
+  int x32 = x31 * x30 + x29;
+  int x33 = x32 * x31 + x30;
+  int x34 = x33 * x32 + x31;
+  int x35 = x34 * x33 + x32;
+  int x36 = x35 * x34 + x33;
+  int x37 = x36 * x35 + x34;
+  int x38 = x37 * x36 + x35;
+  int x39 = x38 * x37 + x36;
+  int x40 = x39 * x38 + x37;
+  int x41 = x40 * x39 + x38;
+  int x42 = x41 * x40 + x39;
+  int x43 = x42 * x41 + x40;
+  int x44 = x43 * x42 + x41;
+  int x45 = x44 * x43 + x42;
+  int x46 = x45 * x44 + x43;
+  int x47 = x46 * x45 + x44;
+  int x48 = x47 * x46 + x45;
+  int x49 = x48 * x47 + x46;
+  int x50 = x49 * x48 + x47;
+  int x51 = x50 * x49 + x48;
+  int x52 = x51 * x50 + x49;
+  int x53 = x52 * x51 + x50;
+  int x54 = x53 * x52 + x51;
+  int x55 = x54 * x53 + x52;
+  int x56 = x55 * x54 + x53;
+  int x57 = x56 * x55 + x54;
+  int x58 = x57 * x56 + x55;
+  int x59 = x58 * x57 + x56;
+  int x60 = x59 * x58 + x57;
+  int x61 = x60 * x59 + x58;
+  int x62 = x61 * x60 + x59;
+  int x63 = x62 * x61 + x60;
+  int x64 = x63 * x62 + x61;
+  int x65 = x64 * x63 + x62;
+  int x66 = x65 * x64 + x63;
+  int x67 = x66 * x65 + x64;
+  int x68 = x67 * x66 + x65;
+  int x69 = x68 * x67 + x66;
+  int x70 = x69 * x68 + x67;
+  int x71 = x70 * x69 + x68;
+  int x72 = x71 * x70 + x69;
+  int x73 = x72 * x71 + x70;
+  int x74 = x73 * x72 + x71;
+  int x75 = x74 * x73 + x72;
+  int x76 = x75 * x74 + x73;
+  int x77 = x76 * x75 + x74;
+  int x78 = x77 * x76 + x75;
+  int x79 = x78 * x77 + x76;
+  int x80 = x79 * x78 + x77;
+  int x81 = x80 * x79 + x78;
+  int x82 = x81 * x80 + x79;
+  int x83 = x82 * x81 + x80;
+  int x84 = x83 * x82 + x81;
+  int x85 = x84 * x83 + x82;
+  int x86 = x85 * x84 + x83;
+  int x87 = x86 * x85 + x84;
+  int x88 = x87 * x86 + x85;
+  int x89 = x88 * x87 + x86;
+  int x90 = x89 * x88 + x87;
+  int x91 = x90 * x89 + x88;
+  int x92 = x91 * x90 + x89;
+  int x93 = x92 * x91 + x90;
+  int x94 = x93 * x92 + x91;
+  int x95 = x94 * x93 + x92;
+  int x96 = x95 * x94 + x93;
+  int x97 = x96 * x95 + x94;
+  int x98 = x97 * x96 + x95;
+  int x99 = x98 * x97 + x96;
+  int x100 = x99 * x98 + x97;
+  int x101 = x100 * x99 + x98;
+  int x102 = x101 * x100 + x99;
+  int x103 = x102 * x101 + x100;
+  int x104 = x103 * x102 + x101;
+  int x105 = x104 * x103 + x102;
+  int x106 = x105 * x104 + x103;
+  int x107 = x106 * x105 + x104;
+  int x108 = x107 * x106 + x105;
+  int x109 = x108 * x107 + x106;
+  int x110 = x109 * x108 + x107;
+  int x111 = x110 * x109 + x108;
+  int x112 = x111 * x110 + x109;
+  int x113 = x112 * x111 + x110;
+  int x114 = x113 * x112 + x111;
+  int x115 = x114 * x113 + x112;
+  int x116 = x115 * x114 + x113;
+  int x117 = x116 * x115 + x114;
+  int x118 = x117 * x116 + x115;
+  int x119 = x118 * x117 + x116;
+  int x120 = x119 * x118 + x117;
+  int x121 = x120 * x119 + x118;
+  int x122 = x121 * x120 + x119;
+  int x123 = x122 * x121 + x120;
+  int x124 = x123 * x122 + x121;
+  int x125 = x124 * x123 + x122;
+  int x126 = x125 * x124 + x123;
+  int x127 = x126 * x

[Lldb-commits] [PATCH] D76806: Remove m_last_file_sp from SourceManager

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

Thanks for noticing the test breakages Pavel.  I fixed the bug, and verified 
that the tests you mentioned pass. PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76806



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

[Lldb-commits] [PATCH] D100977: Use forward type in pointer-to-member

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a reviewer: shafik.
emrekultursay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This change is similar in spirit to the change at:
https://reviews.llvm.org/rG34c697c85e9d0af11a72ac4df5578aac94a627b3

It fixes the problem where the layout of a type was being accessed
while its base classes were not populated yet; which caused an
incorrect layout to be produced and cached.

Bug: 50054


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100977

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1357,7 +1357,7 @@
   dwarf->ResolveTypeUID(attrs.containing_type.Reference(), true);
 
   CompilerType pointee_clang_type = pointee_type->GetForwardCompilerType();
-  CompilerType class_clang_type = class_type->GetLayoutCompilerType();
+  CompilerType class_clang_type = class_type->GetForwardCompilerType();
 
   CompilerType clang_type = TypeSystemClang::CreateMemberPointerType(
   class_clang_type, pointee_clang_type);


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1357,7 +1357,7 @@
   dwarf->ResolveTypeUID(attrs.containing_type.Reference(), true);
 
   CompilerType pointee_clang_type = pointee_type->GetForwardCompilerType();
-  CompilerType class_clang_type = class_type->GetLayoutCompilerType();
+  CompilerType class_clang_type = class_type->GetForwardCompilerType();
 
   CompilerType clang_type = TypeSystemClang::CreateMemberPointerType(
   class_clang_type, pointee_clang_type);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 339483.
emrekultursay added a comment.

Added functionality/lazy-loading test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
  lldb/test/API/functionalities/lazy-loading/main.cpp


Index: lldb/test/API/functionalities/lazy-loading/main.cpp
===
--- lldb/test/API/functionalities/lazy-loading/main.cpp
+++ lldb/test/API/functionalities/lazy-loading/main.cpp
@@ -26,6 +26,9 @@
 struct StaticClassMember { int i; };
 struct UnusedClassMember { int i; };
 struct UnusedClassMemberPtr { int i; };
+struct PointerToMember {
+  int i;
+};
 
 namespace NS {
 class ClassInNamespace {
@@ -36,6 +39,7 @@
   int dummy; // Prevent bug where LLDB always completes first member.
   ClassMember member;
   static StaticClassMember static_member;
+  int(PointerToMember::*ptr_to_member);
   UnusedClassMember unused_member;
   UnusedClassMemberPtr *unused_member_ptr;
   int enteredFunction() {
Index: lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
===
--- lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
+++ lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
@@ -41,6 +41,7 @@
 class_we_enter_decl = [class_decl_kind, "ClassWeEnter"]
 class_member_decl = [struct_decl_kind, "ClassMember"]
 class_static_member_decl = [struct_decl_kind, "StaticClassMember"]
+class_pointer_to_member_decl = [struct_decl_kind, "PointerToMember"]
 unused_class_member_decl = [struct_decl_kind, "UnusedClassMember"]
 unused_class_member_ptr_decl = [struct_decl_kind, "UnusedClassMemberPtr"]
 
@@ -58,6 +59,7 @@
 self.assert_decl_not_loaded(self.class_in_namespace_decl)
 self.assert_decl_not_loaded(self.class_member_decl)
 self.assert_decl_not_loaded(self.class_static_member_decl)
+self.assert_decl_not_loaded(self.class_pointer_to_member_decl)
 self.assert_decl_not_loaded(self.unused_class_member_decl)
 
 def get_ast_dump(self):
@@ -232,6 +234,8 @@
 self.assert_decl_loaded(self.class_member_decl)
 # We didn't load the type of the unused static member.
 self.assert_decl_not_completed(self.class_static_member_decl)
+# We didn't load the type of the unused pointer-to-member member.
+self.assert_decl_not_completed(self.class_pointer_to_member_decl)
 
 # This should not have loaded anything else.
 self.assert_decl_not_loaded(self.other_struct_decl)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1357,7 +1357,7 @@
   dwarf->ResolveTypeUID(attrs.containing_type.Reference(), true);
 
   CompilerType pointee_clang_type = pointee_type->GetForwardCompilerType();
-  CompilerType class_clang_type = class_type->GetLayoutCompilerType();
+  CompilerType class_clang_type = class_type->GetForwardCompilerType();
 
   CompilerType clang_type = TypeSystemClang::CreateMemberPointerType(
   class_clang_type, pointee_clang_type);


Index: lldb/test/API/functionalities/lazy-loading/main.cpp
===
--- lldb/test/API/functionalities/lazy-loading/main.cpp
+++ lldb/test/API/functionalities/lazy-loading/main.cpp
@@ -26,6 +26,9 @@
 struct StaticClassMember { int i; };
 struct UnusedClassMember { int i; };
 struct UnusedClassMemberPtr { int i; };
+struct PointerToMember {
+  int i;
+};
 
 namespace NS {
 class ClassInNamespace {
@@ -36,6 +39,7 @@
   int dummy; // Prevent bug where LLDB always completes first member.
   ClassMember member;
   static StaticClassMember static_member;
+  int(PointerToMember::*ptr_to_member);
   UnusedClassMember unused_member;
   UnusedClassMemberPtr *unused_member_ptr;
   int enteredFunction() {
Index: lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
===
--- lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
+++ lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
@@ -41,6 +41,7 @@
 class_we_enter_decl = [class_decl_kind, "ClassWeEnter"]
 class_member_decl = [struct_decl_kind, "ClassMember"]
 class_static_member_decl = [struct_decl_kind, "StaticClassMember"]
+class_pointer_to_member_decl = [struct_decl_kind, "PointerToMember"]
 unused_class_member_decl = [struct_decl_kind, "UnusedClassMember"]
 unused_class_member_ptr_decl = [struct_decl_kind, "UnusedClassMemberPtr"]
 
@@ -58,6 +59,7 @@
 self.assert_dec

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 339485.
emrekultursay added a comment.

Fix formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
  lldb/test/API/functionalities/lazy-loading/main.cpp


Index: lldb/test/API/functionalities/lazy-loading/main.cpp
===
--- lldb/test/API/functionalities/lazy-loading/main.cpp
+++ lldb/test/API/functionalities/lazy-loading/main.cpp
@@ -26,6 +26,7 @@
 struct StaticClassMember { int i; };
 struct UnusedClassMember { int i; };
 struct UnusedClassMemberPtr { int i; };
+struct PointerToMember { int i; };
 
 namespace NS {
 class ClassInNamespace {
@@ -36,6 +37,7 @@
   int dummy; // Prevent bug where LLDB always completes first member.
   ClassMember member;
   static StaticClassMember static_member;
+  int (PointerToMember::*ptr_to_member);
   UnusedClassMember unused_member;
   UnusedClassMemberPtr *unused_member_ptr;
   int enteredFunction() {
Index: lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
===
--- lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
+++ lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
@@ -41,6 +41,7 @@
 class_we_enter_decl = [class_decl_kind, "ClassWeEnter"]
 class_member_decl = [struct_decl_kind, "ClassMember"]
 class_static_member_decl = [struct_decl_kind, "StaticClassMember"]
+class_pointer_to_member_decl = [struct_decl_kind, "PointerToMember"]
 unused_class_member_decl = [struct_decl_kind, "UnusedClassMember"]
 unused_class_member_ptr_decl = [struct_decl_kind, "UnusedClassMemberPtr"]
 
@@ -58,6 +59,7 @@
 self.assert_decl_not_loaded(self.class_in_namespace_decl)
 self.assert_decl_not_loaded(self.class_member_decl)
 self.assert_decl_not_loaded(self.class_static_member_decl)
+self.assert_decl_not_loaded(self.class_pointer_to_member_decl)
 self.assert_decl_not_loaded(self.unused_class_member_decl)
 
 def get_ast_dump(self):
@@ -232,6 +234,8 @@
 self.assert_decl_loaded(self.class_member_decl)
 # We didn't load the type of the unused static member.
 self.assert_decl_not_completed(self.class_static_member_decl)
+# We didn't load the type of the unused pointer-to-member member.
+self.assert_decl_not_completed(self.class_pointer_to_member_decl)
 
 # This should not have loaded anything else.
 self.assert_decl_not_loaded(self.other_struct_decl)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1357,7 +1357,7 @@
   dwarf->ResolveTypeUID(attrs.containing_type.Reference(), true);
 
   CompilerType pointee_clang_type = pointee_type->GetForwardCompilerType();
-  CompilerType class_clang_type = class_type->GetLayoutCompilerType();
+  CompilerType class_clang_type = class_type->GetForwardCompilerType();
 
   CompilerType clang_type = TypeSystemClang::CreateMemberPointerType(
   class_clang_type, pointee_clang_type);


Index: lldb/test/API/functionalities/lazy-loading/main.cpp
===
--- lldb/test/API/functionalities/lazy-loading/main.cpp
+++ lldb/test/API/functionalities/lazy-loading/main.cpp
@@ -26,6 +26,7 @@
 struct StaticClassMember { int i; };
 struct UnusedClassMember { int i; };
 struct UnusedClassMemberPtr { int i; };
+struct PointerToMember { int i; };
 
 namespace NS {
 class ClassInNamespace {
@@ -36,6 +37,7 @@
   int dummy; // Prevent bug where LLDB always completes first member.
   ClassMember member;
   static StaticClassMember static_member;
+  int (PointerToMember::*ptr_to_member);
   UnusedClassMember unused_member;
   UnusedClassMemberPtr *unused_member_ptr;
   int enteredFunction() {
Index: lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
===
--- lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
+++ lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
@@ -41,6 +41,7 @@
 class_we_enter_decl = [class_decl_kind, "ClassWeEnter"]
 class_member_decl = [struct_decl_kind, "ClassMember"]
 class_static_member_decl = [struct_decl_kind, "StaticClassMember"]
+class_pointer_to_member_decl = [struct_decl_kind, "PointerToMember"]
 unused_class_member_decl = [struct_decl_kind, "UnusedClassMember"]
 unused_class_member_ptr_decl = [struct_decl_kind, "UnusedClassMemberPtr"]
 
@@ -58,6 +59,7 @@
 self.assert_decl_not_loaded(self.class_in_na

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

I added a lazy-loading test. I also attempted to write a test similar to 
`lang/cpp/static_member_type_depending_on_parent_size/TestStaticMemberTypeDependingOnParentSize.py`,
 but I could not get it to produce an incorrect struct size (just like I 
couldn't reduce the reproducer).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

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

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 339694.
emrekultursay added a comment.

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
  lldb/test/API/functionalities/lazy-loading/main.cpp
  
lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
  
lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
  
lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp

Index: lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp
@@ -0,0 +1,35 @@
+// This class just serves as an indirection between LLDB and Clang. LLDB might
+// be tempted to check the member type of DependsOnParam2 for whether it's
+// in some 'currently-loading' state before trying to produce the record layout.
+// By inheriting from ToLayout this will make LLDB just check if
+// DependsOnParam1 is currently being loaded (which it's not) but it won't
+// check if all the types DependsOnParam2 is depending on for its layout are
+// currently parsed.
+template  struct DependsOnParam1 : ToLayoutParam {};
+// This class forces the memory layout of it's type parameter to be created.
+template  struct DependsOnParam2 {
+  DependsOnParam1 m;
+};
+
+// This is the class that LLDB has to generate the record layout for.
+struct ToLayout {
+  // The class part of this pointer-to-member type has a memory layout that
+  // depends on the surrounding class. If LLDB eagerly tries to layout the
+  // class part of a pointer-to-member type while parsing, then layouting this
+  // type should cause a test failure (as we aren't done parsing ToLayout
+  // at this point).
+  int DependsOnParam2::* pointer_to_member_member;
+  // Some dummy member variable. This is only there so that Clang can detect
+  // that the record layout is inconsistent (i.e., the number of fields in the
+  // layout doesn't fit to the fields in the declaration).
+  int some_member;
+};
+
+// Emit the definition of DependsOnParam2. It seems Clang won't
+// emit the definition of a class template if it's only used in the class part
+// of a pointer-to-member type.
+DependsOnParam2 x;
+
+ToLayout test_var;
+
+int main() { return test_var.some_member; }
Index: lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
@@ -0,0 +1,24 @@
+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__)
+
+@no_debug_info_test
+def test(self):
+"""
+This tests a pointer-to-member member which class part is the
+surrounding class. LLDB should *not* try to generate the record layout
+of the class when parsing pointer-to-member types while parsing debug
+info (as the references class might not be complete when the type is
+parsed).
+"""
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+# Force the record layout for 'ToLayout' to be generated by printing
+# a value of it's type.
+self.expect("target variable test_var")
Index: lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/functionalities/lazy-loading/main.cpp
===
--- lldb/test/API/functionalities/lazy-loading/main.cpp
+++ lldb/test/API/functionalities/lazy-loading/main.cpp
@@ -26,6 +26,7 @@
 struct StaticClassMember { int i; };
 struct UnusedClassMember { int i; };
 struct UnusedClassMemberPtr { int i; };
+struct PointerToMember { int i; };
 
 namespace NS {
 class ClassInNamespace {
@@ -36,6 +37,7 @@
   int dummy; // Prevent bug where LLDB always completes first member.
   ClassMember member;
   static StaticClassMember static_member;
+  int (PointerToMember::*ptr_to_member);
   UnusedClassMember unused_member;
   UnusedClassMemberPtr *unused_member_ptr;
   int enteredFunction() {
Index: lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
=

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

> DependsOnParam2 x;

This must have been the magic glue I was missing, Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

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

[Lldb-commits] [PATCH] D79586: Do not list adb devices when a device id is given

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.

On Android, this method gets called twice: first when establishing
a host-server connection, then when attaching to a process id.

Each call takes several seconds to finish (especially slower on Windows)
and eliminating the call for the typical case improves latency significantly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79586

Files:
  lldb/source/Plugins/Platform/Android/AdbClient.cpp
  lldb/unittests/Platform/Android/AdbClientTest.cpp
  lldb/unittests/Platform/Android/CMakeLists.txt
  lldb/unittests/Platform/CMakeLists.txt

Index: lldb/unittests/Platform/CMakeLists.txt
===
--- lldb/unittests/Platform/CMakeLists.txt
+++ lldb/unittests/Platform/CMakeLists.txt
@@ -6,3 +6,5 @@
   LINK_COMPONENTS
 Support
   )
+
+add_subdirectory(Android)
Index: lldb/unittests/Platform/Android/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Platform/Android/CMakeLists.txt
@@ -0,0 +1,8 @@
+include_directories(${LLDB_SOURCE_DIR}/source/Plugins/Platform/Android)
+
+add_lldb_unittest(AdbClientTest
+  AdbClientTest.cpp
+
+  LINK_LIBS
+lldbPluginPlatformAndroid
+  )
Index: lldb/unittests/Platform/Android/AdbClientTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/Android/AdbClientTest.cpp
@@ -0,0 +1,45 @@
+//===-- AdbClientTest.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 "gtest/gtest.h"
+
+#include "Plugins/Platform/Android/AdbClient.h"
+
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace lldb_private {
+namespace platform_android {
+
+class AdbClientTest : public ::testing::Test {
+public:
+  void SetUp() override { putenv("ANDROID_SERIAL="); }
+
+  void TearDown() override { putenv("ANDROID_SERIAL="); }
+};
+
+TEST(AdbClientTest, CreateByDeviceId) {
+  AdbClient adb;
+  Status error = AdbClient::CreateByDeviceID("device1", adb);
+  EXPECT_TRUE(error.Success());
+  EXPECT_EQ("device1", adb.GetDeviceID());
+}
+
+TEST(AdbClientTest, CreateByDeviceId_ByEnvVar) {
+  putenv("ANDROID_SERIAL=device2");
+
+  AdbClient adb;
+  Status error = AdbClient::CreateByDeviceID("", adb);
+  EXPECT_TRUE(error.Success());
+  EXPECT_EQ("device2", adb.GetDeviceID());
+}
+
+} // end namespace platform_android
+} // end namespace lldb_private
\ No newline at end of file
Index: lldb/source/Plugins/Platform/Android/AdbClient.cpp
===
--- lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -94,11 +94,7 @@
 
 Status AdbClient::CreateByDeviceID(const std::string &device_id,
AdbClient &adb) {
-  DeviceIDList connect_devices;
-  auto error = adb.GetDevices(connect_devices);
-  if (error.Fail())
-return error;
-
+  Status error;
   std::string android_serial;
   if (!device_id.empty())
 android_serial = device_id;
@@ -106,18 +102,18 @@
 android_serial = env_serial;
 
   if (android_serial.empty()) {
-if (connect_devices.size() != 1)
+DeviceIDList connected_devices;
+error = adb.GetDevices(connected_devices);
+if (error.Fail())
+  return error;
+
+if (connected_devices.size() != 1)
   return Status("Expected a single connected device, got instead %zu - try "
 "setting 'ANDROID_SERIAL'",
-connect_devices.size());
-adb.SetDeviceID(connect_devices.front());
+connected_devices.size());
+adb.SetDeviceID(connected_devices.front());
   } else {
-auto find_it = std::find(connect_devices.begin(), connect_devices.end(),
- android_serial);
-if (find_it == connect_devices.end())
-  return Status("Device \"%s\" not found", android_serial.c_str());
-
-adb.SetDeviceID(*find_it);
+adb.SetDeviceID(android_serial);
   }
   return error;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D79757: Try IPv4 before IPv6 when creating TCP connection

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added subscribers: lldb-commits, mgrang.
Herald added a project: LLDB.

When connecting to Android, LLDB calls adb#Shell 5 times. At each call,
a TCP connection to "localhost:port" needs to be established. On hosts
that support IPv4 and IPv6, the "localhost" host can map to two
addresses.

It seems like IPv6 typically becomes the addresses[0], and then trying
to connect to it gets rejected (WSAECONNREFUSED), and then the IPv4 one
at addresses[1] succeeds.

Trying to connect to IPv6 first wastes up to 2 seconds per call on
Windows. The adb CLI client also uses an IPv4-first approach.

On Windows, this CL brings total of ~10 second improvement in LLDB
attach latency to Android devices.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79757

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


Index: lldb/source/Host/common/TCPSocket.cpp
===
--- lldb/source/Host/common/TCPSocket.cpp
+++ lldb/source/Host/common/TCPSocket.cpp
@@ -162,6 +162,12 @@
 
   std::vector addresses = SocketAddress::GetAddressInfo(
   host_str.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
+  // IPv4 is more likely to succeed than IPv6. To avoid costly retries,
+  // we sort based on family to prioritize IPv4 over IPv6.
+  std::sort(addresses.begin(), addresses.end(),
+[](SocketAddress a, SocketAddress b) {
+  return a.GetFamily() < b.GetFamily();
+});
   for (SocketAddress &address : addresses) {
 error = CreateSocket(address.GetFamily());
 if (error.Fail())


Index: lldb/source/Host/common/TCPSocket.cpp
===
--- lldb/source/Host/common/TCPSocket.cpp
+++ lldb/source/Host/common/TCPSocket.cpp
@@ -162,6 +162,12 @@
 
   std::vector addresses = SocketAddress::GetAddressInfo(
   host_str.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
+  // IPv4 is more likely to succeed than IPv6. To avoid costly retries,
+  // we sort based on family to prioritize IPv4 over IPv6.
+  std::sort(addresses.begin(), addresses.end(),
+[](SocketAddress a, SocketAddress b) {
+  return a.GetFamily() < b.GetFamily();
+});
   for (SocketAddress &address : addresses) {
 error = CreateSocket(address.GetFamily());
 if (error.Fail())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D79757: Try IPv4 before IPv6 when creating TCP connection

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 263327.
emrekultursay added a comment.

Update commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79757

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


Index: lldb/source/Host/common/TCPSocket.cpp
===
--- lldb/source/Host/common/TCPSocket.cpp
+++ lldb/source/Host/common/TCPSocket.cpp
@@ -162,6 +162,12 @@
 
   std::vector addresses = SocketAddress::GetAddressInfo(
   host_str.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
+  // IPv4 is more likely to succeed than IPv6. To avoid costly retries,
+  // we sort based on family to prioritize IPv4 over IPv6.
+  std::sort(addresses.begin(), addresses.end(),
+[](SocketAddress a, SocketAddress b) {
+  return a.GetFamily() < b.GetFamily();
+});
   for (SocketAddress &address : addresses) {
 error = CreateSocket(address.GetFamily());
 if (error.Fail())


Index: lldb/source/Host/common/TCPSocket.cpp
===
--- lldb/source/Host/common/TCPSocket.cpp
+++ lldb/source/Host/common/TCPSocket.cpp
@@ -162,6 +162,12 @@
 
   std::vector addresses = SocketAddress::GetAddressInfo(
   host_str.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
+  // IPv4 is more likely to succeed than IPv6. To avoid costly retries,
+  // we sort based on family to prioritize IPv4 over IPv6.
+  std::sort(addresses.begin(), addresses.end(),
+[](SocketAddress a, SocketAddress b) {
+  return a.GetFamily() < b.GetFamily();
+});
   for (SocketAddress &address : addresses) {
 error = CreateSocket(address.GetFamily());
 if (error.Fail())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D79757: Try IPv4 before IPv6 when creating TCP connection

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 263599.
emrekultursay added a comment.

Enforce IPv4 usage only in Android code, revert non-Android changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79757

Files:
  lldb/source/Plugins/Platform/Android/AdbClient.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp


Index: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
@@ -188,7 +188,7 @@
 if (error.Success()) {
   m_port_forwards[pid] = local_port;
   std::ostringstream url_str;
-  url_str << "connect://localhost:" << local_port;
+  url_str << "connect://127.0.0.1:" << local_port;
   connect_url = url_str.str();
   break;
 }
Index: lldb/source/Plugins/Platform/Android/AdbClient.cpp
===
--- lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -141,7 +141,7 @@
   if (const char *env_port = std::getenv("ANDROID_ADB_SERVER_PORT")) {
 port = env_port;
   }
-  std::string uri = "connect://localhost:" + port;
+  std::string uri = "connect://127.0.0.1:" + port;
   m_conn->Connect(uri.c_str(), &error);
 
   return error;


Index: lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
@@ -188,7 +188,7 @@
 if (error.Success()) {
   m_port_forwards[pid] = local_port;
   std::ostringstream url_str;
-  url_str << "connect://localhost:" << local_port;
+  url_str << "connect://127.0.0.1:" << local_port;
   connect_url = url_str.str();
   break;
 }
Index: lldb/source/Plugins/Platform/Android/AdbClient.cpp
===
--- lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -141,7 +141,7 @@
   if (const char *env_port = std::getenv("ANDROID_ADB_SERVER_PORT")) {
 port = env_port;
   }
-  std::string uri = "connect://localhost:" + port;
+  std::string uri = "connect://127.0.0.1:" + port;
   m_conn->Connect(uri.c_str(), &error);
 
   return error;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D79757: Try IPv4 before IPv6 when creating TCP connection

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

In D79757#2031186 , @labath wrote:

> ...
>  Where do the addresses that we're connecting to come from (the user or lldb 
> code)? If it's lldb code, maybe we could just replace the relevant 
> "localhost" names with an explicit ipv4 loopback address?


This is great feedback, thank you! yes, it was just two places that had 
"localhost" hard-coded, and just by changing it to ipv4 loopback address, I 
achieved the same improvement. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79757



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

[Lldb-commits] [PATCH] D79757: Use IPv4 for Android connections

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

I admit I don't fully understand the details of that CL and how it may interact 
with this one. However, I can say that I verified this CL with an Android 
device connected over IPv6.

So, I think this CL is ready to be submitted. (I don't have commit access).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79757



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

[Lldb-commits] [PATCH] D79586: Do not list adb devices when a device id is given

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

I don't have commit access. Can someone submit this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79586



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