[Lldb-commits] [PATCH] D154201: [lldb][AArch64] Fix tagged watch test on Graviton 3

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I suspect that's what most people using write watchpoints would actually 
> want, outside of this issue of false watchpoint hits.

Seems like the sensible route.

The other option is understanding every instruction's behaviour, which is in 
theory possible but I wouldn't like to do it for SVE scatter stores for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154201

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

[Lldb-commits] [PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via Phabricator via lldb-commits]

hokein created this revision.
hokein added reviewers: avl, JDevlieghere.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154329

Files:
  lldb/tools/lldb-server/lldb-platform.cpp


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,14 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [&status, &file_spec](const AtomicFileWriteError &E) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: 
%s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
+  if (auto Err = llvm::writeToOutput(file_spec.GetPath(),
+ [&socket_id](llvm::raw_ostream &OS) {
+   OS << socket_id;
+   return llvm::Error::success();
+ }))
 return Status("Failed to atomically write file %s",
   file_spec.GetPath().c_str());
-  }
   return status;
 }
 


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,14 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [&status, &file_spec](const AtomicFileWriteError &E) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: %s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
+  if (auto Err = llvm::writeToOutput(file_spec.GetPath(),
+ [&socket_

[Lldb-commits] [PATCH] D153740: [llvm][Support] Deprecate llvm::writeFileAtomically API

[Haojian Wu via Phabricator via lldb-commits]

hokein updated this revision to Diff 536698.
hokein added a comment.

Restrict the patch to only remove the writeFileAtomically API


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153740

Files:
  llvm/include/llvm/Support/FileUtilities.h
  llvm/lib/Support/FileUtilities.cpp
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/FileUtilitiesTest.cpp

Index: llvm/unittests/Support/FileUtilitiesTest.cpp
===
--- llvm/unittests/Support/FileUtilitiesTest.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-//===- llvm/unittest/Support/FileUtilitiesTest.cpp - unit tests ---===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "llvm/Support/FileUtilities.h"
-#include "llvm/Support/Errc.h"
-#include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/MemoryBuffer.h"
-#include "llvm/Support/Path.h"
-#include "llvm/Testing/Support/SupportHelpers.h"
-#include "gtest/gtest.h"
-#include 
-
-using namespace llvm;
-using namespace llvm::sys;
-
-using llvm::unittest::TempDir;
-
-#define ASSERT_NO_ERROR(x) \
-  if (std::error_code ASSERT_NO_ERROR_ec = x) {\
-SmallString<128> MessageStorage;   \
-raw_svector_ostream Message(MessageStorage);   \
-Message << #x ": did not return errc::success.\n"  \
-<< "error number: " << ASSERT_NO_ERROR_ec.value() << "\n"  \
-<< "error message: " << ASSERT_NO_ERROR_ec.message() << "\n";  \
-GTEST_FATAL_FAILURE_(MessageStorage.c_str());  \
-  } else { \
-  }
-
-namespace {
-TEST(writeFileAtomicallyTest, Test) {
-  // Create unique temporary directory for these tests
-  TempDir RootTestDirectory("writeFileAtomicallyTest", /*Unique*/ true);
-
-  SmallString<128> FinalTestfilePath(RootTestDirectory.path());
-  sys::path::append(FinalTestfilePath, "foo.txt");
-  const std::string TempUniqTestFileModel =
-  std::string(FinalTestfilePath) + "-";
-  const std::string TestfileContent = "fooFOOfoo";
-
-  llvm::Error Err = llvm::writeFileAtomically(TempUniqTestFileModel, FinalTestfilePath, TestfileContent);
-  ASSERT_FALSE(static_cast(Err));
-
-  std::ifstream FinalFileStream(std::string(FinalTestfilePath.str()));
-  std::string FinalFileContent;
-  FinalFileStream >> FinalFileContent;
-  ASSERT_EQ(FinalFileContent, TestfileContent);
-}
-} // anonymous namespace
Index: llvm/unittests/Support/CMakeLists.txt
===
--- llvm/unittests/Support/CMakeLists.txt
+++ llvm/unittests/Support/CMakeLists.txt
@@ -41,7 +41,6 @@
   ExtensibleRTTITest.cpp
   FileCollectorTest.cpp
   FileOutputBufferTest.cpp
-  FileUtilitiesTest.cpp
   FormatVariadicTest.cpp
   FSUniqueIDTest.cpp
   GlobPatternTest.cpp
Index: llvm/lib/Support/FileUtilities.cpp
===
--- llvm/lib/Support/FileUtilities.cpp
+++ llvm/lib/Support/FileUtilities.cpp
@@ -267,64 +267,6 @@
   return CompareFailed;
 }
 
-void llvm::AtomicFileWriteError::log(raw_ostream &OS) const {
-  OS << "atomic_write_error: ";
-  switch (Error) {
-  case atomic_write_error::failed_to_create_uniq_file:
-OS << "failed_to_create_uniq_file";
-return;
-  case atomic_write_error::output_stream_error:
-OS << "output_stream_error";
-return;
-  case atomic_write_error::failed_to_rename_temp_file:
-OS << "failed_to_rename_temp_file";
-return;
-  }
-  llvm_unreachable("unknown atomic_write_error value in "
-   "failed_to_rename_temp_file::log()");
-}
-
-llvm::Error llvm::writeFileAtomically(StringRef TempPathModel,
-  StringRef FinalPath, StringRef Buffer) {
-  return writeFileAtomically(TempPathModel, FinalPath,
- [&Buffer](llvm::raw_ostream &OS) {
-   OS.write(Buffer.data(), Buffer.size());
-   return llvm::Error::success();
- });
-}
-
-llvm::Error llvm::writeFileAtomically(
-StringRef TempPathModel, StringRef FinalPath,
-std::function Writer) {
-  SmallString<128> GeneratedUniqPath;
-  int TempFD;
-  if (sys::fs::createUniqueFile(TempPathModel, TempFD, GeneratedUniqPath)) {
-return llvm::make_error(
-atomic_write_error::failed_to_create_uniq_file);
-  }
-  llvm::FileRemover RemoveTmpFile

[Lldb-commits] [PATCH] D153740: [llvm][Support] Deprecate llvm::writeFileAtomically API

[Haojian Wu via Phabricator via lldb-commits]

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

Now this patch only contains the removal part of the API (I have cleaned all 
usages of `writeFileAtomically` API, except a remaining one in lldb 
https://reviews.llvm.org/D154329).




Comment at: lldb/tools/lldb-server/lldb-platform.cpp:107
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [&status, &file_spec](const AtomicFileWriteError &E) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: 
%s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
+  if (auto Err = handleErrors(file_spec.GetPath(),
+  [&socket_id](llvm::raw_ostream &OS) {

avl wrote:
> the call to llvm::writeToOutput is probably missed here.
oops, good catch, the `handleError` should be `writeToOutput`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153740

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

[Lldb-commits] [lldb] 379b59d - [lldb] Skip apple accelerator table test in DWARF 5 mode

[Felipe de Azevedo Piovezan via lldb-commits]

Author: Felipe de Azevedo Piovezan
Date: 2023-07-03T08:35:55-04:00
New Revision: 379b59d1b0f3e1ddc421f706e9de65e52acad0cb

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

LOG: [lldb] Skip apple accelerator table test in DWARF 5 mode

D68678 added a test that ensures an Apple accelerator lookup is done
efficiently. Since these tables are not used for DWARF 5, we should decorate the
test appropriately.

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

Added: 


Modified: 
lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py 
b/lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
index c3b058b16b6d60..0f21806da6bd32 100644
--- a/lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
+++ b/lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
@@ -7,6 +7,7 @@
 class CPPAcceleratorTableTestCase(TestBase):
 @skipUnlessDarwin
 @skipIf(debug_info=no_match(["dwarf"]))
+@skipIf(dwarf_version=[">=", "5"])
 def test(self):
 """Test that type lookups fail early (performance)"""
 self.build()



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

[Lldb-commits] [PATCH] D154268: [lldb] Skip apple accelerator table test in DWARF 5 mode

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG379b59d1b0f3: [lldb] Skip apple accelerator table test in 
DWARF 5 mode (authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154268

Files:
  lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py


Index: lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
===
--- lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
+++ lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
@@ -7,6 +7,7 @@
 class CPPAcceleratorTableTestCase(TestBase):
 @skipUnlessDarwin
 @skipIf(debug_info=no_match(["dwarf"]))
+@skipIf(dwarf_version=[">=", "5"])
 def test(self):
 """Test that type lookups fail early (performance)"""
 self.build()


Index: lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
===
--- lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
+++ lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
@@ -7,6 +7,7 @@
 class CPPAcceleratorTableTestCase(TestBase):
 @skipUnlessDarwin
 @skipIf(debug_info=no_match(["dwarf"]))
+@skipIf(dwarf_version=[">=", "5"])
 def test(self):
 """Test that type lookups fail early (performance)"""
 self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] f292ca1 - [lldb][NFC] Simplify GetLocation_DW_OP_addr function

[Felipe de Azevedo Piovezan via lldb-commits]

Author: Felipe de Azevedo Piovezan
Date: 2023-07-03T08:36:57-04:00
New Revision: f292ca136240a9c694aa280ad7fd0b961930804a

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

LOG: [lldb][NFC] Simplify GetLocation_DW_OP_addr function

A very old commit (9422dd64f870dd33) changed the signature of this function in a
number of ways. This patch aims to improve it:

1. Reword the documentation, which still mentions old parameters that no longer
exist, and to elaborate upon the behavior of this function.
2. Remove the unnecessary parameter `op_addr_idx`. This parameter is odd in a
couple of ways: we never use it with a value that is non-zero, and the matching
`Update_DW_OP_addr` function doesn't use a similar parameter. We also document
that this new behavior. If we ever decide to handle multiple "DW_OP_addr", we
can introduce the complexity again.

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

Added: 


Modified: 
lldb/include/lldb/Expression/DWARFExpression.h
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/include/lldb/Expression/DWARFExpression.h 
b/lldb/include/lldb/Expression/DWARFExpression.h
index aea41926d848d2..380910ba0ea3d6 100644
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -50,30 +50,22 @@ class DWARFExpression {
   /// Return true if the location expression contains data
   bool IsValid() const;
 
-  /// If a location is not a location list, return true if the location
-  /// contains a DW_OP_addr () opcode in the stream that matches \a file_addr.
-  /// If file_addr is LLDB_INVALID_ADDRESS, the this function will return true
-  /// if the variable there is any DW_OP_addr in a location that (yet still is
-  /// NOT a location list). This helps us detect if a variable is a global or
-  /// static variable since there is no other indication from DWARF debug
-  /// info.
+  /// Return the address specified by the first
+  /// DW_OP_{addr, addrx, GNU_addr_index} in the operation stream.
   ///
   /// \param[in] dwarf_cu
-  /// The dwarf unit this expression belongs to.
-  ///
-  /// \param[in] op_addr_idx
-  /// The DW_OP_addr index to retrieve in case there is more than
-  /// one DW_OP_addr opcode in the location byte stream.
+  /// The dwarf unit this expression belongs to. Only required to resolve
+  /// DW_OP{addrx, GNU_addr_index}.
   ///
   /// \param[out] error
   /// If the location stream contains unknown DW_OP opcodes or the
   /// data is missing, \a error will be set to \b true.
   ///
   /// \return
-  /// LLDB_INVALID_ADDRESS if the location doesn't contain a
-  /// DW_OP_addr for \a op_addr_idx, otherwise a valid file address
+  /// The address specified by the operation, if the operation exists, or
+  /// LLDB_INVALID_ADDRESS otherwise.
   lldb::addr_t GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu,
-  uint32_t op_addr_idx, bool &error) const;
+  bool &error) const;
 
   bool Update_DW_OP_addr(const DWARFUnit *dwarf_cu, lldb::addr_t file_addr);
 

diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 5f71a12456972e..c9524870f316f4 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -355,39 +355,28 @@ static offset_t GetOpcodeDataSize(const DataExtractor 
&data,
 }
 
 lldb::addr_t DWARFExpression::GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu,
- uint32_t op_addr_idx,
  bool &error) const {
   error = false;
   lldb::offset_t offset = 0;
-  uint32_t curr_op_addr_idx = 0;
   while (m_data.ValidOffset(offset)) {
 const uint8_t op = m_data.GetU8(&offset);
 
-if (op == DW_OP_addr) {
-  const lldb::addr_t op_file_addr = m_data.GetAddress(&offset);
-  if (curr_op_addr_idx == op_addr_idx)
-return op_file_addr;
-  ++curr_op_addr_idx;
-} else if (op == DW_OP_GNU_addr_index || op == DW_OP_addrx) {
+if (op == DW_OP_addr)
+  return m_data.GetAddress(&offset);
+if (op == DW_OP_GNU_addr_index || op == DW_OP_addrx) {
   uint64_t index = m_data.GetULEB128(&offset);
-  if (curr_op_addr_idx == op_addr_idx) {
-if (!dwarf_cu) {
-  error = true;
-  break;
-}
-
+  if (dwarf_cu)
 return dwarf_cu->ReadAddressFromDebugAddrSection(index);
-  }
-  ++curr_op_addr_idx;
-} else {
-  const offset_t op_arg_size =
-  GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
-  if (op_arg

[Lldb-commits] [PATCH] D154265: [lldb][NFC] Simplify GetLocation_DW_OP_addr function

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf292ca136240: [lldb][NFC] Simplify GetLocation_DW_OP_addr 
function (authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154265

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3457,8 +3457,8 @@
   bool op_error = false;
   const DWARFExpression* location = location_list.GetAlwaysValidExpr();
   if (location)
-location_DW_OP_addr = location->GetLocation_DW_OP_addr(
-location_form.GetUnit(), 0, op_error);
+location_DW_OP_addr =
+location->GetLocation_DW_OP_addr(location_form.GetUnit(), op_error);
   if (op_error) {
 StreamString strm;
 location->DumpLocation(&strm, eDescriptionLevelFull, nullptr);
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -355,39 +355,28 @@
 }
 
 lldb::addr_t DWARFExpression::GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu,
- uint32_t op_addr_idx,
  bool &error) const {
   error = false;
   lldb::offset_t offset = 0;
-  uint32_t curr_op_addr_idx = 0;
   while (m_data.ValidOffset(offset)) {
 const uint8_t op = m_data.GetU8(&offset);
 
-if (op == DW_OP_addr) {
-  const lldb::addr_t op_file_addr = m_data.GetAddress(&offset);
-  if (curr_op_addr_idx == op_addr_idx)
-return op_file_addr;
-  ++curr_op_addr_idx;
-} else if (op == DW_OP_GNU_addr_index || op == DW_OP_addrx) {
+if (op == DW_OP_addr)
+  return m_data.GetAddress(&offset);
+if (op == DW_OP_GNU_addr_index || op == DW_OP_addrx) {
   uint64_t index = m_data.GetULEB128(&offset);
-  if (curr_op_addr_idx == op_addr_idx) {
-if (!dwarf_cu) {
-  error = true;
-  break;
-}
-
+  if (dwarf_cu)
 return dwarf_cu->ReadAddressFromDebugAddrSection(index);
-  }
-  ++curr_op_addr_idx;
-} else {
-  const offset_t op_arg_size =
-  GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
-  if (op_arg_size == LLDB_INVALID_OFFSET) {
-error = true;
-break;
-  }
-  offset += op_arg_size;
+  error = true;
+  break;
+}
+const offset_t op_arg_size =
+GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
+if (op_arg_size == LLDB_INVALID_OFFSET) {
+  error = true;
+  break;
 }
+offset += op_arg_size;
   }
   return LLDB_INVALID_ADDRESS;
 }
Index: lldb/include/lldb/Expression/DWARFExpression.h
===
--- lldb/include/lldb/Expression/DWARFExpression.h
+++ lldb/include/lldb/Expression/DWARFExpression.h
@@ -50,30 +50,22 @@
   /// Return true if the location expression contains data
   bool IsValid() const;
 
-  /// If a location is not a location list, return true if the location
-  /// contains a DW_OP_addr () opcode in the stream that matches \a file_addr.
-  /// If file_addr is LLDB_INVALID_ADDRESS, the this function will return true
-  /// if the variable there is any DW_OP_addr in a location that (yet still is
-  /// NOT a location list). This helps us detect if a variable is a global or
-  /// static variable since there is no other indication from DWARF debug
-  /// info.
+  /// Return the address specified by the first
+  /// DW_OP_{addr, addrx, GNU_addr_index} in the operation stream.
   ///
   /// \param[in] dwarf_cu
-  /// The dwarf unit this expression belongs to.
-  ///
-  /// \param[in] op_addr_idx
-  /// The DW_OP_addr index to retrieve in case there is more than
-  /// one DW_OP_addr opcode in the location byte stream.
+  /// The dwarf unit this expression belongs to. Only required to resolve
+  /// DW_OP{addrx, GNU_addr_index}.
   ///
   /// \param[out] error
   /// If the location stream contains unknown DW_OP opcodes or the
   /// data is missing, \a error will be set to \b true.
   ///
   /// \return
-  /// LLDB_INVALID_ADDRESS if the location doesn't contain a
-  /// DW_OP_addr for \a op_addr_idx, otherwise a valid file address
+  /// The address specified by the operation, if the operation exists, or
+  /// LLDB_INVALID_ADDRESS otherwise.
   lldb::addr_t GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu,
-  uint32_t op_addr_idx, bool &

[Lldb-commits] [PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Alexey Lapshin via Phabricator via lldb-commits]

avl added inline comments.



Comment at: lldb/tools/lldb-server/lldb-platform.cpp:112
 return Status("Failed to atomically write file %s",
   file_spec.GetPath().c_str());
   return status;

probably, it would be better to add error text here?

```
return Status("Failed to atomically write file %s: %s",
  file_spec.GetPath().c_str(), 
toString(std::move(Err)).c_str());
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154329

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

[Lldb-commits] [PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via Phabricator via lldb-commits]

hokein updated this revision to Diff 536735.
hokein marked an inline comment as done.
hokein added a comment.

address a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154329

Files:
  lldb/tools/lldb-server/lldb-platform.cpp


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,15 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [&status, &file_spec](const AtomicFileWriteError &E) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: 
%s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
-return Status("Failed to atomically write file %s",
-  file_spec.GetPath().c_str());
-  }
+  if (auto Err = llvm::writeToOutput(file_spec.GetPath(),
+ [&socket_id](llvm::raw_ostream &OS) {
+   OS << socket_id;
+   return llvm::Error::success();
+ }))
+return Status("Failed to atomically write file %s: %s",
+  file_spec.GetPath().c_str(),
+  llvm::toString(std::move(Err)).c_str());
   return status;
 }
 


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,15 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [&status, &file_spec](const AtomicFileWriteError &E) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: %s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
-   

[Lldb-commits] [PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via Phabricator via lldb-commits]

hokein added inline comments.



Comment at: lldb/tools/lldb-server/lldb-platform.cpp:112
 return Status("Failed to atomically write file %s",
   file_spec.GetPath().c_str());
   return status;

avl wrote:
> probably, it would be better to add error text here?
> 
> ```
> return Status("Failed to atomically write file %s: %s",
>   file_spec.GetPath().c_str(), 
> toString(std::move(Err)).c_str());
> ```
good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154329

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

[Lldb-commits] [PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Alexey Lapshin via Phabricator via lldb-commits]

avl added a comment.

this LGTM. thanks! Please, wait for approve from Jonas. I think someone from 
lldb needs to check whether new error reporting is OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154329

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

[Lldb-commits] [PATCH] D153905: [lldb][NFCI] Remove unneeded use of ConstString in ASTResultSynthesizer

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG673f91055a41: [lldb][NFCI] Remove unneeded use of 
ConstString in ASTResultSynthesizer (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153905

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
@@ -429,15 +429,10 @@
 return;
 
   StringRef name = D->getName();
-
-  if (name.size() == 0 || name[0] != '$')
+  if (name.empty() || name.front() != '$')
 return;
 
-  Log *log = GetLog(LLDBLog::Expressions);
-
-  ConstString name_cs(name.str().c_str());
-
-  LLDB_LOGF(log, "Recording persistent type %s\n", name_cs.GetCString());
+  LLDB_LOG(GetLog(LLDBLog::Expressions), "Recording persistent type {0}", 
name);
 
   m_decls.push_back(D);
 }
@@ -449,15 +444,10 @@
 return;
 
   StringRef name = D->getName();
-
-  if (name.size() == 0)
+  if (name.empty())
 return;
 
-  Log *log = GetLog(LLDBLog::Expressions);
-
-  ConstString name_cs(name.str().c_str());
-
-  LLDB_LOGF(log, "Recording persistent decl %s\n", name_cs.GetCString());
+  LLDB_LOG(GetLog(LLDBLog::Expressions), "Recording persistent decl {0}", 
name);
 
   m_decls.push_back(D);
 }
@@ -475,7 +465,6 @@
 
   for (clang::NamedDecl *decl : m_decls) {
 StringRef name = decl->getName();
-ConstString name_cs(name.str().c_str());
 
 Decl *D_scratch = persistent_vars->GetClangASTImporter()->DeportDecl(
 &scratch_ts_sp->getASTContext(), decl);
@@ -496,8 +485,8 @@
 }
 
 if (NamedDecl *NamedDecl_scratch = dyn_cast(D_scratch))
-  persistent_vars->RegisterPersistentDecl(name_cs, NamedDecl_scratch,
-  scratch_ts_sp);
+  persistent_vars->RegisterPersistentDecl(ConstString(name),
+  NamedDecl_scratch, 
scratch_ts_sp);
   }
 }
 


Index: lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
@@ -429,15 +429,10 @@
 return;
 
   StringRef name = D->getName();
-
-  if (name.size() == 0 || name[0] != '$')
+  if (name.empty() || name.front() != '$')
 return;
 
-  Log *log = GetLog(LLDBLog::Expressions);
-
-  ConstString name_cs(name.str().c_str());
-
-  LLDB_LOGF(log, "Recording persistent type %s\n", name_cs.GetCString());
+  LLDB_LOG(GetLog(LLDBLog::Expressions), "Recording persistent type {0}", name);
 
   m_decls.push_back(D);
 }
@@ -449,15 +444,10 @@
 return;
 
   StringRef name = D->getName();
-
-  if (name.size() == 0)
+  if (name.empty())
 return;
 
-  Log *log = GetLog(LLDBLog::Expressions);
-
-  ConstString name_cs(name.str().c_str());
-
-  LLDB_LOGF(log, "Recording persistent decl %s\n", name_cs.GetCString());
+  LLDB_LOG(GetLog(LLDBLog::Expressions), "Recording persistent decl {0}", name);
 
   m_decls.push_back(D);
 }
@@ -475,7 +465,6 @@
 
   for (clang::NamedDecl *decl : m_decls) {
 StringRef name = decl->getName();
-ConstString name_cs(name.str().c_str());
 
 Decl *D_scratch = persistent_vars->GetClangASTImporter()->DeportDecl(
 &scratch_ts_sp->getASTContext(), decl);
@@ -496,8 +485,8 @@
 }
 
 if (NamedDecl *NamedDecl_scratch = dyn_cast(D_scratch))
-  persistent_vars->RegisterPersistentDecl(name_cs, NamedDecl_scratch,
-  scratch_ts_sp);
+  persistent_vars->RegisterPersistentDecl(ConstString(name),
+  NamedDecl_scratch, scratch_ts_sp);
   }
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 673f910 - [lldb][NFCI] Remove unneeded use of ConstString in ASTResultSynthesizer

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-07-03T09:31:10-07:00
New Revision: 673f91055a41b2273e159eafe86d0d7d87fa474f

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

LOG: [lldb][NFCI] Remove unneeded use of ConstString in ASTResultSynthesizer

2/3 of the ConstStrings in this class were just to be able to log
something. Putting something in the StringPool just to log it doesn't
make a lot of sense, so let's remove them.

The remaining use is for `RegisterPersistentDecl` which is fine for now.

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

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
index 07cb4c9f027c15..3e2c208bd2018e 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
@@ -429,15 +429,10 @@ void 
ASTResultSynthesizer::MaybeRecordPersistentType(TypeDecl *D) {
 return;
 
   StringRef name = D->getName();
-
-  if (name.size() == 0 || name[0] != '$')
+  if (name.empty() || name.front() != '$')
 return;
 
-  Log *log = GetLog(LLDBLog::Expressions);
-
-  ConstString name_cs(name.str().c_str());
-
-  LLDB_LOGF(log, "Recording persistent type %s\n", name_cs.GetCString());
+  LLDB_LOG(GetLog(LLDBLog::Expressions), "Recording persistent type {0}", 
name);
 
   m_decls.push_back(D);
 }
@@ -449,15 +444,10 @@ void ASTResultSynthesizer::RecordPersistentDecl(NamedDecl 
*D) {
 return;
 
   StringRef name = D->getName();
-
-  if (name.size() == 0)
+  if (name.empty())
 return;
 
-  Log *log = GetLog(LLDBLog::Expressions);
-
-  ConstString name_cs(name.str().c_str());
-
-  LLDB_LOGF(log, "Recording persistent decl %s\n", name_cs.GetCString());
+  LLDB_LOG(GetLog(LLDBLog::Expressions), "Recording persistent decl {0}", 
name);
 
   m_decls.push_back(D);
 }
@@ -475,7 +465,6 @@ void ASTResultSynthesizer::CommitPersistentDecls() {
 
   for (clang::NamedDecl *decl : m_decls) {
 StringRef name = decl->getName();
-ConstString name_cs(name.str().c_str());
 
 Decl *D_scratch = persistent_vars->GetClangASTImporter()->DeportDecl(
 &scratch_ts_sp->getASTContext(), decl);
@@ -496,8 +485,8 @@ void ASTResultSynthesizer::CommitPersistentDecls() {
 }
 
 if (NamedDecl *NamedDecl_scratch = dyn_cast(D_scratch))
-  persistent_vars->RegisterPersistentDecl(name_cs, NamedDecl_scratch,
-  scratch_ts_sp);
+  persistent_vars->RegisterPersistentDecl(ConstString(name),
+  NamedDecl_scratch, 
scratch_ts_sp);
   }
 }
 



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

[Lldb-commits] [lldb] a2ff292 - [lldb][NFCI] TypeSystemClang::CreateStructForIdentifier should take a StringRef

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-07-03T09:35:24-07:00
New Revision: a2ff2921e84aa435e124ad275f70855a185cfb1c

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

LOG: [lldb][NFCI] TypeSystemClang::CreateStructForIdentifier should take a 
StringRef

This doesn't really use fast comparison or string uniqueness. In fact,
all of the current callers pass an empty string for type_name. The only
reason I don't remove it is because it looks like it is used downstream
for swift.

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

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
index 40ffe96737926d..c92ac4bdec1480 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
@@ -78,10 +78,10 @@ class BlockPointerSyntheticFrontEnd : public 
SyntheticChildrenFrontEnd {
 const char *const FuncPtr_name("__FuncPtr");
 
 m_block_struct_type = clang_ast_context->CreateStructForIdentifier(
-ConstString(), {{isa_name, isa_type},
-{flags_name, flags_type},
-{reserved_name, reserved_type},
-{FuncPtr_name, function_pointer_type}});
+llvm::StringRef(), {{isa_name, isa_type},
+{flags_name, flags_type},
+{reserved_name, reserved_type},
+{FuncPtr_name, function_pointer_type}});
   }
 
   ~BlockPointerSyntheticFrontEnd() override = default;

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index de4f23bf95c33c..cb601603a3c43d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -338,7 +338,7 @@ bool 
lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::Update() {
 //+-+
 //
 CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-ConstString(),
+llvm::StringRef(),
 {{"ptr0",
   ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
  {"ptr1",
@@ -503,7 +503,7 @@ bool 
lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
 // +-+
 //
 CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-ConstString(),
+llvm::StringRef(),
 {{"__next_",
   ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
  {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 585ab43b6af07a..092a4120376b7f 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -291,7 +291,7 @@ void 
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
 if (!ast_ctx)
   return;
 CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-ConstString(),
+llvm::StringRef(),
 {{"ptr0", 
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
  {"ptr1", 
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
  {"ptr2", 
ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},

diff  --git a/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp 
b/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
index 3d29739c19adfd..7abc71a1c53fe2 100644
--- a/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
+++ b/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
@@ -244,7 +244,7 @@ CompilerType PlatformFreeBSD::GetSiginfoType(const 
llvm::Triple &triple) {
 
   ast->AddFieldToRecordType(
   union_type, "_fault",
-  ast->CreateStructForIdentifier(ConstString(),
+  ast->CreateStructForIdentifier(llvm::StringRef(),
  {
  {"_trapno", int_type},
  }),
@@ -252,7 +252,7 @@ CompilerType PlatformFre

[Lldb-commits] [PATCH] D153810: [lldb][NFCI] TypeSystemClang::CreateStructForIdentifier should take a StringRef

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa2ff2921e84a: [lldb][NFCI] 
TypeSystemClang::CreateStructForIdentifier should take a StringRef (authored by 
bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153810

Files:
  lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -280,13 +280,13 @@
   }
 
   CompilerType CreateStructForIdentifier(
-  ConstString type_name,
+  llvm::StringRef type_name,
   const std::initializer_list>
   &type_fields,
   bool packed = false);
 
   CompilerType GetOrCreateStructForIdentifier(
-  ConstString type_name,
+  llvm::StringRef type_name,
   const std::initializer_list>
   &type_fields,
   bool packed = false);
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2308,12 +2308,12 @@
 }
 
 CompilerType TypeSystemClang::CreateStructForIdentifier(
-ConstString type_name,
+llvm::StringRef type_name,
 const std::initializer_list>
 &type_fields,
 bool packed) {
   CompilerType type;
-  if (!type_name.IsEmpty() &&
+  if (!type_name.empty() &&
   (type = GetTypeForIdentifier(type_name))
   .IsValid()) {
 lldbassert(0 && "Trying to create a type for an existing name");
@@ -2321,8 +2321,7 @@
   }
 
   type = CreateRecordType(nullptr, OptionalClangModuleID(), lldb::eAccessPublic,
-  type_name.GetCString(), clang::TTK_Struct,
-  lldb::eLanguageTypeC);
+  type_name, clang::TTK_Struct, lldb::eLanguageTypeC);
   StartTagDeclarationDefinition(type);
   for (const auto &field : type_fields)
 AddFieldToRecordType(type, field.first, field.second, lldb::eAccessPublic,
@@ -2334,7 +2333,7 @@
 }
 
 CompilerType TypeSystemClang::GetOrCreateStructForIdentifier(
-ConstString type_name,
+llvm::StringRef type_name,
 const std::initializer_list>
 &type_fields,
 bool packed) {
Index: lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
===
--- lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
+++ lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
@@ -277,7 +277,7 @@
 
   ast->AddFieldToRecordType(
   union_type, "_rt",
-  ast->CreateStructForIdentifier(ConstString(),
+  ast->CreateStructForIdentifier(llvm::StringRef(),
  {
  {"_pid", pid_type},
  {"_uid", uid_type},
@@ -287,7 +287,7 @@
 
   ast->AddFieldToRecordType(
   union_type, "_child",
-  ast->CreateStructForIdentifier(ConstString(),
+  ast->CreateStructForIdentifier(llvm::StringRef(),
  {
  {"_pid", pid_type},
  {"_uid", uid_type},
@@ -299,7 +299,7 @@
 
   ast->AddFieldToRecordType(
   union_type, "_fault",
-  ast->CreateStructForIdentifier(ConstString(),
+  ast->CreateStructForIdentifier(llvm::StringRef(),
  {
  {"_addr", voidp_type},
  {"_trap", int_type},
@@ -310,7 +310,7 @@
 
   ast->AddFieldToRecordType(
   union_type, "_poll",
-  ast->CreateStructForIdentifier(ConstString(),
+  ast->CreateStructForIdentifier(llvm::StringRef(),
  {
  {"_band", long_type},
  {"_fd", int_type},
@@ -319,7 +319,7 @@
 
   ast->AddFieldToRecordType(union_type, "_syscall",
 ast->CreateStructForIdentifier(
-ConstString(),
+llvm::StringRef(),
 {
 {"_sysnum", int_type},
 {"_retval", int_type.GetArrayType(2)},
@@ -330,7 +330,7 @@
 
   ast

[Lldb-commits] [PATCH] D154365: [lldb][nfc] Remove redundant nullptr check

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve created this revision.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, jplehr, sstefan1.
Herald added a project: LLDB.

The make_shared function never returns a nullptr, as such the test for nullptr
is not needed. We also move the empty string check earlier in the if
("oso_object"), as this is cheaper than loading the object file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154365

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


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -471,18 +471,17 @@
   oso_arch, oso_object ? &oso_object : nullptr, 0,
   oso_object ? comp_unit_info->oso_mod_time : 
llvm::sys::TimePoint<>());
 
-  if (!comp_unit_info->oso_sp->module_sp || 
!comp_unit_info->oso_sp->module_sp->GetObjectFile()) {
-if (oso_object && FileSystem::Instance().Exists(oso_file)) {
-  // If we are loading a .o file from a .a file the "oso_object" will
-  // have a valid value name and if the .a file exists, either the .o
-  // file didn't exist in the .a file or the mod time didn't match.
-  comp_unit_info->oso_load_error.SetErrorStringWithFormat(
-  "\"%s\" object from the \"%s\" archive: "
-  "either the .o file doesn't exist in the archive or the "
-  "modification time (0x%8.8x) of the .o file doesn't match",
-  oso_object.AsCString(), oso_file.GetPath().c_str(),
-  (uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time));
-}
+  if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
+  FileSystem::Instance().Exists(oso_file)) {
+// If we are loading a .o file from a .a file the "oso_object" will
+// have a valid value name and if the .a file exists, either the .o
+// file didn't exist in the .a file or the mod time didn't match.
+comp_unit_info->oso_load_error.SetErrorStringWithFormat(
+"\"%s\" object from the \"%s\" archive: "
+"either the .o file doesn't exist in the archive or the "
+"modification time (0x%8.8x) of the .o file doesn't match",
+oso_object.AsCString(), oso_file.GetPath().c_str(),
+(uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time));
   }
 }
   }


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -471,18 +471,17 @@
   oso_arch, oso_object ? &oso_object : nullptr, 0,
   oso_object ? comp_unit_info->oso_mod_time : llvm::sys::TimePoint<>());
 
-  if (!comp_unit_info->oso_sp->module_sp || !comp_unit_info->oso_sp->module_sp->GetObjectFile()) {
-if (oso_object && FileSystem::Instance().Exists(oso_file)) {
-  // If we are loading a .o file from a .a file the "oso_object" will
-  // have a valid value name and if the .a file exists, either the .o
-  // file didn't exist in the .a file or the mod time didn't match.
-  comp_unit_info->oso_load_error.SetErrorStringWithFormat(
-  "\"%s\" object from the \"%s\" archive: "
-  "either the .o file doesn't exist in the archive or the "
-  "modification time (0x%8.8x) of the .o file doesn't match",
-  oso_object.AsCString(), oso_file.GetPath().c_str(),
-  (uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time));
-}
+  if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
+  FileSystem::Instance().Exists(oso_file)) {
+// If we are loading a .o file from a .a file the "oso_object" will
+// have a valid value name and if the .a file exists, either the .o
+// file didn't exist in the .a file or the mod time didn't match.
+comp_unit_info->oso_load_error.SetErrorStringWithFormat(
+"\"%s\" object from the \"%s\" archive: "
+"either the .o file doesn't exist in the archive or the "
+"modification time (0x%8.8x) of the .o file doesn't match",
+oso_object.AsCString(), oso_file.GetPath().c_str(),
+(uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time));
   }
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 1b10288 - [lldb][NFCI] Change return type of GetProcessPluginName

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-07-03T10:03:49-07:00
New Revision: 1b102886c0c33bb01ff8f2360b57c7b8d039abcc

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

LOG: [lldb][NFCI] Change return type of GetProcessPluginName

Instead of just returning a raw `const char *`, I think llvm::StringRef
would make more sense. Most of the time that we use the return value of
`GetProcessPluginName` we're passing it to `CreateProcess` which takes a
StringRef anyway.

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

Added: 


Modified: 
lldb/include/lldb/Host/ProcessLaunchInfo.h
lldb/include/lldb/Target/Process.h
lldb/source/Host/common/ProcessLaunchInfo.cpp
lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/ProcessLaunchInfo.h 
b/lldb/include/lldb/Host/ProcessLaunchInfo.h
index 891f46f650c8c5..25762bc65295d8 100644
--- a/lldb/include/lldb/Host/ProcessLaunchInfo.h
+++ b/lldb/include/lldb/Host/ProcessLaunchInfo.h
@@ -68,7 +68,7 @@ class ProcessLaunchInfo : public ProcessInfo {
 
   void SetWorkingDirectory(const FileSpec &working_dir);
 
-  const char *GetProcessPluginName() const;
+  llvm::StringRef GetProcessPluginName() const;
 
   void SetProcessPluginName(llvm::StringRef plugin);
 

diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index cf0ddeed67e832..b358b7497ff9be 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -147,8 +147,8 @@ class ProcessAttachInfo : public ProcessInstanceInfo {
 
   void SetResumeCount(uint32_t c) { m_resume_count = c; }
 
-  const char *GetProcessPluginName() const {
-return (m_plugin_name.empty() ? nullptr : m_plugin_name.c_str());
+  llvm::StringRef GetProcessPluginName() const {
+return llvm::StringRef(m_plugin_name);
   }
 
   void SetProcessPluginName(llvm::StringRef plugin) {

diff  --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp 
b/lldb/source/Host/common/ProcessLaunchInfo.cpp
index e737815750b2c0..0e2c3da11ba9fb 100644
--- a/lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -126,8 +126,8 @@ void ProcessLaunchInfo::SetWorkingDirectory(const FileSpec 
&working_dir) {
   m_working_dir = working_dir;
 }
 
-const char *ProcessLaunchInfo::GetProcessPluginName() const {
-  return (m_plugin_name.empty() ? nullptr : m_plugin_name.c_str());
+llvm::StringRef ProcessLaunchInfo::GetProcessPluginName() const {
+  return llvm::StringRef(m_plugin_name);
 }
 
 void ProcessLaunchInfo::SetProcessPluginName(llvm::StringRef plugin) {

diff  --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp 
b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
index 017318fe4b4204..1301a301f2111d 100644
--- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -533,9 +533,9 @@ lldb::ProcessSP PlatformWindows::Attach(ProcessAttachInfo 
&attach_info,
   if (!target || error.Fail())
 return process_sp;
 
-  const char *plugin_name = attach_info.GetProcessPluginName();
-  process_sp = target->CreateProcess(
-  attach_info.GetListenerForProcess(debugger), plugin_name, nullptr, 
false);
+  process_sp =
+  target->CreateProcess(attach_info.GetListenerForProcess(debugger),
+attach_info.GetProcessPluginName(), nullptr, 
false);
 
   process_sp->HijackProcessEvents(attach_info.GetHijackListener());
   if (process_sp)

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 1444bb6d1217ec..17181569cbca29 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3209,8 +3209,8 @@ Status Target::Launch(ProcessLaunchInfo &launch_info, 
Stream *stream) {
   assert(m_process_sp);
 } else {
   // Use a Process plugin to construct the process.
-  const char *plugin_name = launch_info.GetProcessPluginName();
-  CreateProcess(launch_info.GetListener(), plugin_name, nullptr, false);
+  CreateProcess(launch_info.GetListener(),
+launch_info.GetProcessPluginName(), nullptr, false);
 }
 
 // Since we didn't have a platform launch the process, launch it here.
@@ -3371,14 +3371,14 @@ Status Target::Attach(ProcessAttachInfo &attach_info, 
Stream *stream) {
   } else {
 if (state != eStateConnected) {
   SaveScriptedLaunchInfo(attach_info);
-  const char *plugin_name = attach_info.GetProcessPluginName();
+  llvm::StringRef plugin_name = attach_info.GetProcessPluginName();
   process_sp =
   CreateProcess(attach_info.GetListenerForProcess(GetDebugger()),
 plugin_name, nullptr, false);
-   

[Lldb-commits] [PATCH] D153825: [lldb][NFCI] Change return type of GetProcessPluginName

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1b102886c0c3: [lldb][NFCI] Change return type of 
GetProcessPluginName (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153825

Files:
  lldb/include/lldb/Host/ProcessLaunchInfo.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3209,8 +3209,8 @@
   assert(m_process_sp);
 } else {
   // Use a Process plugin to construct the process.
-  const char *plugin_name = launch_info.GetProcessPluginName();
-  CreateProcess(launch_info.GetListener(), plugin_name, nullptr, false);
+  CreateProcess(launch_info.GetListener(),
+launch_info.GetProcessPluginName(), nullptr, false);
 }
 
 // Since we didn't have a platform launch the process, launch it here.
@@ -3371,14 +3371,14 @@
   } else {
 if (state != eStateConnected) {
   SaveScriptedLaunchInfo(attach_info);
-  const char *plugin_name = attach_info.GetProcessPluginName();
+  llvm::StringRef plugin_name = attach_info.GetProcessPluginName();
   process_sp =
   CreateProcess(attach_info.GetListenerForProcess(GetDebugger()),
 plugin_name, nullptr, false);
-  if (process_sp == nullptr) {
-error.SetErrorStringWithFormat(
-"failed to create process using plugin %s",
-(plugin_name) ? plugin_name : "null");
+  if (!process_sp) {
+error.SetErrorStringWithFormatv(
+"failed to create process using plugin '{0}'",
+plugin_name.empty() ? "" : plugin_name);
 return error;
   }
 }
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -533,9 +533,9 @@
   if (!target || error.Fail())
 return process_sp;
 
-  const char *plugin_name = attach_info.GetProcessPluginName();
-  process_sp = target->CreateProcess(
-  attach_info.GetListenerForProcess(debugger), plugin_name, nullptr, 
false);
+  process_sp =
+  target->CreateProcess(attach_info.GetListenerForProcess(debugger),
+attach_info.GetProcessPluginName(), nullptr, 
false);
 
   process_sp->HijackProcessEvents(attach_info.GetHijackListener());
   if (process_sp)
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -126,8 +126,8 @@
   m_working_dir = working_dir;
 }
 
-const char *ProcessLaunchInfo::GetProcessPluginName() const {
-  return (m_plugin_name.empty() ? nullptr : m_plugin_name.c_str());
+llvm::StringRef ProcessLaunchInfo::GetProcessPluginName() const {
+  return llvm::StringRef(m_plugin_name);
 }
 
 void ProcessLaunchInfo::SetProcessPluginName(llvm::StringRef plugin) {
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -147,8 +147,8 @@
 
   void SetResumeCount(uint32_t c) { m_resume_count = c; }
 
-  const char *GetProcessPluginName() const {
-return (m_plugin_name.empty() ? nullptr : m_plugin_name.c_str());
+  llvm::StringRef GetProcessPluginName() const {
+return llvm::StringRef(m_plugin_name);
   }
 
   void SetProcessPluginName(llvm::StringRef plugin) {
Index: lldb/include/lldb/Host/ProcessLaunchInfo.h
===
--- lldb/include/lldb/Host/ProcessLaunchInfo.h
+++ lldb/include/lldb/Host/ProcessLaunchInfo.h
@@ -68,7 +68,7 @@
 
   void SetWorkingDirectory(const FileSpec &working_dir);
 
-  const char *GetProcessPluginName() const;
+  llvm::StringRef GetProcessPluginName() const;
 
   void SetProcessPluginName(llvm::StringRef plugin);
 


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3209,8 +3209,8 @@
   assert(m_process_sp);
 } else {
   // Use a Process plugin to construct the process.
-  const char *plugin_name = launch_info.GetProcessPluginName();
-  CreateProcess(launch_info.GetListener(), plugin_name, nullptr, false);
+  CreateProcess(launch_info.GetListener(),
+launch_info.GetProcessPluginName(), nullptr, false);
 }
 
 // Since we didn't have a pl

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 536850.
mib added a comment.

After chatting with Jonas, it turns out that my previous implementation didn't 
actually address the underflow issue using atomics, so we have to revert to 
using a `std::mutex` when incrementing/decrementing the python lock counter.


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

https://reviews.llvm.org/D154271

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));
Index: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -380,9 +380,13 @@
 
   uint32_t IsExecutingPython() const { return m_lock_count > 0; }
 
-  uint32_t IncrementLockCount() { return ++m_lock_count; }
+  uint32_t IncrementLockCount() {
+std::lock_guard guard(m_mutex);
+return ++m_lock_count;
+  }
 
   uint32_t DecrementLockCount() {
+std::lock_guard guard(m_mutex);
 if (m_lock_count > 0)
   --m_lock_count;
 return m_lock_count;
@@ -422,6 +426,7 @@
   bool m_pty_secondary_is_open;
   bool m_valid_session;
   uint32_t m_lock_count;
+  std::mutex m_mutex;
   PyThreadState *m_command_thread_state;
 };
 


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -380,9 +380,13 @@
 
   uint32_t IsExecutingPython() const { return m_lock_count > 0; }
 
-  uint32_t IncrementLockCount() { return ++m_lock_count; }
+  uint32_t IncrementLockCount() {
+std::lock_guard guard(m_mutex);
+return ++m_lock_count;
+  }
 
   uint32_t DecrementLockCount() {
+std::lock_guard guard(m_mutex);
 if (m_lock_count > 0)
   --m_lock_count;
 return m_lock_count;
@@ -422,6 +426,7 @@
   bool m_pty_secondary_is_open;
   bool m_valid_session;
   uint32_t m_lock_count;
+  std::mutex m_mutex;
   PyThreadState *m_command_thread_state;
 };
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e3921b8 - [lldb][NFCI] Remove use of ConstString from ProcessElfCore

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-07-03T11:13:50-07:00
New Revision: e3921b8bff693649602760a6221cd1150420a287

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

LOG: [lldb][NFCI] Remove use of ConstString from ProcessElfCore

I'm not convinced that it makes sense for the paths to be ConstStrings. We're
going to be putting them into FileSpecs (which are backed by
ConstStrings, for now) but otherwise there's no need to store them as
ConstStrings upfront.

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

Added: 


Modified: 
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index 9c1b30d5f8024c..bfb59eceb2d025 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -258,8 +258,8 @@ Status ProcessElfCore::DoLoadCore() {
 if (!m_nt_file_entries.empty()) {
   ModuleSpec exe_module_spec;
   exe_module_spec.GetArchitecture() = arch;
-  exe_module_spec.GetFileSpec().SetFile(
-  m_nt_file_entries[0].path.GetCString(), FileSpec::Style::native);
+  exe_module_spec.GetFileSpec().SetFile(m_nt_file_entries[0].path,
+FileSpec::Style::native);
   if (exe_module_spec.GetFileSpec()) {
 exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec, 
   true /* notify */);
@@ -938,7 +938,7 @@ llvm::Error 
ProcessElfCore::parseLinuxNotes(llvm::ArrayRef notes) {
   for (uint64_t i = 0; i < count; ++i) {
 const char *path = note.data.GetCStr(&offset);
 if (path && path[0])
-  m_nt_file_entries[i].path.SetCString(path);
+  m_nt_file_entries[i].path.assign(path);
   }
   break;
 }

diff  --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h 
b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
index 03c23378e3c165..1454e8735a677b 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -20,7 +20,6 @@
 #include 
 
 #include "lldb/Target/PostMortemProcess.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
 
 #include "Plugins/ObjectFile/ELF/ELFHeader.h"
@@ -117,7 +116,7 @@ class ProcessElfCore : public 
lldb_private::PostMortemProcess {
 lldb::addr_t start;
 lldb::addr_t end;
 lldb::addr_t file_ofs;
-lldb_private::ConstString path;
+std::string path;
   };
 
   // For ProcessElfCore only



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

[Lldb-commits] [PATCH] D153827: [lldb][NFCI] Remove use of ConstString from ProcessElfCore

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe3921b8bff69: [lldb][NFCI] Remove use of ConstString from 
ProcessElfCore (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153827

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.h


Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -20,7 +20,6 @@
 #include 
 
 #include "lldb/Target/PostMortemProcess.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
 
 #include "Plugins/ObjectFile/ELF/ELFHeader.h"
@@ -117,7 +116,7 @@
 lldb::addr_t start;
 lldb::addr_t end;
 lldb::addr_t file_ofs;
-lldb_private::ConstString path;
+std::string path;
   };
 
   // For ProcessElfCore only
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -258,8 +258,8 @@
 if (!m_nt_file_entries.empty()) {
   ModuleSpec exe_module_spec;
   exe_module_spec.GetArchitecture() = arch;
-  exe_module_spec.GetFileSpec().SetFile(
-  m_nt_file_entries[0].path.GetCString(), FileSpec::Style::native);
+  exe_module_spec.GetFileSpec().SetFile(m_nt_file_entries[0].path,
+FileSpec::Style::native);
   if (exe_module_spec.GetFileSpec()) {
 exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec, 
   true /* notify */);
@@ -938,7 +938,7 @@
   for (uint64_t i = 0; i < count; ++i) {
 const char *path = note.data.GetCStr(&offset);
 if (path && path[0])
-  m_nt_file_entries[i].path.SetCString(path);
+  m_nt_file_entries[i].path.assign(path);
   }
   break;
 }


Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -20,7 +20,6 @@
 #include 
 
 #include "lldb/Target/PostMortemProcess.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
 
 #include "Plugins/ObjectFile/ELF/ELFHeader.h"
@@ -117,7 +116,7 @@
 lldb::addr_t start;
 lldb::addr_t end;
 lldb::addr_t file_ofs;
-lldb_private::ConstString path;
+std::string path;
   };
 
   // For ProcessElfCore only
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -258,8 +258,8 @@
 if (!m_nt_file_entries.empty()) {
   ModuleSpec exe_module_spec;
   exe_module_spec.GetArchitecture() = arch;
-  exe_module_spec.GetFileSpec().SetFile(
-  m_nt_file_entries[0].path.GetCString(), FileSpec::Style::native);
+  exe_module_spec.GetFileSpec().SetFile(m_nt_file_entries[0].path,
+FileSpec::Style::native);
   if (exe_module_spec.GetFileSpec()) {
 exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec, 
   true /* notify */);
@@ -938,7 +938,7 @@
   for (uint64_t i = 0; i < count; ++i) {
 const char *path = note.data.GetCStr(&offset);
 if (path && path[0])
-  m_nt_file_entries[i].path.SetCString(path);
+  m_nt_file_entries[i].path.assign(path);
   }
   break;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

[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 modulo one more lock.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h:381
 
   uint32_t IsExecutingPython() const { return m_lock_count > 0; }
 

We should probably take the lock here as well. It won't be super meaningful 
(the value can change after the function returns) but I think TSan will 
complain about the potential race. 


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

https://reviews.llvm.org/D154271

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

[Lldb-commits] [lldb] ebec53e - [lldb] Introduce a macro to mark methods as unsupported with no replacement

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-07-03T11:17:48-07:00
New Revision: ebec53e2d7c79b519aec455d048f5f03fc2e7abe

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

LOG: [lldb] Introduce a macro to mark methods as unsupported with no replacement

We already have LLDB_DEPRECATED which is used to mark methods as
deprecated with a message and an alternative to use instead. This is
expresses an intent of "We recognize this functionality is useful but
there are some pitfalls with the interface we have exposed."

In other cases, there are no "alternative" methods to use and the code should be
refactored to avoid using a method entirely. For example,
`SBValue::Cast` should be avoided in favor of using the expression
evaluator to perform a cast. There isn't a mechanical solution, the
recommendation is to instead refactor your code.

This commit renames the existing `LLDB_DEPRECATED` to
`LLDB_DEPRECATED_FIXME`, and adds a `LLDB_DEPRECATED` macro to cover the
second scenario.

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

Added: 


Modified: 
lldb/include/lldb/API/SBCommandReturnObject.h
lldb/include/lldb/API/SBDebugger.h
lldb/include/lldb/API/SBDefines.h
lldb/include/lldb/API/SBFileSpec.h
lldb/include/lldb/API/SBProcess.h
lldb/include/lldb/API/SBStructuredData.h
lldb/include/lldb/API/SBTarget.h
lldb/include/lldb/API/SBValue.h
lldb/include/lldb/lldb-defines.h

Removed: 




diff  --git a/lldb/include/lldb/API/SBCommandReturnObject.h 
b/lldb/include/lldb/API/SBCommandReturnObject.h
index ce93b28ba84cb1..f96384a4710b16 100644
--- a/lldb/include/lldb/API/SBCommandReturnObject.h
+++ b/lldb/include/lldb/API/SBCommandReturnObject.h
@@ -47,8 +47,8 @@ class LLDB_API SBCommandReturnObject {
   const char *GetError();
 
 #ifndef SWIG
-  LLDB_DEPRECATED("Use PutOutput(SBFile) or PutOutput(FileSP)",
-  "PutOutput(SBFile)")
+  LLDB_DEPRECATED_FIXME("Use PutOutput(SBFile) or PutOutput(FileSP)",
+"PutOutput(SBFile)")
   size_t PutOutput(FILE *fh);
 #endif
 
@@ -61,8 +61,8 @@ class LLDB_API SBCommandReturnObject {
   size_t GetErrorSize();
 
 #ifndef SWIG
-  LLDB_DEPRECATED("Use PutError(SBFile) or PutError(FileSP)",
-  "PutError(SBFile)")
+  LLDB_DEPRECATED_FIXME("Use PutError(SBFile) or PutError(FileSP)",
+"PutError(SBFile)")
   size_t PutError(FILE *fh);
 #endif
 
@@ -87,22 +87,22 @@ class LLDB_API SBCommandReturnObject {
   bool GetDescription(lldb::SBStream &description);
 
 #ifndef SWIG
-  LLDB_DEPRECATED(
+  LLDB_DEPRECATED_FIXME(
   "Use SetImmediateOutputFile(SBFile) or SetImmediateOutputFile(FileSP)",
   "SetImmediateOutputFile(SBFile)")
   void SetImmediateOutputFile(FILE *fh);
 
-  LLDB_DEPRECATED(
+  LLDB_DEPRECATED_FIXME(
   "Use SetImmediateErrorFile(SBFile) or SetImmediateErrorFile(FileSP)",
   "SetImmediateErrorFile(SBFile)")
   void SetImmediateErrorFile(FILE *fh);
 
-  LLDB_DEPRECATED(
+  LLDB_DEPRECATED_FIXME(
   "Use SetImmediateOutputFile(SBFile) or SetImmediateOutputFile(FileSP)",
   "SetImmediateOutputFile(SBFile)")
   void SetImmediateOutputFile(FILE *fh, bool transfer_ownership);
 
-  LLDB_DEPRECATED(
+  LLDB_DEPRECATED_FIXME(
   "Use SetImmediateErrorFile(SBFile) or SetImmediateErrorFile(FileSP)",
   "SetImmediateErrorFile(SBFile)")
   void SetImmediateErrorFile(FILE *fh, bool transfer_ownership);

diff  --git a/lldb/include/lldb/API/SBDebugger.h 
b/lldb/include/lldb/API/SBDebugger.h
index 5a2ea634d882d5..57199d3fcb5e68 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -115,7 +115,7 @@ class LLDB_API SBDebugger {
 
   static void Terminate();
 
-  LLDB_DEPRECATED("Use one of the other Create variants", "Create(bool)")
+  LLDB_DEPRECATED_FIXME("Use one of the other Create variants", "Create(bool)")
   static lldb::SBDebugger Create();
 
   static lldb::SBDebugger Create(bool source_init_files);
@@ -208,14 +208,13 @@ class LLDB_API SBDebugger {
   lldb::SBListener GetListener();
 
 #ifndef SWIG
-  LLDB_DEPRECATED(
+  LLDB_DEPRECATED_FIXME(
   "Use HandleProcessEvent(const SBProcess &, const SBEvent &, SBFile, "
   "SBFile) or HandleProcessEvent(const SBProcess &, const SBEvent &, "
   "FileSP, FileSP)",
   "HandleProcessEvent(const SBProcess &, const SBEvent &, SBFile, SBFile)")
   void HandleProcessEvent(const lldb::SBProcess &process,
-  const lldb::SBEvent &event, FILE *out,
-  FILE *err);
+  const lldb::SBEvent &event, FILE *out, FILE *err);
 #endif
 
   void HandleProcessEvent(const lldb::SBProcess &process,
@@ -332,8 +331,8 @@ class LLDB_API SBDebugger {
   void *baton);
 
 #

[Lldb-commits] [PATCH] D153928: [lldb] Introduce a macro to mark methods as unsupported with no replacement

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGebec53e2d7c7: [lldb] Introduce a macro to mark methods as 
unsupported with no replacement (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153928

Files:
  lldb/include/lldb/API/SBCommandReturnObject.h
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBFileSpec.h
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/API/SBValue.h
  lldb/include/lldb/lldb-defines.h

Index: lldb/include/lldb/lldb-defines.h
===
--- lldb/include/lldb/lldb-defines.h
+++ lldb/include/lldb/lldb-defines.h
@@ -125,10 +125,13 @@
 
 #define UNUSED_IF_ASSERT_DISABLED(x) ((void)(x))
 
+#define LLDB_DEPRECATED(MSG)   \
+  [[deprecated("This method is no longer supported: " MSG)]]
+
 #if defined(__clang__)
-#define LLDB_DEPRECATED(MSG, FIX) __attribute__((deprecated(MSG, FIX)))
+#define LLDB_DEPRECATED_FIXME(MSG, FIX) __attribute__((deprecated(MSG, FIX)))
 #else
-#define LLDB_DEPRECATED(MSG, FIX) [[deprecated(MSG)]]
+#define LLDB_DEPRECATED_FIXME(MSG, FIX) LLDB_DEPRECATED(MSG)
 #endif
 
 #endif // LLDB_LLDB_DEFINES_H
Index: lldb/include/lldb/API/SBValue.h
===
--- lldb/include/lldb/API/SBValue.h
+++ lldb/include/lldb/API/SBValue.h
@@ -105,8 +105,8 @@
 
   const char *GetLocation();
 
-  LLDB_DEPRECATED("Use the variant that takes an SBError &",
-  "SetValueFromCString(const char *, SBError &)")
+  LLDB_DEPRECATED_FIXME("Use the variant that takes an SBError &",
+"SetValueFromCString(const char *, SBError &)")
   bool SetValueFromCString(const char *value_str);
 
   bool SetValueFromCString(const char *value_str, lldb::SBError &error);
@@ -124,7 +124,7 @@
   lldb::SBValue CreateChildAtOffset(const char *name, uint32_t offset,
 lldb::SBType type);
 
-  LLDB_DEPRECATED("Use the expression evaluator to perform type casting", "")
+  LLDB_DEPRECATED("Use the expression evaluator to perform type casting")
   lldb::SBValue Cast(lldb::SBType type);
 
   lldb::SBValue CreateValueFromExpression(const char *name,
@@ -295,7 +295,7 @@
 
   lldb::SBValue Dereference();
 
-  LLDB_DEPRECATED("Use GetType().IsPointerType() instead", "")
+  LLDB_DEPRECATED("Use GetType().IsPointerType() instead")
   bool TypeIsPointerType();
 
   lldb::SBType GetType();
Index: lldb/include/lldb/API/SBTarget.h
===
--- lldb/include/lldb/API/SBTarget.h
+++ lldb/include/lldb/API/SBTarget.h
@@ -396,8 +396,8 @@
   /// \return
   /// An error to indicate success, fail, and any reason for
   /// failure.
-  LLDB_DEPRECATED("Use SetModuleLoadAddress(lldb::SBModule, uint64_t)",
-  "SetModuleLoadAddress(lldb::SBModule, uint64_t)")
+  LLDB_DEPRECATED_FIXME("Use SetModuleLoadAddress(lldb::SBModule, uint64_t)",
+"SetModuleLoadAddress(lldb::SBModule, uint64_t)")
   lldb::SBError SetModuleLoadAddress(lldb::SBModule module,
  int64_t sections_offset);
 #endif
Index: lldb/include/lldb/API/SBStructuredData.h
===
--- lldb/include/lldb/API/SBStructuredData.h
+++ lldb/include/lldb/API/SBStructuredData.h
@@ -72,8 +72,9 @@
   /// Return the integer value if this data structure is an integer type.
   int64_t GetSignedIntegerValue(int64_t fail_value = 0) const;
 
-  LLDB_DEPRECATED("Specify if the value is signed or unsigned",
-  "uint64_t GetUnsignedIntegerValue(uint64_t fail_value = 0)")
+  LLDB_DEPRECATED_FIXME(
+  "Specify if the value is signed or unsigned",
+  "uint64_t GetUnsignedIntegerValue(uint64_t fail_value = 0)")
   uint64_t GetIntegerValue(uint64_t fail_value = 0) const;
 
   /// Return the floating point value if this data structure is a floating
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -48,7 +48,7 @@
 
   const char *GetPluginName();
 
-  LLDB_DEPRECATED("Use GetPluginName()", "GetPluginName()")
+  LLDB_DEPRECATED_FIXME("Use GetPluginName()", "GetPluginName()")
   const char *GetShortPluginName();
 
   void Clear();
Index: lldb/include/lldb/API/SBFileSpec.h
===
--- lldb/include/lldb/API/SBFileSpec.h
+++ lldb/include/lldb/API/SBFileSpec.h
@@ -19,9 +19,10 @@
 
   SBFileSpec(const lldb::SBFileSpec &rhs);
 
-  LLDB_DEPRECATED("Use the other constructor to determine if this the 

[Lldb-commits] [PATCH] D154365: [lldb][nfc] Remove redundant nullptr check

[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/D154365/new/

https://reviews.llvm.org/D154365

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

[Lldb-commits] [PATCH] D153834: [lldb] Add two-level caching in the source manager

[Alex Langford via Phabricator via lldb-commits]

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

LGTM!


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

https://reviews.llvm.org/D153834

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

[Lldb-commits] [lldb] b709149 - [lldb][NFCI] Target::StopHook::GetDescription should take a Stream ref instead of pointer

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-07-03T11:39:38-07:00
New Revision: b709149b76277e8bea4a3d7977ccb4e818499b7b

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

LOG: [lldb][NFCI] Target::StopHook::GetDescription should take a Stream ref 
instead of pointer

We always assume that this is valid anyway, might as well take a
reference.

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

Added: 


Modified: 
lldb/include/lldb/Target/Target.h
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index eb5e87dcacb395..50cd315b0fdfa7 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1310,8 +1310,8 @@ class Target : public 
std::enable_shared_from_this,
 
 bool GetAutoContinue() const { return m_auto_continue; }
 
-void GetDescription(Stream *s, lldb::DescriptionLevel level) const;
-virtual void GetSubclassDescription(Stream *s,
+void GetDescription(Stream &s, lldb::DescriptionLevel level) const;
+virtual void GetSubclassDescription(Stream &s,
 lldb::DescriptionLevel level) const = 
0;
 
   protected:
@@ -1334,7 +1334,7 @@ class Target : public 
std::enable_shared_from_this,
 
 StopHookResult HandleStop(ExecutionContext &exc_ctx,
   lldb::StreamSP output_sp) override;
-void GetSubclassDescription(Stream *s,
+void GetSubclassDescription(Stream &s,
 lldb::DescriptionLevel level) const override;
 
   private:
@@ -1356,7 +1356,7 @@ class Target : public 
std::enable_shared_from_this,
 Status SetScriptCallback(std::string class_name,
  StructuredData::ObjectSP extra_args_sp);
 
-void GetSubclassDescription(Stream *s,
+void GetSubclassDescription(Stream &s,
 lldb::DescriptionLevel level) const override;
 
   private:

diff  --git a/lldb/source/Commands/CommandCompletions.cpp 
b/lldb/source/Commands/CommandCompletions.cpp
index 35237d419d469b..dad63b6aaa1744 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -754,7 +754,7 @@ void CommandCompletions::StopHookIDs(CommandInterpreter 
&interpreter,
 // neater.
 strm.SetIndentLevel(11);
 const Target::StopHookSP stophook_sp = target_sp->GetStopHookAtIndex(idx);
-stophook_sp->GetDescription(&strm, lldb::eDescriptionLevelInitial);
+stophook_sp->GetDescription(strm, lldb::eDescriptionLevelInitial);
 request.TryCompleteCurrentArg(std::to_string(stophook_sp->GetID()),
   strm.GetString());
   }

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 22045948f88ba9..b186c8e6c1ab8f 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -5057,7 +5057,7 @@ class CommandObjectTargetStopHookList : public 
CommandObjectParsed {
 Target::StopHookSP this_hook = target.GetStopHookAtIndex(i);
 if (i > 0)
   result.GetOutputStream().PutCString("\n");
-this_hook->GetDescription(&(result.GetOutputStream()),
+this_hook->GetDescription(result.GetOutputStream(),
   eDescriptionLevelFull);
   }
 }

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 17181569cbca29..d9cd0f0a3d1570 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2892,7 +2892,7 @@ bool Target::RunStopHooks() {
 
   if (print_hook_header && !any_thread_matched) {
 StreamString s;
-cur_hook_sp->GetDescription(&s, eDescriptionLevelBrief);
+cur_hook_sp->GetDescription(s, eDescriptionLevelBrief);
 if (s.GetSize() != 0)
   output_sp->Printf("\n- Hook %" PRIu64 " (%s)\n", 
cur_hook_sp->GetID(),
 s.GetData());
@@ -3651,7 +3651,7 @@ bool Target::StopHook::ExecutionContextPasses(const 
ExecutionContext &exc_ctx) {
   return will_run;
 }
 
-void Target::StopHook::GetDescription(Stream *s,
+void Target::StopHook::GetDescription(Stream &s,
   lldb::DescriptionLevel level) const {
 
   // For brief descriptions, only print the subclass description:
@@ -3660,55 +3660,55 @@ void Target::StopHook::GetDescription(Stream *s,
 return;
   }
 
-  unsigned indent_level = s->GetIndentLevel();
+  unsigned indent_level = s.GetIndentLevel();
 
-  s->SetIndentLevel(indent_level + 2);
+  s.SetIndentLevel(indent_level + 2);
 
-  s->Printf("H

[Lldb-commits] [PATCH] D153917: [lldb][NFCI] Target::StopHook::GetDescription should take a Stream ref instead of pointer

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb709149b7627: [lldb][NFCI] Target::StopHook::GetDescription 
should take a Stream ref instead… (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153917

Files:
  lldb/include/lldb/Target/Target.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2892,7 +2892,7 @@
 
   if (print_hook_header && !any_thread_matched) {
 StreamString s;
-cur_hook_sp->GetDescription(&s, eDescriptionLevelBrief);
+cur_hook_sp->GetDescription(s, eDescriptionLevelBrief);
 if (s.GetSize() != 0)
   output_sp->Printf("\n- Hook %" PRIu64 " (%s)\n", cur_hook_sp->GetID(),
 s.GetData());
@@ -3651,7 +3651,7 @@
   return will_run;
 }
 
-void Target::StopHook::GetDescription(Stream *s,
+void Target::StopHook::GetDescription(Stream &s,
   lldb::DescriptionLevel level) const {
 
   // For brief descriptions, only print the subclass description:
@@ -3660,55 +3660,55 @@
 return;
   }
 
-  unsigned indent_level = s->GetIndentLevel();
+  unsigned indent_level = s.GetIndentLevel();
 
-  s->SetIndentLevel(indent_level + 2);
+  s.SetIndentLevel(indent_level + 2);
 
-  s->Printf("Hook: %" PRIu64 "\n", GetID());
+  s.Printf("Hook: %" PRIu64 "\n", GetID());
   if (m_active)
-s->Indent("State: enabled\n");
+s.Indent("State: enabled\n");
   else
-s->Indent("State: disabled\n");
+s.Indent("State: disabled\n");
 
   if (m_auto_continue)
-s->Indent("AutoContinue on\n");
+s.Indent("AutoContinue on\n");
 
   if (m_specifier_sp) {
-s->Indent();
-s->PutCString("Specifier:\n");
-s->SetIndentLevel(indent_level + 4);
-m_specifier_sp->GetDescription(s, level);
-s->SetIndentLevel(indent_level + 2);
+s.Indent();
+s.PutCString("Specifier:\n");
+s.SetIndentLevel(indent_level + 4);
+m_specifier_sp->GetDescription(&s, level);
+s.SetIndentLevel(indent_level + 2);
   }
 
   if (m_thread_spec_up) {
 StreamString tmp;
-s->Indent("Thread:\n");
+s.Indent("Thread:\n");
 m_thread_spec_up->GetDescription(&tmp, level);
-s->SetIndentLevel(indent_level + 4);
-s->Indent(tmp.GetString());
-s->PutCString("\n");
-s->SetIndentLevel(indent_level + 2);
+s.SetIndentLevel(indent_level + 4);
+s.Indent(tmp.GetString());
+s.PutCString("\n");
+s.SetIndentLevel(indent_level + 2);
   }
   GetSubclassDescription(s, level);
 }
 
 void Target::StopHookCommandLine::GetSubclassDescription(
-Stream *s, lldb::DescriptionLevel level) const {
+Stream &s, lldb::DescriptionLevel level) const {
   // The brief description just prints the first command.
   if (level == eDescriptionLevelBrief) {
 if (m_commands.GetSize() == 1)
-  s->PutCString(m_commands.GetStringAtIndex(0));
+  s.PutCString(m_commands.GetStringAtIndex(0));
 return;
   }
-  s->Indent("Commands: \n");
-  s->SetIndentLevel(s->GetIndentLevel() + 4);
+  s.Indent("Commands: \n");
+  s.SetIndentLevel(s.GetIndentLevel() + 4);
   uint32_t num_commands = m_commands.GetSize();
   for (uint32_t i = 0; i < num_commands; i++) {
-s->Indent(m_commands.GetStringAtIndex(i));
-s->PutCString("\n");
+s.Indent(m_commands.GetStringAtIndex(i));
+s.PutCString("\n");
   }
-  s->SetIndentLevel(s->GetIndentLevel() - 4);
+  s.SetIndentLevel(s.GetIndentLevel() - 4);
 }
 
 // Target::StopHookCommandLine
@@ -3796,13 +3796,13 @@
 }
 
 void Target::StopHookScripted::GetSubclassDescription(
-Stream *s, lldb::DescriptionLevel level) const {
+Stream &s, lldb::DescriptionLevel level) const {
   if (level == eDescriptionLevelBrief) {
-s->PutCString(m_class_name);
+s.PutCString(m_class_name);
 return;
   }
-  s->Indent("Class:");
-  s->Printf("%s\n", m_class_name.c_str());
+  s.Indent("Class:");
+  s.Printf("%s\n", m_class_name.c_str());
 
   // Now print the extra args:
   // FIXME: We should use StructuredData.GetDescription on the m_extra_args
@@ -3821,20 +3821,20 @@
   if (num_keys == 0)
 return;
 
-  s->Indent("Args:\n");
-  s->SetIndentLevel(s->GetIndentLevel() + 4);
+  s.Indent("Args:\n");
+  s.SetIndentLevel(s.GetIndentLevel() + 4);
 
   auto print_one_element = [&s](ConstString key,
 StructuredData::Object *object) {
-s->Indent();
-s->Printf("%s : %s\n", key.GetCString(),
+s.Indent();
+s.Printf("%s : %s\n", key.GetCString(),
   object->GetStringValue().str().c_str());
 return true;
   };
 
   as_dict->ForEach(print_one_element);
 
-  s->SetIndentLevel(s->GetIndentLevel() - 4);
+  s.Set

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 536859.
mib added a comment.

Address @JDevlieghere last comment.


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

https://reviews.llvm.org/D154271

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));
Index: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -378,11 +378,18 @@
 
   void LeaveSession();
 
-  uint32_t IsExecutingPython() const { return m_lock_count > 0; }
+  uint32_t IsExecutingPython() {
+std::lock_guard guard(m_mutex);
+return m_lock_count > 0;
+  }
 
-  uint32_t IncrementLockCount() { return ++m_lock_count; }
+  uint32_t IncrementLockCount() {
+std::lock_guard guard(m_mutex);
+return ++m_lock_count;
+  }
 
   uint32_t DecrementLockCount() {
+std::lock_guard guard(m_mutex);
 if (m_lock_count > 0)
   --m_lock_count;
 return m_lock_count;
@@ -422,6 +429,7 @@
   bool m_pty_secondary_is_open;
   bool m_valid_session;
   uint32_t m_lock_count;
+  std::mutex m_mutex;
   PyThreadState *m_command_thread_state;
 };
 


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -378,11 +378,18 @@
 
   void LeaveSession();
 
-  uint32_t IsExecutingPython() const { return m_lock_count > 0; }
+  uint32_t IsExecutingPython() {
+std::lock_guard guard(m_mutex);
+return m_lock_count > 0;
+  }
 
-  uint32_t IncrementLockCount() { return ++m_lock_count; }
+  uint32_t IncrementLockCount() {
+std::lock_guard guard(m_mutex);
+return ++m_lock_count;
+  }
 
   uint32_t DecrementLockCount() {
+std::lock_guard guard(m_mutex);
 if (m_lock_count > 0)
   --m_lock_count;
 return m_lock_count;
@@ -422,6 +429,7 @@
   bool m_pty_secondary_is_open;
   bool m_valid_session;
   uint32_t m_lock_count;
+  std::mutex m_mutex;
   PyThreadState *m_command_thread_state;
 };
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 9987646 - [lldb] Fix data race when interacting with python scripts

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2023-07-03T11:49:11-07:00
New Revision: 9987646057d9748514662f96c869a5f87e463bc6

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

LOG: [lldb] Fix data race when interacting with python scripts

This patch should fix some data races when a python script (i.e. a
Scripted Process) has a nested call to another python script (i.e. a
OperatingSystem Plugin), which can cause concurrent writes to the python
lock count.

This patch also fixes a data race happening when resetting the operating
system unique pointer.

To address these issues, both accesses is guarded by a mutex.

rdar://109413039

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/source/Target/Process.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h 
b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
index fe75f69fa2a03a..55ade49a1245c7 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -378,11 +378,18 @@ class ScriptInterpreterPythonImpl : public 
ScriptInterpreterPython {
 
   void LeaveSession();
 
-  uint32_t IsExecutingPython() const { return m_lock_count > 0; }
+  uint32_t IsExecutingPython() {
+std::lock_guard guard(m_mutex);
+return m_lock_count > 0;
+  }
 
-  uint32_t IncrementLockCount() { return ++m_lock_count; }
+  uint32_t IncrementLockCount() {
+std::lock_guard guard(m_mutex);
+return ++m_lock_count;
+  }
 
   uint32_t DecrementLockCount() {
+std::lock_guard guard(m_mutex);
 if (m_lock_count > 0)
   --m_lock_count;
 return m_lock_count;
@@ -422,6 +429,7 @@ class ScriptInterpreterPythonImpl : public 
ScriptInterpreterPython {
   bool m_pty_secondary_is_open;
   bool m_valid_session;
   uint32_t m_lock_count;
+  std::mutex m_mutex;
   PyThreadState *m_command_thread_state;
 };
 

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 4059660e942667..ca2eac7d8e5b04 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@ Process::WaitForProcessStopPrivate(EventSP &event_sp,
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));



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

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9987646057d9: [lldb] Fix data race when interacting with 
python scripts (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154271

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));
Index: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -378,11 +378,18 @@
 
   void LeaveSession();
 
-  uint32_t IsExecutingPython() const { return m_lock_count > 0; }
+  uint32_t IsExecutingPython() {
+std::lock_guard guard(m_mutex);
+return m_lock_count > 0;
+  }
 
-  uint32_t IncrementLockCount() { return ++m_lock_count; }
+  uint32_t IncrementLockCount() {
+std::lock_guard guard(m_mutex);
+return ++m_lock_count;
+  }
 
   uint32_t DecrementLockCount() {
+std::lock_guard guard(m_mutex);
 if (m_lock_count > 0)
   --m_lock_count;
 return m_lock_count;
@@ -422,6 +429,7 @@
   bool m_pty_secondary_is_open;
   bool m_valid_session;
   uint32_t m_lock_count;
+  std::mutex m_mutex;
   PyThreadState *m_command_thread_state;
 };
 


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -378,11 +378,18 @@
 
   void LeaveSession();
 
-  uint32_t IsExecutingPython() const { return m_lock_count > 0; }
+  uint32_t IsExecutingPython() {
+std::lock_guard guard(m_mutex);
+return m_lock_count > 0;
+  }
 
-  uint32_t IncrementLockCount() { return ++m_lock_count; }
+  uint32_t IncrementLockCount() {
+std::lock_guard guard(m_mutex);
+return ++m_lock_count;
+  }
 
   uint32_t DecrementLockCount() {
+std::lock_guard guard(m_mutex);
 if (m_lock_count > 0)
   --m_lock_count;
 return m_lock_count;
@@ -422,6 +429,7 @@
   bool m_pty_secondary_is_open;
   bool m_valid_session;
   uint32_t m_lock_count;
+  std::mutex m_mutex;
   PyThreadState *m_command_thread_state;
 };
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153900: [lldb] Deprecate SBHostOS threading functionality

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 536864.
bulbazord added a comment.

Reword deprecation message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153900

Files:
  lldb/include/lldb/API/SBHostOS.h
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -795,8 +795,6 @@
   // Setup LLDB signal handlers once the debugger has been initialized.
   SBDebugger::PrintDiagnosticsOnError();
 
-  SBHostOS::ThreadCreated("");
-
   signal(SIGINT, sigint_handler);
 #if !defined(_WIN32)
   signal(SIGPIPE, SIG_IGN);
Index: lldb/include/lldb/API/SBHostOS.h
===
--- lldb/include/lldb/API/SBHostOS.h
+++ lldb/include/lldb/API/SBHostOS.h
@@ -24,15 +24,26 @@
 
   static lldb::SBFileSpec GetUserHomeDirectory();
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static void ThreadCreated(const char *name);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static lldb::thread_t ThreadCreate(const char *name,
  lldb::thread_func_t thread_function,
  void *thread_arg, lldb::SBError *err);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadCancel(lldb::thread_t thread, lldb::SBError *err);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadDetach(lldb::thread_t thread, lldb::SBError *err);
+
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadJoin(lldb::thread_t thread, lldb::thread_result_t *result,
  lldb::SBError *err);
 


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -795,8 +795,6 @@
   // Setup LLDB signal handlers once the debugger has been initialized.
   SBDebugger::PrintDiagnosticsOnError();
 
-  SBHostOS::ThreadCreated("");
-
   signal(SIGINT, sigint_handler);
 #if !defined(_WIN32)
   signal(SIGPIPE, SIG_IGN);
Index: lldb/include/lldb/API/SBHostOS.h
===
--- lldb/include/lldb/API/SBHostOS.h
+++ lldb/include/lldb/API/SBHostOS.h
@@ -24,15 +24,26 @@
 
   static lldb::SBFileSpec GetUserHomeDirectory();
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static void ThreadCreated(const char *name);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static lldb::thread_t ThreadCreate(const char *name,
  lldb::thread_func_t thread_function,
  void *thread_arg, lldb::SBError *err);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadCancel(lldb::thread_t thread, lldb::SBError *err);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadDetach(lldb::thread_t thread, lldb::SBError *err);
+
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadJoin(lldb::thread_t thread, lldb::thread_result_t *result,
  lldb::SBError *err);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153918: [lldb][NFCI] Deprecate SBValue::GetOpaqueType

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 536865.
bulbazord added a comment.

Address feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153918

Files:
  lldb/include/lldb/API/SBValue.h


Index: lldb/include/lldb/API/SBValue.h
===
--- lldb/include/lldb/API/SBValue.h
+++ lldb/include/lldb/API/SBValue.h
@@ -283,6 +283,7 @@
 
   uint32_t GetNumChildren(uint32_t max);
 
+  LLDB_DEPRECATED("SBValue::GetOpaqueType() is deprecated.")
   void *GetOpaqueType();
 
   lldb::SBTarget GetTarget();


Index: lldb/include/lldb/API/SBValue.h
===
--- lldb/include/lldb/API/SBValue.h
+++ lldb/include/lldb/API/SBValue.h
@@ -283,6 +283,7 @@
 
   uint32_t GetNumChildren(uint32_t max);
 
+  LLDB_DEPRECATED("SBValue::GetOpaqueType() is deprecated.")
   void *GetOpaqueType();
 
   lldb::SBTarget GetTarget();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154386: [lldb][NFCI] Remove use of ConstString from OptionValue

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

No need to create a ConstString, `GetName` already returns a StringRef.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154386

Files:
  lldb/source/Interpreter/OptionValue.cpp


Index: lldb/source/Interpreter/OptionValue.cpp
===
--- lldb/source/Interpreter/OptionValue.cpp
+++ lldb/source/Interpreter/OptionValue.cpp
@@ -534,8 +534,8 @@
 if (m_parent_sp->DumpQualifiedName(strm))
   dumped_something = true;
   }
-  ConstString name(GetName());
-  if (name) {
+  llvm::StringRef name(GetName());
+  if (!name.empty()) {
 if (dumped_something)
   strm.PutChar('.');
 else


Index: lldb/source/Interpreter/OptionValue.cpp
===
--- lldb/source/Interpreter/OptionValue.cpp
+++ lldb/source/Interpreter/OptionValue.cpp
@@ -534,8 +534,8 @@
 if (m_parent_sp->DumpQualifiedName(strm))
   dumped_something = true;
   }
-  ConstString name(GetName());
-  if (name) {
+  llvm::StringRef name(GetName());
+  if (!name.empty()) {
 if (dumped_something)
   strm.PutChar('.');
 else
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154386: [lldb][NFCI] Remove use of ConstString from OptionValue

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 536873.
bulbazord added a comment.

Small fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154386

Files:
  lldb/source/Interpreter/OptionValue.cpp


Index: lldb/source/Interpreter/OptionValue.cpp
===
--- lldb/source/Interpreter/OptionValue.cpp
+++ lldb/source/Interpreter/OptionValue.cpp
@@ -534,8 +534,8 @@
 if (m_parent_sp->DumpQualifiedName(strm))
   dumped_something = true;
   }
-  ConstString name(GetName());
-  if (name) {
+  llvm::StringRef name(GetName());
+  if (!name.empty()) {
 if (dumped_something)
   strm.PutChar('.');
 else


Index: lldb/source/Interpreter/OptionValue.cpp
===
--- lldb/source/Interpreter/OptionValue.cpp
+++ lldb/source/Interpreter/OptionValue.cpp
@@ -534,8 +534,8 @@
 if (m_parent_sp->DumpQualifiedName(strm))
   dumped_something = true;
   }
-  ConstString name(GetName());
-  if (name) {
+  llvm::StringRef name(GetName());
+  if (!name.empty()) {
 if (dumped_something)
   strm.PutChar('.');
 else
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154387: [lldb][NFCI] Minor cleanup of default OptionValue::GetSubValue implementation

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This does 2 things:

- Corrects a minor typo (`value subvalue` -> `valid subvalue`)
- Removes an unnecessary instance of `str().c_str()` (creating a temporary 
std::string from a StringRef just to get a valid null-terminated string).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154387

Files:
  lldb/include/lldb/Interpreter/OptionValue.h


Index: lldb/include/lldb/Interpreter/OptionValue.h
===
--- lldb/include/lldb/Interpreter/OptionValue.h
+++ lldb/include/lldb/Interpreter/OptionValue.h
@@ -114,8 +114,7 @@
   virtual lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
   llvm::StringRef name,
   Status &error) const {
-error.SetErrorStringWithFormat("'%s' is not a value subvalue",
-   name.str().c_str());
+error.SetErrorStringWithFormatv("'{0}' is not a valid subvalue", name);
 return lldb::OptionValueSP();
   }
 


Index: lldb/include/lldb/Interpreter/OptionValue.h
===
--- lldb/include/lldb/Interpreter/OptionValue.h
+++ lldb/include/lldb/Interpreter/OptionValue.h
@@ -114,8 +114,7 @@
   virtual lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
   llvm::StringRef name,
   Status &error) const {
-error.SetErrorStringWithFormat("'%s' is not a value subvalue",
-   name.str().c_str());
+error.SetErrorStringWithFormatv("'{0}' is not a valid subvalue", name);
 return lldb::OptionValueSP();
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153900: [lldb] Deprecate SBHostOS threading functionality

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153900

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

[Lldb-commits] [PATCH] D153900: [lldb] Deprecate SBHostOS threading functionality

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

LGTM! @bulbazord when you land this, make sure to close @teemperor's diff 
(D104231 ). Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153900

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

[Lldb-commits] [lldb] f470ab7 - [lldb][nfc] Remove redundant nullptr check

[Felipe de Azevedo Piovezan via lldb-commits]

Author: Felipe de Azevedo Piovezan
Date: 2023-07-03T17:12:06-04:00
New Revision: f470ab734c9bae9464faf6cff6530beb32ea44ba

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

LOG: [lldb][nfc] Remove redundant nullptr check

The make_shared function never returns a nullptr, as such the test for nullptr
is not needed. We also move the empty string check earlier in the if
("oso_object"), as this is cheaper than loading the object file.

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
index 1d79b13a6633f0..131933777bf31c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -471,18 +471,17 @@ Module *SymbolFileDWARFDebugMap::GetModuleByCompUnitInfo(
   oso_arch, oso_object ? &oso_object : nullptr, 0,
   oso_object ? comp_unit_info->oso_mod_time : 
llvm::sys::TimePoint<>());
 
-  if (!comp_unit_info->oso_sp->module_sp || 
!comp_unit_info->oso_sp->module_sp->GetObjectFile()) {
-if (oso_object && FileSystem::Instance().Exists(oso_file)) {
-  // If we are loading a .o file from a .a file the "oso_object" will
-  // have a valid value name and if the .a file exists, either the .o
-  // file didn't exist in the .a file or the mod time didn't match.
-  comp_unit_info->oso_load_error.SetErrorStringWithFormat(
-  "\"%s\" object from the \"%s\" archive: "
-  "either the .o file doesn't exist in the archive or the "
-  "modification time (0x%8.8x) of the .o file doesn't match",
-  oso_object.AsCString(), oso_file.GetPath().c_str(),
-  (uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time));
-}
+  if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
+  FileSystem::Instance().Exists(oso_file)) {
+// If we are loading a .o file from a .a file the "oso_object" will
+// have a valid value name and if the .a file exists, either the .o
+// file didn't exist in the .a file or the mod time didn't match.
+comp_unit_info->oso_load_error.SetErrorStringWithFormat(
+"\"%s\" object from the \"%s\" archive: "
+"either the .o file doesn't exist in the archive or the "
+"modification time (0x%8.8x) of the .o file doesn't match",
+oso_object.AsCString(), oso_file.GetPath().c_str(),
+(uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time));
   }
 }
   }



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

[Lldb-commits] [PATCH] D154365: [lldb][nfc] Remove redundant nullptr check

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf470ab734c9b: [lldb][nfc] Remove redundant nullptr check 
(authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154365

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


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -471,18 +471,17 @@
   oso_arch, oso_object ? &oso_object : nullptr, 0,
   oso_object ? comp_unit_info->oso_mod_time : 
llvm::sys::TimePoint<>());
 
-  if (!comp_unit_info->oso_sp->module_sp || 
!comp_unit_info->oso_sp->module_sp->GetObjectFile()) {
-if (oso_object && FileSystem::Instance().Exists(oso_file)) {
-  // If we are loading a .o file from a .a file the "oso_object" will
-  // have a valid value name and if the .a file exists, either the .o
-  // file didn't exist in the .a file or the mod time didn't match.
-  comp_unit_info->oso_load_error.SetErrorStringWithFormat(
-  "\"%s\" object from the \"%s\" archive: "
-  "either the .o file doesn't exist in the archive or the "
-  "modification time (0x%8.8x) of the .o file doesn't match",
-  oso_object.AsCString(), oso_file.GetPath().c_str(),
-  (uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time));
-}
+  if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
+  FileSystem::Instance().Exists(oso_file)) {
+// If we are loading a .o file from a .a file the "oso_object" will
+// have a valid value name and if the .a file exists, either the .o
+// file didn't exist in the .a file or the mod time didn't match.
+comp_unit_info->oso_load_error.SetErrorStringWithFormat(
+"\"%s\" object from the \"%s\" archive: "
+"either the .o file doesn't exist in the archive or the "
+"modification time (0x%8.8x) of the .o file doesn't match",
+oso_object.AsCString(), oso_file.GetPath().c_str(),
+(uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time));
   }
 }
   }


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -471,18 +471,17 @@
   oso_arch, oso_object ? &oso_object : nullptr, 0,
   oso_object ? comp_unit_info->oso_mod_time : llvm::sys::TimePoint<>());
 
-  if (!comp_unit_info->oso_sp->module_sp || !comp_unit_info->oso_sp->module_sp->GetObjectFile()) {
-if (oso_object && FileSystem::Instance().Exists(oso_file)) {
-  // If we are loading a .o file from a .a file the "oso_object" will
-  // have a valid value name and if the .a file exists, either the .o
-  // file didn't exist in the .a file or the mod time didn't match.
-  comp_unit_info->oso_load_error.SetErrorStringWithFormat(
-  "\"%s\" object from the \"%s\" archive: "
-  "either the .o file doesn't exist in the archive or the "
-  "modification time (0x%8.8x) of the .o file doesn't match",
-  oso_object.AsCString(), oso_file.GetPath().c_str(),
-  (uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time));
-}
+  if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
+  FileSystem::Instance().Exists(oso_file)) {
+// If we are loading a .o file from a .a file the "oso_object" will
+// have a valid value name and if the .a file exists, either the .o
+// file didn't exist in the .a file or the mod time didn't match.
+comp_unit_info->oso_load_error.SetErrorStringWithFormat(
+"\"%s\" object from the \"%s\" archive: "
+"either the .o file doesn't exist in the archive or the "
+"modification time (0x%8.8x) of the .o file doesn't match",
+oso_object.AsCString(), oso_file.GetPath().c_str(),
+(uint32_t)llvm::sys::toTimeT(comp_unit_info->oso_mod_time));
   }
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 51944e7 - [lldb] Add two-level caching in the source manager

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-07-03T14:12:39-07:00
New Revision: 51944e78bb3760768d62b79e972a8e65baaa1518

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

LOG: [lldb] Add two-level caching in the source manager

We recently saw an uptick in internal reports complaining that LLDB is
slow when sources on network file systems are inaccessible. I looked at
the SourceManger and its cache and I think there’s some room for
improvement in terms of reducing file system accesses:

 1. We always resolve the path.
 2. We always check the timestamp.
 3. We always recheck the file system for negative cache hits.

D153726 fixes (1) but (2) and (3) are necessary because of the cache’s
current design. Source files are cached at the debugger level which
means that the source file cache can span multiple targets and
processes. It wouldn't be correct to not reload a modified or new file
from disk.

We can however significantly reduce the number of file system accesses
by using a two level cache design: one cache at the debugger level and
one at the process level:

 - The cache at the debugger level works the way it does today. There is
   no negative cache: if we can't find the file on disk, we'll try again
   next time the cache is queried. If a cached file's timestamp changes
   or if its path remapping changes, the cached file is evicted and we
   reload it from disk.
 - The cache at the process level is design to avoid accessing the file
   system. It doesn't check the file's modification time. It caches
   negative results, so if a file didn't exist, it doesn't try to reread
   it from disk. Checking if the path remapping changed is cheap
   (doesn't involve checking the file system) and is the only way for a
   file to get evicted from the process cache.

The result of this patch is that LLDB will not show you new content if a
file is modified or created while a process is running. I would argue
that this is what most people would expect, but it is a change from how
LLDB behaves today.

For an average stop, we query the source cache 4 times. With the current
implementation, that's 4 stats to get the modification time, If the file
doesn't exist on disk, that's an additional 4 stats. Before D153726, if
the path starts with a ~ there are another additional 4 calls to
realpath. When debugging sources on a slow (network) file system, this
quickly adds up.

In addition to the two level caching, this patch also adds a source
logging channel and synchronization to the source file cache. The
logging was helpful during development and hopefully will help us triage
issues in the future. The synchronization isn't a new requirement: as
the cache is shared across targets, there is no guarantees that it can't
be accessed concurrently. The patch also fixes a bug where we would only
set the source remapping ID if the un-remapped file didn't exist, which
led to the file getting evicted from the cache on every access.

rdar://110787562

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

Added: 


Modified: 
lldb/include/lldb/Core/SourceManager.h
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Utility/LLDBLog.h
lldb/source/Commands/CommandObjectSource.cpp
lldb/source/Core/SourceManager.cpp
lldb/source/Utility/LLDBLog.cpp
lldb/test/API/source-manager/TestSourceManager.py
lldb/unittests/Core/SourceManagerTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/SourceManager.h 
b/lldb/include/lldb/Core/SourceManager.h
index 24a567747bda8b..5239ac6f4055f5 100644
--- a/lldb/include/lldb/Core/SourceManager.h
+++ b/lldb/include/lldb/Core/SourceManager.h
@@ -14,6 +14,7 @@
 #include "lldb/lldb-forward.h"
 
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/RWMutex.h"
 
 #include 
 #include 
@@ -36,11 +37,11 @@ class SourceManager {
const SourceManager::File &rhs);
 
   public:
-File(const FileSpec &file_spec, Target *target);
+File(const FileSpec &file_spec, lldb::TargetSP target_sp);
 File(const FileSpec &file_spec, lldb::DebuggerSP debugger_sp);
-~File() = default;
 
-void UpdateIfNeeded();
+bool ModificationTimeIsStale() const;
+bool PathRemappingIsStale() const;
 
 size_t DisplaySourceLines(uint32_t line, std::optional column,
   uint32_t context_before, uint32_t context_after,
@@ -89,22 +90,29 @@ class SourceManager {
 typedef std::vector LineOffsets;
 LineOffsets m_offsets;
 lldb::DebuggerWP m_debugger_wp;
+lldb::TargetWP m_target_wp;
 
   private:
-void CommonInitializer(const FileSpec &file_spec, Target *target);
+void CommonInitializer(const FileSpec &file_spec, lldb::TargetSP 
target_sp);
   };
 
   typedef std::shared_ptr FileSP;
 
-  // The 

[Lldb-commits] [PATCH] D153834: [lldb] Add two-level caching in the source manager

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG51944e78bb37: [lldb] Add two-level caching in the source 
manager (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153834

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/LLDBLog.h
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Utility/LLDBLog.cpp
  lldb/test/API/source-manager/TestSourceManager.py
  lldb/unittests/Core/SourceManagerTest.cpp

Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- lldb/unittests/Core/SourceManagerTest.cpp
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -30,7 +30,7 @@
   // Insert: foo
   FileSpec foo_file_spec("foo");
   auto foo_file_sp =
-  std::make_shared(foo_file_spec, nullptr);
+  std::make_shared(foo_file_spec, lldb::DebuggerSP());
   cache.AddSourceFile(foo_file_spec, foo_file_sp);
 
   // Query: foo, expect found.
@@ -44,7 +44,7 @@
   // Insert: foo
   FileSpec foo_file_spec("foo");
   auto foo_file_sp =
-  std::make_shared(foo_file_spec, nullptr);
+  std::make_shared(foo_file_spec, lldb::DebuggerSP());
   cache.AddSourceFile(foo_file_spec, foo_file_sp);
 
   // Query: bar, expect not found.
@@ -62,8 +62,8 @@
   FileSystem::Instance().Resolve(resolved_foo_file_spec);
 
   // Create the file with the resolved file spec.
-  auto foo_file_sp =
-  std::make_shared(resolved_foo_file_spec, nullptr);
+  auto foo_file_sp = std::make_shared(
+  resolved_foo_file_spec, lldb::DebuggerSP());
 
   // Cache the result with the unresolved file spec.
   cache.AddSourceFile(foo_file_spec, foo_file_sp);
Index: lldb/test/API/source-manager/TestSourceManager.py
===
--- lldb/test/API/source-manager/TestSourceManager.py
+++ lldb/test/API/source-manager/TestSourceManager.py
@@ -10,6 +10,7 @@
 """
 
 import os
+import io
 import stat
 
 import lldb
@@ -37,6 +38,33 @@
 self.file = self.getBuildArtifact("main-copy.c")
 self.line = line_number("main.c", "// Set break point at this line.")
 
+def modify_content(self):
+
+# Read the main.c file content.
+with io.open(self.file, "r", newline="\n") as f:
+original_content = f.read()
+if self.TraceOn():
+print("original content:", original_content)
+
+# Modify the in-memory copy of the original source code.
+new_content = original_content.replace("Hello world", "Hello lldb", 1)
+
+# Modify the source code file.
+# If the source was read only, the copy will also be read only.
+# Run "chmod u+w" on it first so we can modify it.
+statinfo = os.stat(self.file)
+os.chmod(self.file, statinfo.st_mode | stat.S_IWUSR)
+
+with io.open(self.file, "w", newline="\n") as f:
+time.sleep(1)
+f.write(new_content)
+if self.TraceOn():
+print("new content:", new_content)
+print(
+"os.path.getmtime() after writing new content:",
+os.path.getmtime(self.file),
+)
+
 def get_expected_stop_column_number(self):
 """Return the 1-based column number of the first non-whitespace
 character in the breakpoint source line."""
@@ -234,32 +262,20 @@
 self.fail("Fail to display source level breakpoints")
 self.assertTrue(int(m.group(1)) > 0)
 
-# Read the main.c file content.
-with io.open(self.file, "r", newline="\n") as f:
-original_content = f.read()
-if self.TraceOn():
-print("original content:", original_content)
-
-# Modify the in-memory copy of the original source code.
-new_content = original_content.replace("Hello world", "Hello lldb", 1)
+# Modify content
+self.modify_content()
 
-# Modify the source code file.
-# If the source was read only, the copy will also be read only.
-# Run "chmod u+w" on it first so we can modify it.
-statinfo = os.stat(self.file)
-os.chmod(self.file, statinfo.st_mode | stat.S_IWUSR)
+# Display the source code again. We should not see the updated line.
+self.expect(
+"source list -f main-copy.c -l %d" % self.line,
+SOURCE_DISPLAYED_CORRECTLY,
+substrs=["Hello world"],
+)
 
-with io.open(self.file, "w", newline="\n") as f:
-time.sleep(1)
-f.write(new_content)
-if self.TraceOn():
-print("new content:", new_content)
-print(
-"os.path.getmtime() after writing new content:",
-  

[Lldb-commits] [PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[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/D154329/new/

https://reviews.llvm.org/D154329

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

[Lldb-commits] [PATCH] D153740: [llvm][Support] Deprecate llvm::writeFileAtomically API

[Alexey Lapshin via Phabricator via lldb-commits]

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

this LGTM, assuming D154329  is landed. Thank 
you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153740

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

[Lldb-commits] [lldb] cec93c6 - [lldb] SymbolFileJSON: Update outdates Text -> JSON (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-07-03T14:28:02-07:00
New Revision: cec93c6589fe6e6fab99aa5c11819ff841a469b2

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

LOG: [lldb] SymbolFileJSON: Update outdates Text -> JSON (NFC)

Originally the symbol file format was going to textual, before we
decided to use JSON. Updated the plugin name and the header guard.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.cpp
lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.cpp 
b/lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.cpp
index f10ff59f3186d6..1f3ca27417e4ab 100644
--- a/lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.cpp
+++ b/lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.cpp
@@ -48,7 +48,7 @@ void SymbolFileJSON::Terminate() {
 }
 
 llvm::StringRef SymbolFileJSON::GetPluginDescriptionStatic() {
-  return "Reads debug symbols from a textual symbol table.";
+  return "Reads debug symbols from a JSON symbol table.";
 }
 
 SymbolFile *SymbolFileJSON::CreateInstance(ObjectFileSP objfile_sp) {

diff  --git a/lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.h 
b/lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.h
index 1fc41c739c027e..4dd0d65da46583 100644
--- a/lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.h
+++ b/lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.h
@@ -6,8 +6,8 @@
 //
 
//===--===//
 
-#ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_JSON_SYMBOLFILETEXT_H
-#define LLDB_SOURCE_PLUGINS_SYMBOLFILE_JSON_SYMBOLFILETEXT_H
+#ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_JSON_SYMBOLFILEJSON_H
+#define LLDB_SOURCE_PLUGINS_SYMBOLFILE_JSON_SYMBOLFILEJSON_H
 
 #include 
 #include 
@@ -37,7 +37,7 @@ class SymbolFileJSON : public lldb_private::SymbolFileCommon {
 
   static void Terminate();
 
-  static llvm::StringRef GetPluginNameStatic() { return "text"; }
+  static llvm::StringRef GetPluginNameStatic() { return "JSON"; }
 
   static llvm::StringRef GetPluginDescriptionStatic();
 
@@ -107,4 +107,4 @@ class SymbolFileJSON : public 
lldb_private::SymbolFileCommon {
 };
 } // namespace lldb_private
 
-#endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_JSON_SYMBOLFILETEXT_H
+#endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_JSON_SYMBOLFILEJSON_H



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

[Lldb-commits] [lldb] 6977c1c - [lldb] Remove old commented out code (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-07-03T14:41:03-07:00
New Revision: 6977c1caf43fa22988cfd811d65dce39f1720c34

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

LOG: [lldb] Remove old commented out code (NFC)

Move to `DumpAddress` in c4a8a76048e91baecb5746b80b9733e4af299937.

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index b186c8e6c1ab8f..868dc8726650f9 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1507,28 +1507,6 @@ static bool LookupAddressInModule(CommandInterpreter 
&interpreter, Stream &strm,
 ExecutionContextScope *exe_scope =
 interpreter.GetExecutionContext().GetBestExecutionContextScope();
 DumpAddress(exe_scope, so_addr, verbose, all_ranges, strm);
-//strm.IndentMore();
-//strm.Indent ("Address: ");
-//so_addr.Dump (&strm, exe_scope,
-//Address::DumpStyleModuleWithFileAddress);
-//strm.PutCString (" (");
-//so_addr.Dump (&strm, exe_scope,
-//Address::DumpStyleSectionNameOffset);
-//strm.PutCString (")\n");
-//strm.Indent ("Summary: ");
-//const uint32_t save_indent = strm.GetIndentLevel ();
-//strm.SetIndentLevel (save_indent + 13);
-//so_addr.Dump (&strm, exe_scope,
-//Address::DumpStyleResolvedDescription);
-//strm.SetIndentLevel (save_indent);
-//// Print out detailed address information when verbose is enabled
-//if (verbose)
-//{
-//strm.EOL();
-//so_addr.Dump (&strm, exe_scope,
-//Address::DumpStyleDetailedSymbolContext);
-//}
-//strm.IndentLess();
 return true;
   }
 



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

[Lldb-commits] [PATCH] D153918: [lldb][NFCI] Deprecate SBValue::GetOpaqueType

[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/D153918/new/

https://reviews.llvm.org/D153918

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

[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Chuanqi Xu via Phabricator via lldb-commits]

ChuanqiXu added a comment.

Now I think the feature may be important for the performance of modules. And I 
feel we should work on the ASTWriter side instead of ASTReader side. Since the 
size of BMIs is a problem now also it shows that it is not free to load the 
large BMIs. So while it is semantical correct to work on the reader side, it is 
better for the performance to work on the writer side.

I'd like to finish the idea. And for the current patch, I'd like to refactor it 
a little bit:

1. Test it by unittest instead of by matching the dump result.
2. Remove the Serialization part. So it will be a NFC patch.
3. Some other trivial polishment.

Of course, I'll still mark you as the author.

How do you feel about this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Iain Sandoe via Phabricator via lldb-commits]

iains added a comment.

In D126694#4470139 , @ChuanqiXu wrote:

> Now I think the feature may be important for the performance of modules. And 
> I feel we should work on the ASTWriter side instead of ASTReader side. Since 
> the size of BMIs is a problem now also it shows that it is not free to load 
> the large BMIs. So while it is semantical correct to work on the reader side, 
> it is better for the performance to work on the writer side.
>
> I'd like to finish the idea. And for the current patch, I'd like to refactor 
> it a little bit:
>
> 1. Test it by unittest instead of by matching the dump result.

fine with me

> 2. Remove the Serialization part. So it will be a NFC patch.

how do you plan to identify "elided" decls (we agreed to leave them in the BMI 
to keep diagnostics quality up, but we need to be able to ignore them)

> 3. Some other trivial polishment.



4. I wanted to take another look at using visitors to implement the checks.

> Of course, I'll still mark you as the author.
>
> How do you feel about this?

Do you plan to try this before clang-17?
 (if so, then go ahead - my next priority for 17 is the lookup bug)

Otherwise, this is on my TODO for clang-18.

BTW, I think that there are other opportunities to reduce the BMI size that we 
could realistically aim for clang-18
 (but let us make that discussion separately from this patch)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Chuanqi Xu via Phabricator via lldb-commits]

ChuanqiXu added a comment.

In D126694#4470235 , @iains wrote:

> In D126694#4470139 , @ChuanqiXu 
> wrote:
>
>> Now I think the feature may be important for the performance of modules. And 
>> I feel we should work on the ASTWriter side instead of ASTReader side. Since 
>> the size of BMIs is a problem now also it shows that it is not free to load 
>> the large BMIs. So while it is semantical correct to work on the reader 
>> side, it is better for the performance to work on the writer side.
>>
>> I'd like to finish the idea. And for the current patch, I'd like to refactor 
>> it a little bit:
>>
>> 1. Test it by unittest instead of by matching the dump result.
>
> fine with me
>
>> 2. Remove the Serialization part. So it will be a NFC patch.
>
> how do you plan to identify "elided" decls (we agreed to leave them in the 
> BMI to keep diagnostics quality up, but we need to be able to ignore them)

No. Now I feel it is not good to keep these redundant things in the BMI. They 
slow down  the performance. The enlarge the size of the BMI. They make the 
model more complex. They've brought a lot of pain points but they bring few 
benefits. I really don't think we should keep them. And this is the reason why 
I re-looked at this.

>> 3. Some other trivial polishment.
>
>
>
> 4. I wanted to take another look at using visitors to implement the checks.

Yeah, this is what I called polishment.

>> Of course, I'll still mark you as the author.
>>
>> How do you feel about this?
>
> Do you plan to try this before clang-17?
>  (if so, then go ahead - my next priority for 17 is the lookup bug)
>
> Otherwise, this is on my TODO for clang-18.

I want to give it a try for clang-17. The direct motivation is that I found the 
performance of the std module is worse than the direct #include in a small 
program (~20 lines). After many analysising, I think this one may be the most 
important technique to improve that. But it is only possible if we elide them 
in the BMI when writing. It doesn't help if we still keep them in the BMI.

And I feel it pretty bad. This is the reason why I want to make it into 
clang-17.

> BTW, I think that there are other opportunities to reduce the BMI size that 
> we could realistically aim for clang-18
>  (but let us make that discussion separately from this patch)

Yeah. I tried to look at it a little bit but it looks not so straight forward. 
A main part I found now is the source managers in serializer.  But let's 
discuss it separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Chuanqi Xu via Phabricator via lldb-commits]

ChuanqiXu added a comment.

BTW, in my experience for talking about modules to users, they mainly/mostly 
care about the compilation performance. And I can't image how many people would 
like to use modules if they know they won't get a compilation performance win.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Iain Sandoe via Phabricator via lldb-commits]

iains added a comment.

In D126694#4470250 , @ChuanqiXu wrote:

> BTW, in my experience for talking about modules to users, they mainly/mostly 
> care about the compilation performance. And I can't image how many people 
> would like to use modules if they know they won't get a compilation 
> performance win.

That is clearly a big motivation - I will ask the folks we were talking to at 
WG21 if that is their priority - or maybe they care about language isolation 
etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[Lldb-commits] [PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via Phabricator via lldb-commits]

hokein added a comment.

thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154329

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

[Lldb-commits] [lldb] 2a579db - [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via lldb-commits]

Author: Haojian Wu
Date: 2023-07-04T08:52:45+02:00
New Revision: 2a579db32a7a0a5f10ddd989eb4cb14974c17c26

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

LOG: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

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

Added: 


Modified: 
lldb/tools/lldb-server/lldb-platform.cpp

Removed: 




diff  --git a/lldb/tools/lldb-server/lldb-platform.cpp 
b/lldb/tools/lldb-server/lldb-platform.cpp
index 21b433511c5657..3e126584eb25b4 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,15 @@ static Status save_socket_id_to_file(const std::string 
&socket_id,
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [&status, &file_spec](const AtomicFileWriteError &E) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: 
%s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
-return Status("Failed to atomically write file %s",
-  file_spec.GetPath().c_str());
-  }
+  if (auto Err = llvm::writeToOutput(file_spec.GetPath(),
+ [&socket_id](llvm::raw_ostream &OS) {
+   OS << socket_id;
+   return llvm::Error::success();
+ }))
+return Status("Failed to atomically write file %s: %s",
+  file_spec.GetPath().c_str(),
+  llvm::toString(std::move(Err)).c_str());
   return status;
 }
 



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

[Lldb-commits] [PATCH] D154329: [lldb] Replace llvm::writeFileAtomically with llvm::writeToOutput API.

[Haojian Wu via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a579db32a7a: [lldb] Replace llvm::writeFileAtomically with 
llvm::writeToOutput API. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154329

Files:
  lldb/tools/lldb-server/lldb-platform.cpp


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,15 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [&status, &file_spec](const AtomicFileWriteError &E) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: 
%s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   ErrorMsgBuffer.c_str());
-   break;
- }
-   })) {
-return Status("Failed to atomically write file %s",
-  file_spec.GetPath().c_str());
-  }
+  if (auto Err = llvm::writeToOutput(file_spec.GetPath(),
+ [&socket_id](llvm::raw_ostream &OS) {
+   OS << socket_id;
+   return llvm::Error::success();
+ }))
+return Status("Failed to atomically write file %s: %s",
+  file_spec.GetPath().c_str(),
+  llvm::toString(std::move(Err)).c_str());
   return status;
 }
 


Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,7 +22,6 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -103,38 +102,15 @@
 return Status("Failed to create directory %s: %s",
   temp_file_spec.GetPath().c_str(), error.AsCString());
 
-  llvm::SmallString<64> temp_file_path;
-  temp_file_spec.AppendPathComponent("port-file.%%");
-  temp_file_path = temp_file_spec.GetPath();
-
   Status status;
-  if (auto Err =
-  handleErrors(llvm::writeFileAtomically(
-   temp_file_path, file_spec.GetPath(), socket_id),
-   [&status, &file_spec](const AtomicFileWriteError &E) {
- std::string ErrorMsgBuffer;
- llvm::raw_string_ostream S(ErrorMsgBuffer);
- E.log(S);
-
- switch (E.Error) {
- case atomic_write_error::failed_to_create_uniq_file:
-   status = Status("Failed to create temp file: %s",
-   ErrorMsgBuffer.c_str());
-   break;
- case atomic_write_error::output_stream_error:
-   status = Status("Failed to write to port file.");
-   break;
- case atomic_write_error::failed_to_rename_temp_file:
-   status = Status("Failed to rename file %s to %s: %s",
-   ErrorMsgBuffer.c_str(),
-   file_spec.GetPath().c_str(),
-   Er

[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Chuanqi Xu via Phabricator via lldb-commits]

ChuanqiXu added a comment.

> That is clearly a big motivation - I will ask the folks we were talking to at 
> WG21 if that is their priority - or maybe they care about language isolation 
> etc.

Yeah, I know the folks in WG21 prefer the language isolation. But you know, 
there are many folks who are not in WG21...

Oh, the thing I want to say, in this case we have a chance to improve the 
compilation speed significantly and the so called diagnose quality became a 
blocker for us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[Lldb-commits] [PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Iain Sandoe via Phabricator via lldb-commits]

iains added a comment.

In D126694#4470261 , @ChuanqiXu wrote:

>> That is clearly a big motivation - I will ask the folks we were talking to 
>> at WG21 if that is their priority - or maybe they care about language 
>> isolation etc.
>
> Yeah, I know the folks in WG21 prefer the language isolation. But you know, 
> there are many folks who are not in WG21...

indeed :)

> Oh, the thing I want to say, in this case we have a chance to improve the 
> compilation speed significantly and the so called diagnose quality became a 
> blocker for us. Also it is beneficial to remove them out of the BMI to 
> improve the language isolation feature.

Yes, that was the decision at the last time we looked - because removing decls 
would degrade this - if we have new information that changes our preferred 
design, then fine.  
One solution is to place the elision behind a flag so that the user can choose 
slower compilation with better diagnostics or faster compilation but maybe 
harder-to-find errors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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