date:20220302

[Lldb-commits] [PATCH] D120755: Fix race condition when launching and attaching.This is a modified version of a previous patch that was reverted: https://reviews.llvm.org/D119797This version only wait

2022-03-02 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.
Herald added a project: All.

I'm not exactly exactly thrilled by the sleep-based implementation, but 
otherwise, the patch seems fine.




Comment at: lldb/tools/lldb-vscode/VSCode.cpp:551
+  auto timeout_time =
+  std::chrono::high_resolution_clock::now() + 
std::chrono::seconds(seconds);
+  while (std::chrono::high_resolution_clock::now() < timeout_time) {

`steady_clock` would be better here (no time travel there)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120755

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

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

This seems like a better approach, but I'm not sure how does it actually work 
(see inline comment). Are you sure that we're still sending input to the 
process (I'm not sure how much test coverage for this do we have)?




Comment at: lldb/source/Target/Process.cpp:4343
+std::lock_guard guard(m_mutex);
+if (GetIsDone())
+  break;

I'm confused. How come this does not immediately terminate due to GetIsDone 
(through SetIsDone(true) via SetIsRunning(true) on line 4339) returning true?


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

https://reviews.llvm.org/D120762

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

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

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



Comment at: lldb/source/Target/Process.cpp:4343
+std::lock_guard guard(m_mutex);
+if (GetIsDone())
+  break;

labath wrote:
> I'm confused. How come this does not immediately terminate due to GetIsDone 
> (through SetIsDone(true) via SetIsRunning(true) on line 4339) returning true?
Yeah this is bogus, that line should read `SetIsDone(!running);`. I originally 
had these 3 lines inline and made a typo when extracting the function and 
didn't rerun the tests.


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

https://reviews.llvm.org/D120762

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

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 412337.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

Fix typo in `SetIsRunning`.


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

https://reviews.llvm.org/D120762

Files:
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4311,6 +4311,12 @@
 
   ~IOHandlerProcessSTDIO() override = default;
 
+  void SetIsRunning(bool running) {
+std::lock_guard guard(m_mutex);
+SetIsDone(!running);
+m_is_running = running;
+  }
+
   // Each IOHandler gets to run until it is done. It should read data from the
   // "in" and place output into "out" and "err and return when done.
   void Run() override {
@@ -4330,49 +4336,52 @@
 // FD_ZERO, FD_SET are not supported on windows
 #ifndef _WIN32
 const int pipe_read_fd = m_pipe.GetReadFileDescriptor();
-m_is_running = true;
-while (!GetIsDone()) {
+SetIsRunning(true);
+while (true) {
+  {
+std::lock_guard guard(m_mutex);
+if (GetIsDone())
+  break;
+  }
+
   SelectHelper select_helper;
   select_helper.FDSetRead(read_fd);
   select_helper.FDSetRead(pipe_read_fd);
   Status error = select_helper.Select();
 
-  if (error.Fail()) {
-SetIsDone(true);
-  } else {
-char ch = 0;
-size_t n;
-if (select_helper.FDIsSetRead(read_fd)) {
-  n = 1;
-  if (m_read_file.Read(&ch, n).Success() && n == 1) {
-if (m_write_file.Write(&ch, n).Fail() || n != 1)
-  SetIsDone(true);
-  } else
-SetIsDone(true);
-}
+  if (error.Fail())
+break;
+
+  char ch = 0;
+  size_t n;
+  if (select_helper.FDIsSetRead(read_fd)) {
+n = 1;
+if (m_read_file.Read(&ch, n).Success() && n == 1) {
+  if (m_write_file.Write(&ch, n).Fail() || n != 1)
+break;
+} else
+  break;
+
 if (select_helper.FDIsSetRead(pipe_read_fd)) {
   size_t bytes_read;
   // Consume the interrupt byte
   Status error = m_pipe.Read(&ch, 1, bytes_read);
   if (error.Success()) {
-switch (ch) {
-case 'q':
-  SetIsDone(true);
+if (ch == 'q')
   break;
-case 'i':
+if (ch == 'i')
   if (StateIsRunningState(m_process->GetState()))
 m_process->SendAsyncInterrupt();
-  break;
-}
   }
 }
   }
 }
-m_is_running = false;
+SetIsRunning(false);
 #endif
   }
 
   void Cancel() override {
+std::lock_guard guard(m_mutex);
 SetIsDone(true);
 // Only write to our pipe to cancel if we are in
 // IOHandlerProcessSTDIO::Run(). We can end up with a python command that
@@ -4429,7 +4438,8 @@
   NativeFile m_write_file; // Write to this file (usually the primary pty for
// getting io to debuggee)
   Pipe m_pipe;
-  std::atomic m_is_running{false};
+  std::mutex m_mutex;
+  bool m_is_running = false;
 };
 
 void Process::SetSTDIOFileDescriptor(int fd) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D120762#3353655 , @labath wrote:

> Are you sure that we're still sending input to the process (I'm not sure how 
> much test coverage for this do we have)?

I'll rerun the tests tomorrow with my typo and see if anything catches this. If 
not I can add a shell/pexpect test with the little test program I was using 
which echos whatever it reads via stdin.

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

https://reviews.llvm.org/D120762

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

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D120762#3353686 , @JDevlieghere 
wrote:

> In D120762#3353655 , @labath wrote:
>
>> Are you sure that we're still sending input to the process (I'm not sure how 
>> much test coverage for this do we have)?
>
> I'll rerun the tests tomorrow with my typo and see if anything catches this. 
> If not I can add a shell/pexpect test with the little test program I was 
> using which echos whatever it reads via stdin.

Cool.


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

https://reviews.llvm.org/D120762

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

[Lldb-commits] [PATCH] D120803: [lldb] Don't print trailing nuls in char arrays

2022-03-02 Thread Pavel Labath via Phabricator via lldb-commits

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

Embedded nul characters are still printed, and they don't terminate the
string. See also D111634 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120803

Files:
  lldb/source/Core/ValueObject.cpp
  
lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
  lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s


Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
@@ -17,7 +17,7 @@
 ## Variables specified using string forms. This behavior purely speculative -- 
I
 ## don't know of any compiler that would represent character strings this way.
 # CHECK: (char[7]) string = "string"
-# CHECK: (char[7]) strp = "strp\0\0"
+# CHECK: (char[7]) strp = "strp"
 ## Bogus attribute form. Let's make sure we don't crash at least.
 # CHECK: (char[7]) ref4 = 
 ## A variable of pointer type.
Index: lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -29,6 +29,7 @@
 
 int main (int argc, char const *argv[])
 {
+const char manytrailingnuls[] = "F\0OO\0BA\0R\0\0\0\0";
 A a, b, c;
 // Deliberately write past the end of data to test that the formatter stops
 // at the end of array.
@@ -59,6 +60,7 @@
 //% self.expect_var_path("a.data", summary='"FOOB"')
 //% self.expect_var_path("b.data", summary=r'"FO\0B"')
 //% self.expect_var_path("c.data", summary=r'"F\0O"')
+//% self.expect_var_path("manytrailingnuls", summary=r'"F\0OO\0BA\0R"')
 //%
 //% for c in ["", "const"]:
 //%   for v in ["", "volatile"]:
Index: 
lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
===
--- 
lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
+++ 
lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
@@ -90,8 +90,8 @@
 
 # Different character arrays.
 # FIXME: Passing a 'const char *' will ignore any given format,
-self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09\0"', 
self.getFormatted("character array", "cstring"))
-self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09\0"', 
self.getFormatted("c-string", "cstring"))
+self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09"', 
self.getFormatted("character array", "cstring"))
+self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09"', 
self.getFormatted("c-string", "cstring"))
 self.assertIn(' = " \\e\\a\\b\\f\\n\\r\\t\\vaA09" " 
\\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n',
   self.getFormatted("c-string", "(char *)cstring"))
 self.assertIn('=\n', self.getFormatted("c-string", 
"(__UINT64_TYPE__)0"))
@@ -131,10 +131,10 @@
 self.assertIn('= 0x2007080c0a0d090b415a617a30391b00\n', 
self.getFormatted("OSType", string_expr))
 
 # bytes
-self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09\0"', 
self.getFormatted("bytes", "cstring"))
+self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09"', 
self.getFormatted("bytes", "cstring"))
 
 # bytes with ASCII
-self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09\0"', 
self.getFormatted("bytes with ASCII", "cstring"))
+self.assertIn(r'= " \U001b\a\b\f\n\r\t\vaA09"', 
self.getFormatted("bytes with ASCII", "cstring"))
 
 # unicode8
 self.assertIn('= 0x78 0x56 0x34 0x12\n', self.getFormatted("unicode8", 
"0x12345678"))
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -850,7 +850,7 @@
 static bool CopyStringDataToBufferSP(const StreamString &source,
  lldb::DataBufferSP &destination) {
   llvm::StringRef src = source.GetString();
-  src.consume_back(llvm::StringRef("\0", 1));
+  src = src.rtrim('\0');
   destination = std::make_shared(src.size(), 0);
   memcpy(destination->GetBytes(), src.data(), src.size());
   return true;


Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s
@@ -17,7 +17,7 @@
 ## Variables specified using str

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

2022-03-02 Thread 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

2022-03-02 Thread 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] [lldb] d2edca6 - [lldb/Platform] Prepare decouple instance and plugin names

2022-03-02 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2022-03-02T14:57:01+01:00
New Revision: d2edca6276d1715a02d1144eae577b3d79673d67

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

LOG: [lldb/Platform] Prepare decouple instance and plugin names

This patch changes the return value of Platform::GetName() to a
StringRef, and uses the opportunity (compile errors) to change some
callsites to use GetPluginName() instead. The two methods still remain
hardwired to return the same thing, but this will change once the ideas
in

are implemented.

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

Added: 


Modified: 
lldb/include/lldb/Target/Platform.h
lldb/source/API/SBPlatform.cpp
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Interpreter/OptionGroupPlatform.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/TargetList.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index b804d4877c59c..b3855f0bb5071 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -217,7 +217,7 @@ class Platform : public PluginInterface {
   llvm::Optional GetOSKernelDescription();
 
   // Returns the name of the platform
-  ConstString GetName();
+  llvm::StringRef GetName() { return GetPluginName(); }
 
   virtual const char *GetHostname();
 
@@ -508,17 +508,17 @@ class Platform : public PluginInterface {
 
   virtual uint64_t ReadFile(lldb::user_id_t fd, uint64_t offset, void *dst,
 uint64_t dst_len, Status &error) {
-error.SetErrorStringWithFormat(
-"Platform::ReadFile() is not supported in the %s platform",
-GetName().GetCString());
+error.SetErrorStringWithFormatv(
+"Platform::ReadFile() is not supported in the {0} platform",
+GetPluginName());
 return -1;
   }
 
   virtual uint64_t WriteFile(lldb::user_id_t fd, uint64_t offset,
  const void *src, uint64_t src_len, Status &error) 
{
-error.SetErrorStringWithFormat(
-"Platform::WriteFile() is not supported in the %s platform",
-GetName().GetCString());
+error.SetErrorStringWithFormatv(
+"Platform::WriteFile() is not supported in the {0} platform",
+GetPluginName());
 return -1;
   }
 

diff  --git a/lldb/source/API/SBPlatform.cpp b/lldb/source/API/SBPlatform.cpp
index d521a38b30e8d..eeb56e9c6ed9b 100644
--- a/lldb/source/API/SBPlatform.cpp
+++ b/lldb/source/API/SBPlatform.cpp
@@ -342,7 +342,7 @@ const char *SBPlatform::GetName() {
 
   PlatformSP platform_sp(GetSP());
   if (platform_sp)
-return platform_sp->GetName().GetCString();
+return ConstString(platform_sp->GetName()).AsCString();
   return nullptr;
 }
 

diff  --git a/lldb/source/Commands/CommandObjectPlatform.cpp 
b/lldb/source/Commands/CommandObjectPlatform.cpp
index 4c18465c868a0..bf586adb2bbd1 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1281,16 +1281,15 @@ class CommandObjectPlatformProcessList : public 
CommandObjectParsed {
 result.AppendErrorWithFormatv(
 "no processes were found that {0} \"{1}\" on the \"{2}\" "
 "platform\n",
-match_desc, match_name, platform_sp->GetPluginName());
+match_desc, match_name, platform_sp->GetName());
   else
 result.AppendErrorWithFormatv(
 "no processes were found on the \"{0}\" platform\n",
-platform_sp->GetPluginName());
+platform_sp->GetName());
 } else {
-  result.AppendMessageWithFormat(
-  "%u matching process%s found on \"%s\"", matches,
-  matches > 1 ? "es were" : " was",
-  platform_sp->GetName().GetCString());
+  result.AppendMessageWithFormatv(
+  "{0} matching process{1} found on \"{2}\"", matches,
+  matches > 1 ? "es were" : " was", platform_sp->GetName());
   if (match_desc)
 result.AppendMessageWithFormat(" whose name %s \"%s\"",

[Lldb-commits] [PATCH] D119146: [lldb/Platform] Prepare decouple instance and plugin names

2022-03-02 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd2edca6276d1: [lldb/Platform] Prepare decouple instance and 
plugin names (authored by labath).
Herald added a project: All.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119146

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Interpreter/OptionGroupPlatform.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/TargetList.cpp

Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -242,7 +242,7 @@
 platform_set.end()) {
   if (!platform_set.empty())
 error_strm.PutCString(", ");
-  error_strm.PutCString(the_platform_sp->GetName().GetCString());
+  error_strm.PutCString(the_platform_sp->GetName());
   platform_set.insert(the_platform_sp.get());
 }
   }
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2895,11 +2895,10 @@
   if (platform_sp) {
 GetTarget().SetPlatform(platform_sp);
 GetTarget().SetArchitecture(platform_arch);
-LLDB_LOGF(log,
-  "Process::%s switching platform to %s and architecture "
-  "to %s based on info from attach",
-  __FUNCTION__, platform_sp->GetName().AsCString(""),
-  platform_arch.GetTriple().getTriple().c_str());
+LLDB_LOG(log,
+ "switching platform to {0} and architecture to {1} based on "
+ "info from attach",
+ platform_sp->GetName(), platform_arch.GetTriple().getTriple());
   }
 } else if (!process_arch.IsValid()) {
   ProcessInstanceInfo process_info;
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -280,7 +280,7 @@
 
 std::lock_guard guard(GetPlatformListMutex());
 for (const auto &platform_sp : GetPlatformList()) {
-  if (platform_sp->GetName() == name)
+  if (platform_sp->GetName() == name.GetStringRef())
 return platform_sp;
 }
   }
@@ -786,8 +786,6 @@
   }
 }
 
-ConstString Platform::GetName() { return ConstString(GetPluginName()); }
-
 const char *Platform::GetHostname() {
   if (IsHost())
 return "127.0.0.1";
@@ -1728,7 +1726,7 @@
 
 FileSpec Platform::GetModuleCacheRoot() {
   auto dir_spec = GetGlobalPlatformProperties().GetModuleCacheDirectory();
-  dir_spec.AppendPathComponent(GetName().AsCString());
+  dir_spec.AppendPathComponent(GetPluginName());
   return dir_spec;
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2404,9 +2404,8 @@
   m_public_state.GetValue() != eStateRunning) {
 PlatformSP platform_sp = GetTarget().GetPlatform();
 
-if (platform_sp && platform_sp->GetName() &&
-platform_sp->GetName().GetStringRef() ==
-PlatformRemoteiOS::GetPluginNameStatic()) {
+if (platform_sp && platform_sp->GetPluginName() ==
+   PlatformRemoteiOS::GetPluginNameStatic()) {
   if (m_destroy_tried_resuming) {
 if (log)
   log->PutCString("ProcessGDBRemote::DoDestroy() - Tried resuming to "
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -24,7 +24,7 @@
 
 class PlatformDarwin : public PlatformPOSIX {
 public:
-  PlatformDarwin(bool is_host);
+  using PlatformPOSIX::PlatformPOSIX;
 
   ~PlatformDarwin() override;
 
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -48,9 +48,6 @@
 using namespace lldb;
 using

[Lldb-commits] [PATCH] D120810: [lldb] Remove the global platform list

2022-03-02 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: JDevlieghere, clayborg, jingham.
Herald added subscribers: mgorny, emaste.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

This patch moves the platform creation and selection logic into the
per-debugger platform lists. I've tried to keep functional changes to a
minimum -- the main (only) observable difference in this change is that
APIs, which select a platform by name (e.g.,
Debugger::SetCurrentPlatform) will not automatically pick up a platform
associated with another debugger (or no debugger at all).

I've also added several tests for this functionality -- one of the
pleasant consequences of the debugger isolation is that it is now
possible to test the platform selection and creation logic.

This is a product of the discussion at
https://discourse.llvm.org/t/multiple-platforms-with-the-same-name/59594.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120810

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBPlatform.cpp
  lldb/source/Interpreter/OptionGroupPlatform.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/Platform/POSIX/CMakeLists.txt
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Platform/Windows/CMakeLists.txt
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/source/Plugins/Platform/gdb-server/CMakeLists.txt
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetList.cpp
  lldb/test/API/python_api/debugger/TestDebuggerAPI.py
  lldb/test/API/python_api/debugger/elf.yaml
  lldb/test/API/python_api/debugger/macho.yaml
  lldb/test/API/python_api/sbplatform/TestSBPlatform.py

Index: lldb/test/API/python_api/sbplatform/TestSBPlatform.py
===
--- lldb/test/API/python_api/sbplatform/TestSBPlatform.py
+++ lldb/test/API/python_api/sbplatform/TestSBPlatform.py
@@ -25,6 +25,29 @@
 plat = lldb.SBPlatform("remote-linux") # arbitrary choice
 self.assertTrue(plat)
 plat.SetSDKRoot(self.getBuildDir())
-self.dbg.SetCurrentPlatform("remote-linux")
+self.dbg.SetSelectedPlatform(plat)
 self.expect("platform status",
 substrs=["Sysroot:", self.getBuildDir()])
+
+def test_SetCurrentPlatform_floating(self):
+# floating platforms cannot be referenced by name until they are
+# associated with a debugger
+floating_platform = lldb.SBPlatform("remote-netbsd")
+floating_platform.SetWorkingDirectory(self.getBuildDir())
+self.assertSuccess(self.dbg.SetCurrentPlatform("remote-netbsd"))
+dbg_platform = self.dbg.GetSelectedPlatform()
+self.assertEqual(dbg_platform.GetName(), "remote-netbsd")
+self.assertIsNone(dbg_platform.GetWorkingDirectory())
+
+def test_SetCurrentPlatform_associated(self):
+# associated platforms are found by name-based lookup
+floating_platform = lldb.SBPlatform("remote-netbsd")
+floating_platform.SetWorkingDirectory(self.getBuildDir())
+orig_platform = self.dbg.GetSelectedPlatform()
+
+self.dbg.SetSelectedPlatform(floating_platform)
+self.dbg.SetSelectedPlatform(orig_platform)
+self.assertSuccess(self.dbg.SetCurrentPlatform("remote-netbsd"))
+dbg_platform = self.dbg.GetSelectedPlatform()
+self.assertEqual(dbg_platform.GetName(), "remote-netbsd")
+self.assertEqual(dbg_platform.GetWorkingDirectory(), self.getBuildDir())
Index: lldb/test/API/python_api/debugger/macho.yaml
===
--- /dev/null
+++ lldb/test/API/python_api/debugger/macho.yaml
@@ -0,0 +1,42 @@
+--- !mach-o
+FileHeader:  
+  magic:   0xFEEDFACF
+  cputype: 0x0107
+  cpusubtype:  0x0003
+  filetype:0x0001
+  ncmds:   4
+  sizeofcmds:  1160
+  flags:   0x2000
+  reserved:0x
+LoadCommands:
+  - cmd: LC_SEGMENT_64
+cmdsize: 1032
+segname: ''
+vmaddr:  0
+vmsize:  744
+fileoff: 1192
+filesize:744
+maxprot: 7
+initprot:7
+nsects:  12
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x
+size:22
+offset:  0x04A8
+align:   4
+reloff:  0x
+nreloc:  0
+flags:   0x8400
+reserved1:   0x
+reserved2:   0x
+reserved3:   0x
+  - cmd: LC_BUILD_VERSION
+cmdsize: 24
+platform:

[Lldb-commits] [PATCH] D120803: [lldb] Don't print trailing nuls in char arrays

2022-03-02 Thread Dave Lee via Phabricator via lldb-commits

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

thanks for doing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120803

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

[Lldb-commits] [PATCH] D120792: [lldb] Fix python errors in gdbremote.py

2022-03-02 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added inline comments.



Comment at: lldb/examples/python/gdbremote.py:1224-1225
+print(json.dumps(json_tree, indent=4, separators=(',', ': ')))
+except json.JSONDecodeError:
+return
 

I don't know this tool, but should it print the original json (`rsp`) if 
there's an issue when decoding the json? Or should it print a message saying 
invalid json was given?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120792

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

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 412479.
JDevlieghere added a comment.

Unsurprisingly no tests failed with the typo. Added a test case to cover 
reading from stdin through the `IOHandlerProcessSTDIO`.


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

https://reviews.llvm.org/D120762

Files:
  lldb/source/Target/Process.cpp
  lldb/test/API/iohandler/stdio/Makefile
  lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
  lldb/test/API/iohandler/stdio/main.cpp

Index: lldb/test/API/iohandler/stdio/main.cpp
===
--- /dev/null
+++ lldb/test/API/iohandler/stdio/main.cpp
@@ -0,0 +1,8 @@
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  for (std::string line; std::getline(std::cin, line);)
+std::cout << "stdout: " << line << '\n';
+  return 0;
+}
Index: lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
===
--- /dev/null
+++ lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
@@ -0,0 +1,30 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestIOHandlerProcessSTDIO(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+def test(self):
+self.build()
+self.launch(executable=self.getBuildArtifact("a.out"), dimensions=(100,500))
+self.child.sendline("run")
+
+self.child.send("foo\n")
+self.child.expect_exact("stdout: foo")
+
+self.child.send("bar\n")
+self.child.expect_exact("stdout: bar")
+
+self.child.send("baz\n")
+self.child.expect_exact("stdout: baz")
+
+self.child.sendcontrol('d')
+self.expect_prompt()
+self.quit()
Index: lldb/test/API/iohandler/stdio/Makefile
===
--- /dev/null
+++ lldb/test/API/iohandler/stdio/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4311,6 +4311,12 @@
 
   ~IOHandlerProcessSTDIO() override = default;
 
+  void SetIsRunning(bool running) {
+std::lock_guard guard(m_mutex);
+SetIsDone(!running);
+m_is_running = running;
+  }
+
   // Each IOHandler gets to run until it is done. It should read data from the
   // "in" and place output into "out" and "err and return when done.
   void Run() override {
@@ -4330,49 +4336,52 @@
 // FD_ZERO, FD_SET are not supported on windows
 #ifndef _WIN32
 const int pipe_read_fd = m_pipe.GetReadFileDescriptor();
-m_is_running = true;
-while (!GetIsDone()) {
+SetIsRunning(true);
+while (true) {
+  {
+std::lock_guard guard(m_mutex);
+if (GetIsDone())
+  break;
+  }
+
   SelectHelper select_helper;
   select_helper.FDSetRead(read_fd);
   select_helper.FDSetRead(pipe_read_fd);
   Status error = select_helper.Select();
 
-  if (error.Fail()) {
-SetIsDone(true);
-  } else {
-char ch = 0;
-size_t n;
-if (select_helper.FDIsSetRead(read_fd)) {
-  n = 1;
-  if (m_read_file.Read(&ch, n).Success() && n == 1) {
-if (m_write_file.Write(&ch, n).Fail() || n != 1)
-  SetIsDone(true);
-  } else
-SetIsDone(true);
-}
+  if (error.Fail())
+break;
+
+  char ch = 0;
+  size_t n;
+  if (select_helper.FDIsSetRead(read_fd)) {
+n = 1;
+if (m_read_file.Read(&ch, n).Success() && n == 1) {
+  if (m_write_file.Write(&ch, n).Fail() || n != 1)
+break;
+} else
+  break;
+
 if (select_helper.FDIsSetRead(pipe_read_fd)) {
   size_t bytes_read;
   // Consume the interrupt byte
   Status error = m_pipe.Read(&ch, 1, bytes_read);
   if (error.Success()) {
-switch (ch) {
-case 'q':
-  SetIsDone(true);
+if (ch == 'q')
   break;
-case 'i':
+if (ch == 'i')
   if (StateIsRunningState(m_process->GetState()))
 m_process->SendAsyncInterrupt();
-  break;
-}
   }
 }
   }
 }
-m_is_running = false;
+SetIsRunning(false);
 #endif
   }
 
   void Cancel() override {
+std::lock_guard guard(m_mutex);
 SetIsDone(true);
 // Only write to our pipe to cancel if we are in
 // IOHandlerProcessSTDIO::Run(). We can end up with a python command that
@@ -4429,7 +4438,8 @@
   NativeFile m_write_file; // Write to this file (u

[Lldb-commits] [PATCH] D120755: Fix race condition when launching and attaching.This is a modified version of a previous patch that was reverted: https://reviews.llvm.org/D119797This version only wait

2022-03-02 Thread jeffrey tan via Phabricator via lldb-commits

yinghuitan added inline comments.



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:532-533
+lldb::SBError VSCode::WaitForProcessToStop(uint32_t seconds) {
+  // Wait for the process hit a stopped state. When running a launch (with or
+  // without "launchCommands") or attach (with or without)= "attachCommands"),
+  // the calls might take some time to stop at the entry point since the 
command

The same: update the comment to reflect latest state.



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:551
+  auto timeout_time =
+  std::chrono::high_resolution_clock::now() + 
std::chrono::seconds(seconds);
+  while (std::chrono::high_resolution_clock::now() < timeout_time) {

labath wrote:
> `steady_clock` would be better here (no time travel there)
Agreed.



Comment at: lldb/tools/lldb-vscode/VSCode.h:249-251
+  /// continue with the launch or attach. This is needed since we no longer 
play
+  /// with the synchronous mode in the debugger for launching (with or without
+  /// "launchCommands") or attaching (with or without "attachCommands").

This comment is not true in this diff anymore. We only use async mode in 
launchCommands/attachCommands.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:459
+// event by calling the SendThreadStoppedEvent().
+if (process.GetStopID() > 1) {
+  // Only report a stopped event if the process was not restarted.

This assumes there is only one stop event sent before 
`request_configuarationDone`. This is true in our detected situation but I like 
the other logic of not sending any stop event before 
`request_configuarationDone`. That is more accurately aligned with synchronous 
launch/attach mode withotu relying on one stop event assumption for some future 
unknown situation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120755

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

[Lldb-commits] [PATCH] D120810: [lldb] Remove the global platform list

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

It's always great to see changes that enable more testing. I left one inline 
comment/nit but besides that this LGTM.




Comment at: lldb/source/Target/Platform.cpp:1891
+
+PlatformSP PlatformList::GetOrCreate(const ArchSpec &arch,
+ ArchSpec *platform_arch_ptr,

[Nit/pedantic] We have a few calls to `IsCompatibleArchitecture` in this 
function. Based on the existing comments you can infer that that's what the 
second argument is for. Normally I'd suggest an inline comment, but given the 
that there's a bunch of calls, could we have either have two constants:

```
enum ArchMatch : bool {
  ExactArchMatch = true,
  CompatibleArchMatch = false,
};
```

Or maybe even better, can we change the signature of that function to take an 
enum with those values? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120810

___
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

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Thanks for cleaning up the implementation file. LGMT module removing the added 
`lldb_private::` which I believe we no longer need.




Comment at: lldb/include/lldb/Core/DataFileCache.h:164
   ///   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);

I assume the header was never a problem and you did this to match the signature 
in the implementation file (which no longer needs the namespace qualifier 
without the `using namespace llvm`), unless one of the headers included here 
(transitively) includes a header that has the "😱 😱 😱" top level `using 
namespace llvm;`? 


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] D120594: Improve error messages for command that need a stopped process

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120594

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

[Lldb-commits] [lldb] 9f37775 - [cmake] Prefix gtest and gtest_main with "llvm_".

2022-03-02 Thread Stella Laurenzo via lldb-commits


Author: Stella Laurenzo
Date: 2022-03-02T10:53:32-08:00
New Revision: 9f37775472b45986b0ecce5243bd6ce119e5bd69

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

LOG: [cmake] Prefix gtest and gtest_main with "llvm_".

The upstream project ships CMake rules for building vanilla gtest/gmock which 
conflict with the names chosen by LLVM. Since LLVM's build rules here are quite 
specific to LLVM, prefixing them to avoid collision is the right thing (i.e. 
there does not appear to be a path to letting someone *replace* LLVM's 
googletest with one they bring, so co-existence should be the goal).

This allows LLVM to be included with testing enabled within projects that 
themselves have a dependency on an official gtest release.

Reviewed By: mehdi_amini

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

Added: 


Modified: 
compiler-rt/lib/asan/tests/CMakeLists.txt
compiler-rt/lib/fuzzer/tests/CMakeLists.txt
compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
compiler-rt/lib/interception/tests/CMakeLists.txt
compiler-rt/lib/msan/tests/CMakeLists.txt
compiler-rt/lib/orc/unittests/CMakeLists.txt
compiler-rt/lib/sanitizer_common/tests/CMakeLists.txt
compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
compiler-rt/lib/tsan/tests/CMakeLists.txt
compiler-rt/lib/xray/tests/CMakeLists.txt
flang/CMakeLists.txt
libc/benchmarks/CMakeLists.txt
libc/test/utils/tools/CMakeLists.txt
lldb/unittests/CMakeLists.txt
llvm/cmake/modules/AddLLVM.cmake
llvm/lib/Testing/Support/CMakeLists.txt
llvm/runtimes/CMakeLists.txt
llvm/unittests/Support/CommandLineInit/CMakeLists.txt
llvm/utils/unittest/CMakeLists.txt
llvm/utils/unittest/UnitTestMain/CMakeLists.txt

Removed: 




diff  --git a/compiler-rt/lib/asan/tests/CMakeLists.txt 
b/compiler-rt/lib/asan/tests/CMakeLists.txt
index 047a3fa282c40..b2e27f66b7312 100644
--- a/compiler-rt/lib/asan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/asan/tests/CMakeLists.txt
@@ -172,7 +172,7 @@ function(add_asan_tests arch test_runtime)
   function(generate_asan_tests test_objects test_suite testname)
 generate_compiler_rt_tests(${test_objects} ${test_suite} ${testname} 
${arch}
   COMPILE_DEPS ${ASAN_UNITTEST_HEADERS} ${ASAN_IGNORELIST_FILE}
-  DEPS gtest asan
+  DEPS llvm_gtest asan
   KIND ${TEST_KIND}
   ${ARGN}
   )

diff  --git a/compiler-rt/lib/fuzzer/tests/CMakeLists.txt 
b/compiler-rt/lib/fuzzer/tests/CMakeLists.txt
index 85369990889ec..10fcfbaa083e5 100644
--- a/compiler-rt/lib/fuzzer/tests/CMakeLists.txt
+++ b/compiler-rt/lib/fuzzer/tests/CMakeLists.txt
@@ -74,7 +74,7 @@ if(COMPILER_RT_DEFAULT_TARGET_ARCH IN_LIST 
FUZZER_SUPPORTED_ARCH)
 FuzzerUnitTests "Fuzzer-${arch}-Test" ${arch}
 SOURCES FuzzerUnittest.cpp ${COMPILER_RT_GTEST_SOURCE}
 RUNTIME ${LIBFUZZER_TEST_RUNTIME}
-DEPS gtest ${LIBFUZZER_TEST_RUNTIME_DEPS} 
+DEPS llvm_gtest ${LIBFUZZER_TEST_RUNTIME_DEPS}
 CFLAGS ${LIBFUZZER_UNITTEST_CFLAGS} ${LIBFUZZER_TEST_RUNTIME_CFLAGS}
 LINK_FLAGS ${LIBFUZZER_UNITTEST_LINK_FLAGS} 
${LIBFUZZER_TEST_RUNTIME_LINK_FLAGS})
   set_target_properties(FuzzerUnitTests PROPERTIES
@@ -84,7 +84,7 @@ if(COMPILER_RT_DEFAULT_TARGET_ARCH IN_LIST 
FUZZER_SUPPORTED_ARCH)
   generate_compiler_rt_tests(FuzzedDataProviderTestObjects
 FuzzedDataProviderUnitTests "FuzzerUtils-${arch}-Test" ${arch}
 SOURCES FuzzedDataProviderUnittest.cpp ${COMPILER_RT_GTEST_SOURCE}
-DEPS gtest ${LIBFUZZER_TEST_RUNTIME_DEPS} 
${COMPILER_RT_SOURCE_DIR}/include/fuzzer/FuzzedDataProvider.h
+DEPS llvm_gtest ${LIBFUZZER_TEST_RUNTIME_DEPS} 
${COMPILER_RT_SOURCE_DIR}/include/fuzzer/FuzzedDataProvider.h
 CFLAGS ${LIBFUZZER_UNITTEST_CFLAGS} ${LIBFUZZER_TEST_RUNTIME_CFLAGS}
 LINK_FLAGS ${LIBFUZZER_UNITTEST_LINK_FLAGS} 
${LIBFUZZER_TEST_RUNTIME_LINK_FLAGS})
   set_target_properties(FuzzedDataProviderUnitTests PROPERTIES

diff  --git a/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt 
b/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
index abc02a49637cc..8b4ee7d553b82 100644
--- a/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
@@ -66,7 +66,7 @@ if(COMPILER_RT_DEFAULT_TARGET_ARCH IN_LIST 
GWP_ASAN_SUPPORTED_ARCH)
 GwpAsanUnitTests "GwpAsan-${arch}-Test" ${arch}
 SOURCES ${GWP_ASAN_UNITTESTS} ${COMPILER_RT_GTEST_SOURCE}
 RUNTIME ${GWP_ASAN_TEST_RUNTIME}
-DEPS gtest ${GWP_ASAN_UNIT_TEST_HEADERS}
+DEPS llvm_gtest ${GWP_ASAN_UNIT_TEST_HEADERS}
 CFLAGS ${GWP_ASAN_UNITTEST_CFLAGS}
 LINK_FLAGS ${GWP_ASAN_UNITTEST_LINK_FLAGS})
   set_target_properties(GwpAsanUnitTests PROPERTIES

diff  --git a/compiler-rt/lib/interception/tests/CMakeLists.txt 
b/compiler-rt/lib/interception/tests/CMakeLists.txt
index 06184ee

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a comment.

I used `using namespace` in a few functions but I am not committed to this 
approach. So I am happy to hear feedback on whether we want to just use fully 
qualified names everywhere instead or nail down a criteria as to when it is 
acceptable.

We also have `using namespace lldb` in a few header files but I wanted to 
tackle that separately based on the feedback on this.


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

https://reviews.llvm.org/D120836

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

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added a comment.

For the .cpp files with hundreds of lines modified, what do you think about 
adding `using namespace llvm::dwarf;` to those? For starters 
`DWARFExpression.cpp`, `DWARFAbbreviationDeclaration.cpp`, and 
`SymbolFileDWARF.cpp`.


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

https://reviews.llvm.org/D120836

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

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added a comment.

sorry Shafik I see that you've asked that very question. I'll give a +1 to 
making use of `using namespace` within .cpp files, especially where a namespace 
is used pervasively. When a file uses it only a namespace infrequently, I think 
explicit is reasonable.


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

https://reviews.llvm.org/D120836

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

[Lldb-commits] [lldb] 7cdda6b - Revert "[cmake] Prefix gtest and gtest_main with "llvm_"."

2022-03-02 Thread Stella Laurenzo via lldb-commits


Author: Stella Laurenzo
Date: 2022-03-02T11:13:46-08:00
New Revision: 7cdda6b8ce49ae3c90c068cff4dc355bba5d77f2

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

LOG: Revert "[cmake] Prefix gtest and gtest_main with "llvm_"."

lldb buildbot failure. will investigate and roll forward.

This reverts commit 9f37775472b45986b0ecce5243bd6ce119e5bd69.

Added: 


Modified: 
compiler-rt/lib/asan/tests/CMakeLists.txt
compiler-rt/lib/fuzzer/tests/CMakeLists.txt
compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
compiler-rt/lib/interception/tests/CMakeLists.txt
compiler-rt/lib/msan/tests/CMakeLists.txt
compiler-rt/lib/orc/unittests/CMakeLists.txt
compiler-rt/lib/sanitizer_common/tests/CMakeLists.txt
compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
compiler-rt/lib/tsan/tests/CMakeLists.txt
compiler-rt/lib/xray/tests/CMakeLists.txt
flang/CMakeLists.txt
libc/benchmarks/CMakeLists.txt
libc/test/utils/tools/CMakeLists.txt
lldb/unittests/CMakeLists.txt
llvm/cmake/modules/AddLLVM.cmake
llvm/lib/Testing/Support/CMakeLists.txt
llvm/runtimes/CMakeLists.txt
llvm/unittests/Support/CommandLineInit/CMakeLists.txt
llvm/utils/unittest/CMakeLists.txt
llvm/utils/unittest/UnitTestMain/CMakeLists.txt

Removed: 




diff  --git a/compiler-rt/lib/asan/tests/CMakeLists.txt 
b/compiler-rt/lib/asan/tests/CMakeLists.txt
index b2e27f66b7312..047a3fa282c40 100644
--- a/compiler-rt/lib/asan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/asan/tests/CMakeLists.txt
@@ -172,7 +172,7 @@ function(add_asan_tests arch test_runtime)
   function(generate_asan_tests test_objects test_suite testname)
 generate_compiler_rt_tests(${test_objects} ${test_suite} ${testname} 
${arch}
   COMPILE_DEPS ${ASAN_UNITTEST_HEADERS} ${ASAN_IGNORELIST_FILE}
-  DEPS llvm_gtest asan
+  DEPS gtest asan
   KIND ${TEST_KIND}
   ${ARGN}
   )

diff  --git a/compiler-rt/lib/fuzzer/tests/CMakeLists.txt 
b/compiler-rt/lib/fuzzer/tests/CMakeLists.txt
index 10fcfbaa083e5..85369990889ec 100644
--- a/compiler-rt/lib/fuzzer/tests/CMakeLists.txt
+++ b/compiler-rt/lib/fuzzer/tests/CMakeLists.txt
@@ -74,7 +74,7 @@ if(COMPILER_RT_DEFAULT_TARGET_ARCH IN_LIST 
FUZZER_SUPPORTED_ARCH)
 FuzzerUnitTests "Fuzzer-${arch}-Test" ${arch}
 SOURCES FuzzerUnittest.cpp ${COMPILER_RT_GTEST_SOURCE}
 RUNTIME ${LIBFUZZER_TEST_RUNTIME}
-DEPS llvm_gtest ${LIBFUZZER_TEST_RUNTIME_DEPS}
+DEPS gtest ${LIBFUZZER_TEST_RUNTIME_DEPS} 
 CFLAGS ${LIBFUZZER_UNITTEST_CFLAGS} ${LIBFUZZER_TEST_RUNTIME_CFLAGS}
 LINK_FLAGS ${LIBFUZZER_UNITTEST_LINK_FLAGS} 
${LIBFUZZER_TEST_RUNTIME_LINK_FLAGS})
   set_target_properties(FuzzerUnitTests PROPERTIES
@@ -84,7 +84,7 @@ if(COMPILER_RT_DEFAULT_TARGET_ARCH IN_LIST 
FUZZER_SUPPORTED_ARCH)
   generate_compiler_rt_tests(FuzzedDataProviderTestObjects
 FuzzedDataProviderUnitTests "FuzzerUtils-${arch}-Test" ${arch}
 SOURCES FuzzedDataProviderUnittest.cpp ${COMPILER_RT_GTEST_SOURCE}
-DEPS llvm_gtest ${LIBFUZZER_TEST_RUNTIME_DEPS} 
${COMPILER_RT_SOURCE_DIR}/include/fuzzer/FuzzedDataProvider.h
+DEPS gtest ${LIBFUZZER_TEST_RUNTIME_DEPS} 
${COMPILER_RT_SOURCE_DIR}/include/fuzzer/FuzzedDataProvider.h
 CFLAGS ${LIBFUZZER_UNITTEST_CFLAGS} ${LIBFUZZER_TEST_RUNTIME_CFLAGS}
 LINK_FLAGS ${LIBFUZZER_UNITTEST_LINK_FLAGS} 
${LIBFUZZER_TEST_RUNTIME_LINK_FLAGS})
   set_target_properties(FuzzedDataProviderUnitTests PROPERTIES

diff  --git a/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt 
b/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
index 8b4ee7d553b82..abc02a49637cc 100644
--- a/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
+++ b/compiler-rt/lib/gwp_asan/tests/CMakeLists.txt
@@ -66,7 +66,7 @@ if(COMPILER_RT_DEFAULT_TARGET_ARCH IN_LIST 
GWP_ASAN_SUPPORTED_ARCH)
 GwpAsanUnitTests "GwpAsan-${arch}-Test" ${arch}
 SOURCES ${GWP_ASAN_UNITTESTS} ${COMPILER_RT_GTEST_SOURCE}
 RUNTIME ${GWP_ASAN_TEST_RUNTIME}
-DEPS llvm_gtest ${GWP_ASAN_UNIT_TEST_HEADERS}
+DEPS gtest ${GWP_ASAN_UNIT_TEST_HEADERS}
 CFLAGS ${GWP_ASAN_UNITTEST_CFLAGS}
 LINK_FLAGS ${GWP_ASAN_UNITTEST_LINK_FLAGS})
   set_target_properties(GwpAsanUnitTests PROPERTIES

diff  --git a/compiler-rt/lib/interception/tests/CMakeLists.txt 
b/compiler-rt/lib/interception/tests/CMakeLists.txt
index 8d5c4abb7af0c..06184ee77447f 100644
--- a/compiler-rt/lib/interception/tests/CMakeLists.txt
+++ b/compiler-rt/lib/interception/tests/CMakeLists.txt
@@ -95,7 +95,7 @@ macro(add_interception_tests_for_arch arch)
 RUNTIME ${INTERCEPTION_COMMON_LIB}
 SOURCES ${INTERCEPTION_UNITTESTS} ${COMPILER_RT_GTEST_SOURCE}
 COMPILE_DEPS ${INTERCEPTION_TEST_HEADERS}
-DEPS llvm_gtest
+DEPS gtest
 CFLAGS ${INTERCEPTION_TEST_CFLAGS_COMMON}
 LINK_FLAGS ${INTERCEP

[Lldb-commits] [lldb] daba823 - Refine error msgs from CommandObject & Disassemble

2022-03-02 Thread Jason Molenda via lldb-commits


Author: Jason Molenda
Date: 2022-03-02T11:17:48-08:00
New Revision: daba82362228b4aa460c26079c028ebf832066fd

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

LOG: Refine error msgs from CommandObject & Disassemble

Make it clearer for end users why a command cannot be used
when a process is not stopped, etc.

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandObject.h
lldb/source/Commands/CommandObjectDisassemble.cpp
lldb/test/Shell/Commands/command-disassemble.s

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandObject.h 
b/lldb/include/lldb/Interpreter/CommandObject.h
index ce741296dce75..45fc47b02c046 100644
--- a/lldb/include/lldb/Interpreter/CommandObject.h
+++ b/lldb/include/lldb/Interpreter/CommandObject.h
@@ -326,15 +326,20 @@ class CommandObject {
   }
 
   virtual const char *GetInvalidProcessDescription() {
-return "invalid process";
+return "Command requires a current process.";
   }
 
-  virtual const char *GetInvalidThreadDescription() { return "invalid thread"; 
}
+  virtual const char *GetInvalidThreadDescription() {
+return "Command requires a process which is currently stopped.";
+  }
 
-  virtual const char *GetInvalidFrameDescription() { return "invalid frame"; }
+  virtual const char *GetInvalidFrameDescription() {
+return "Command requires a process, which is currently stopped.";
+  }
 
   virtual const char *GetInvalidRegContextDescription() {
-return "invalid frame, no registers";
+return "invalid frame, no registers, command requires a process which is "
+   "currently stopped.";
   }
 
   // This is for use in the command interpreter, when you either want the

diff  --git a/lldb/source/Commands/CommandObjectDisassemble.cpp 
b/lldb/source/Commands/CommandObjectDisassemble.cpp
index e3c40ed73cf63..b02e071e7ac4f 100644
--- a/lldb/source/Commands/CommandObjectDisassemble.cpp
+++ b/lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -278,11 +278,20 @@ CommandObjectDisassemble::GetContainingAddressRanges() {
 
 llvm::Expected>
 CommandObjectDisassemble::GetCurrentFunctionRanges() {
+  Process *process = m_exe_ctx.GetProcessPtr();
   StackFrame *frame = m_exe_ctx.GetFramePtr();
   if (!frame) {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Cannot disassemble around the current "
-   "function without a selected frame.\n");
+if (process) {
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Cannot disassemble around the current "
+  "function without the process being stopped.\n");
+} else {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Cannot disassemble around the current "
+ "function without a selected frame: "
+ "no currently running process.\n");
+}
   }
   SymbolContext sc(
   frame->GetSymbolContext(eSymbolContextFunction | eSymbolContextSymbol));
@@ -301,11 +310,20 @@ CommandObjectDisassemble::GetCurrentFunctionRanges() {
 
 llvm::Expected>
 CommandObjectDisassemble::GetCurrentLineRanges() {
+  Process *process = m_exe_ctx.GetProcessPtr();
   StackFrame *frame = m_exe_ctx.GetFramePtr();
   if (!frame) {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Cannot disassemble around the current "
-   "line without a selected frame.\n");
+if (process) {
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Cannot disassemble around the current "
+  "function without the process being stopped.\n");
+} else {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Cannot disassemble around the current "
+ "line without a selected frame: "
+ "no currently running process.\n");
+}
   }
 
   LineEntry pc_line_entry(
@@ -361,11 +379,20 @@ 
CommandObjectDisassemble::GetNameRanges(CommandReturnObject &result) {
 
 llvm::Expected>
 CommandObjectDisassemble::GetPCRanges() {
+  Process *process = m_exe_ctx.GetProcessPtr();
   StackFrame *frame = m_exe_ctx.GetFramePtr();
   if (!frame) {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Cannot disassemble around the current "
-   "PC without a selected frame.\n");
+if (process) {
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(

[Lldb-commits] [PATCH] D120594: Improve error messages for command that need a stopped process

2022-03-02 Thread Jason Molenda via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdaba82362228: Refine error msgs from CommandObject & 
Disassemble (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120594

Files:
  lldb/include/lldb/Interpreter/CommandObject.h
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/test/Shell/Commands/command-disassemble.s

Index: lldb/test/Shell/Commands/command-disassemble.s
===
--- lldb/test/Shell/Commands/command-disassemble.s
+++ lldb/test/Shell/Commands/command-disassemble.s
@@ -6,13 +6,13 @@
 # RUN:   -s %S/Inputs/command-disassemble.lldbinit -o exit 2>&1 | FileCheck %s
 
 # CHECK:  (lldb) disassemble
-# CHECK-NEXT: error: Cannot disassemble around the current function without a selected frame.
+# CHECK-NEXT: error: Cannot disassemble around the current function without a selected frame: no currently running process.
 # CHECK-NEXT: (lldb) disassemble --line
-# CHECK-NEXT: error: Cannot disassemble around the current line without a selected frame.
+# CHECK-NEXT: error: Cannot disassemble around the current line without a selected frame: no currently running process.
 # CHECK-NEXT: (lldb) disassemble --frame
-# CHECK-NEXT: error: Cannot disassemble around the current function without a selected frame.
+# CHECK-NEXT: error: Cannot disassemble around the current function without a selected frame: no currently running process.
 # CHECK-NEXT: (lldb) disassemble --pc
-# CHECK-NEXT: error: Cannot disassemble around the current PC without a selected frame.
+# CHECK-NEXT: error: Cannot disassemble around the current PC without a selected frame: no currently running process.
 # CHECK-NEXT: (lldb) disassemble --start-address 0x0
 # CHECK-NEXT: command-disassemble.s.tmp`foo:
 # CHECK-NEXT: command-disassemble.s.tmp[0x0] <+0>:   int$0x10
Index: lldb/source/Commands/CommandObjectDisassemble.cpp
===
--- lldb/source/Commands/CommandObjectDisassemble.cpp
+++ lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -278,11 +278,20 @@
 
 llvm::Expected>
 CommandObjectDisassemble::GetCurrentFunctionRanges() {
+  Process *process = m_exe_ctx.GetProcessPtr();
   StackFrame *frame = m_exe_ctx.GetFramePtr();
   if (!frame) {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Cannot disassemble around the current "
-   "function without a selected frame.\n");
+if (process) {
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Cannot disassemble around the current "
+  "function without the process being stopped.\n");
+} else {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Cannot disassemble around the current "
+ "function without a selected frame: "
+ "no currently running process.\n");
+}
   }
   SymbolContext sc(
   frame->GetSymbolContext(eSymbolContextFunction | eSymbolContextSymbol));
@@ -301,11 +310,20 @@
 
 llvm::Expected>
 CommandObjectDisassemble::GetCurrentLineRanges() {
+  Process *process = m_exe_ctx.GetProcessPtr();
   StackFrame *frame = m_exe_ctx.GetFramePtr();
   if (!frame) {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Cannot disassemble around the current "
-   "line without a selected frame.\n");
+if (process) {
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Cannot disassemble around the current "
+  "function without the process being stopped.\n");
+} else {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Cannot disassemble around the current "
+ "line without a selected frame: "
+ "no currently running process.\n");
+}
   }
 
   LineEntry pc_line_entry(
@@ -361,11 +379,20 @@
 
 llvm::Expected>
 CommandObjectDisassemble::GetPCRanges() {
+  Process *process = m_exe_ctx.GetProcessPtr();
   StackFrame *frame = m_exe_ctx.GetFramePtr();
   if (!frame) {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Cannot disassemble around the current "
-   "PC without a selected frame.\n");
+if (process) {
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Cannot disassemble around the current "
+  "function without the process being stopped.\n");
+} else {
+  return llvm::createStringError(llvm::inconvertibleErrorCode

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I think it's reasonable to be able to refer to the dwarf constants from within 
the dwarf plugin via their base names alone. The implementation -- a top-level 
`using namespace llvm::dwarf` -- is not reasonable, but that's because the 
plugin is very old, and completely unnamespaced.

For the newer plugins, we've started putting them in a sub-namespace of 
lldb_private, which means they cannot be accidentally referenced from the core 
code, but they can easily reference (without qualification) symbols defined in 
the core libraries.

If we put the dwarf plugin into a (e.g.) `lldb_private::dwarf` namespace, then 
I think it would be ok to put a `using namespace llvm::dwarf` into that 
namespace, even in a header -- because those symbols would only be visible to 
symbols which are in that namespace.

In other words, I'd have the dwarf plugin do what the minidump plugin is doing 
(disclaimer: I wrote most of that plugin).

Anyway, I'm not married to that approach, but that's what I would do...


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

https://reviews.llvm.org/D120836

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

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Pavel Labath via Phabricator via lldb-commits

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

Looks good. Thanks for the test.




Comment at: lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py:16
+self.build()
+self.launch(executable=self.getBuildArtifact("a.out"), 
dimensions=(100,500))
+self.child.sendline("run")

The default should be fine here.


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

https://reviews.llvm.org/D120762

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

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

2022-03-02 Thread Michał Górny via Phabricator via lldb-commits

mgorny added inline comments.
Herald added a project: All.



Comment at: lldb/packages/Python/lldbsuite/test/lldbpexpect.py:27
+def launch(self, executable=None, extra_args=None, timeout=60,
+dimensions=None, run_under=None, post_spawn=None):
 logfile = getattr(sys.stdout, 'buffer',

I think we should follow PEP8 for Python code, i.e. align the first param with 
the first param from line above.



Comment at: lldb/packages/Python/lldbsuite/test/lldbpexpect.py:32
+args = []
+if run_under:
+args += run_under





Comment at: lldb/packages/Python/lldbsuite/test/lldbpexpect.py:51
+
+if post_spawn:
+post_spawn()





Comment at: lldb/test/API/driver/job_control/shell.py:9
+import os
+
+def preexec_fn():





Comment at: lldb/test/API/driver/job_control/shell.py:23
+signal.pthread_sigmask(signal.SIG_SETMASK, orig_mask)
+
+if __name__ == "__main__":




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120320

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

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Thorsten via Phabricator via lldb-commits

tschuett added a comment.

I found this:
https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions


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

https://reviews.llvm.org/D120836

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

[Lldb-commits] [PATCH] D119146: [lldb/Platform] Prepare decouple instance and plugin names

2022-03-02 Thread Stella Stamenova via Phabricator via lldb-commits

stella.stamenova added a comment.

It looks like this broke the windows lldb bot:

https://lab.llvm.org/buildbot/#/builders/83/builds/15999


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119146

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

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a comment.

In D120836#3355167 , @labath wrote:

> I think it's reasonable to be able to refer to the dwarf constants from 
> within the dwarf plugin via their base names alone. The implementation -- a 
> top-level `using namespace llvm::dwarf` -- is not reasonable, but that's 
> because the plugin is very old, and completely unnamespaced.
>
> For the newer plugins, we've started putting them in a sub-namespace of 
> lldb_private, which means they cannot be accidentally referenced from the 
> core code, but they can easily reference (without qualification) symbols 
> defined in the core libraries.
>
> If we put the dwarf plugin into a (e.g.) `lldb_private::dwarf` namespace, 
> then I think it would be ok to put a `using namespace llvm::dwarf` into that 
> namespace, even in a header -- because those symbols would only be visible to 
> symbols which are in that namespace.
>
> In other words, I'd have the dwarf plugin do what the minidump plugin is 
> doing (disclaimer: I wrote most of that plugin).
>
> Anyway, I'm not married to that approach, but that's what I would do...

If I understand correctly you are proposing that I do:

  namespace lldb_private {
  namespace dwarf {
using namespace llvm::dwarf;
  }
  }

in `lldb/include/lldb/Core/dwarf.h` and then in the `.cpp` files:

  using namespace lldb_private;
  using namespace dwarf;

That sounds reasonable to me.

Then I would revert the changes to the minidump files, since they are already 
using this strategy.


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

https://reviews.llvm.org/D120836

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

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D120836#3355245 , @shafik wrote:

> In D120836#3355167 , @labath wrote:
>
>> I think it's reasonable to be able to refer to the dwarf constants from 
>> within the dwarf plugin via their base names alone. The implementation -- a 
>> top-level `using namespace llvm::dwarf` -- is not reasonable, but that's 
>> because the plugin is very old, and completely unnamespaced.
>>
>> For the newer plugins, we've started putting them in a sub-namespace of 
>> lldb_private, which means they cannot be accidentally referenced from the 
>> core code, but they can easily reference (without qualification) symbols 
>> defined in the core libraries.
>>
>> If we put the dwarf plugin into a (e.g.) `lldb_private::dwarf` namespace, 
>> then I think it would be ok to put a `using namespace llvm::dwarf` into that 
>> namespace, even in a header -- because those symbols would only be visible 
>> to symbols which are in that namespace.
>>
>> In other words, I'd have the dwarf plugin do what the minidump plugin is 
>> doing (disclaimer: I wrote most of that plugin).
>>
>> Anyway, I'm not married to that approach, but that's what I would do...
>
> If I understand correctly you are proposing that I do:
>
>   namespace lldb_private {
>   namespace dwarf {
> using namespace llvm::dwarf;
>   }
>   }
>
> in `lldb/include/lldb/Core/dwarf.h` and then in the `.cpp` files:
>
>   using namespace lldb_private;
>   using namespace dwarf;

Yes, that's the general idea. I'm not entirely sure how to apply that to the 
DWARFExpression class, since it is not really a part of the dwarf plugin. But 
all of the uses of dwarf constants are in the cpp file, so I suppose a `using 
namespace llvm::dwarf` in DWARFExpression.cpp file should suffice.


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

https://reviews.llvm.org/D120836

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

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

How is using m_mutex better than just using the std::atomic? Just 
protecting the modification of the value with a mutex doesn't seem like it 
would do much more than the atomic already did unless we are using the lock the 
protect the value for longer that just the modification.




Comment at: lldb/source/Target/Process.cpp:4384
   void Cancel() override {
+std::lock_guard guard(m_mutex);
 SetIsDone(true);

Won't this cause a deadlock? You aren't using a recursive mutex here and you 
are locking the "m_mutex" and then calling SetIsDone(true) which will try and 
lock the mutex as well?



Comment at: lldb/source/Target/Process.cpp:4441
   Pipe m_pipe;
-  std::atomic m_is_running{false};
+  std::mutex m_mutex;
+  bool m_is_running = false;

Does this need to be a recursive mutex? We have the same thread locking this 
mutex multiple times it seems from the code?


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

https://reviews.llvm.org/D120762

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

[Lldb-commits] [lldb] 4977da2 - [lldb] Explicitly declare the default constructor in PlatformAndroidRemoteGDBServer

2022-03-02 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2022-03-02T22:35:54+01:00
New Revision: 4977da2c555b0a8d49be68bad4c54b816d7cc908

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

LOG: [lldb] Explicitly declare the default constructor in 
PlatformAndroidRemoteGDBServer

MSVC does not seem to like the inheriting constructor.

Added: 


Modified: 
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h 
b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
index 675cdc3d74e94..0b3e4c65c4b2c 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
@@ -24,7 +24,7 @@ namespace platform_android {
 class PlatformAndroidRemoteGDBServer
 : public platform_gdb_server::PlatformRemoteGDBServer {
 public:
-  using PlatformRemoteGDBServer::PlatformRemoteGDBServer;
+  PlatformAndroidRemoteGDBServer() = default;
 
   ~PlatformAndroidRemoteGDBServer() override;
 



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

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

2022-03-02 Thread Zequan Wu via Phabricator via lldb-commits

zequanwu updated this revision to Diff 412532.
zequanwu marked 3 inline comments as done.
zequanwu added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119963

Files:
  lldb/include/lldb/Core/Address.h
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/include/lldb/Symbol/Variable.h
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Address.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_loclists_base.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc_and_loclists.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwo.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwp.s
  lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
  lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test

Index: lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
+++ lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
@@ -14,35 +14,35 @@
 # at the inlined function entry.
 image lookup -v -s break_at_inlined_f_in_main
 # CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main
-# CHECK: name = "unused1", type = "void *", location = 
-# CHECK: name = "used", type = "int", location = DW_OP_consts +42
-# CHECK: name = "unused2", type = "int", location = 
-# CHECK: name = "partial", type = "int", location = DW_OP_reg4 RSI
-# CHECK: name = "unused3", type = "int", location = 
+# CHECK: name = "unused1", type = "void *", valid ranges = , location = 
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0011, 0x0014) -> DW_OP_consts +42
+# CHECK: name = "unused2", type = "int", valid ranges = , location = 
+# CHECK: name = "partial", type = "int", valid ranges = , location = [0x0011, 0x0019) -> DW_OP_reg4 RSI
+# CHECK: name = "unused3", type = "int", valid ranges = , location = 
 
 # Show variables outsid of the live range of the 'partial' parameter
 # and verify that the output is as expected.
 image lookup -v -s break_at_inlined_f_in_main_between_printfs
 # CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main_between_printfs
-# CHECK: name = "unused1", type = "void *", location = 
-# CHECK: name = "used", type = "int", location = DW_OP_reg3 RBX
-# CHECK: name = "unused2", type = "int", location = 
+# CHECK: name = "unused1", type = "void *", valid ranges = , location = 
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0014, 0x001e) -> DW_OP_reg3 RBX
+# CHECK: name = "unused2", type = "int", valid ranges = , location = 
 # Note: image lookup does not show variables outside of their
 #   location, so |partial| is missing here.
 # CHECK-NOT: partial
-# CHECK: name = "unused3", type = "int", location = 
+# CHECK: name = "unused3", type = "int", valid ranges = , location = 
 
 # Check that we show parameters even if all of them are compiled away.
 image lookup -v -s  break_at_inlined_g_in_main
 # CHECK-LABEL: image lookup -v -s  break_at_inlined_g_in_main
-# CHECK: name = "unused", type = "int", location = 
+# CHECK: name = "unused", type = "int", valid ranges = , location = 
 
 # Check that even the other inlined instance of f displays the correct
 # parameters.
 image lookup -v -s  break_at_inlined_f_in_other
 # CHECK-LABEL: image lookup -v -s  break_at_inlined_f_in_other
-# CHECK: name = "unused1", type = "void *", location = 
-# CHECK: name = "used", type = "int", location = DW_OP_consts +1
-# CHECK: name = "unused2", type = "int", location = 
-# CHECK: name = "partial", type = "int", location =  DW_OP_consts +2
-# CHECK: name = "unused3", type = "int", location = 
+# CHECK: name = "unused1", type = "void *", valid ranges = , location = 
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0001, 0x000b) -> DW_OP_consts +1
+# CHECK: name = "unused2", type = "int", valid ranges = , location = 
+# CHECK: name = "partial", type = "int", valid ranges = , location = [0x0001, 0x0006) -> DW_OP_consts +2
+# CHECK: name = "unused3", type = "int", valid ranges = , location = 
Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
@@ -12,7 +12,7 @@
 # CHECK-LABEL: image lookup -v -n F1
 # CHECK: CompileUnit: id = {0x0001}, file = "1.c", language = ""
 # CHECK: Function: {{.*}}, name = "F1", range = [0x0001-0x0002)
-# CHECK: Variable: {{.*}}, name = "x", type = "i

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

2022-03-02 Thread Zequan Wu via Phabricator via lldb-commits

zequanwu added inline comments.



Comment at: lldb/source/Commands/Options.td:962
+"\\x01">, GroupRange<1, 6>, Desc<"Dump valid ranges of variables (must be "
+"used in conjunction with --verbose">;
   def target_modules_lookup_verbose : Option<"verbose", "v">,

jingham wrote:
> I missed where you return an error if somebody specifies show-variable-ranges 
> == true w/o specifying verbose.  Did I miss that?
If `show-variable-ranges` is given but not `-v`, then `show-variable-ranges` 
has no effect. I don't know if there is way to make it gives an error in that 
case.
I tried to make `show-variable-ranges` group 7 and `verbose` GroupRange <1,7>, 
then `-v` and `show-variable-ranges` combination is invalid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119963

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

[Lldb-commits] [lldb] 15983c2 - [LLDB] Dump valid ranges of variables

2022-03-02 Thread Zequan Wu via lldb-commits


Author: Zequan Wu
Date: 2022-03-02T13:44:19-08:00
New Revision: 15983c28aa819031e08a2b3fe49d02c41839b22c

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

LOG: [LLDB] Dump valid ranges of variables

This allows `image lookup -a ... -v` to print variables only if the given
address is covered by the valid ranges of the variables. Since variables created
in dwarf plugin always has empty scope range, print the variable if it has
empty scope.

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

Added: 


Modified: 
lldb/include/lldb/Core/Address.h
lldb/include/lldb/Expression/DWARFExpression.h
lldb/include/lldb/Symbol/Variable.h
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/Options.td
lldb/source/Core/Address.cpp
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Symbol/Variable.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_loclists_base.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc_and_loclists.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwo.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwp.s
lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test

Removed: 




diff  --git a/lldb/include/lldb/Core/Address.h 
b/lldb/include/lldb/Core/Address.h
index f77ffc2fd25e6..4121f6c07ae2b 100644
--- a/lldb/include/lldb/Core/Address.h
+++ b/lldb/include/lldb/Core/Address.h
@@ -229,6 +229,14 @@ class Address {
   /// \param[in] fallback_style
   /// The display style for the address.
   ///
+  /// \param[in] addr_byte_size
+  /// The address byte size for the address.
+  ///
+  /// \param[in] all_ranges
+  /// If true, dump all valid ranges and value ranges for the variable that
+  /// contains the address, otherwise dumping the range that contains the
+  /// address.
+  ///
   /// \return
   /// Returns \b true if the address was able to be displayed.
   /// File and load addresses may be unresolved and it may not be
@@ -238,7 +246,8 @@ class Address {
   /// \see Address::DumpStyle
   bool Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style,
 DumpStyle fallback_style = DumpStyleInvalid,
-uint32_t addr_byte_size = UINT32_MAX) const;
+uint32_t addr_byte_size = UINT32_MAX,
+bool all_ranges = false) const;
 
   AddressClass GetAddressClass() const;
 

diff  --git a/lldb/include/lldb/Expression/DWARFExpression.h 
b/lldb/include/lldb/Expression/DWARFExpression.h
index 1490ac2d614a8..96a0e8e02da13 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -15,6 +15,7 @@
 #include "lldb/Utility/Scalar.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private.h"
+#include "llvm/DebugInfo/DWARF/DWARFLocationExpression.h"
 #include 
 
 class DWARFUnit;
@@ -55,18 +56,10 @@ class DWARFExpression {
   /// \param[in] level
   /// The level of verbosity to use.
   ///
-  /// \param[in] location_list_base_addr
-  /// If this is a location list based expression, this is the
-  /// address of the object that owns it. NOTE: this value is
-  /// 
diff erent from the DWARF version of the location list base
-  /// address which is compile unit relative. This base address
-  /// is the address of the object that owns the location list.
-  ///
   /// \param[in] abi
   /// An optional ABI plug-in that can be used to resolve register
   /// names.
-  void GetDescription(Stream *s, lldb::DescriptionLevel level,
-  lldb::addr_t location_list_base_addr, ABI *abi) const;
+  void GetDescription(Stream *s, lldb::DescriptionLevel level, ABI *abi) const;
 
   /// Return true if the location expression contains data
   bool IsValid() const;
@@ -217,6 +210,13 @@ class DWARFExpression {
   lldb::addr_t func_load_addr, lldb::addr_t 
address,
   ABI *abi);
 
+  bool DumpLocations(Stream *s, lldb::DescriptionLevel level,
+ lldb::addr_t func_load_addr, lldb::addr_t addr, ABI *abi);
+
+  bool GetLocationExpressions(
+  lldb::addr_t load_function_start,
+  llvm::function_ref callback) const;
+
   bool MatchesOperand(StackFrame &frame, const Instruction::Operand &op);
 
   llvm::Optional

diff  --git a/lldb/include/lldb/Symbol/Variable.h 
b/lldb/include/lldb/Symbol/Variable.h
index ccd5072c3d234..88a975df39928 100644
--- a/lldb/include/lldb/Symbol/Variable.h
+++ b/lldb/include/lldb/Symbol/Variable.h
@@ -77,7 +77,9 @@ class Variable : public UserID, public 
std::enable_shared_from_this {

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

2022-03-02 Thread Zequan Wu via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG15983c28aa81: [LLDB] Dump valid ranges of variables 
(authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119963

Files:
  lldb/include/lldb/Core/Address.h
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/include/lldb/Symbol/Variable.h
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Address.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_loclists_base.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc_and_loclists.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwo.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwp.s
  lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
  lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test

Index: lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
+++ lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
@@ -14,35 +14,35 @@
 # at the inlined function entry.
 image lookup -v -s break_at_inlined_f_in_main
 # CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main
-# CHECK: name = "unused1", type = "void *", location = 
-# CHECK: name = "used", type = "int", location = DW_OP_consts +42
-# CHECK: name = "unused2", type = "int", location = 
-# CHECK: name = "partial", type = "int", location = DW_OP_reg4 RSI
-# CHECK: name = "unused3", type = "int", location = 
+# CHECK: name = "unused1", type = "void *", valid ranges = , location = 
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0011, 0x0014) -> DW_OP_consts +42
+# CHECK: name = "unused2", type = "int", valid ranges = , location = 
+# CHECK: name = "partial", type = "int", valid ranges = , location = [0x0011, 0x0019) -> DW_OP_reg4 RSI
+# CHECK: name = "unused3", type = "int", valid ranges = , location = 
 
 # Show variables outsid of the live range of the 'partial' parameter
 # and verify that the output is as expected.
 image lookup -v -s break_at_inlined_f_in_main_between_printfs
 # CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main_between_printfs
-# CHECK: name = "unused1", type = "void *", location = 
-# CHECK: name = "used", type = "int", location = DW_OP_reg3 RBX
-# CHECK: name = "unused2", type = "int", location = 
+# CHECK: name = "unused1", type = "void *", valid ranges = , location = 
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0014, 0x001e) -> DW_OP_reg3 RBX
+# CHECK: name = "unused2", type = "int", valid ranges = , location = 
 # Note: image lookup does not show variables outside of their
 #   location, so |partial| is missing here.
 # CHECK-NOT: partial
-# CHECK: name = "unused3", type = "int", location = 
+# CHECK: name = "unused3", type = "int", valid ranges = , location = 
 
 # Check that we show parameters even if all of them are compiled away.
 image lookup -v -s  break_at_inlined_g_in_main
 # CHECK-LABEL: image lookup -v -s  break_at_inlined_g_in_main
-# CHECK: name = "unused", type = "int", location = 
+# CHECK: name = "unused", type = "int", valid ranges = , location = 
 
 # Check that even the other inlined instance of f displays the correct
 # parameters.
 image lookup -v -s  break_at_inlined_f_in_other
 # CHECK-LABEL: image lookup -v -s  break_at_inlined_f_in_other
-# CHECK: name = "unused1", type = "void *", location = 
-# CHECK: name = "used", type = "int", location = DW_OP_consts +1
-# CHECK: name = "unused2", type = "int", location = 
-# CHECK: name = "partial", type = "int", location =  DW_OP_consts +2
-# CHECK: name = "unused3", type = "int", location = 
+# CHECK: name = "unused1", type = "void *", valid ranges = , location = 
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0001, 0x000b) -> DW_OP_consts +1
+# CHECK: name = "unused2", type = "int", valid ranges = , location = 
+# CHECK: name = "partial", type = "int", valid ranges = , location = [0x0001, 0x0006) -> DW_OP_consts +2
+# CHECK: name = "unused3", type = "int", valid ranges = , location = 
Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
@@ -12,7 +12,7 @@
 # CHECK-LABEL: image lookup -v -n F1
 # CHECK: CompileUnit: id = {0x0001}, file = "1.c", language = ""
 # CHECK: Function: {{.*}}, name = "F1", range = [0x0001-0x0002)
-# CHECK: Vari

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

2022-03-02 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

In D119963#3355501 , @zequanwu wrote:

> Address comments

This sort of thing is hard to check in the option groups w/o blowing out the 
number of groups you need to have.

The way to check after all the option parsing is done that the options are 
coherent is to have the CommandOptions implement OptionParsingFinished, check 
the if the all_ranges is set verbose is also set, and if it isn't return a 
suitable error.

Can you do this (even though I see you've checked it in already.)  Silently 
having some combination of options not work isn't a good thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119963

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

[Lldb-commits] [PATCH] D120755: Fix race condition when launching and attaching.This is a modified version of a previous patch that was reverted: https://reviews.llvm.org/D119797This version only wait

2022-03-02 Thread Greg Clayton via Phabricator via lldb-commits

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

Fixes:

- use a "configuration_done_sent" boolean to track when the configurationDone 
packet has already been sent and stop all process events from being sent before 
this happens. This allows "launchCommands" and "attachCommands" to have more 
than one stop if desired before we sync up and make sure the process is stopped 
after running these commands
- fixed comments to reflect the fact that we are only synchronizing with the 
process if "attachCommands" or "launchCommands" are used
- use steady clock


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120755

Files:
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -215,7 +215,7 @@
 			},
 			"launchCommands": {
 "type": "array",
-"description": "Custom commands that are executed instead of launching a process. A target will be created with the launch arguments prior to executing these commands. The commands may optionally create a new target and must perform a launch. A valid process must exist after these commands complete or the \"launch\" will fail.",
+"description": "Custom commands that are executed instead of launching a process. A target will be created with the launch arguments prior to executing these commands. The commands may optionally create a new target and must perform a launch. A valid process must exist after these commands complete or the \"launch\" will fail. Launch the process with \"process launch -s\" to make the process to at the entry point since lldb-vscode will auto resume if necessary.",
 "default": []
 			},
 			"stopCommands": {
@@ -232,6 +232,10 @@
 "type": "boolean",
 "description": "Launch the program inside an integrated terminal in the IDE. Useful for debugging interactive command line programs",
 "default": false
+			},
+			"timeout": {
+"type": "string",
+"description": "The time in seconds to wait for a program to stop at entry point when launching with \"launchCommands\". Defaults to 30 seconds."
 			}
 		}
 	},
@@ -307,6 +311,10 @@
 			"coreFile": {
 "type": "string",
 "description": "Path to the core file to debug."
+			},
+			"timeout": {
+"type": "string",
+"description": "The time in seconds to wait for a program to stop when attaching using \"attachCommands\". Defaults to 30 seconds."
 			}
 		}
 	}
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -449,10 +449,18 @@
   case lldb::eStateSuspended:
 break;
   case lldb::eStateStopped:
-// Only report a stopped event if the process was not restarted.
-if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
-  SendStdOutStdErr(process);
-  SendThreadStoppedEvent();
+// We launch and attach in synchronous mode then the first stop
+// event will not be delivered. If we use "launchCommands" during a
+// launch or "attachCommands" during an attach we might some process
+// stop events which we do not want to send an event for. We will
+// manually send a stopped event in request_configurationDone(...)
+// so don't send any before then.
+if (g_vsc.configuration_done_sent) {
+  // Only report a stopped event if the process was not restarted.
+  if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
+SendStdOutStdErr(process);
+SendThreadStoppedEvent();
+  }
 }
 break;
   case lldb::eStateRunning:
@@ -600,6 +608,7 @@
   g_vsc.terminate_commands = GetStrings(arguments, "terminateCommands");
   auto attachCommands = GetStrings(arguments, "attachCommands");
   llvm::StringRef core_file = GetString(arguments, "coreFile");
+  const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
   g_vsc.stop_at_entry =
   core_file.empty() ? GetBoolean(arguments, "stopOnEntry", false) : true;
   std::vector postRunCommands =
@@ -657,6 +666,10 @@
 // The custom commands might have created a new target so we should use the
 // selected target after these commands are run.
 g_vsc.target = g_vsc.debugger.GetSelectedTarget();
+
+// Make sure the process is attached and stopped before proceed

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

I think you might be looking at a combination of the old and the new patch. The 
new mutex protects the whole `Cancel` and `SetIsRunning`. I don't think this 
needs to be a recursive mutex because these functions are not calling each 
other. They both indirectly protect access to `SetIsDone`.


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

https://reviews.llvm.org/D120762

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

[Lldb-commits] [lldb] 302d717 - [LLDB] Add error message when using --show-variable-ranges without -v

2022-03-02 Thread Zequan Wu via lldb-commits


Author: Zequan Wu
Date: 2022-03-02T14:20:14-08:00
New Revision: 302d7179e101ebbf48c386a632828a1150a18769

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

LOG: [LLDB] Add error message when using --show-variable-ranges without -v

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 3af2c73b113cb..39f3af7f6df26 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3626,6 +3626,15 @@ class CommandObjectTargetModulesLookup : public 
CommandObjectParsed {
   m_print_all = false;
 }
 
+Status OptionParsingFinished(ExecutionContext *execution_context) override 
{
+  Status status;
+  if (m_all_ranges && !m_verbose) {
+status.SetErrorString("--show-variable-ranges must be used in "
+  "conjunction with --verbose.");
+  }
+  return status;
+}
+
 llvm::ArrayRef GetDefinitions() override {
   return llvm::makeArrayRef(g_target_modules_lookup_options);
 }



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

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

2022-03-02 Thread Zequan Wu via Phabricator via lldb-commits

zequanwu added a comment.

In D119963#3355563 , @jingham wrote:

> In D119963#3355501 , @zequanwu 
> wrote:
>
>> Address comments
>
> This sort of thing is hard to check in the option groups w/o blowing out the 
> number of groups you need to have.
>
> The way to check after all the option parsing is done that the options are 
> coherent is to have the CommandOptions implement OptionParsingFinished, check 
> the if the all_ranges is set verbose is also set, and if it isn't return a 
> suitable error.
>
> Can you do this (even though I see you've checked it in already.)  Silently 
> having some combination of options not work isn't a good thing.

Thanks. I added it at 
https://reviews.llvm.org/rG302d7179e101ebbf48c386a632828a1150a18769.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119963

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

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik updated this revision to Diff 412548.
shafik added a comment.

Updating diff based on comments


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

https://reviews.llvm.org/D120836

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Symbol/DWARFCallFrameInfo.cpp
  lldb/source/Symbol/PostfixExpression.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
  lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -39,6 +39,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace dwarf;
 
 class SymbolFileDWARFTests : public testing::Test {
   SubsystemRAII Evaluate(llvm::ArrayRef expr,
lldb::ModuleSP module_sp = {},
Index: lldb/source/Symbol/PostfixExpression.cpp
===
--- lldb/source/Symbol/PostfixExpression.cpp
+++ lldb/source/Symbol/PostfixExpression.cpp
@@ -18,6 +18,7 @@
 
 using namespace lldb_private;
 using namespace lldb_private::postfix;
+using namespace llvm::dwarf;
 
 static llvm::Optional
 GetBinaryOpType(llvm::StringRef token) {
Index: lldb/source/Symbol/DWARFCallFrameInfo.cpp
===
--- lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -24,6 +24,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace llvm::dwarf;
 
 // GetDwarfEHPtr
 //
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -80,6 +80,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace dwarf;
 using namespace clang;
 using llvm::StringSwitch;
 
Index: lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -25,6 +25,7 @@
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::npdb;
+using namespace lldb_private::dwarf;
 using namespace llvm::pdb;
 
 static std::unique_ptr
Index: lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -10,6 +10,8 @@
 
 #include "lldb/Core/Declaration.h"
 
+using namespace lldb_private::dwarf;
+
 bool UniqueDWARFASTTypeList::Find(const DWARFDIE &die,
   const lldb_private::Declaration &decl,
   const int32_t byte_size,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -96,6 +96,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace dwarf;
 
 LLDB_PLUGIN_DEFINE(SymbolFileDWARF)
 
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/Symbol

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

2022-03-02 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

Excellent, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119963

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

[Lldb-commits] [lldb] 37eb15a - [lldb] Devirtualize IOHandler::{IsActive, SetIsDone, GetIsDone} (NFC)

2022-03-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-03-02T15:55:49-08:00
New Revision: 37eb15ad7ab25e0135b759f3ecdd6ada5ce82330

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

LOG: [lldb] Devirtualize IOHandler::{IsActive,SetIsDone,GetIsDone} (NFC)

There are no implementations overriding these methods.

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

Added: 


Modified: 
lldb/include/lldb/Core/IOHandler.h

Removed: 




diff  --git a/lldb/include/lldb/Core/IOHandler.h 
b/lldb/include/lldb/Core/IOHandler.h
index 7011dd1e8e047..c717d1d872564 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -85,11 +85,11 @@ class IOHandler {
 
   virtual void GotEOF() = 0;
 
-  virtual bool IsActive() { return m_active && !m_done; }
+  bool IsActive() { return m_active && !m_done; }
 
-  virtual void SetIsDone(bool b) { m_done = b; }
+  void SetIsDone(bool b) { m_done = b; }
 
-  virtual bool GetIsDone() { return m_done; }
+  bool GetIsDone() { return m_done; }
 
   Type GetType() const { return m_type; }
 



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

[Lldb-commits] [lldb] 1022276 - [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-03-02T15:55:49-08:00
New Revision: 10222764a9a3b09678013d3d13b66790ea3298d9

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

LOG: [lldb] Avoid data race in IOHandlerProcessSTDIO

This patch fixes a data race in IOHandlerProcessSTDIO. The race is
happens between the main thread and the event handling thread. The main
thread is running the IOHandler (IOHandlerProcessSTDIO::Run()) when an
event comes in that makes us pop the process IO handler which involves
cancelling the IOHandler (IOHandlerProcessSTDIO::Cancel). The latter
calls SetIsDone(true) which modifies m_is_done. At the same time, we
have the main thread reading the variable through GetIsDone().

This patch avoids the race by using a mutex to synchronize the two
threads. On the event thread, in IOHandlerProcessSTDIO ::Cancel method,
we obtain the lock before changing the value of m_is_done. On the main
thread, in IOHandlerProcessSTDIO::Run(), we obtain the lock before
reading the value of m_is_done. Additionally, we delay calling SetIsDone
until after the loop exists, to avoid a potential race between the two
writes.

  Write of size 1 at 0x00010b66bb68 by thread T7 (mutexes: write M2862, write 
M718324145051843688):
#0 lldb_private::IOHandler::SetIsDone(bool) IOHandler.h:90 
(liblldb.15.0.0git.dylib:arm64+0x971d84)
#1 IOHandlerProcessSTDIO::Cancel() Process.cpp:4382 
(liblldb.15.0.0git.dylib:arm64+0x5ddfec)
#2 
lldb_private::Debugger::PopIOHandler(std::__1::shared_ptr
 const&) Debugger.cpp:1156 (liblldb.15.0.0git.dylib:arm64+0x3cb2a8)
#3 
lldb_private::Debugger::RemoveIOHandler(std::__1::shared_ptr
 const&) Debugger.cpp:1063 (liblldb.15.0.0git.dylib:arm64+0x3cbd2c)
#4 lldb_private::Process::PopProcessIOHandler() Process.cpp:4487 
(liblldb.15.0.0git.dylib:arm64+0x5c583c)
#5 
lldb_private::Debugger::HandleProcessEvent(std::__1::shared_ptr
 const&) Debugger.cpp:1549 (liblldb.15.0.0git.dylib:arm64+0x3ceabc)
#6 lldb_private::Debugger::DefaultEventHandler() Debugger.cpp:1622 
(liblldb.15.0.0git.dylib:arm64+0x3cf2c0)
#7 
std::__1::__function::__func, 
void* ()>::operator()() function.h:352 (liblldb.15.0.0git.dylib:arm64+0x3d1bd8)
#8 lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) 
HostNativeThreadBase.cpp:62 (liblldb.15.0.0git.dylib:arm64+0x4c71ac)
#9 lldb_private::HostThreadMacOSX::ThreadCreateTrampoline(void*) 
HostThreadMacOSX.mm:18 (liblldb.15.0.0git.dylib:arm64+0x29ef544)

  Previous read of size 1 at 0x00010b66bb68 by main thread:
#0 lldb_private::IOHandler::GetIsDone() IOHandler.h:92 
(liblldb.15.0.0git.dylib:arm64+0x971db8)
#1 IOHandlerProcessSTDIO::Run() Process.cpp:4339 
(liblldb.15.0.0git.dylib:arm64+0x5ddc7c)
#2 lldb_private::Debugger::RunIOHandlers() Debugger.cpp:982 
(liblldb.15.0.0git.dylib:arm64+0x3cb48c)
#3 
lldb_private::CommandInterpreter::RunCommandInterpreter(lldb_private::CommandInterpreterRunOptions&)
 CommandInterpreter.cpp:3298 (liblldb.15.0.0git.dylib:arm64+0x506478)
#4 lldb::SBDebugger::RunCommandInterpreter(bool, bool) SBDebugger.cpp:1166 
(liblldb.15.0.0git.dylib:arm64+0x53604)
#5 Driver::MainLoop() Driver.cpp:634 (lldb:arm64+0x16294)
#6 main Driver.cpp:853 (lldb:arm64+0x17344)

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

Added: 
lldb/test/API/iohandler/stdio/Makefile
lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
lldb/test/API/iohandler/stdio/main.cpp

Modified: 
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 70717b332b755..3cfda7129e624 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4310,6 +4310,12 @@ class IOHandlerProcessSTDIO : public IOHandler {
 
   ~IOHandlerProcessSTDIO() override = default;
 
+  void SetIsRunning(bool running) {
+std::lock_guard guard(m_mutex);
+SetIsDone(!running);
+m_is_running = running;
+  }
+
   // Each IOHandler gets to run until it is done. It should read data from the
   // "in" and place output into "out" and "err and return when done.
   void Run() override {
@@ -4329,49 +4335,52 @@ class IOHandlerProcessSTDIO : public IOHandler {
 // FD_ZERO, FD_SET are not supported on windows
 #ifndef _WIN32
 const int pipe_read_fd = m_pipe.GetReadFileDescriptor();
-m_is_running = true;
-while (!GetIsDone()) {
+SetIsRunning(true);
+while (true) {
+  {
+std::lock_guard guard(m_mutex);
+if (GetIsDone())
+  break;
+  }
+
   SelectHelper select_helper;
   select_helper.FDSetRead(read_fd);
   select_helper.FDSetRead(pipe_read_fd);
   Status error = select_helper.Select();
 
-  if (error.Fail()) {
-SetIsDone(tr

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG10222764a9a3: [lldb] Avoid data race in 
IOHandlerProcessSTDIO (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D120762?vs=412479&id=412567#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120762

Files:
  lldb/source/Target/Process.cpp
  lldb/test/API/iohandler/stdio/Makefile
  lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
  lldb/test/API/iohandler/stdio/main.cpp

Index: lldb/test/API/iohandler/stdio/main.cpp
===
--- /dev/null
+++ lldb/test/API/iohandler/stdio/main.cpp
@@ -0,0 +1,8 @@
+#include 
+#include 
+
+int main(int argc, char **argv) {
+  for (std::string line; std::getline(std::cin, line);)
+std::cout << "stdout: " << line << '\n';
+  return 0;
+}
Index: lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
===
--- /dev/null
+++ lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
@@ -0,0 +1,30 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestIOHandlerProcessSTDIO(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+def test(self):
+self.build()
+self.launch(executable=self.getBuildArtifact("a.out"))
+self.child.sendline("run")
+
+self.child.send("foo\n")
+self.child.expect_exact("stdout: foo")
+
+self.child.send("bar\n")
+self.child.expect_exact("stdout: bar")
+
+self.child.send("baz\n")
+self.child.expect_exact("stdout: baz")
+
+self.child.sendcontrol('d')
+self.expect_prompt()
+self.quit()
Index: lldb/test/API/iohandler/stdio/Makefile
===
--- /dev/null
+++ lldb/test/API/iohandler/stdio/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4310,6 +4310,12 @@
 
   ~IOHandlerProcessSTDIO() override = default;
 
+  void SetIsRunning(bool running) {
+std::lock_guard guard(m_mutex);
+SetIsDone(!running);
+m_is_running = running;
+  }
+
   // Each IOHandler gets to run until it is done. It should read data from the
   // "in" and place output into "out" and "err and return when done.
   void Run() override {
@@ -4329,49 +4335,52 @@
 // FD_ZERO, FD_SET are not supported on windows
 #ifndef _WIN32
 const int pipe_read_fd = m_pipe.GetReadFileDescriptor();
-m_is_running = true;
-while (!GetIsDone()) {
+SetIsRunning(true);
+while (true) {
+  {
+std::lock_guard guard(m_mutex);
+if (GetIsDone())
+  break;
+  }
+
   SelectHelper select_helper;
   select_helper.FDSetRead(read_fd);
   select_helper.FDSetRead(pipe_read_fd);
   Status error = select_helper.Select();
 
-  if (error.Fail()) {
-SetIsDone(true);
-  } else {
-char ch = 0;
-size_t n;
-if (select_helper.FDIsSetRead(read_fd)) {
-  n = 1;
-  if (m_read_file.Read(&ch, n).Success() && n == 1) {
-if (m_write_file.Write(&ch, n).Fail() || n != 1)
-  SetIsDone(true);
-  } else
-SetIsDone(true);
-}
+  if (error.Fail())
+break;
+
+  char ch = 0;
+  size_t n;
+  if (select_helper.FDIsSetRead(read_fd)) {
+n = 1;
+if (m_read_file.Read(&ch, n).Success() && n == 1) {
+  if (m_write_file.Write(&ch, n).Fail() || n != 1)
+break;
+} else
+  break;
+
 if (select_helper.FDIsSetRead(pipe_read_fd)) {
   size_t bytes_read;
   // Consume the interrupt byte
   Status error = m_pipe.Read(&ch, 1, bytes_read);
   if (error.Success()) {
-switch (ch) {
-case 'q':
-  SetIsDone(true);
+if (ch == 'q')
   break;
-case 'i':
+if (ch == 'i')
   if (StateIsRunningState(m_process->GetState()))
 m_process->SendAsyncInterrupt();
-  break;
-}
   }
 }
   }
 }
-m_is_running = false;
+SetIsRunning(false);
 #endif
   }
 
   void Cancel() override {
+std::lock_guard guard(m_mutex);
 SetIsDone(true);
 // Only write to our pipe to cancel if we are in
 // IOHandlerProcessSTDIO::Run(). We can end u

[Lldb-commits] [PATCH] D120766: [lldb] Devirtualize IOHandler::{IsActive, SetIsDone, GetIsDone} (NFC)

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG37eb15ad7ab2: [lldb] Devirtualize 
IOHandler::{IsActive,SetIsDone,GetIsDone} (NFC) (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D120766?vs=412216&id=412568#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120766

Files:
  lldb/include/lldb/Core/IOHandler.h


Index: lldb/include/lldb/Core/IOHandler.h
===
--- lldb/include/lldb/Core/IOHandler.h
+++ lldb/include/lldb/Core/IOHandler.h
@@ -85,11 +85,11 @@
 
   virtual void GotEOF() = 0;
 
-  virtual bool IsActive() { return m_active && !m_done; }
+  bool IsActive() { return m_active && !m_done; }
 
-  virtual void SetIsDone(bool b) { m_done = b; }
+  void SetIsDone(bool b) { m_done = b; }
 
-  virtual bool GetIsDone() { return m_done; }
+  bool GetIsDone() { return m_done; }
 
   Type GetType() const { return m_type; }
 


Index: lldb/include/lldb/Core/IOHandler.h
===
--- lldb/include/lldb/Core/IOHandler.h
+++ lldb/include/lldb/Core/IOHandler.h
@@ -85,11 +85,11 @@
 
   virtual void GotEOF() = 0;
 
-  virtual bool IsActive() { return m_active && !m_done; }
+  bool IsActive() { return m_active && !m_done; }
 
-  virtual void SetIsDone(bool b) { m_done = b; }
+  void SetIsDone(bool b) { m_done = b; }
 
-  virtual bool GetIsDone() { return m_done; }
+  bool GetIsDone() { return m_done; }
 
   Type GetType() const { return m_type; }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 77bfdeb - [lldb] Update error messages in TestMemoryHistory.py

2022-03-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-03-02T15:56:45-08:00
New Revision: 77bfdeb092d186179efd7e032ba1c11fc0a6a444

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

LOG: [lldb] Update error messages in TestMemoryHistory.py

Update TestMemoryHistory.py for daba82362228b4aa460c26079c028ebf832066fd
which changes the CommandObject & Disassemble error messages .

Added: 


Modified: 
lldb/test/API/functionalities/asan/TestMemoryHistory.py

Removed: 




diff  --git a/lldb/test/API/functionalities/asan/TestMemoryHistory.py 
b/lldb/test/API/functionalities/asan/TestMemoryHistory.py
index 2ba3a92c9b553..185b3ecba5fa2 100644
--- a/lldb/test/API/functionalities/asan/TestMemoryHistory.py
+++ b/lldb/test/API/functionalities/asan/TestMemoryHistory.py
@@ -40,7 +40,7 @@ def asan_tests(self):
 # "memory history" command should not work without a process
 self.expect("memory history 0",
 error=True,
-substrs=["invalid process"])
+substrs=["Command requires a current process"])
 
 self.runCmd("run")
 



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

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:46
 using namespace lldb_private;
+using namespace llvm::dwarf;
 

Why not `lldb_private::dwarf`? 



Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:19
 using namespace lldb;
+using namespace dwarf;
 

Wouldn't it be more consistent to use `lldb_private::dwarf` everywhere? 


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

https://reviews.llvm.org/D120836

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

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:35
+using namespace dwarf;
 using namespace std;
 extern int g_verbose;

[[ https://llvm.org/docs/CodingStandards.html#do-not-use-using-namespace-std | 
Oh no... ]]


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

https://reviews.llvm.org/D120836

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

[Lldb-commits] [PATCH] D120792: [lldb] Fix python errors in gdbremote.py

2022-03-02 Thread Dominic Chen via Phabricator via lldb-commits

ddcc added inline comments.



Comment at: lldb/examples/python/gdbremote.py:1224-1225
+print(json.dumps(json_tree, indent=4, separators=(',', ': ')))
+except json.JSONDecodeError:
+return
 

kastiglione wrote:
> I don't know this tool, but should it print the original json (`rsp`) if 
> there's an issue when decoding the json? Or should it print a message saying 
> invalid json was given?
The contents of each packet are already printed earlier, this change only 
affects the "jThreadsInfo" message which is supposed to return a JSON response. 
But servers are allowed to return an empty response if they don't support a 
protocol message, which will fail to decode as valid JSON.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120792

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

[Lldb-commits] [lldb] 42db8bf - [lldb] Skip check for the lldb prompt in TestIOHandlerProcessSTDIO

2022-03-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-03-02T16:44:14-08:00
New Revision: 42db8bfa20d956f06f4c19ddfa6f1688bd29f8d4

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

LOG: [lldb] Skip check for the lldb prompt in TestIOHandlerProcessSTDIO

The check for the prompt isn't essential for this test. The check fail
on the lldb-arm-ubuntu because of what appears to be a missing space
after the prompt. Rather than disabling the test, let's see if we can
get it to pass without it.

Added: 


Modified: 
lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py

Removed: 




diff  --git a/lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py 
b/lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
index 5a77c0cb2b7c6..0db9c56f37f93 100644
--- a/lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
+++ b/lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py
@@ -26,5 +26,4 @@ def test(self):
 self.child.expect_exact("stdout: baz")
 
 self.child.sendcontrol('d')
-self.expect_prompt()
 self.quit()



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

[Lldb-commits] [PATCH] D120792: [lldb] Fix python errors in gdbremote.py

2022-03-02 Thread Dave Lee via Phabricator via lldb-commits

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

other than the comment about the json, looks all good to me!




Comment at: lldb/examples/python/gdbremote.py:1224-1225
+print(json.dumps(json_tree, indent=4, separators=(',', ': ')))
+except json.JSONDecodeError:
+return
 

ddcc wrote:
> kastiglione wrote:
> > I don't know this tool, but should it print the original json (`rsp`) if 
> > there's an issue when decoding the json? Or should it print a message 
> > saying invalid json was given?
> The contents of each packet are already printed earlier, this change only 
> affects the "jThreadsInfo" message which is supposed to return a JSON 
> response. But servers are allowed to return an empty response if they don't 
> support a protocol message, which will fail to decode as valid JSON.
thanks for the explanation. My only suggestion is to handle that case 
explicitly, by checking that the response is not empty before decoding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120792

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

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:46
 using namespace lldb_private;
+using namespace llvm::dwarf;
 

JDevlieghere wrote:
> Why not `lldb_private::dwarf`? 
I think I started out with this file and must have written that by mistake.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:19
 using namespace lldb;
+using namespace dwarf;
 

JDevlieghere wrote:
> Wouldn't it be more consistent to use `lldb_private::dwarf` everywhere? 
Yeah, I was trying to match the style of  how it was done for minidump, see 
`source/Plugins/Process/minidump/MinidumpParser.cpp`.

Since we need access to both namespaces but in files that did already have 
`using namespace lldb_private;` I just used the less verbose version.

I can see arguments for just being verbose everywhere.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:35
+using namespace dwarf;
 using namespace std;
 extern int g_verbose;

JDevlieghere wrote:
> [[ https://llvm.org/docs/CodingStandards.html#do-not-use-using-namespace-std 
> | Oh no... ]]
Yeah I noticed that was going to put that on a list of things to do, didn't 
feel right mixing that in here as well.


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

https://reviews.llvm.org/D120836

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

[Lldb-commits] [PATCH] D120792: [lldb] Fix python errors in gdbremote.py

2022-03-02 Thread Dominic Chen via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcd89f94aa904: [lldb] Fix python errors in gdbremote.py 
(authored by ddcc).

Changed prior to commit:
  https://reviews.llvm.org/D120792?vs=412327&id=412594#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120792

Files:
  lldb/examples/python/gdbremote.py


Index: lldb/examples/python/gdbremote.py
===
--- lldb/examples/python/gdbremote.py
+++ lldb/examples/python/gdbremote.py
@@ -260,6 +260,12 @@
 dest='verbose',
 help='display verbose debug info',
 default=False)
+parser.add_option(
+'--plot',
+action='store_true',
+dest='plot',
+help='plot packet latencies by packet type',
+default=False)
 parser.add_option(
 '-q',
 '--quiet',
@@ -556,11 +562,11 @@
 return kvp
 
 def split(self, ch):
-return string.split(self.str, ch)
+return self.str.split(ch)
 
 def split_hex(self, ch, byte_order):
 hex_values = list()
-strings = string.split(self.str, ch)
+strings = self.str.split(ch)
 for str in strings:
 hex_values.append(Packet(str).get_hex_uint(byte_order))
 return hex_values
@@ -888,7 +894,7 @@
 
 
 def cmd_vAttach(options, cmd, args):
-(extra_command, args) = string.split(args, ';')
+(extra_command, args) = args.split(';')
 if extra_command:
 print("%s%s(%s)" % (cmd, extra_command, args))
 else:
@@ -1212,9 +1218,13 @@
 
 def rsp_json(options, cmd, cmd_args, rsp):
 print('%s() reply:' % (cmd))
-json_tree = json.loads(rsp)
-print(json.dumps(json_tree, indent=4, separators=(',', ': ')))
-
+if not rsp:
+return
+try:
+json_tree = json.loads(rsp)
+print(json.dumps(json_tree, indent=4, separators=(',', ': ')))
+except json.JSONDecodeError:
+return
 
 def rsp_jGetLoadedDynamicLibrariesInfos(options, cmd, cmd_args, rsp):
 if cmd_args:
@@ -1541,7 +1551,7 @@
 print("  %24s %11.6f  %5.2f%% %6d %9.6f" % (
 item, packet_total_time, packet_percent, packet_count,
 float(packet_total_time) / float(packet_count)))
-if options.plot:
+if options and options.plot:
 plot_latencies(packet_times)
 
 if __name__ == '__main__':
@@ -1558,12 +1568,6 @@
 dest='verbose',
 help='display verbose debug info',
 default=False)
-parser.add_option(
-'--plot',
-action='store_true',
-dest='plot',
-help='plot packet latencies by packet type',
-default=False)
 parser.add_option(
 '-q',
 '--quiet',


Index: lldb/examples/python/gdbremote.py
===
--- lldb/examples/python/gdbremote.py
+++ lldb/examples/python/gdbremote.py
@@ -260,6 +260,12 @@
 dest='verbose',
 help='display verbose debug info',
 default=False)
+parser.add_option(
+'--plot',
+action='store_true',
+dest='plot',
+help='plot packet latencies by packet type',
+default=False)
 parser.add_option(
 '-q',
 '--quiet',
@@ -556,11 +562,11 @@
 return kvp
 
 def split(self, ch):
-return string.split(self.str, ch)
+return self.str.split(ch)
 
 def split_hex(self, ch, byte_order):
 hex_values = list()
-strings = string.split(self.str, ch)
+strings = self.str.split(ch)
 for str in strings:
 hex_values.append(Packet(str).get_hex_uint(byte_order))
 return hex_values
@@ -888,7 +894,7 @@
 
 
 def cmd_vAttach(options, cmd, args):
-(extra_command, args) = string.split(args, ';')
+(extra_command, args) = args.split(';')
 if extra_command:
 print("%s%s(%s)" % (cmd, extra_command, args))
 else:
@@ -1212,9 +1218,13 @@
 
 def rsp_json(options, cmd, cmd_args, rsp):
 print('%s() reply:' % (cmd))
-json_tree = json.loads(rsp)
-print(json.dumps(json_tree, indent=4, separators=(',', ': ')))
-
+if not rsp:
+return
+try:
+json_tree = json.loads(rsp)
+print(json.dumps(json_tree, indent=4, separators=(',', ': ')))
+except json.JSONDecodeError:
+return
 
 def rsp_jGetLoadedDynamicLibrariesInfos(options, cmd, cmd_args, rsp):
 if cmd_args:
@@ -1541,7 +1551,7 @@
 print("  %24s %11.6f  %5.2f%% %6d %9.6f" % (
 item, packet_total_time, packet_percent, packet_count,
 float(packet_total_time) / float(packet_count)))
-if options.plot:
+if options and options.plot:
 plot_latencies(packet_times)
 
 if __name__ == '__main__':
@@ -1558,12 +1568,6 @@
 dest='ver

[Lldb-commits] [lldb] cd89f94 - [lldb] Fix python errors in gdbremote.py

2022-03-02 Thread Dominic Chen via lldb-commits


Author: Dominic Chen
Date: 2022-03-02T19:47:51-08:00
New Revision: cd89f94aa9048d59120d5d89ac84b5119bad45ab

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

LOG: [lldb] Fix python errors in gdbremote.py

Fix exceptions encountered while debugging gdb protocol

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

Added: 


Modified: 
lldb/examples/python/gdbremote.py

Removed: 




diff  --git a/lldb/examples/python/gdbremote.py 
b/lldb/examples/python/gdbremote.py
index 3045bb0170886..8b6a268496214 100755
--- a/lldb/examples/python/gdbremote.py
+++ b/lldb/examples/python/gdbremote.py
@@ -260,6 +260,12 @@ def stop_gdb_log(debugger, command, result, dict):
 dest='verbose',
 help='display verbose debug info',
 default=False)
+parser.add_option(
+'--plot',
+action='store_true',
+dest='plot',
+help='plot packet latencies by packet type',
+default=False)
 parser.add_option(
 '-q',
 '--quiet',
@@ -556,11 +562,11 @@ def get_key_value_pairs(self):
 return kvp
 
 def split(self, ch):
-return string.split(self.str, ch)
+return self.str.split(ch)
 
 def split_hex(self, ch, byte_order):
 hex_values = list()
-strings = string.split(self.str, ch)
+strings = self.str.split(ch)
 for str in strings:
 hex_values.append(Packet(str).get_hex_uint(byte_order))
 return hex_values
@@ -888,7 +894,7 @@ def rsp_vCont(options, cmd, cmd_args, rsp):
 
 
 def cmd_vAttach(options, cmd, args):
-(extra_command, args) = string.split(args, ';')
+(extra_command, args) = args.split(';')
 if extra_command:
 print("%s%s(%s)" % (cmd, extra_command, args))
 else:
@@ -1212,9 +1218,13 @@ def decode_packet(s, start_index=0):
 
 def rsp_json(options, cmd, cmd_args, rsp):
 print('%s() reply:' % (cmd))
-json_tree = json.loads(rsp)
-print(json.dumps(json_tree, indent=4, separators=(',', ': ')))
-
+if not rsp:
+return
+try:
+json_tree = json.loads(rsp)
+print(json.dumps(json_tree, indent=4, separators=(',', ': ')))
+except json.JSONDecodeError:
+return
 
 def rsp_jGetLoadedDynamicLibrariesInfos(options, cmd, cmd_args, rsp):
 if cmd_args:
@@ -1541,7 +1551,7 @@ def parse_gdb_log(file, options):
 print("  %24s %11.6f  %5.2f%% %6d %9.6f" % (
 item, packet_total_time, packet_percent, packet_count,
 float(packet_total_time) / float(packet_count)))
-if options.plot:
+if options and options.plot:
 plot_latencies(packet_times)
 
 if __name__ == '__main__':
@@ -1558,12 +1568,6 @@ def parse_gdb_log(file, options):
 dest='verbose',
 help='display verbose debug info',
 default=False)
-parser.add_option(
-'--plot',
-action='store_true',
-dest='plot',
-help='plot packet latencies by packet type',
-default=False)
 parser.add_option(
 '-q',
 '--quiet',



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

[Lldb-commits] [lldb] 03dae31 - [lldb] Update TestBasicEntryValues.py for `image lookup` output

2022-03-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-03-02T22:35:34-08:00
New Revision: 03dae31aca63cef0584dc25cc494005f1f241f99

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

LOG: [lldb] Update TestBasicEntryValues.py for `image lookup` output

Update TestBasicEntryValues.py for 15983c28aa81 which changes the output
for `image lookup -v`. This should fix the debian and macos build bots.

Added: 


Modified: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp 
b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
index a1804254931cf..91769e8768017 100644
--- a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
+++ b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
@@ -20,8 +20,8 @@ __attribute__((noinline)) void func1(int &sink) {
   ++global;
   //% prefix = "FUNC1-GNU" if "GNU" in self.name else "FUNC1-V5"
   //% self.filecheck("image lookup -v -a $pc", "main.cpp", 
"-check-prefix="+prefix)
-  // FUNC1-GNU: name = "sink", type = "int &", location = DW_OP_GNU_entry_value
-  // FUNC1-V5: name = "sink", type = "int &", location = DW_OP_entry_value
+  // FUNC1-GNU: name = "sink", type = "int &", valid ranges = {{.*}}, location 
= {{.*}} DW_OP_GNU_entry_value
+  // FUNC1-V5: name = "sink", type = "int &", valid ranges = {{.*}}, location 
= {{.*}} DW_OP_entry_value
 }
 
 __attribute__((noinline)) void func2(int &sink, int x) {



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

[Lldb-commits] [lldb] ef0de5d - [lldb] Update the CI docs

2022-03-02 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-03-02T22:40:05-08:00
New Revision: ef0de5dce7ead61fc890476ee2666cb32f5125a5

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

LOG: [lldb] Update the CI docs

Remove the reproducer bot and the fedora bot.

Added: 


Modified: 
lldb/docs/resources/bots.rst

Removed: 




diff  --git a/lldb/docs/resources/bots.rst b/lldb/docs/resources/bots.rst
index 8a43f53bd2502..f2deda52851ed 100644
--- a/lldb/docs/resources/bots.rst
+++ b/lldb/docs/resources/bots.rst
@@ -11,9 +11,8 @@ LLVM Buildbot is the place where volunteers provide build 
machines. Everyone can
 * `lldb-x86_64-debian `_
 * `lldb-aarch64-ubuntu `_
 * `lldb-arm-ubuntu `_
-* `lldb-x86_64-fedora `_
 
-An overview of all LLDB builders (except Fedora) can be found here:
+An overview of all LLDB builders can be found here:
 
 `https://lab.llvm.org/buildbot/#/builders?tags=lldb 
`_
 
@@ -25,7 +24,6 @@ GreenDragon builds and tests LLDB on macOS. It has a 
`dedicated tab
 
 * `lldb-cmake `_
 * `lldb-cmake-matrix 
`_
-* `lldb-cmake-reproducers 
`_
 * `lldb-cmake-standalone 
`_
 * `lldb-cmake-sanitized 
`_
 



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

Re: [Lldb-commits] [lldb] 2dc6e90 - [lldb/Host] Fix crash in FileSystem::IsLocal

2022-03-02 Thread Jonas Devlieghere via lldb-commits

FileSystem has a bunch of methods that unconditionally use `m_fs`. There's
no reason `IsLocal` is special in that regard. If this crashes(which is
what the radar is about) it's because the FileSystem is not properly
initialized and we should figure out why instead.

On Fri, Feb 25, 2022 at 6:41 PM Med Ismail Bennani via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

>
> Author: Med Ismail Bennani
> Date: 2022-02-25T18:33:31-08:00
> New Revision: 2dc6e906b09deb092c15a726c06d0ecbeec1eb18
>
> URL:
> https://github.com/llvm/llvm-project/commit/2dc6e906b09deb092c15a726c06d0ecbeec1eb18
> DIFF:
> https://github.com/llvm/llvm-project/commit/2dc6e906b09deb092c15a726c06d0ecbeec1eb18.diff
>
> LOG: [lldb/Host] Fix crash in FileSystem::IsLocal
>
> This checks `m_fs` before dereferencing it to access its`isLocal` method.
>
> rdar://67410058
>
> Signed-off-by: Med Ismail Bennani 
>
> Added:
>
>
> Modified:
> lldb/source/Host/common/FileSystem.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/lldb/source/Host/common/FileSystem.cpp
> b/lldb/source/Host/common/FileSystem.cpp
> index 1e4a24abe3017..26a98fa0a4ec4 100644
> --- a/lldb/source/Host/common/FileSystem.cpp
> +++ b/lldb/source/Host/common/FileSystem.cpp
> @@ -192,7 +192,8 @@ bool FileSystem::IsDirectory(const FileSpec
> &file_spec) const {
>
>  bool FileSystem::IsLocal(const Twine &path) const {
>bool b = false;
> -  m_fs->isLocal(path, b);
> +  if (m_fs)
> +m_fs->isLocal(path, b);
>return b;
>  }
>
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

2022-03-02 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:19
 using namespace lldb;
+using namespace dwarf;
 

shafik wrote:
> JDevlieghere wrote:
> > Wouldn't it be more consistent to use `lldb_private::dwarf` everywhere? 
> Yeah, I was trying to match the style of  how it was done for minidump, see 
> `source/Plugins/Process/minidump/MinidumpParser.cpp`.
> 
> Since we need access to both namespaces but in files that did already have 
> `using namespace lldb_private;` I just used the less verbose version.
> 
> I can see arguments for just being verbose everywhere.
Maybe it's just preference? Curious to hear from @labath why he did it that 
way. Personally I prefer the slightly more verbose one because it's self 
contained. 



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:35
+using namespace dwarf;
 using namespace std;
 extern int g_verbose;

shafik wrote:
> JDevlieghere wrote:
> > [[ 
> > https://llvm.org/docs/CodingStandards.html#do-not-use-using-namespace-std | 
> > Oh no... ]]
> Yeah I noticed that was going to put that on a list of things to do, didn't 
> feel right mixing that in here as well.
Totally!


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

https://reviews.llvm.org/D120836

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

63 matches

Mail list logo