[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/source/Host/common/File.cpp:609
   num_bytes = bytes_written;
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 bytes_written = ::fwrite(buf, 1, num_bytes, m_stream);

augusto2112 wrote:
> bulbazord wrote:
> > augusto2112 wrote:
> > > One thing that's not super obvious for people reading this is that 
> > > `descriptor_guard` is still in scope in the else branch (due to C++'s 
> > > weird scoping rules when it comes to declaring variables inside `if` 
> > > statements), and will keep the descriptor mutex locked which is probably 
> > > not what you intended. I don't think this affects the correctness of this 
> > > implementation at the moment, but could be dangerous with further changes 
> > > on top of this one.
> > If that's true, then this change needs further adjustment. `GetStream` 
> > acquires the `ValueGuard`s in the opposite order, meaning Thread 1 can call 
> > this function and acquire `descriptor_guard` while Thread 2 can call 
> > `GetStream` and acquire `stream_guard` at the same time. Then they're both 
> > waiting on the other lock (deadlock).
> You're right, nice catch.
Thanks, I didn't know about that and that was certainly not the intention. 


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

https://reviews.llvm.org/D157347

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


[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 548849.
JDevlieghere marked 4 inline comments as done.
JDevlieghere added a comment.

Limit scope of `ValueGuard` by avoiding `else if`.


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

https://reviews.llvm.org/D157347

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Host/common/File.cpp

Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -219,7 +219,7 @@
 size_t File::PrintfVarArg(const char *format, va_list args) {
   llvm::SmallString<0> s;
   if (VASprintf(s, format, args)) {
-size_t written = s.size();;
+size_t written = s.size();
 Write(s.data(), written);
 return written;
   }
@@ -247,15 +247,20 @@
   return file_stats.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
 }
 
+bool NativeFile::IsValid() const {
+  return DescriptorIsValid() || StreamIsValid();
+}
+
 Expected NativeFile::GetOptions() const { return m_options; }
 
 int NativeFile::GetDescriptor() const {
-  if (DescriptorIsValid())
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 return m_descriptor;
+  }
 
   // Don't open the file descriptor if we don't need to, just get it from the
   // stream if we have one.
-  if (StreamIsValid()) {
+  if (ValueGuard stream_guard = StreamIsValid()) {
 #if defined(_WIN32)
 return _fileno(m_stream);
 #else
@@ -272,8 +277,9 @@
 }
 
 FILE *NativeFile::GetStream() {
-  if (!StreamIsValid()) {
-if (DescriptorIsValid()) {
+  ValueGuard stream_guard = StreamIsValid();
+  if (!stream_guard) {
+if (ValueGuard descriptor_guard = DescriptorIsValid()) {
   auto mode = GetStreamOpenModeFromOptions(m_options);
   if (!mode)
 llvm::consumeError(mode.takeError());
@@ -282,9 +288,9 @@
 // We must duplicate the file descriptor if we don't own it because when you
 // call fdopen, the stream will own the fd
 #ifdef _WIN32
-  m_descriptor = ::_dup(GetDescriptor());
+  m_descriptor = ::_dup(m_descriptor);
 #else
-  m_descriptor = dup(GetDescriptor());
+  m_descriptor = dup(m_descriptor);
 #endif
   m_own_descriptor = true;
 }
@@ -306,8 +312,11 @@
 }
 
 Status NativeFile::Close() {
+  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
+
   Status error;
-  if (StreamIsValid()) {
+
+  if (StreamIsValidUnlocked()) {
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
@@ -322,15 +331,17 @@
   }
 }
   }
-  if (DescriptorIsValid() && m_own_descriptor) {
+
+  if (DescriptorIsValidUnlocked() && m_own_descriptor) {
 if (::close(m_descriptor) != 0)
   error.SetErrorToErrno();
   }
-  m_descriptor = kInvalidDescriptor;
+
   m_stream = kInvalidStream;
-  m_options = OpenOptions(0);
   m_own_stream = false;
+  m_descriptor = kInvalidDescriptor;
   m_own_descriptor = false;
+  m_options = OpenOptions(0);
   m_is_interactive = eLazyBoolCalculate;
   m_is_real_terminal = eLazyBoolCalculate;
   return error;
@@ -374,7 +385,7 @@
 
 off_t NativeFile::SeekFromStart(off_t offset, Status *error_ptr) {
   off_t result = 0;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -383,7 +394,10 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+return result;
+  }
+
+  if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -392,15 +406,17 @@
   else
 error_ptr->Clear();
 }
-  } else if (error_ptr) {
-error_ptr->SetErrorString("invalid file handle");
+return result;
   }
+
+  if (error_ptr)
+error_ptr->SetErrorString("invalid file handle");
   return result;
 }
 
 off_t NativeFile::SeekFromCurrent(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -409,7 +425,10 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+return result;
+  }
+
+  if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -418,15 +437,17 @@
   else
 error_ptr->Clear();
 }
-  } else if (error_ptr) {
-error_ptr->SetErrorString("invalid file handle");
+return result;
   }
+
+  if (error_ptr)
+error_ptr->SetErrorString("invalid file handle");
   return result;
 }
 
 off_t NativeFile::SeekFromEnd(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_END);
 
 if (error_ptr) {
@@ -435,7 +456,10 @@
   else
 

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-09 Thread Augusto Noronha via Phabricator via lldb-commits
augusto2112 added inline comments.



Comment at: lldb/source/Host/common/File.cpp:609
   num_bytes = bytes_written;
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 bytes_written = ::fwrite(buf, 1, num_bytes, m_stream);

bulbazord wrote:
> augusto2112 wrote:
> > One thing that's not super obvious for people reading this is that 
> > `descriptor_guard` is still in scope in the else branch (due to C++'s weird 
> > scoping rules when it comes to declaring variables inside `if` statements), 
> > and will keep the descriptor mutex locked which is probably not what you 
> > intended. I don't think this affects the correctness of this implementation 
> > at the moment, but could be dangerous with further changes on top of this 
> > one.
> If that's true, then this change needs further adjustment. `GetStream` 
> acquires the `ValueGuard`s in the opposite order, meaning Thread 1 can call 
> this function and acquire `descriptor_guard` while Thread 2 can call 
> `GetStream` and acquire `stream_guard` at the same time. Then they're both 
> waiting on the other lock (deadlock).
You're right, nice catch.


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

https://reviews.llvm.org/D157347

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


[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
bulbazord requested changes to this revision.
bulbazord added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/source/Host/common/File.cpp:609
   num_bytes = bytes_written;
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 bytes_written = ::fwrite(buf, 1, num_bytes, m_stream);

augusto2112 wrote:
> One thing that's not super obvious for people reading this is that 
> `descriptor_guard` is still in scope in the else branch (due to C++'s weird 
> scoping rules when it comes to declaring variables inside `if` statements), 
> and will keep the descriptor mutex locked which is probably not what you 
> intended. I don't think this affects the correctness of this implementation 
> at the moment, but could be dangerous with further changes on top of this one.
If that's true, then this change needs further adjustment. `GetStream` acquires 
the `ValueGuard`s in the opposite order, meaning Thread 1 can call this 
function and acquire `descriptor_guard` while Thread 2 can call `GetStream` and 
acquire `stream_guard` at the same time. Then they're both waiting on the other 
lock (deadlock).


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

https://reviews.llvm.org/D157347

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


[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-09 Thread Augusto Noronha via Phabricator via lldb-commits
augusto2112 accepted this revision.
augusto2112 added a comment.

I have one nit that I'll leave up to you whether it should be addressed now or 
not.




Comment at: lldb/source/Host/common/File.cpp:545
   num_bytes = bytes_read;
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard file_lock = StreamIsValid()) {
 bytes_read = ::fread(buf, 1, num_bytes, m_stream);

I believe `descriptor_guard` will still be in scope in the else branch (weird 
scoping rules), but 



Comment at: lldb/source/Host/common/File.cpp:609
   num_bytes = bytes_written;
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 bytes_written = ::fwrite(buf, 1, num_bytes, m_stream);

One thing that's not super obvious for people reading this is that 
`descriptor_guard` is still in scope in the else branch (due to C++'s weird 
scoping rules when it comes to declaring variables inside `if` statements), and 
will keep the descriptor mutex locked which is probably not what you intended. 
I don't think this affects the correctness of this implementation at the 
moment, but could be dangerous with further changes on top of this one.


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

https://reviews.llvm.org/D157347

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


[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf2d32ddcec82: [lldb] Sink StreamFile into lldbHost (authored 
by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157460

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/StreamFile.h
  lldb/include/lldb/Host/StreamFile.h
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/API/SBBreakpoint.cpp
  lldb/source/API/SBBreakpointLocation.cpp
  lldb/source/API/SBBreakpointOptionCommon.cpp
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBEvent.cpp
  lldb/source/API/SBFrame.cpp
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBSourceManager.cpp
  lldb/source/API/SBStream.cpp
  lldb/source/API/SBThread.cpp
  lldb/source/API/SBThreadPlan.cpp
  lldb/source/API/SBValue.cpp
  lldb/source/API/SBWatchpoint.cpp
  lldb/source/Core/CMakeLists.txt
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/EmulateInstruction.cpp
  lldb/source/Core/IOHandler.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/StreamFile.cpp
  lldb/source/Expression/LLVMUserExpression.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Expression/UtilityFunction.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/StreamFile.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Target/DynamicRegisterInfo.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -12,7 +12,6 @@
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/DumpRegisterValue.h"
 #include "lldb/Core/Module.h"
-#include "lldb/Core/StreamFile.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/TypeSystem.h"
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -23,7 +23,6 @@
 #include "lldb/Core/SearchFilter.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/SourceManager.h"
-#include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
@@ -34,6 +33,7 @@
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/PosixApi.h"
+#include "lldb/Host/StreamFile.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionGroupWatchpoint.h"
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -11,7 +11,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/SourceManager.h"
-#include "lldb/Core/StreamFile.h"
+#include "lldb/Host/StreamFile.h"
 #include "lldb/Symbol/Block.h"
 

[Lldb-commits] [lldb] f2d32dd - [lldb] Sink StreamFile into lldbHost

2023-08-09 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-08-09T17:17:18-07:00
New Revision: f2d32ddcec82c20582c6aa32558b82ca7c3d3c50

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

LOG: [lldb] Sink StreamFile into lldbHost

StreamFile subclasses Stream (from lldbUtility) and is backed by a File
(from lldbHost). It does not depend on anything from lldbCore or any of its
sibling libraries, so I think it makes sense for this to live in
lldbHost instead.

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

Added: 
lldb/include/lldb/Host/StreamFile.h
lldb/source/Host/common/StreamFile.cpp

Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Interpreter/CommandReturnObject.h
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/source/API/SBBreakpoint.cpp
lldb/source/API/SBBreakpointLocation.cpp
lldb/source/API/SBBreakpointOptionCommon.cpp
lldb/source/API/SBDebugger.cpp
lldb/source/API/SBEvent.cpp
lldb/source/API/SBFrame.cpp
lldb/source/API/SBInstruction.cpp
lldb/source/API/SBInstructionList.cpp
lldb/source/API/SBProcess.cpp
lldb/source/API/SBSourceManager.cpp
lldb/source/API/SBStream.cpp
lldb/source/API/SBThread.cpp
lldb/source/API/SBThreadPlan.cpp
lldb/source/API/SBValue.cpp
lldb/source/API/SBWatchpoint.cpp
lldb/source/Core/CMakeLists.txt
lldb/source/Core/Debugger.cpp
lldb/source/Core/EmulateInstruction.cpp
lldb/source/Core/IOHandler.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Expression/LLVMUserExpression.cpp
lldb/source/Expression/REPL.cpp
lldb/source/Expression/UserExpression.cpp
lldb/source/Expression/UtilityFunction.cpp
lldb/source/Host/CMakeLists.txt
lldb/source/Interpreter/CommandInterpreter.cpp

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp

lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp

lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp

lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
lldb/source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Symbol/CompilerType.cpp
lldb/source/Target/DynamicRegisterInfo.cpp
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/ThreadPlanTracer.cpp

Removed: 
lldb/include/lldb/Core/StreamFile.h
lldb/source/Core/StreamFile.cpp



diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index be16e742a72099..35703d6f885aa1 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -19,9 +19,9 @@
 #include "lldb/Core/FormatEntity.h"
 #include "lldb/Core/IOHandler.h"
 #include "lldb/Core/SourceManager.h"
-#include "lldb/Core/StreamFile.h"
 #include "lldb/Core/UserSettingsController.h"
 #include "lldb/Host/HostThread.h"
+#include "lldb/Host/StreamFile.h"
 #include "lldb/Host/Terminal.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Platform.h"

diff  --git a/lldb/include/lldb/Core/StreamFile.h 
b/lldb/include/lldb/Host/StreamFile.h
similarity index 94%
rename from lldb/include/lldb/Core/StreamFile.h
rename to lldb/include/lldb/Host/StreamFile.h
index dba4042b6ec99a..2c96e13565a007 100644
--- a/lldb/include/lldb/Core/StreamFile.h
+++ b/lldb/include/lldb/Host/StreamFile.h
@@ -6,8 +6,8 @@
 //
 
//===--===//
 
-#ifndef LLDB_CORE_STREAMFILE_H
-#define LLDB_CORE_STREAMFILE_H
+#ifndef 

[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
bulbazord updated this revision to Diff 548828.
bulbazord added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157460

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Core/StreamFile.h
  lldb/include/lldb/Host/StreamFile.h
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/API/SBBreakpoint.cpp
  lldb/source/API/SBBreakpointLocation.cpp
  lldb/source/API/SBBreakpointOptionCommon.cpp
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBEvent.cpp
  lldb/source/API/SBFrame.cpp
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBSourceManager.cpp
  lldb/source/API/SBStream.cpp
  lldb/source/API/SBThread.cpp
  lldb/source/API/SBThreadPlan.cpp
  lldb/source/API/SBValue.cpp
  lldb/source/API/SBWatchpoint.cpp
  lldb/source/Core/CMakeLists.txt
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/EmulateInstruction.cpp
  lldb/source/Core/IOHandler.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/StreamFile.cpp
  lldb/source/Expression/LLVMUserExpression.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Expression/UtilityFunction.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/StreamFile.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Target/DynamicRegisterInfo.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -12,7 +12,6 @@
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/DumpRegisterValue.h"
 #include "lldb/Core/Module.h"
-#include "lldb/Core/StreamFile.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Symbol/TypeList.h"
 #include "lldb/Symbol/TypeSystem.h"
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -23,7 +23,6 @@
 #include "lldb/Core/SearchFilter.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/SourceManager.h"
-#include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
@@ -34,6 +33,7 @@
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/PosixApi.h"
+#include "lldb/Host/StreamFile.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionGroupWatchpoint.h"
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -11,7 +11,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/SourceManager.h"
-#include "lldb/Core/StreamFile.h"
+#include "lldb/Host/StreamFile.h"
 #include "lldb/Symbol/Block.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
Index: 

[Lldb-commits] [PATCH] D157463: [lldb] Sink StreamBuffer into lldbUtility

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG20c77a578933: [lldb] Sink StreamBuffer into lldbUtility 
(authored by bulbazord).

Changed prior to commit:
  https://reviews.llvm.org/D157463?vs=548419=548826#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157463

Files:
  lldb/include/lldb/Core/StreamBuffer.h
  lldb/include/lldb/Utility/StreamBuffer.h
  lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h
  lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp

Index: lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
===
--- lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
+++ lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
@@ -10,10 +10,10 @@
 
 #include "Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.h"
 
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/StreamBuffer.h"
 #include "lldb/Utility/StreamString.h"
 #include "llvm/DebugInfo/DIContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFExpression.h"
Index: lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
@@ -9,11 +9,11 @@
 #include "PDBLocationToDWARFExpression.h"
 
 #include "lldb/Core/Section.h"
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Core/dwarf.h"
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Symbol/Variable.h"
 #include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/StreamBuffer.h"
 
 #include "llvm/DebugInfo/CodeView/CodeView.h"
 #include "llvm/DebugInfo/PDB/IPDBSession.h"
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -14,7 +14,6 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/LineTable.h"
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
@@ -9,7 +9,6 @@
 #include "PdbFPOProgramToDWARFExpression.h"
 #include "CodeViewRegisterMapping.h"
 
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Symbol/PostfixExpression.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Stream.h"
Index: lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
@@ -10,10 +10,10 @@
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/StreamBuffer.h"
 
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
Index: lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
===
--- lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
+++ lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
@@ -10,7 +10,6 @@
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
@@ -26,6 +25,7 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegularExpression.h"
+#include "lldb/Utility/StreamBuffer.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 #include "llvm/Support/MemoryBuffer.h"
Index: 

[Lldb-commits] [lldb] 20c77a5 - [lldb] Sink StreamBuffer into lldbUtility

2023-08-09 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-08-09T17:10:13-07:00
New Revision: 20c77a5789331956cd47d1804df7885a82094964

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

LOG: [lldb] Sink StreamBuffer into lldbUtility

lldbUtility seems like a more suitable place for StreamBuffer.

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

Added: 
lldb/include/lldb/Utility/StreamBuffer.h

Modified: 
lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h
lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp

Removed: 
lldb/include/lldb/Core/StreamBuffer.h



diff  --git a/lldb/include/lldb/Core/StreamBuffer.h 
b/lldb/include/lldb/Utility/StreamBuffer.h
similarity index 100%
rename from lldb/include/lldb/Core/StreamBuffer.h
rename to lldb/include/lldb/Utility/StreamBuffer.h

diff  --git a/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h 
b/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h
index 5e11b2d32bfed4..dba8947f9d386b 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h
@@ -14,9 +14,9 @@
 #include 
 
 #include "lldb/Core/Communication.h"
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Utility/Listener.h"
 #include "lldb/Utility/Predicate.h"
+#include "lldb/Utility/StreamBuffer.h"
 #include "lldb/lldb-private.h"
 
 class CommunicationKDP : public lldb_private::Communication {

diff  --git a/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp 
b/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
index 89329d1abf5dcb..62353be170cfb0 100644
--- a/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
+++ b/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
@@ -10,7 +10,6 @@
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
@@ -26,6 +25,7 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegularExpression.h"
+#include "lldb/Utility/StreamBuffer.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 #include "llvm/Support/MemoryBuffer.h"

diff  --git 
a/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
index 9623daa416bbc3..75adf7302f00cd 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
@@ -10,10 +10,10 @@
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/StreamBuffer.h"
 
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/DebugInfo/CodeView/TypeDeserializer.h"

diff  --git 
a/lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
index c45db174e5349a..f28509acbf79fa 100644
--- 
a/lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
+++ 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
@@ -9,7 +9,6 @@
 #include "PdbFPOProgramToDWARFExpression.h"
 #include "CodeViewRegisterMapping.h"
 
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Symbol/PostfixExpression.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Stream.h"

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index f14fb32621b308..89d0f1e91eaa63 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -14,7 +14,6 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
-#include "lldb/Core/StreamBuffer.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/LineTable.h"

diff  --git 
a/lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp 
b/lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
index 94023737b2a2ef..95add31385df6f 

[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/include/lldb/Target/Process.h:348-357
 eBroadcastBitStateChanged = (1 << 0),
 eBroadcastBitInterrupt = (1 << 1),
 eBroadcastBitSTDOUT = (1 << 2),
 eBroadcastBitSTDERR = (1 << 3),
 eBroadcastBitProfileData = (1 << 4),
 eBroadcastBitStructuredData = (1 << 5),
   };

I wonder if it might be better to add a new element to the enum, something like 
`eBroadcastBitAll = eBroadcastBitStageChanged | ... | 
eBroadcastBitStructuredData,`.

If we have to add a new element to this enumeration, I'm not sure everyone will 
realize that `g_all_event_bits` needs to be updated in a separate file (even 
though it's right below the enum definition).



Comment at: lldb/include/lldb/Target/Process.h:616-619
+  void SetShadowListener(lldb::ListenerSP shadow_listener_sp) {
+if (shadow_listener_sp)
+  AddListener(shadow_listener_sp, g_all_event_bits);
+  }

Why was this moved down?



Comment at: lldb/source/Utility/Broadcaster.cpp:246
+// listeners.
+if (!is_hijack) {
+  for (auto  : GetListeners()) {

mib wrote:
> Then, as a follow-up to my suggestion, you could check `hijacking_listener_sp 
> ` instead of checking the boolean.
Instead of having the bool flag for `is_hijack`, you can compare 
`primary_listener_sp` against `hijacking_listener_sp`.

Then the setup above gets less complex, like so:
```
ListenerSP primary_listener_sp = hijacking_listener_sp;
if (!primary_listener_sp)
  primary_listener_sp = m_primary_listener_sp;
```



Comment at: lldb/source/Utility/Broadcaster.cpp:247
+if (!is_hijack) {
+  for (auto  : GetListeners()) {
+if (!(pair.second & event_type))

I wonder if it might be useful to introduce a variant of `GetListeners` which 
takes an `event_type` and only gives you back the ones you want. That would 
simplify this loop (and the one in the other branching path). You could also 
just pass that vector along to the `Event` instead of filtering and adding them 
one at a time (so you're not paying for potential vector resizing costs).



Comment at: lldb/source/Utility/Broadcaster.cpp:253
+  continue;
+if (pair.first != hijacking_listener_sp 
+&& pair.first != m_primary_listener_sp)

nit: Checking to see if `pair.first` is `hijacking_listener_sp` is redundant. 
If you've gotten to this point, `hijacking_listener_sp` must be pointing to 
`nullptr`.



Comment at: lldb/source/Utility/Broadcaster.cpp:254
+if (pair.first != hijacking_listener_sp 
+&& pair.first != m_primary_listener_sp)
+  event_sp->AddPendingListener(pair.first);

Why might `pair.first` be `m_primary_listener_sp`? I would assume that if a 
primary listener is set, it wouldn't be among the other listeners in a 
broadcaster But I suppose nothing stops you from doing that.



Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py:26
 @skipUnlessDarwin
-@skipIfDarwin
+#@skipIfDarwin
 def test_passthrough_launch(self):

Remove this comment?



Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py:38-44
+self, self.dbg.GetListener(), self.mux_process.GetBroadcaster()
+)
+self.assertState(lldb.SBProcess.GetStateFromEvent(event), 
lldb.eStateRunning)
+event = lldbutil.fetch_next_event(
+self, self.dbg.GetListener(), self.mux_process.GetBroadcaster()
+)
+self.assertState(lldb.SBProcess.GetStateFromEvent(event), 
lldb.eStateStopped)

This is to make sure that the primary listener is getting the run/stop events 
before the `mux_process_listener` right?



Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py:56
 @skipUnlessDarwin
-@skipIfDarwin
+#@skipIfDarwin
 def test_multiplexed_launch(self):

delete?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157556

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


[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

2023-08-09 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

LGTM with some comments :)




Comment at: lldb/source/Target/Process.cpp:372
 
+const int Process::g_all_event_bits = eBroadcastBitStateChanged 
+  | eBroadcastBitInterrupt

nit: this could probably be `constexpr`



Comment at: lldb/source/Utility/Broadcaster.cpp:229-235
+  ListenerSP primary_listener_sp = hijacking_listener_sp;
+  bool is_hijack = false;
+  
+  if (primary_listener_sp)
+is_hijack = true;
+  else
+primary_listener_sp = m_primary_listener_sp;

nit: We could simplify the logic here a little bit



Comment at: lldb/source/Utility/Broadcaster.cpp:246
+// listeners.
+if (!is_hijack) {
+  for (auto  : GetListeners()) {

Then, as a follow-up to my suggestion, you could check `hijacking_listener_sp ` 
instead of checking the boolean.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157556

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


[Lldb-commits] [PATCH] D157455: [lldb][NFCI] Remove MappedHash.h

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG44c876fd75c2: [lldb][NFCI] Remove MappedHash.h (authored by 
bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157455

Files:
  lldb/include/lldb/Core/MappedHash.h
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp

Index: lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
@@ -10,7 +10,6 @@
 #include "ObjCLanguageRuntime.h"
 
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
-#include "lldb/Core/MappedHash.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/ValueObject.h"
Index: lldb/include/lldb/Core/MappedHash.h
===
--- lldb/include/lldb/Core/MappedHash.h
+++ /dev/null
@@ -1,308 +0,0 @@
-//===-- MappedHash.h *- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLDB_CORE_MAPPEDHASH_H
-#define LLDB_CORE_MAPPEDHASH_H
-
-#include 
-#include 
-
-#include 
-#include 
-#include 
-#include 
-
-#include "lldb/Utility/DataExtractor.h"
-#include "lldb/Utility/Stream.h"
-#include "llvm/Support/DJB.h"
-
-class MappedHash {
-public:
-  enum HashFunctionType {
-eHashFunctionDJB = 0u // Daniel J Bernstein hash function that is also used
-  // by the ELF GNU_HASH sections
-  };
-
-  static uint32_t HashString(uint32_t hash_function, llvm::StringRef s) {
-switch (hash_function) {
-case MappedHash::eHashFunctionDJB:
-  return llvm::djbHash(s);
-
-default:
-  break;
-}
-llvm_unreachable("Invalid hash function index");
-  }
-
-  static const uint32_t HASH_MAGIC = 0x48415348u;
-  static const uint32_t HASH_CIGAM = 0x48534148u;
-
-  template  struct Header {
-typedef T HeaderData;
-
-uint32_t
-magic; // HASH_MAGIC or HASH_CIGAM magic value to allow endian detection
-uint16_t version = 1; // Version number
-uint16_t hash_function =
-eHashFunctionDJB;  // The hash function enumeration that was used
-uint32_t bucket_count = 0; // The number of buckets in this hash table
-uint32_t hashes_count = 0; // The total number of unique hash values and
-   // hash data offsets in this table
-uint32_t header_data_len; // The size in bytes of the "header_data" template
-  // member below
-HeaderData header_data;   //
-
-Header() : magic(HASH_MAGIC), header_data_len(sizeof(T)), header_data() {}
-
-virtual ~Header() = default;
-
-size_t GetByteSize() const {
-  return sizeof(magic) + sizeof(version) + sizeof(hash_function) +
- sizeof(bucket_count) + sizeof(hashes_count) +
- sizeof(header_data_len) + header_data_len;
-}
-
-virtual size_t GetByteSize(const HeaderData _data) = 0;
-
-void SetHeaderDataByteSize(uint32_t header_data_byte_size) {
-  header_data_len = header_data_byte_size;
-}
-
-void Dump(lldb_private::Stream ) {
-  s.Printf("header.magic  = 0x%8.8x\n", magic);
-  s.Printf("header.version= 0x%4.4x\n", version);
-  s.Printf("header.hash_function  = 0x%4.4x\n", hash_function);
-  s.Printf("header.bucket_count   = 0x%8.8x %u\n", bucket_count,
-   bucket_count);
-  s.Printf("header.hashes_count   = 0x%8.8x %u\n", hashes_count,
-   hashes_count);
-  s.Printf("header.header_data_len= 0x%8.8x %u\n", header_data_len,
-   header_data_len);
-}
-
-virtual lldb::offset_t Read(lldb_private::DataExtractor ,
-lldb::offset_t offset) {
-  if (data.ValidOffsetForDataOfSize(
-  offset, sizeof(magic) + sizeof(version) + sizeof(hash_function) +
-  sizeof(bucket_count) + sizeof(hashes_count) +
-  sizeof(header_data_len))) {
-magic = data.GetU32();
-if (magic != HASH_MAGIC) {
-  if (magic == HASH_CIGAM) {
-switch (data.GetByteOrder()) {
-case lldb::eByteOrderBig:
-  data.SetByteOrder(lldb::eByteOrderLittle);
-  break;
-case lldb::eByteOrderLittle:
-  data.SetByteOrder(lldb::eByteOrderBig);
-  break;
-default:
-  return LLDB_INVALID_OFFSET;
-}
-  } else 

[Lldb-commits] [lldb] 44c876f - [lldb][NFCI] Remove MappedHash.h

2023-08-09 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-08-09T15:48:23-07:00
New Revision: 44c876fd75c26482d197a5c5312eac6f43fe

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

LOG: [lldb][NFCI] Remove MappedHash.h

This is no longer used as of 8e71d14972b48df8c5b701a9aec19af3194f9a5e.

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

Added: 


Modified: 
lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp

Removed: 
lldb/include/lldb/Core/MappedHash.h



diff  --git a/lldb/include/lldb/Core/MappedHash.h 
b/lldb/include/lldb/Core/MappedHash.h
deleted file mode 100644
index 7bd3c3a7844900..00
--- a/lldb/include/lldb/Core/MappedHash.h
+++ /dev/null
@@ -1,308 +0,0 @@
-//===-- MappedHash.h *- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLDB_CORE_MAPPEDHASH_H
-#define LLDB_CORE_MAPPEDHASH_H
-
-#include 
-#include 
-
-#include 
-#include 
-#include 
-#include 
-
-#include "lldb/Utility/DataExtractor.h"
-#include "lldb/Utility/Stream.h"
-#include "llvm/Support/DJB.h"
-
-class MappedHash {
-public:
-  enum HashFunctionType {
-eHashFunctionDJB = 0u // Daniel J Bernstein hash function that is also used
-  // by the ELF GNU_HASH sections
-  };
-
-  static uint32_t HashString(uint32_t hash_function, llvm::StringRef s) {
-switch (hash_function) {
-case MappedHash::eHashFunctionDJB:
-  return llvm::djbHash(s);
-
-default:
-  break;
-}
-llvm_unreachable("Invalid hash function index");
-  }
-
-  static const uint32_t HASH_MAGIC = 0x48415348u;
-  static const uint32_t HASH_CIGAM = 0x48534148u;
-
-  template  struct Header {
-typedef T HeaderData;
-
-uint32_t
-magic; // HASH_MAGIC or HASH_CIGAM magic value to allow endian 
detection
-uint16_t version = 1; // Version number
-uint16_t hash_function =
-eHashFunctionDJB;  // The hash function enumeration that was used
-uint32_t bucket_count = 0; // The number of buckets in this hash table
-uint32_t hashes_count = 0; // The total number of unique hash values and
-   // hash data offsets in this table
-uint32_t header_data_len; // The size in bytes of the "header_data" 
template
-  // member below
-HeaderData header_data;   //
-
-Header() : magic(HASH_MAGIC), header_data_len(sizeof(T)), header_data() {}
-
-virtual ~Header() = default;
-
-size_t GetByteSize() const {
-  return sizeof(magic) + sizeof(version) + sizeof(hash_function) +
- sizeof(bucket_count) + sizeof(hashes_count) +
- sizeof(header_data_len) + header_data_len;
-}
-
-virtual size_t GetByteSize(const HeaderData _data) = 0;
-
-void SetHeaderDataByteSize(uint32_t header_data_byte_size) {
-  header_data_len = header_data_byte_size;
-}
-
-void Dump(lldb_private::Stream ) {
-  s.Printf("header.magic  = 0x%8.8x\n", magic);
-  s.Printf("header.version= 0x%4.4x\n", version);
-  s.Printf("header.hash_function  = 0x%4.4x\n", hash_function);
-  s.Printf("header.bucket_count   = 0x%8.8x %u\n", bucket_count,
-   bucket_count);
-  s.Printf("header.hashes_count   = 0x%8.8x %u\n", hashes_count,
-   hashes_count);
-  s.Printf("header.header_data_len= 0x%8.8x %u\n", header_data_len,
-   header_data_len);
-}
-
-virtual lldb::offset_t Read(lldb_private::DataExtractor ,
-lldb::offset_t offset) {
-  if (data.ValidOffsetForDataOfSize(
-  offset, sizeof(magic) + sizeof(version) + sizeof(hash_function) +
-  sizeof(bucket_count) + sizeof(hashes_count) +
-  sizeof(header_data_len))) {
-magic = data.GetU32();
-if (magic != HASH_MAGIC) {
-  if (magic == HASH_CIGAM) {
-switch (data.GetByteOrder()) {
-case lldb::eByteOrderBig:
-  data.SetByteOrder(lldb::eByteOrderLittle);
-  break;
-case lldb::eByteOrderLittle:
-  data.SetByteOrder(lldb::eByteOrderBig);
-  break;
-default:
-  return LLDB_INVALID_OFFSET;
-}
-  } else {
-// Magic bytes didn't match
-version = 0;
-return LLDB_INVALID_OFFSET;
-  }
-}
-
-version = data.GetU16();
-if (version != 1) {
-  

[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

2023-08-09 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision.
jingham added reviewers: mib, bulbazord, clayborg, JDevlieghere.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Allow a Broadcaster to have one "Primary" listener, and potentially many 
secondary Listeners.  The primary listener is guaranteed to run the DoOnRemoval 
first, and only when that is complete will secondary Listeners receive the 
event.

Prior to Ismail's addition of the notion of a "Secondary" listener, you could 
only safely observe process events from the Listener you passed in to the 
Process creation.  That was because the act of pulling the process event off 
the event queue triggered the change of the public state of the process (thus 
keeping the event stream and the state in sync).

Ismail added a privileged "shadow listener" that could also be informed of the 
event.  That wasn't quite the right model, however, because the real singleton 
is the primary listener.

That's what this patch implements.

This isn't quite the full solution for the Process Listener.  The goal is that 
the primary Process Listener not only gets to drive the event stream, but that 
the primary listener gets to react to each event before any other Listener can 
hear about it.  That will allow the process execution logic to run before any 
other agent gets a chance to change the process state out from under it.  For 
that, I'll need to add a receipt mechanism to the event delivery, and have the 
forwarding to the pending listeners happen only after the receipt is 
acknowledged.  But that will be a follow on patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157556

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/include/lldb/Utility/Event.h
  lldb/source/Target/Process.cpp
  lldb/source/Utility/Broadcaster.cpp
  lldb/source/Utility/Event.cpp
  lldb/source/Utility/Listener.cpp
  
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py
  lldb/test/API/python_api/event/TestEvents.py

Index: lldb/test/API/python_api/event/TestEvents.py
===
--- lldb/test/API/python_api/event/TestEvents.py
+++ lldb/test/API/python_api/event/TestEvents.py
@@ -313,3 +313,121 @@
 self.assertEqual(
 self.state, "stopped", "Both expected state changed events received"
 )
+
+def wait_for_next_event(self, expected_state, test_shadow = False):
+"""Wait for an event from self.primary & self.shadow listener.
+   If test_shadow is true, we also check that the shadow listener only 
+   receives events AFTER the primary listener does."""
+# Waiting on the shadow listener shouldn't have events yet because
+# we haven't fetched them for the primary listener yet:
+event = lldb.SBEvent()
+
+if test_shadow:
+success = self.shadow_listener.WaitForEvent(1, event)
+self.assertFalse(success, "Shadow listener doesn't pull events")
+
+# But there should be an event for the primary listener:
+success = self.primary_listener.WaitForEvent(5, event)
+self.assertTrue(success, "Primary listener got the event")
+
+state = lldb.SBProcess.GetStateFromEvent(event)
+restart = False
+if state == lldb.eStateStopped:
+restart = lldb.SBProcess.GetRestartedFromEvent(event)
+
+if expected_state != None:
+self.assertEqual(state, expected_state, "Primary thread got the correct event")
+
+# And after pulling that one there should be an equivalent event for the shadow
+# listener:
+success = self.shadow_listener.WaitForEvent(5, event)
+self.assertTrue(success, "Shadow listener got event too")
+self.assertEqual(state, lldb.SBProcess.GetStateFromEvent(event), "It was the same event")
+self.assertEqual(restart, lldb.SBProcess.GetRestartedFromEvent(event), "It was the same restarted")
+
+return state, restart
+
+def test_shadow_listener(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+# Create a target by the debugger.
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Now create a breakpoint on main.c by name 'c'.
+bkpt1 = target.BreakpointCreateByName("c", "a.out")
+self.trace("breakpoint:", bkpt1)
+self.assertTrue(bkpt1.GetNumLocations() == 1, VALID_BREAKPOINT)
+
+self.primary_listener = lldb.SBListener("my listener")
+self.shadow_listener = lldb.SBListener("shadow listener")
+
+self.cur_thread = None
+
+error = lldb.SBError()
+launch_info = target.GetLaunchInfo()
+launch_info.SetListener(self.primary_listener)
+

[Lldb-commits] [PATCH] D157361: [lldb] FIx data race in ThreadedCommunication

2023-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a8d9a7657bb: [lldb] Fix data race in ThreadedCommunication 
(authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157361

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


Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -155,6 +156,8 @@
 }
 
 bool ThreadedCommunication::StartReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (error_ptr)
 error_ptr->Clear();
 
@@ -189,6 +192,8 @@
 }
 
 bool ThreadedCommunication::StopReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
@@ -199,13 +204,13 @@
 
   BroadcastEvent(eBroadcastBitReadThreadShouldExit, nullptr);
 
-  // error = m_read_thread.Cancel();
-
   Status error = m_read_thread.Join(nullptr);
   return error.Success();
 }
 
 bool ThreadedCommunication::JoinReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
Index: lldb/include/lldb/Core/ThreadedCommunication.h
===
--- lldb/include/lldb/Core/ThreadedCommunication.h
+++ lldb/include/lldb/Core/ThreadedCommunication.h
@@ -223,10 +223,22 @@
   }
 
 protected:
-  HostThread m_read_thread; ///< The read thread handle in case we need to
-/// cancel the thread.
+  /// The read thread handle in case we need to cancel the thread.
+  /// @{
+  HostThread m_read_thread;
+  std::mutex m_read_thread_mutex;
+  /// @}
+
+  /// Whether the read thread is enabled. This cannot be guarded by the read
+  /// thread mutex becuase it is used as the control variable to exit the read
+  /// thread.
   std::atomic m_read_thread_enabled;
+
+  /// Whether the read thread is enabled. Technically this could be guarded by
+  /// the read thread mutex but that needlessly complicates things to
+  /// check this variables momentary value.
   std::atomic m_read_thread_did_exit;
+
   std::string
   m_bytes; ///< A buffer to cache bytes read in the ReadThread function.
   std::recursive_mutex m_bytes_mutex;   ///< A mutex to protect multi-threaded


Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -155,6 +156,8 @@
 }
 
 bool ThreadedCommunication::StartReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (error_ptr)
 error_ptr->Clear();
 
@@ -189,6 +192,8 @@
 }
 
 bool ThreadedCommunication::StopReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
@@ -199,13 +204,13 @@
 
   BroadcastEvent(eBroadcastBitReadThreadShouldExit, nullptr);
 
-  // error = m_read_thread.Cancel();
-
   Status error = m_read_thread.Join(nullptr);
   return error.Success();
 }
 
 bool ThreadedCommunication::JoinReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
Index: lldb/include/lldb/Core/ThreadedCommunication.h
===
--- lldb/include/lldb/Core/ThreadedCommunication.h
+++ lldb/include/lldb/Core/ThreadedCommunication.h
@@ -223,10 +223,22 @@
   }
 
 protected:
-  HostThread m_read_thread; ///< The read thread handle in case we need to
-/// cancel the thread.
+  /// The read thread handle in case we need to cancel the thread.
+  /// @{
+  HostThread m_read_thread;
+  std::mutex m_read_thread_mutex;
+  /// @}
+
+  /// Whether the read thread is enabled. This cannot be guarded by the read
+  /// thread mutex becuase it is used as the control variable to exit the read
+  /// thread.
   std::atomic m_read_thread_enabled;
+
+  /// Whether the read thread is enabled. Technically this could be guarded by
+  /// the read thread mutex but that needlessly complicates things to
+  /// check this variables momentary value.
   std::atomic m_read_thread_did_exit;
+
   std::string
   m_bytes; ///< A buffer to cache bytes read in the ReadThread function.
   std::recursive_mutex m_bytes_mutex;   ///< A mutex to protect multi-threaded
___
lldb-commits mailing list

[Lldb-commits] [lldb] 1a8d9a7 - [lldb] Fix data race in ThreadedCommunication

2023-08-09 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2023-08-09T15:15:42-07:00
New Revision: 1a8d9a7657bba79099e6e2a6b0568db53d1e9a23

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

LOG: [lldb] Fix data race in ThreadedCommunication

TSan reports the following race:

  Write of size 8 at 0x000107707ee8 by main thread:
#0 lldb_private::ThreadedCommunication::StartReadThread(...) 
ThreadedCommunication.cpp:175
#1 lldb_private::Process::SetSTDIOFileDescriptor(...) Process.cpp:4533
#2 lldb_private::Platform::DebugProcess(...) Platform.cpp:1121
#3 lldb_private::PlatformDarwin::DebugProcess(...) PlatformDarwin.cpp:711
#4 lldb_private::Target::Launch(...) Target.cpp:3235
#5 CommandObjectProcessLaunch::DoExecute(...) CommandObjectProcess.cpp:256
#6 lldb_private::CommandObjectParsed::Execute(...) CommandObject.cpp:751
#7 lldb_private::CommandInterpreter::HandleCommand(...) 
CommandInterpreter.cpp:2054

  Previous read of size 8 at 0x000107707ee8 by thread T5:
#0 lldb_private::HostThread::IsJoinable(...) const HostThread.cpp:30
#1 lldb_private::ThreadedCommunication::StopReadThread(...) 
ThreadedCommunication.cpp:192
#2 lldb_private::Process::ShouldBroadcastEvent(...) Process.cpp:3420
#3 lldb_private::Process::HandlePrivateEvent(...) Process.cpp:3728
#4 lldb_private::Process::RunPrivateStateThread(...) Process.cpp:3914
#5 
std::__1::__function::__funchttps://reviews.llvm.org/D157361

Added: 


Modified: 
lldb/include/lldb/Core/ThreadedCommunication.h
lldb/source/Core/ThreadedCommunication.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ThreadedCommunication.h 
b/lldb/include/lldb/Core/ThreadedCommunication.h
index 2e3afde3c05826..7ebb77beb77f3d 100644
--- a/lldb/include/lldb/Core/ThreadedCommunication.h
+++ b/lldb/include/lldb/Core/ThreadedCommunication.h
@@ -223,10 +223,22 @@ class ThreadedCommunication : public Communication, 
public Broadcaster {
   }
 
 protected:
-  HostThread m_read_thread; ///< The read thread handle in case we need to
-/// cancel the thread.
+  /// The read thread handle in case we need to cancel the thread.
+  /// @{
+  HostThread m_read_thread;
+  std::mutex m_read_thread_mutex;
+  /// @}
+
+  /// Whether the read thread is enabled. This cannot be guarded by the read
+  /// thread mutex becuase it is used as the control variable to exit the read
+  /// thread.
   std::atomic m_read_thread_enabled;
+
+  /// Whether the read thread is enabled. Technically this could be guarded by
+  /// the read thread mutex but that needlessly complicates things to
+  /// check this variables momentary value.
   std::atomic m_read_thread_did_exit;
+
   std::string
   m_bytes; ///< A buffer to cache bytes read in the ReadThread function.
   std::recursive_mutex m_bytes_mutex;   ///< A mutex to protect multi-threaded

diff  --git a/lldb/source/Core/ThreadedCommunication.cpp 
b/lldb/source/Core/ThreadedCommunication.cpp
index 755a158a5359e9..7d8aae5d8ff689 100644
--- a/lldb/source/Core/ThreadedCommunication.cpp
+++ b/lldb/source/Core/ThreadedCommunication.cpp
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -155,6 +156,8 @@ size_t ThreadedCommunication::Read(void *dst, size_t 
dst_len,
 }
 
 bool ThreadedCommunication::StartReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (error_ptr)
 error_ptr->Clear();
 
@@ -189,6 +192,8 @@ bool ThreadedCommunication::StartReadThread(Status 
*error_ptr) {
 }
 
 bool ThreadedCommunication::StopReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
@@ -199,13 +204,13 @@ bool ThreadedCommunication::StopReadThread(Status 
*error_ptr) {
 
   BroadcastEvent(eBroadcastBitReadThreadShouldExit, nullptr);
 
-  // error = m_read_thread.Cancel();
-
   Status error = m_read_thread.Join(nullptr);
   return error.Success();
 }
 
 bool ThreadedCommunication::JoinReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 



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


[Lldb-commits] [PATCH] D156970: Fix crash in lldb-vscode when missing function name

2023-08-09 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:699-703
+  // `function_name` can be a nullptr, which throws an error when assigned to 
an
+  // `std::string`.
+  const char *function_name = frame.GetDisplayFunctionName();
+  std::string frame_name =
+  function_name == nullptr ? std::string() : function_name;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156970

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


[Lldb-commits] [PATCH] D157455: [lldb][NFCI] Remove MappedHash.h

2023-08-09 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/D157455/new/

https://reviews.llvm.org/D157455

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


[Lldb-commits] [PATCH] D157538: [lldb][NFCI] Remove use of ifdef __cpluplus where unneeded

2023-08-09 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/D157538/new/

https://reviews.llvm.org/D157538

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


[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

Looks right to me now. Thanks!


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

https://reviews.llvm.org/D157347

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


[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 548729.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

Address @bulbazord's code review feedback


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

https://reviews.llvm.org/D157347

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Host/common/File.cpp

Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -219,7 +219,7 @@
 size_t File::PrintfVarArg(const char *format, va_list args) {
   llvm::SmallString<0> s;
   if (VASprintf(s, format, args)) {
-size_t written = s.size();;
+size_t written = s.size();
 Write(s.data(), written);
 return written;
   }
@@ -247,15 +247,20 @@
   return file_stats.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
 }
 
+bool NativeFile::IsValid() const {
+  return DescriptorIsValid() || StreamIsValid();
+}
+
 Expected NativeFile::GetOptions() const { return m_options; }
 
 int NativeFile::GetDescriptor() const {
-  if (DescriptorIsValid())
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 return m_descriptor;
+  }
 
   // Don't open the file descriptor if we don't need to, just get it from the
   // stream if we have one.
-  if (StreamIsValid()) {
+  if (ValueGuard stream_guard = StreamIsValid()) {
 #if defined(_WIN32)
 return _fileno(m_stream);
 #else
@@ -272,8 +277,9 @@
 }
 
 FILE *NativeFile::GetStream() {
-  if (!StreamIsValid()) {
-if (DescriptorIsValid()) {
+  ValueGuard stream_guard = StreamIsValid();
+  if (!stream_guard) {
+if (ValueGuard descriptor_guard = DescriptorIsValid()) {
   auto mode = GetStreamOpenModeFromOptions(m_options);
   if (!mode)
 llvm::consumeError(mode.takeError());
@@ -306,8 +312,11 @@
 }
 
 Status NativeFile::Close() {
+  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
+
   Status error;
-  if (StreamIsValid()) {
+
+  if (StreamIsValidUnlocked()) {
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
@@ -322,15 +331,17 @@
   }
 }
   }
-  if (DescriptorIsValid() && m_own_descriptor) {
+
+  if (DescriptorIsValidUnlocked() && m_own_descriptor) {
 if (::close(m_descriptor) != 0)
   error.SetErrorToErrno();
   }
-  m_descriptor = kInvalidDescriptor;
+
   m_stream = kInvalidStream;
-  m_options = OpenOptions(0);
   m_own_stream = false;
+  m_descriptor = kInvalidDescriptor;
   m_own_descriptor = false;
+  m_options = OpenOptions(0);
   m_is_interactive = eLazyBoolCalculate;
   m_is_real_terminal = eLazyBoolCalculate;
   return error;
@@ -374,7 +385,7 @@
 
 off_t NativeFile::SeekFromStart(off_t offset, Status *error_ptr) {
   off_t result = 0;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -383,7 +394,7 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -400,7 +411,7 @@
 
 off_t NativeFile::SeekFromCurrent(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -409,7 +420,7 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -426,7 +437,7 @@
 
 off_t NativeFile::SeekFromEnd(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_END);
 
 if (error_ptr) {
@@ -435,7 +446,7 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_END);
 
 if (error_ptr) {
@@ -452,18 +463,23 @@
 
 Status NativeFile::Flush() {
   Status error;
-  if (StreamIsValid()) {
+  if (ValueGuard stream_guard = StreamIsValid()) {
 if (llvm::sys::RetryAfterSignal(EOF, ::fflush, m_stream) == EOF)
   error.SetErrorToErrno();
-  } else if (!DescriptorIsValid()) {
-error.SetErrorString("invalid file handle");
+return error;
+  }
+
+  {
+ValueGuard descriptor_guard = DescriptorIsValid();
+if (!descriptor_guard)
+  error.SetErrorString("invalid file handle");
   }
   return error;
 }
 
 Status NativeFile::Sync() {
   Status error;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 #ifdef _WIN32
 int err = FlushFileBuffers((HANDLE)_get_osfhandle(m_descriptor));
 if (err == 0)

[Lldb-commits] [PATCH] D157538: [lldb][NFCI] Remove use of ifdef __cpluplus where unneeded

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jasonmolenda, jingham.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Just about every file in the lldb project is built with C++ enabled.
Unless I've missed something, these macro guards don't really accomplish
very much.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157538

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/include/lldb/Host/Editline.h
  lldb/include/lldb/Host/Terminal.h
  lldb/include/lldb/Host/posix/PipePosix.h
  lldb/include/lldb/Utility/DataBuffer.h
  lldb/include/lldb/Utility/DataEncoder.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/include/lldb/lldb-private-types.h
  lldb/include/lldb/lldb-private.h
  lldb/source/Host/macosx/cfcpp/CFCReleaser.h
  lldb/source/Host/macosx/objcxx/PosixSpawnResponsible.h
  lldb/tools/debugserver/source/DNBLog.h
  lldb/tools/debugserver/source/DNBRegisterInfo.h
  lldb/tools/debugserver/source/MacOSX/CFUtils.h

Index: lldb/tools/debugserver/source/MacOSX/CFUtils.h
===
--- lldb/tools/debugserver/source/MacOSX/CFUtils.h
+++ lldb/tools/debugserver/source/MacOSX/CFUtils.h
@@ -15,8 +15,6 @@
 
 #include 
 
-#ifdef __cplusplus
-
 // Templatized CF helper class that can own any CF pointer and will
 // call CFRelease() on any valid pointer it owns unless that pointer is
 // explicitly released using the release() member function.
@@ -71,5 +69,4 @@
   element_type _ptr;
 };
 
-#endif // #ifdef __cplusplus
 #endif // LLDB_TOOLS_DEBUGSERVER_SOURCE_MACOSX_CFUTILS_H
Index: lldb/tools/debugserver/source/DNBRegisterInfo.h
===
--- lldb/tools/debugserver/source/DNBRegisterInfo.h
+++ lldb/tools/debugserver/source/DNBRegisterInfo.h
@@ -18,12 +18,10 @@
 #include 
 
 struct DNBRegisterValueClass : public DNBRegisterValue {
-#ifdef __cplusplus
   DNBRegisterValueClass(const DNBRegisterInfo *regInfo = NULL);
   void Clear();
   void Dump(const char *pre, const char *post) const;
   bool IsValid() const;
-#endif
 };
 
 #endif
Index: lldb/tools/debugserver/source/DNBLog.h
===
--- lldb/tools/debugserver/source/DNBLog.h
+++ lldb/tools/debugserver/source/DNBLog.h
@@ -17,9 +17,7 @@
 #include 
 #include 
 
-#ifdef __cplusplus
 extern "C" {
-#endif
 
 // Flags that get filled in automatically before calling the log callback
 // function
@@ -144,9 +142,6 @@
 #define DNBLogCloseLogFile() ((void)0)
 
 #endif // #else defined(DNBLOG_ENABLED)
-
-#ifdef __cplusplus
 }
-#endif
 
 #endif // LLDB_TOOLS_DEBUGSERVER_SOURCE_DNBLOG_H
Index: lldb/source/Host/macosx/objcxx/PosixSpawnResponsible.h
===
--- lldb/source/Host/macosx/objcxx/PosixSpawnResponsible.h
+++ lldb/source/Host/macosx/objcxx/PosixSpawnResponsible.h
@@ -31,9 +31,7 @@
 static dispatch_once_t pred;
 dispatch_once(, ^{
   responsibility_spawnattrs_setdisclaim_ptr =
-#ifdef __cplusplus
   reinterpret_cast<__typeof__(_spawnattrs_setdisclaim)>
-#endif
   (dlsym(RTLD_DEFAULT, "responsibility_spawnattrs_setdisclaim"));
 });
 if (responsibility_spawnattrs_setdisclaim_ptr)
Index: lldb/source/Host/macosx/cfcpp/CFCReleaser.h
===
--- lldb/source/Host/macosx/cfcpp/CFCReleaser.h
+++ lldb/source/Host/macosx/cfcpp/CFCReleaser.h
@@ -11,8 +11,6 @@
 
 #include 
 
-#ifdef __cplusplus
-
 #include 
 
 // Templatized CF helper class that can own any CF pointer and will
@@ -105,5 +103,4 @@
   T _ptr;
 };
 
-#endif // #ifdef __cplusplus
 #endif // LLDB_SOURCE_HOST_MACOSX_CFCPP_CFCRELEASER_H
Index: lldb/include/lldb/lldb-private.h
===
--- lldb/include/lldb/lldb-private.h
+++ lldb/include/lldb/lldb-private.h
@@ -9,13 +9,9 @@
 #ifndef LLDB_LLDB_PRIVATE_H
 #define LLDB_LLDB_PRIVATE_H
 
-#if defined(__cplusplus)
-
 #include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-private-interfaces.h"
 #include "lldb/lldb-private-types.h"
 #include "lldb/lldb-public.h"
 
-#endif // defined(__cplusplus)
-
 #endif // LLDB_LLDB_PRIVATE_H
Index: lldb/include/lldb/lldb-private-types.h
===
--- lldb/include/lldb/lldb-private-types.h
+++ lldb/include/lldb/lldb-private-types.h
@@ -9,8 +9,6 @@
 #ifndef LLDB_LLDB_PRIVATE_TYPES_H
 #define LLDB_LLDB_PRIVATE_TYPES_H
 
-#if defined(__cplusplus)
-
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/ArrayRef.h"
@@ -125,6 +123,4 @@
 void *baton, const char **argv, lldb_private::CommandReturnObject );
 } // namespace lldb_private
 
-#endif // #if defined(__cplusplus)
-
 #endif // LLDB_LLDB_PRIVATE_TYPES_H
Index: 

[Lldb-commits] [PATCH] D157450: [lldb][NFCI] Remove unused IOStreamMacros

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG335d0e3a6e8d: [lldb][NFCI] Remove unused IOStreamMacros 
(authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157450

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


Index: lldb/include/lldb/Core/IOStreamMacros.h
===
--- lldb/include/lldb/Core/IOStreamMacros.h
+++ /dev/null
@@ -1,41 +0,0 @@
-//===-- IOStreamMacros.h *- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef liblldb_IOStreamMacros_h_
-#define liblldb_IOStreamMacros_h_
-#if defined(__cplusplus)
-
-#include 
-
-#define RAW_HEXBASE std::setfill('0') << std::hex << std::right
-#define HEXBASE '0' << 'x' << RAW_HEXBASE
-#define RAWHEX8(x) RAW_HEXBASE << std::setw(2) << ((uint32_t)(x))
-#define RAWHEX16 RAW_HEXBASE << std::setw(4)
-#define RAWHEX32 RAW_HEXBASE << std::setw(8)
-#define RAWHEX64 RAW_HEXBASE << std::setw(16)
-#define HEX8(x) HEXBASE << std::setw(2) << ((uint32_t)(x))
-#define HEX16 HEXBASE << std::setw(4)
-#define HEX32 HEXBASE << std::setw(8)
-#define HEX64 HEXBASE << std::setw(16)
-#define RAW_HEX(x) RAW_HEXBASE << std::setw(sizeof(x) * 2) << (x)
-#define HEX(x) HEXBASE << std::setw(sizeof(x) * 2) << (x)
-#define HEX_SIZE(x, sz) HEXBASE << std::setw((sz)) << (x)
-#define STRING_WIDTH(w) std::setfill(' ') << std::setw(w)
-#define LEFT_STRING_WIDTH(s, w)
\
-  std::left << std::setfill(' ') << std::setw(w) << (s) << std::right
-#define DECIMAL std::dec << std::setfill(' ')
-#define DECIMAL_WIDTH(w) DECIMAL << std::setw(w)
-//#define FLOAT(n, d)   std::setfill(' ') << std::setw((n)+(d)+1) <<
-//std::setprecision(d) << std::showpoint << std::fixed
-#define INDENT_WITH_SPACES(iword_idx)  
\
-  std::setfill(' ') << std::setw((iword_idx)) << ""
-#define INDENT_WITH_TABS(iword_idx)
\
-  std::setfill('\t') << std::setw((iword_idx)) << ""
-
-#endif // #if defined(__cplusplus)
-#endif // liblldb_IOStreamMacros_h_


Index: lldb/include/lldb/Core/IOStreamMacros.h
===
--- lldb/include/lldb/Core/IOStreamMacros.h
+++ /dev/null
@@ -1,41 +0,0 @@
-//===-- IOStreamMacros.h *- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef liblldb_IOStreamMacros_h_
-#define liblldb_IOStreamMacros_h_
-#if defined(__cplusplus)
-
-#include 
-
-#define RAW_HEXBASE std::setfill('0') << std::hex << std::right
-#define HEXBASE '0' << 'x' << RAW_HEXBASE
-#define RAWHEX8(x) RAW_HEXBASE << std::setw(2) << ((uint32_t)(x))
-#define RAWHEX16 RAW_HEXBASE << std::setw(4)
-#define RAWHEX32 RAW_HEXBASE << std::setw(8)
-#define RAWHEX64 RAW_HEXBASE << std::setw(16)
-#define HEX8(x) HEXBASE << std::setw(2) << ((uint32_t)(x))
-#define HEX16 HEXBASE << std::setw(4)
-#define HEX32 HEXBASE << std::setw(8)
-#define HEX64 HEXBASE << std::setw(16)
-#define RAW_HEX(x) RAW_HEXBASE << std::setw(sizeof(x) * 2) << (x)
-#define HEX(x) HEXBASE << std::setw(sizeof(x) * 2) << (x)
-#define HEX_SIZE(x, sz) HEXBASE << std::setw((sz)) << (x)
-#define STRING_WIDTH(w) std::setfill(' ') << std::setw(w)
-#define LEFT_STRING_WIDTH(s, w)\
-  std::left << std::setfill(' ') << std::setw(w) << (s) << std::right
-#define DECIMAL std::dec << std::setfill(' ')
-#define DECIMAL_WIDTH(w) DECIMAL << std::setw(w)
-//#define FLOAT(n, d)   std::setfill(' ') << std::setw((n)+(d)+1) <<
-//std::setprecision(d) << std::showpoint << std::fixed
-#define INDENT_WITH_SPACES(iword_idx)  \
-  std::setfill(' ') << std::setw((iword_idx)) << ""
-#define INDENT_WITH_TABS(iword_idx)\
-  std::setfill('\t') << std::setw((iword_idx)) << ""
-
-#endif // #if defined(__cplusplus)
-#endif // liblldb_IOStreamMacros_h_
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 335d0e3 - [lldb][NFCI] Remove unused IOStreamMacros

2023-08-09 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-08-09T11:42:41-07:00
New Revision: 335d0e3a6e8de8cccd535d9dbaa3df5600884ad7

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

LOG: [lldb][NFCI] Remove unused IOStreamMacros

These are unused AFAICT.

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

Added: 


Modified: 


Removed: 
lldb/include/lldb/Core/IOStreamMacros.h



diff  --git a/lldb/include/lldb/Core/IOStreamMacros.h 
b/lldb/include/lldb/Core/IOStreamMacros.h
deleted file mode 100644
index 45bde88f94410b..00
--- a/lldb/include/lldb/Core/IOStreamMacros.h
+++ /dev/null
@@ -1,41 +0,0 @@
-//===-- IOStreamMacros.h *- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef liblldb_IOStreamMacros_h_
-#define liblldb_IOStreamMacros_h_
-#if defined(__cplusplus)
-
-#include 
-
-#define RAW_HEXBASE std::setfill('0') << std::hex << std::right
-#define HEXBASE '0' << 'x' << RAW_HEXBASE
-#define RAWHEX8(x) RAW_HEXBASE << std::setw(2) << ((uint32_t)(x))
-#define RAWHEX16 RAW_HEXBASE << std::setw(4)
-#define RAWHEX32 RAW_HEXBASE << std::setw(8)
-#define RAWHEX64 RAW_HEXBASE << std::setw(16)
-#define HEX8(x) HEXBASE << std::setw(2) << ((uint32_t)(x))
-#define HEX16 HEXBASE << std::setw(4)
-#define HEX32 HEXBASE << std::setw(8)
-#define HEX64 HEXBASE << std::setw(16)
-#define RAW_HEX(x) RAW_HEXBASE << std::setw(sizeof(x) * 2) << (x)
-#define HEX(x) HEXBASE << std::setw(sizeof(x) * 2) << (x)
-#define HEX_SIZE(x, sz) HEXBASE << std::setw((sz)) << (x)
-#define STRING_WIDTH(w) std::setfill(' ') << std::setw(w)
-#define LEFT_STRING_WIDTH(s, w)
\
-  std::left << std::setfill(' ') << std::setw(w) << (s) << std::right
-#define DECIMAL std::dec << std::setfill(' ')
-#define DECIMAL_WIDTH(w) DECIMAL << std::setw(w)
-//#define FLOAT(n, d)   std::setfill(' ') << std::setw((n)+(d)+1) <<
-//std::setprecision(d) << std::showpoint << std::fixed
-#define INDENT_WITH_SPACES(iword_idx)  
\
-  std::setfill(' ') << std::setw((iword_idx)) << ""
-#define INDENT_WITH_TABS(iword_idx)
\
-  std::setfill('\t') << std::setw((iword_idx)) << ""
-
-#endif // #if defined(__cplusplus)
-#endif // liblldb_IOStreamMacros_h_



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


[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

2023-08-09 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib accepted this revision.
mib added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/include/lldb/Host/StreamFile.h:12
 
 #include "lldb/Host/File.h"
 #include "lldb/Utility/Stream.h"

bulbazord wrote:
> mib wrote:
> > bulbazord wrote:
> > > mib wrote:
> > > > Do we really need this header ? We could forward declare FileSP do 
> > > > avoid having to include it here 
> > > We're actually using it in this header (look at `GetFile()`).
> > There is already a `GetFileSP` we could use that instead and remove 
> > `GetFile`
> Yeah, that would be a good idea. Especially since `GetFile()` itself doesn't 
> check the validity of the `FileSP` it has anyway... And it doesn't seem like 
> there's an obvious way to check for validity before calling `GetFile()`. Is 
> it ok if I do that in a follow-up? I don't want a revert to move the file 
> back to `lldbCore` in case something goes wrong.
Sure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157460

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


[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added inline comments.



Comment at: lldb/include/lldb/Host/StreamFile.h:12
 
 #include "lldb/Host/File.h"
 #include "lldb/Utility/Stream.h"

mib wrote:
> bulbazord wrote:
> > mib wrote:
> > > Do we really need this header ? We could forward declare FileSP do avoid 
> > > having to include it here 
> > We're actually using it in this header (look at `GetFile()`).
> There is already a `GetFileSP` we could use that instead and remove `GetFile`
Yeah, that would be a good idea. Especially since `GetFile()` itself doesn't 
check the validity of the `FileSP` it has anyway... And it doesn't seem like 
there's an obvious way to check for validity before calling `GetFile()`. Is it 
ok if I do that in a follow-up? I don't want a revert to move the file back to 
`lldbCore` in case something goes wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157460

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


[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

2023-08-09 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added inline comments.



Comment at: lldb/include/lldb/Host/StreamFile.h:12
 
 #include "lldb/Host/File.h"
 #include "lldb/Utility/Stream.h"

bulbazord wrote:
> mib wrote:
> > Do we really need this header ? We could forward declare FileSP do avoid 
> > having to include it here 
> We're actually using it in this header (look at `GetFile()`).
There is already a `GetFileSP` we could use that instead and remove `GetFile`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157460

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


[Lldb-commits] [PATCH] D157512: [lldb][PlatformDarwin] Only parse SDK from debug-info if target language is Objective-C

2023-08-09 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

It's fairly common for AppKit developers to use access to static methods under 
NSApplication to see the state of the application.  Since you aren't actually 
accessing the frame variables for this purpose, it's also pretty common for 
those folks to do:

(lldb) process interrupt
(lldb) p [{NSApplication sharedApplication] someRequest]

That works today w/o having to specify a language.  It would be good if this 
change didn't break that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157512

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


[Lldb-commits] [PATCH] D157460: [lldb] Sink StreamFile into lldbHost

2023-08-09 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

In D157460#4571772 , @mib wrote:

> Shouldn't this be next to the other `Stream` class in `lldbUtility` ?

`lldbUtility` is the lowest layer and does not depend on anything in 
`lldbHost`. That being said, `StreamFile` uses `File` and `FileSystem` from 
`lldbHost`, so that is the lowest layer it can sit on without a layering 
violation.




Comment at: lldb/include/lldb/Host/StreamFile.h:12
 
 #include "lldb/Host/File.h"
 #include "lldb/Utility/Stream.h"

mib wrote:
> Do we really need this header ? We could forward declare FileSP do avoid 
> having to include it here 
We're actually using it in this header (look at `GetFile()`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157460

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


[Lldb-commits] [PATCH] D157512: [lldb][PlatformDarwin] Only parse SDK from debug-info if target language is Objective-C

2023-08-09 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a subscriber: kadircet.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added subscribers: lldb-commits, ilya-biryukov.
Herald added a project: LLDB.

The call to `HostInfoMacOSX::GetSDKRoot` can noticeably slow down the first
expression evaluation in an LLDB session. For C++ expressions we don't
actually ever use the results of the `ClangDeclVendor`, so getting the
flags right isn't necessary. Thus skip SDK parsing logic for C++
targets.

An alternative would be to entirely omit the setup of the
ClangDeclVendor (and other Objective-C machinery) for C++ targets.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157512

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Breakpoint/BreakpointSite.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
@@ -1097,7 +1098,7 @@
 
   FileSpec sysroot_spec;
 
-  if (target) {
+  if (target && Language::LanguageIsObjC(target->GetLanguage())) {
 if (ModuleSP exe_module_sp = target->GetExecutableModule()) {
   auto path_or_err = ResolveSDKPathFromDebugInfo(*exe_module_sp);
   if (path_or_err) {


Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Breakpoint/BreakpointSite.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
@@ -1097,7 +1098,7 @@
 
   FileSpec sysroot_spec;
 
-  if (target) {
+  if (target && Language::LanguageIsObjC(target->GetLanguage())) {
 if (ModuleSP exe_module_sp = target->GetExecutableModule()) {
   auto path_or_err = ResolveSDKPathFromDebugInfo(*exe_module_sp);
   if (path_or_err) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

2023-08-09 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added a comment.

In D156774#4572967 , @Endill wrote:

> In D156774#4572947 , @Michael137 
> wrote:
>
>> Are you still planning on moving this forward? Otherwise I could commandeer 
>> the revision to get this in. I do think it's a useful bug to address
>
> I do. Locally I've been preparing additional changes on top of this patch 
> that expose enums in SBType, so that I can do end-to-end test to ensure this 
> sufficiently addresses my use case.
> On top of that, I'm planning to redo this patch after the example of 
> `DW_TAG_subprogram` per your suggestion.
>
> Unfortunately, I didn't have time last week for this, and now I'm knee deep 
> into triaging old Clang bugs. I'm not finished with that until number of open 
> issues is brought under 20k :)

Sounds good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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


[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

2023-08-09 Thread Vlad Serebrennikov via Phabricator via lldb-commits
Endill added a comment.

In D156774#4572947 , @Michael137 
wrote:

> Are you still planning on moving this forward? Otherwise I could commandeer 
> the revision to get this in. I do think it's a useful bug to address

I do. Locally I've been preparing additional changes on top of this patch that 
expose enums in SBType, so that I can do end-to-end test to ensure this 
sufficiently addresses my use case.
On top of that, I'm planning to redo this patch after the example of 
`DW_TAG_subprogram` per your suggestion.

Unfortunately, I didn't have time last week for this, and now I'm knee deep 
into triaging old Clang bugs. I'm not finished with that until number of open 
issues is brought under 20k :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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


[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

2023-08-09 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added a comment.

Are you still planning on moving this forward? Otherwise I could commandeer the 
revision to get this in. I do think it's a useful bug to address


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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


[Lldb-commits] [PATCH] D157059: [lldb][PECOFF] Exclude alignment padding when reading section data

2023-08-09 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added a comment.

Hi, this is failing on swift-ci (runs on x86 bots) with the following error:

  
/Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/build/Ninja-ReleaseAssert+stdlib-Release/lldb-macosx-x86_64/unittests/ObjectFile/PECOFF/./ObjectFilePECOFFTests
 --gtest_filter=SectionSizeTest.NoAlignmentPadding
  --
  YAML:12:5: error: unknown key 'SizeOfRawData'
  SizeOfRawData:   512
  ^
  
/Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/llvm-project/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp:49:
 Failure
  Value of: llvm::detail::TakeExpected(ExpectedFile)
  Expected: succeeded
Actual: failed  (convertYAML() failed)
  
  
/Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/llvm-project/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp:49
  Value of: llvm::detail::TakeExpected(ExpectedFile)
  Expected: succeeded
Actual: failed  (convertYAML() failed)

https://ci.swift.org/view/LLDB/job/oss-lldb-incremental-macos-cmake/

Could you take a look please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157059

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


[Lldb-commits] [PATCH] D157488: [lldb][AArch64] Add testing of save/restore for Linux MTE control register

2023-08-09 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This has always worked but had no coverage. Adding testing now so that
later I can refactor the save/restore code safely.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157488

Files:
  lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
  
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
  lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c


Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
@@ -0,0 +1,21 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int setup_mte() {
+  return prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | 
PR_MTE_TCF_SYNC,
+   0, 0, 0);
+}
+
+int main(int argc, char const *argv[]) {
+  if (!(getauxval(AT_HWCAP2) & HWCAP2_MTE))
+return 1;
+
+  if (setup_mte())
+return 1;
+
+  return 0; // Set a break point here.
+}
Index: 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
===
--- /dev/null
+++ 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
@@ -0,0 +1,50 @@
+"""
+Test that LLDB correctly reads, writes and restores the MTE control register on
+AArch64 Linux.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MTECtrlRegisterTestCase(TestBase):
+@no_debug_info_test
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(["linux"]))
+def test_mte_ctrl_register(self):
+if not self.isAArch64MTE():
+self.skipTest("Target must support MTE.")
+
+self.build()
+self.line = line_number("main.c", "// Set a break point here.")
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1
+)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"process status",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."],
+)
+
+# Bit 0 = tagged addressing enabled
+# Bit 1 = synchronous faults
+# Bit 2 = asynchronous faults
+# We start enabled with synchronous faults.
+self.expect("register read mte_ctrl", substrs=["0x0003"])
+
+# Change to asynchronous faults.
+self.runCmd("register write mte_ctrl 5")
+self.expect("register read mte_ctrl", substrs=["0x0005"])
+
+# This would return to synchronous faults if we did not restore the
+# previous value.
+self.expect("expression setup_mte()", substrs=["= 0"])
+self.expect("register read mte_ctrl", substrs=["0x0005"])
\ No newline at end of file
Index: 
lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules


Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/main.c
@@ -0,0 +1,21 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int setup_mte() {
+  return prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC,
+   0, 0, 0);
+}
+
+int main(int argc, char const *argv[]) {
+  if (!(getauxval(AT_HWCAP2) & HWCAP2_MTE))
+return 1;
+
+  if (setup_mte())
+return 1;
+
+  return 0; // Set a break point here.
+}
Index: lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_mte_ctrl_register/TestMTECtrlRegister.py
@@ -0,0 +1,50 @@
+"""
+Test that LLDB correctly reads, writes and restores the MTE control register on
+AArch64 Linux.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MTECtrlRegisterTestCase(TestBase):
+@no_debug_info_test
+@skipIf(archs=no_match(["aarch64"]))
+

[Lldb-commits] [PATCH] D157486: Triple Patch

2023-08-09 Thread Evgeniy Makarev via Phabricator via lldb-commits
Pivnoy created this revision.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
Pivnoy requested review of this revision.
Herald added projects: clang, LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157486

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  lldb/source/Utility/ArchSpec.cpp
  llvm/tools/dsymutil/DwarfLinkerForBinary.cpp


Index: llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
===
--- llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -801,7 +801,8 @@
   return error(toString(std::move(E)));
   }
 
-  if (llvm::TripleUtils::isOSDarwin(Map.getTriple()) && 
!Map.getBinaryPath().empty() &&
+  if (llvm::TripleUtils::isOSDarwin(Map.getTriple()) &&
+  !Map.getBinaryPath().empty() &&
   ObjectType == Linker::OutputFileType::Object)
 return MachOUtils::generateDsymCompanion(
 Options.VFS, Map, Options.Translator,
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -1416,7 +1416,8 @@
 
   const unsigned unspecified = 0;
   const llvm::Triple  = GetTriple();
-  if (llvm::TripleUtils::isOSDarwin(triple) && triple.getOSMajorVersion() == 
unspecified)
+  if (llvm::TripleUtils::isOSDarwin(triple) &&
+  triple.getOSMajorVersion() == unspecified)
 return false;
 
   return true;
Index: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
===
--- clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
+++ clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
@@ -111,8 +111,9 @@
 GTEST_SKIP();
 
   // FIXME: libunwind on darwin is broken, see PR49692.
-  if (llvm::TripleUtils::isOSDarwin(Triple) && (Triple.getArch() == 
llvm::Triple::aarch64 ||
-  Triple.getArch() == llvm::Triple::aarch64_32))
+  if (llvm::TripleUtils::isOSDarwin(Triple) &&
+  (Triple.getArch() == llvm::Triple::aarch64 ||
+   Triple.getArch() == llvm::Triple::aarch64_32))
 GTEST_SKIP();
 
   llvm::cantFail(Interp->ParseAndExecute(ExceptionCode));
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -45,7 +45,7 @@
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MathExtras.h"
-#include "llvm/Support/raw_ostream.h" 
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/TripleUtils.h"
 #include 
 
@@ -3195,7 +3195,8 @@
   S.Diag(AL.getLoc(), diag::warn_attribute_invalid_on_definition)
 << "weak_import";
 else if (isa(D) || isa(D) ||
- 
(llvm::TripleUtils::isOSDarwin(S.Context.getTargetInfo().getTriple()) &&
+ (llvm::TripleUtils::isOSDarwin(
+  S.Context.getTargetInfo().getTriple()) &&
   (isa(D) || isa(D {
   // Nothing to warn about here.
 } else
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12165,7 +12165,8 @@
   // Darwin passes an undocumented fourth argument of type char**.  If
   // other platforms start sprouting these, the logic below will start
   // getting shifty.
-  if (nparams == 4 && 
llvm::TripleUtils::isOSDarwin(Context.getTargetInfo().getTriple()))
+  if (nparams == 4 &&
+  llvm::TripleUtils::isOSDarwin(Context.getTargetInfo().getTriple()))
 HasExtraParameters = false;
 
   if (HasExtraParameters) {


Index: llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
===
--- llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -801,7 +801,8 @@
   return error(toString(std::move(E)));
   }
 
-  if (llvm::TripleUtils::isOSDarwin(Map.getTriple()) && !Map.getBinaryPath().empty() &&
+  if (llvm::TripleUtils::isOSDarwin(Map.getTriple()) &&
+  !Map.getBinaryPath().empty() &&
   ObjectType == Linker::OutputFileType::Object)
 return MachOUtils::generateDsymCompanion(
 Options.VFS, Map, Options.Translator,
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -1416,7 +1416,8 @@
 
   const unsigned unspecified = 0;
   const llvm::Triple  = GetTriple();
-  if (llvm::TripleUtils::isOSDarwin(triple) && triple.getOSMajorVersion() ==