[Lldb-commits] [PATCH] D120593: [Support] Allow the ability to change WithColor's auto detection function

2022-03-01 Thread Tomas Matheson via Phabricator via lldb-commits 
tmatheson added inline comments.



Comment at: llvm/include/llvm/Support/WithColor.h:142
+  /// Retrieve the default color auto detection function.
+  static AutoDetectFunctionType defaultAutoDetectFunction();
+

This introduces a global constructor call into libSupport, which it should not. 
See https://reviews.llvm.org/rG5eb2e9aa64b7be7cd8ed7f36de19c2c9bdf1977c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120593

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

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

2022-03-01 Thread Emre Kultursay via Phabricator via lldb-commits 
emrekultursay created this revision.
emrekultursay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120718

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


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


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

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

2022-03-01 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, I'm sorry this took so long.

Jim, do you have anything to add?




Comment at: lldb/source/Symbol/Variable.cpp:439-440
+bool Variable::DumpLocations(Stream *s, const Address &address) {
+  // When given invalid address, it dumps all locations. Otherwise it only 
dumps
+  // the location that contains this address.
+  SymbolContext sc;

Maybe move into a header?


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] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

2022-03-01 Thread Andrew Turner via Phabricator via lldb-commits 
andrew added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:128
+
+#ifdef NT_ARM_ADDR_MASK
+  if (m_addr_mask_is_valid)

emaste wrote:
> This `#define` is coming from our headers? We need to provide this constant 
> within LLVM so this will be available on non-FreeBSD hosts.
My understanding is this file is used on FreeBSD as it's used when debugging a 
currently running process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120485

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

[Lldb-commits] [PATCH] D120595: [WIP][trace][intelpt] Add TSC to Nanosecond conversion for IntelPT traces

2022-03-01 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:74
+struct TraceIntelPTGetStateResponse : TraceGetStateResponse {
+  /// `nullptr` if no tsc conversion rate exists.
+  llvm::Optional tsc_rate;

wallace wrote:
> avoid using nullptr when you have an Optional. 
Ideally this would be a plain `TimestampCounterRateSP` variable and nullptr 
would denote emptyness. Since (shared) pointers already have a natural empty 
state, it's best to use that. Adding an extra Optional layer just creates 
ambiguity.



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:22
 #include "llvm/ADT/None.h"
+#include 
 

wallace wrote:
> jj10306 wrote:
> > 
> memory should be defined as a new include block between lines 9 and 11. 
> That's just the pattern we follow
It's actually a pattern we should not follow. When all of the includes are 
grouped in one block, then clang-format automatically reorders them to conform 
to the [[https://llvm.org/docs/CodingStandards.html#include-style | llvm coding 
standards ]].


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

https://reviews.llvm.org/D120595

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

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

2022-03-01 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D117928#3349234 , @ljmf00 wrote:

> Meanwhile, a commit will land Linux stable branches soon to fix this issue: 
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=44cad52cc14ae10062f142ec16ede489bccf4469
>  . Do we have an alternative way to fix this, e.g. a custom XFAIL flag for 
> specific linux versions?

We don't. That's why I said this:

In D117928#3269040 , @labath wrote:

> I'd say it all comes down to how much you care about this particular 
> configuration having a clean test run. My position is that I'd rather have a 
> workaround in place than to have to figure out how to skip this test for this 
> exact setup.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117928

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

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

2022-03-01 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik accepted this revision.
shafik 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/D120718/new/

https://reviews.llvm.org/D120718

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

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

2022-03-01 Thread Emre Kultursay via Phabricator via lldb-commits 
emrekultursay added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120718

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

[Lldb-commits] [PATCH] 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-01 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere, jingham, aadsm, wallace.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

...and doesn't play with the async mode when doing normal launch/attach.

We discovered that when using "launchCommands" or "attachCommands" that there 
was an issue where these commands were not being run synchronously. There were 
further problems in this case where we would get thread events for the process 
that was just launched or attached before the IDE was ready, which is after 
"configurationDone" was sent to lldb-vscode.

This fix introduces the ability to wait for the process to stop after 
"launchCommands" or "attachCommands" are run to ensure that we have a stopped 
process point that is ready for the debug session to proceed. We spin up the 
thread that listens for process events before we start the launch or attach, 
but we don't want stop events being delivered through the DAP protocol until 
the "configurationDone" packet is received. We now always ignore the stop event 
with a stop ID of 1, which is the first stop. All normal launch and attach 
scenarios use the synchronous mode, and "launchCommands and "attachCommands" 
run an array of LLDB commands in async mode.

This should make our launch with "launchCommands" and attach with 
"attachCommands" avoid a race condition when the process is being launched or 
attached.


Repository:
  rG LLVM Github Monorepo

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,19 @@
   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 get the
+// first process stop event which we do not want to send an event
+// for. The first stop event is always sent in
+// request_configuarationDone() where  it will manually deliver the
+// event by calling the SendThreadStoppedEvent().
+if (process.GetStopID() > 1) {
+  // Only report a stopped event if the process was not restarted.
+  if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
+

[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-01 Thread walter erquinigo via Phabricator via lldb-commits 
wallace accepted this revision.
wallace added a comment.
This revision is now accepted and ready to land.

I'm assuming this passes all tests locally


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] D120607: [lldb/test] Re-enable TestEvents.py on Darwin and fix crashes

2022-03-01 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/D120607/new/

https://reviews.llvm.org/D120607

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

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

2022-03-01 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.
Herald added a project: All.

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


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] D120762: [lldb] Protect control variables in IOHandler with a mutex to avoid a data race

2022-03-01 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added reviewers: clayborg, jingham, labath.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
JDevlieghere requested review of this revision.

Fixes a data race between the main thread and the process handling thread:

  WARNING: ThreadSanitizer: data race (pid=13380)
Write of size 1 at 0x0001067646e8 by thread T8 (mutexes: write M2862):
  #0 lldb_private::IOHandler::SetIsDone(bool) IOHandler.h:90 
(liblldb.15.0.0git.dylib:arm64+0x971d84)
  #1 IOHandlerProcessSTDIO::Cancel() Process.cpp:4376 
(liblldb.15.0.0git.dylib:arm64+0x5de0ec)
  #2 
lldb_private::Debugger::PopIOHandler(std::__1::shared_ptr
 const&) Debugger.cpp:1156 (liblldb.15.0.0git.dylib:arm64+0x3cb264)
  #3 
lldb_private::Debugger::RemoveIOHandler(std::__1::shared_ptr
 const&) Debugger.cpp:1063 (liblldb.15.0.0git.dylib:arm64+0x3cbce8)
  #4 lldb_private::Process::PopProcessIOHandler() Process.cpp:4480 
(liblldb.15.0.0git.dylib:arm64+0x5c57f8)
  #5 
lldb_private::Debugger::HandleProcessEvent(std::__1::shared_ptr
 const&) Debugger.cpp:1549 (liblldb.15.0.0git.dylib:arm64+0x3cea78)
  #6 lldb_private::Debugger::DefaultEventHandler() Debugger.cpp:1622 
(liblldb.15.0.0git.dylib:arm64+0x3cf27c)
  #7 
std::__1::__function::__func, 
void* ()>::operator()() function.h:352 (liblldb.15.0.0git.dylib:arm64+0x3d1b94)
  #8 lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) 
HostNativeThreadBase.cpp:62 (liblldb.15.0.0git.dylib:arm64+0x4c7168)
  #9 lldb_private::HostThreadMacOSX::ThreadCreateTrampoline(void*) 
HostThreadMacOSX.mm:18 (liblldb.15.0.0git.dylib:arm64+0x29ef544)
  
Previous write of size 1 at 0x0001067646e8 by main thread:
  #0 lldb_private::IOHandler::SetIsDone(bool) IOHandler.h:90 
(liblldb.15.0.0git.dylib:arm64+0x971d84)
  #1 IOHandlerProcessSTDIO::Run() Process.cpp:4323 
(liblldb.15.0.0git.dylib:arm64+0x5ddc2c)
  #2 lldb_private::Debugger::RunIOHandlers() Debugger.cpp:982 
(liblldb.15.0.0git.dylib:arm64+0x3cb448)
  #3 
lldb_private::CommandInterpreter::RunCommandInterpreter(lldb_private::CommandInterpreterRunOptions&)
 CommandInterpreter.cpp:3298 (liblldb.15.0.0git.dylib:arm64+0x506434)
  #4 lldb::SBDebugger::RunCommandInterpreter(bool, bool) 
SBDebugger.cpp:1166 (liblldb.15.0.0git.dylib:arm64+0x535c0)
  #5 Driver::MainLoop() Driver.cpp:634 (lldb:arm64+0x16294)
  #6 main Driver.cpp:853 (lldb:arm64+0x17344)


https://reviews.llvm.org/D120762

Files:
  lldb/include/lldb/Core/IOHandler.h
  lldb/source/Core/IOHandler.cpp
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4398,7 +4398,7 @@
 // interrupt the IOHandlerProcessSTDIO::Run() and we can look at the byte
 // that was written to the pipe and then call
 // m_process->SendAsyncInterrupt() from a much safer location in code.
-if (m_active) {
+if (IsActive()) {
   char ch = 'i'; // Send 'i' for interrupt
   size_t bytes_written = 0;
   Status result = m_pipe.Write(&ch, 1, bytes_written);
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -79,6 +79,31 @@
 
 IOHandler::~IOHandler() = default;
 
+bool IOHandler::IsActive() {
+  std::lock_guard guard(m_control_mutex);
+  return m_active && !m_done;
+}
+
+void IOHandler::SetIsDone(bool b) {
+  std::lock_guard guard(m_control_mutex);
+  m_done = b;
+}
+
+bool IOHandler::GetIsDone() {
+  std::lock_guard guard(m_control_mutex);
+  return m_done;
+}
+
+void IOHandler::Activate() {
+  std::lock_guard guard(m_control_mutex);
+  m_active = true;
+}
+
+void IOHandler::Deactivate() {
+  std::lock_guard guard(m_control_mutex);
+  m_active = false;
+}
+
 int IOHandler::GetInputFD() {
   return (m_input_sp ? m_input_sp->GetDescriptor() : -1);
 }
@@ -558,7 +583,7 @@
   StringList lines;
   if (GetLines(lines, interrupted)) {
 if (interrupted) {
-  m_done = m_interrupt_exits;
+  SetIsDone(m_interrupt_exits);
   m_delegate.IOHandlerInputInterrupted(*this, line);
 
 } else {
@@ -566,7 +591,7 @@
   m_delegate.IOHandlerInputComplete(*this, line);
 }
   } else {
-m_done = true;
+SetIsDone(true);
   }
 } else {
   if (GetLine(line, interrupted)) {
@@ -575,7 +600,7 @@
 else
   m_delegate.IOHandlerInputComplete(*this, line);
   } else {
-m_done = true;
+SetIsDone(true);
   }
 }
   }
Index: lldb/include/lldb/Core/IOHandler.h
===
--- lldb/include/lldb/Core/IOHandler.h
+++ lldb/include/lldb/Core/IOHandler.h
@@ -85,17 +85,17 @@
 
   virtual void GotEOF() = 0;
 
-  virtual bool IsActive() { return m_

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

2022-03-01 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added reviewers: clayborg, jingham, labath, mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

There are no implementations overriding these methods.


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();
+  bool IsActive();
 
-  virtual void SetIsDone(bool b);
+  void SetIsDone(bool b);
 
-  virtual bool GetIsDone();
+  bool GetIsDone();
 
   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();
+  bool IsActive();
 
-  virtual void SetIsDone(bool b);
+  void SetIsDone(bool b);
 
-  virtual bool GetIsDone();
+  bool GetIsDone();
 
   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] [PATCH] D120762: [lldb] Protect control variables in IOHandler with a mutex to avoid a data race

2022-03-01 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

These kinds of changes rarely fix a bug -- usually they just change a 
(detectable) data race into some nondeterministic runtime behavior.

That's because, even though e.g. `IsActive` can compute its result in a 
race-free manner, there's nothing preventing someone from invalidating it one 
nanosecond after the function returns (and before anyone manages to act on it). 
And if it isn't possible for someone to concurrently change the values that 
IsActive depends on, then we wouldn't have a data race in the first place.

Most of the time, that means the result of the function is useless. I haven't 
checked tried (yet) to figure out how could that problem manifest itself in 
this case (the iohandler logic is fairly complex), but I'm pretty sure that 
this should be fixed at a higher level.




Comment at: lldb/source/Target/Process.cpp:4396
   bool Interrupt() override {
 // Do only things that are safe to do in an interrupt context (like in a
 // SIGINT handler), like write 1 byte to a file descriptor. This will

Locking a mutex in a signal handler is not usually safe.


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] D120766: [lldb] Devirtualize IOHandler::{IsActive, SetIsDone, GetIsDone} (NFC)

2022-03-01 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.

good good


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

https://reviews.llvm.org/D120766

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

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

2022-03-01 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik 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.

That is a good point. I did a quick look at the file `DataFileCache.h` and none 
of the files it includes directly use `using namespace llvm;`.

It looks like across llvm-project we have 54 headers that use `using namespace 
llvm;` I thought it was a lot more.

I personally dislike it even in `.cpp` files using `llvm::` or `lldb_private::` 
does not feel like an undue burden. Although the count for `.cpp` files is 
4000+ 😱


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120718

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

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

2022-03-01 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D120718#3352635 , @shafik wrote:

> 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.
>
> That is a good point. I did a quick look at the file `DataFileCache.h` and 
> none of the files it includes directly use `using namespace llvm;`.
>
> It looks like across llvm-project we have 54 headers that use `using 
> namespace llvm;` I thought it was a lot more.

It looks like at least some of those are inside functions, which is kind of OK. 
But what's worse, some of those are not inside functions, which is  😱 😱 😱 😱 😱 😱 
😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱 😱. using directives in 
headers completely mangle the symbol lookups in any file which includes them.


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] D120690: [PDB] Add char8_t type

2022-03-01 Thread Zequan Wu via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5c9e20d7d0a7: [PDB] Add char8_t type (authored by zequanwu).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120690

Files:
  lld/test/COFF/pdb_char8_t.ll
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  llvm/docs/PDB/TpiStream.rst
  llvm/include/llvm/DebugInfo/CodeView/TypeIndex.h
  llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/DebugInfo/CodeView/TypeIndex.cpp
  llvm/lib/DebugInfo/PDB/Native/NativeTypeEnum.cpp
  llvm/lib/DebugInfo/PDB/Native/SymbolCache.cpp
  llvm/lib/DebugInfo/PDB/PDBExtras.cpp
  llvm/tools/llvm-pdbutil/PrettyBuiltinDumper.cpp

Index: llvm/tools/llvm-pdbutil/PrettyBuiltinDumper.cpp
===
--- llvm/tools/llvm-pdbutil/PrettyBuiltinDumper.cpp
+++ llvm/tools/llvm-pdbutil/PrettyBuiltinDumper.cpp
@@ -90,6 +90,8 @@
 return "char16_t";
   case PDB_BuiltinType::Char32:
 return "char32_t";
+  case PDB_BuiltinType::Char8:
+return "char8_t";
   case PDB_BuiltinType::None:
 return "...";
   }
Index: llvm/lib/DebugInfo/PDB/PDBExtras.cpp
===
--- llvm/lib/DebugInfo/PDB/PDBExtras.cpp
+++ llvm/lib/DebugInfo/PDB/PDBExtras.cpp
@@ -63,6 +63,7 @@
 CASE_OUTPUT_ENUM_CLASS_NAME(PDB_BuiltinType, HResult, OS)
 CASE_OUTPUT_ENUM_CLASS_NAME(PDB_BuiltinType, Char16, OS)
 CASE_OUTPUT_ENUM_CLASS_NAME(PDB_BuiltinType, Char32, OS)
+CASE_OUTPUT_ENUM_CLASS_NAME(PDB_BuiltinType, Char8, OS)
   }
   return OS;
 }
Index: llvm/lib/DebugInfo/PDB/Native/SymbolCache.cpp
===
--- llvm/lib/DebugInfo/PDB/Native/SymbolCache.cpp
+++ llvm/lib/DebugInfo/PDB/Native/SymbolCache.cpp
@@ -67,6 +67,7 @@
 {codeview::SimpleTypeKind::WideCharacter, PDB_BuiltinType::WCharT, 2},
 {codeview::SimpleTypeKind::Character16, PDB_BuiltinType::Char16, 2},
 {codeview::SimpleTypeKind::Character32, PDB_BuiltinType::Char32, 4},
+{codeview::SimpleTypeKind::Character8, PDB_BuiltinType::Char8, 1},
 {codeview::SimpleTypeKind::SignedCharacter, PDB_BuiltinType::Char, 1},
 {codeview::SimpleTypeKind::UnsignedCharacter, PDB_BuiltinType::UInt, 1},
 {codeview::SimpleTypeKind::Float32, PDB_BuiltinType::Float, 4},
Index: llvm/lib/DebugInfo/PDB/Native/NativeTypeEnum.cpp
===
--- llvm/lib/DebugInfo/PDB/Native/NativeTypeEnum.cpp
+++ llvm/lib/DebugInfo/PDB/Native/NativeTypeEnum.cpp
@@ -205,6 +205,8 @@
 return PDB_BuiltinType::Char16;
   case SimpleTypeKind::Character32:
 return PDB_BuiltinType::Char32;
+  case SimpleTypeKind::Character8:
+return PDB_BuiltinType::Char8;
   case SimpleTypeKind::Int128:
   case SimpleTypeKind::Int128Oct:
   case SimpleTypeKind::Int16:
Index: llvm/lib/DebugInfo/CodeView/TypeIndex.cpp
===
--- llvm/lib/DebugInfo/CodeView/TypeIndex.cpp
+++ llvm/lib/DebugInfo/CodeView/TypeIndex.cpp
@@ -33,6 +33,7 @@
 {"wchar_t*", SimpleTypeKind::WideCharacter},
 {"char16_t*", SimpleTypeKind::Character16},
 {"char32_t*", SimpleTypeKind::Character32},
+{"char8_t*", SimpleTypeKind::Character8},
 {"__int8*", SimpleTypeKind::SByte},
 {"unsigned __int8*", SimpleTypeKind::Byte},
 {"short*", SimpleTypeKind::Int16Short},
Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -1826,6 +1826,7 @@
 break;
   case dwarf::DW_ATE_UTF:
 switch (ByteSize) {
+case 1: STK = SimpleTypeKind::Character8; break;
 case 2: STK = SimpleTypeKind::Character16; break;
 case 4: STK = SimpleTypeKind::Character32; break;
 }
Index: llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
===
--- llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
+++ llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
@@ -352,7 +352,8 @@
   BSTR = 30,
   HResult = 31,
   Char16 = 32,
-  Char32 = 33
+  Char32 = 33,
+  Char8 = 34,
 };
 
 /// These values correspond to the flags that can be combined to control the
Index: llvm/include/llvm/DebugInfo/CodeView/TypeIndex.h
===
--- llvm/include/llvm/DebugInfo/CodeView/TypeIndex.h
+++ llvm/include/llvm/DebugInfo/CodeView/TypeIndex.h
@@ -35,6 +35,7 @@
   WideCharacter = 0x0071, // wide char
   Character16 = 0x007a,

[Lldb-commits] [lldb] 5c9e20d - [PDB] Add char8_t type

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

Author: Zequan Wu
Date: 2022-03-01T13:39:51-08:00
New Revision: 5c9e20d7d0a71439a95875ba6067f9c0fc7a4e04

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

LOG: [PDB] Add char8_t type

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

Added: 
lld/test/COFF/pdb_char8_t.ll

Modified: 
lldb/include/lldb/lldb-enumerations.h
lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
llvm/docs/PDB/TpiStream.rst
llvm/include/llvm/DebugInfo/CodeView/TypeIndex.h
llvm/include/llvm/DebugInfo/PDB/PDBTypes.h
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/DebugInfo/CodeView/TypeIndex.cpp
llvm/lib/DebugInfo/PDB/Native/NativeTypeEnum.cpp
llvm/lib/DebugInfo/PDB/Native/SymbolCache.cpp
llvm/lib/DebugInfo/PDB/PDBExtras.cpp
llvm/tools/llvm-pdbutil/PrettyBuiltinDumper.cpp

Removed: 




diff  --git a/lld/test/COFF/pdb_char8_t.ll b/lld/test/COFF/pdb_char8_t.ll
new file mode 100644
index 0..0bae6e03a6178
--- /dev/null
+++ b/lld/test/COFF/pdb_char8_t.ll
@@ -0,0 +1,45 @@
+; RUN: llc -mtriple x86_64-windows-msvc -filetype obj -o %t.obj %s
+; RUN: lld-link /nodefaultlib /noentry /dll /debug /out:%t.exe /pdb:%t.pdb 
%t.obj
+; RUN: llvm-pdbutil dump -type-index=0x7c %t.pdb
+
+; CHECK: 0x007C (char8_t) | char8_t
+
+define dso_local i32 @main() #0 !dbg !9 {
+  %1 = alloca i32, align 4
+  %2 = alloca i8, align 1
+  store i32 0, i32* %1, align 4
+  call void @llvm.dbg.declare(metadata i8* %2, metadata !13, metadata 
!DIExpression()), !dbg !15
+  store i8 0, i8* %2, align 1, !dbg !15
+  %3 = load i8, i8* %2, align 1, !dbg !16
+  %4 = zext i8 %3 to i32, !dbg !16
+  ret i32 %4, !dbg !16
+}
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { mustprogress noinline norecurse nounwind optnone uwtable 
"frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" 
"stack-protector-buffer-size"="8" "target-cpu"="x86-64" 
"target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { nofree nosync nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.linker.options = !{}
+!llvm.module.flags = !{!3, !4, !5, !6, !7}
+!llvm.ident = !{!8}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, 
producer: "clang version 13.0.0", isOptimized: false, runtimeVersion: 0, 
emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: 
None)
+!1 = !DIFile(filename: "pdb_char8_t.cpp", directory: "C:\\src", checksumkind: 
CSK_MD5, checksum: "a00748d29f4e59003184945cd3e17ee3")
+!2 = !{}
+!3 = !{i32 2, !"CodeView", i32 1}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 1}
+!8 = !{!"clang version 13.0.0"}
+!9 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 1, type: 
!10, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: 
!0, retainedNodes: !2)
+!10 = !DISubroutineType(types: !11)
+!11 = !{!12}
+!12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!13 = !DILocalVariable(name: "local", scope: !9, file: !1, line: 3, type: !14)
+!14 = !DIBasicType(name: "char8_t", size: 8, encoding: DW_ATE_UTF)
+!15 = !DILocation(line: 3, scope: !9)
+!16 = !DILocation(line: 4, scope: !9)

diff  --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index f5cabc02bd84e..529ab001a761f 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -749,6 +749,7 @@ enum BasicType {
   eBasicTypeUnsignedWChar,
   eBasicTypeChar16,
   eBasicTypeChar32,
+  eBasicTypeChar8,
   eBasicTypeShort,
   eBasicTypeUnsignedShort,
   eBasicTypeInt,

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
index b5a16447d9cfb..6efad1219852c 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
@@ -704,6 +704,8 @@ 
lldb_private::npdb::GetCompilerTypeForSimpleKind(SimpleTypeKind kind) {
 return lldb::eBasicTypeChar16;
   case SimpleTypeKind::Character32:
 return lldb::eBasicTypeChar32;
+  case SimpleTypeKind::Character8:
+return lldb::eBasicTypeChar8;
   case SimpleTypeKind::Complex80:
 return lldb::eBasicTypeLongDoubleComplex;
   case SimpleTypeKind::Complex64:
@@ -796,6 +798,7 @@ size_t 
lldb_private::npdb::GetTypeSizeForSimpleKind(SimpleTypeKind kind) {
   case SimpleTypeKind::NarrowCharacter:
   case SimpleTypeKind::SignedCharacter:
   case SimpleTypeKind::SByte:
+

[Lldb-commits] [PATCH] D113616: [lldb] Hyphenate Objective-C exception breakpoint labels ✍️

2022-03-01 Thread Stephane Moore via Phabricator via lldb-commits 
stephanemoore added a comment.
Herald added a project: All.

In D113616#3347980 , @clayborg wrote:

> Very easy change as VS Code IDE displays these strings and no one references 
> these by name.

I thought so! Thanks for confirming!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113616

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

[Lldb-commits] [lldb] b901c40 - [PDB] add missing char8_t for 5c9e20d

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

Author: Zequan Wu
Date: 2022-03-01T14:24:21-08:00
New Revision: b901c404a838400983ece83e25583b42737cfaac

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

LOG: [PDB] add missing char8_t for 5c9e20d

Added: 


Modified: 
lldb/docs/python_api_enums.rst
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/unittests/Symbol/TestTypeSystemClang.cpp

Removed: 




diff  --git a/lldb/docs/python_api_enums.rst b/lldb/docs/python_api_enums.rst
index 58100b2aa9a2..1c363381a11b 100644
--- a/lldb/docs/python_api_enums.rst
+++ b/lldb/docs/python_api_enums.rst
@@ -1017,6 +1017,7 @@ BasicType
 .. py:data:: eBasicTypeWChar
 .. py:data:: eBasicTypeSignedWChar
 .. py:data:: eBasicTypeUnsignedWChar
+.. py:data:: eBasicTypeChar8
 .. py:data:: eBasicTypeChar16
 .. py:data:: eBasicTypeChar32
 .. py:data:: eBasicTypeShort

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 962d778bcc38..c6d2e6d27622 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2019,6 +2019,8 @@ TypeSystemClang::GetOpaqueCompilerType(clang::ASTContext 
*ast,
 return ast->getSignedWCharType().getAsOpaquePtr();
   case eBasicTypeUnsignedWChar:
 return ast->getUnsignedWCharType().getAsOpaquePtr();
+  case eBasicTypeChar8:
+return ast->Char8Ty.getAsOpaquePtr();
   case eBasicTypeChar16:
 return ast->Char16Ty.getAsOpaquePtr();
   case eBasicTypeChar32:
@@ -5480,6 +5482,8 @@ 
TypeSystemClang::GetBasicTypeEnumeration(lldb::opaque_compiler_type_t type) {
 return eBasicTypeSignedChar;
   case clang::BuiltinType::Char_U:
 return eBasicTypeUnsignedChar;
+  case clang::BuiltinType::Char8:
+return eBasicTypeChar8;
   case clang::BuiltinType::Char16:
 return eBasicTypeChar16;
   case clang::BuiltinType::Char32:

diff  --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp 
b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
index 5b1154d2cd8b..e78a084f59f0 100644
--- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -53,6 +53,8 @@ TEST_F(TestTypeSystemClang, TestGetBasicTypeFromEnum) {
   context.hasSameType(GetBasicQualType(eBasicTypeBool), context.BoolTy));
   EXPECT_TRUE(
   context.hasSameType(GetBasicQualType(eBasicTypeChar), context.CharTy));
+  EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeChar8),
+  context.Char8Ty));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeChar16),
   context.Char16Ty));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeChar32),



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

[Lldb-commits] [lldb] 0ff2905 - [lldb] Hyphenate Objective-C exception breakpoint labels ✍️

2022-03-01 Thread Stephane Moore via lldb-commits 

Author: Stephane Moore
Date: 2022-03-01T15:02:44-08:00
New Revision: 0ff290570965b920d0fb8495c0e4b43e6e9ccbf0

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

LOG: [lldb] Hyphenate Objective-C exception breakpoint labels ✍️

Objective-C is officially hyphenated:
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ObjectiveC/Introduction/introObjectiveC.html#//apple_ref/doc/uid/TP30001163

Reviewed By: mwyman, clayborg

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

Added: 


Modified: 
lldb/tools/lldb-vscode/VSCode.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/VSCode.cpp 
b/lldb/tools/lldb-vscode/VSCode.cpp
index 3209eea4a897f..8e43ea3b771e6 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -34,8 +34,8 @@ VSCode::VSCode()
   exception_breakpoints(
   {{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus},
{"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
-   {"objc_catch", "Objective C Catch", lldb::eLanguageTypeObjC},
-   {"objc_throw", "Objective C Throw", lldb::eLanguageTypeObjC},
+   {"objc_catch", "Objective-C Catch", lldb::eLanguageTypeObjC},
+   {"objc_throw", "Objective-C Throw", lldb::eLanguageTypeObjC},
{"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
{"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),
   focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),



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

[Lldb-commits] [PATCH] D113616: [lldb] Hyphenate Objective-C exception breakpoint labels ✍️

2022-03-01 Thread Stephane Moore via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0ff290570965: [lldb] Hyphenate Objective-C exception 
breakpoint labels ✍️ (authored by stephanemoore).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113616

Files:
  lldb/tools/lldb-vscode/VSCode.cpp


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -34,8 +34,8 @@
   exception_breakpoints(
   {{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus},
{"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
-   {"objc_catch", "Objective C Catch", lldb::eLanguageTypeObjC},
-   {"objc_throw", "Objective C Throw", lldb::eLanguageTypeObjC},
+   {"objc_catch", "Objective-C Catch", lldb::eLanguageTypeObjC},
+   {"objc_throw", "Objective-C Throw", lldb::eLanguageTypeObjC},
{"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
{"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),
   focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -34,8 +34,8 @@
   exception_breakpoints(
   {{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus},
{"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
-   {"objc_catch", "Objective C Catch", lldb::eLanguageTypeObjC},
-   {"objc_throw", "Objective C Throw", lldb::eLanguageTypeObjC},
+   {"objc_catch", "Objective-C Catch", lldb::eLanguageTypeObjC},
+   {"objc_throw", "Objective-C Throw", lldb::eLanguageTypeObjC},
{"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
{"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),
   focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),
___
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-01 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda updated this revision to Diff 412288.
jasonmolenda added a comment.
Herald added a project: All.

Integrate Jonas' suggestion, update the disassemble shell test to match the new 
error messages.


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::

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

2022-03-01 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.
Herald added a project: All.

Yeah, just a couple little things in the comments.  Otherwise this looks good.




Comment at: lldb/include/lldb/Core/Address.h:242
+uint32_t addr_byte_size = UINT32_MAX,
+bool all_ranges = false) const;
 

You should document all_ranges.



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">,

I missed where you return an error if somebody specifies show-variable-ranges 
== true w/o specifying verbose.  Did I miss that?



Comment at: lldb/source/Expression/DWARFExpression.cpp:2741
   };
   llvm::Error E = loctable_up->visitAbsoluteLocationList(
   offset, 
llvm::object::SectionedAddress{m_loclist_addresses->cu_file_addr},

Local variables in lldb are all lower case and describe what they are.


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] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

2022-03-01 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 412316.
JDevlieghere retitled this revision from "[lldb] Protect control variables in 
IOHandler with a mutex to avoid a data race" to "[lldb] Avoid data race in 
IOHandlerProcessSTDIO".
JDevlieghere edited the summary of this revision.

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-01 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D120762#3352628 , @labath wrote:

> These kinds of changes rarely fix a bug -- usually they just change a 
> (detectable) data race into some nondeterministic runtime behavior.
>
> That's because, even though e.g. `IsActive` can compute its result in a 
> race-free manner, there's nothing preventing someone from invalidating it one 
> nanosecond after the function returns (and before anyone manages to act on 
> it). And if it isn't possible for someone to concurrently change the values 
> that IsActive depends on, then we wouldn't have a data race in the first 
> place.
>
> Most of the time, that means the result of the function is useless. I haven't 
> checked tried (yet) to figure out how could that problem manifest itself in 
> this case (the iohandler logic is fairly complex), but I'm pretty sure that 
> this should be fixed at a higher level.

I think in this particular case the race is fairly "harmless". In the 
read/write scenario, at worst we execute the run-loop in 
`IOHandlerProcessSTDIO` once more than we should. In case of a write/write race 
we are concurrently writing the same value. Anyway, that's not an excuse not to 
fix the underlying issue. I think the updated patch should take care of that.


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] D120792: [lldb] Fix python errors in gdbremote.py

2022-03-01 Thread Dominic Chen via Phabricator via lldb-commits 
ddcc created this revision.
ddcc added reviewers: kastiglione, jingham, JDevlieghere, clayborg.
Herald added a project: All.
ddcc requested review of this revision.
Herald added a project: LLDB.

Fix exceptions encountered while debugging gdb protocol


Repository:
  rG LLVM Github Monorepo

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,11 @@
 
 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=(',', ': ')))
-
+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 +1549,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 +1566,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,11 @@
 
 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=(',', ': ')))
-
+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 +1549,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 +1566,6 @@
 dest='verbose',
 help='display verbose debug info',
 default=False)
-parser.add_option(
-'--plot',
-action=