date:20230606

[Lldb-commits] [PATCH] D152225: [lldb] fix dangling reference in `ClangHost.cpp`

2023-06-06 Thread Junchang Liu via Phabricator via lldb-commits

paperchalice added a comment.

If this patch fixes the CI failure 
,
 I need someone to commit this change, thanks in advance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152225

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

[Lldb-commits] [PATCH] D150805: Rate limit progress reporting

2023-06-06 Thread Sterling Augustine via Phabricator via lldb-commits

saugustine added a comment.

In D150805#4397940 , @rupprecht wrote:

> In D150805#4350849 , @JDevlieghere 
> wrote:
>
>> I also like Jordan's rate limiting idea. In my mind that should be a 
>> property of the broadcaster. Different tools (e.g. vscode vs the command 
>> line) could specify different values when register a listener.
>
> This makes sense: we could augment `lldb_private::Listener` with additional 
> members that keep track of when the last broadcast time was, and if we're 
> rate limiting. Then we could change the implementation of 
> `Listener::GetEvent(lldb::EventSP &event_sp, const Timeout 
> &timeout)` to continuously churn through `m_events`, returning the most 
> recent one by the time the rate limiting window is over, and discarding any 
> intermediate ones in between.
>
> One thing I'm not sure of though is how we'll avoid an unnecessary pause for 
> rate limiting on the last item. This patch avoids that because it checks 
> `data->GetCompleted() != data->GetTotal()` to decide if we should actually 
> rate limit. In the generic case, how does the listener know that an event it 
> returns is the final one, and that it should ignore the rate limiting delay?
>
> I think we could address that by adding a `bool m_important` member to 
> `lldb::Event`, and then it would be up to the broadcaster to set that to true 
> for the last one (or any intermediate ones that are similarly important & 
> should be immediately shown, e.g. warnings/errors). Would that be reasonable?

The later a decision the decision not to update is made, the more work is 
wasted. Even the fairly simple solution that checked time in a somewhat 
expensive way (via the misnamed getCurrentTime that also gets memory used) 
ended up being slower overall. In my opinion, the problem is that a single-die 
is too small a unit of work to be worth reporting on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150805

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

[Lldb-commits] [lldb] 6c02e36 - [lldb] fix dangling reference in `ClangHost.cpp`

2023-06-06 Thread Adrian Prantl via lldb-commits


Author: paperchalice
Date: 2023-06-06T08:11:01-07:00
New Revision: 6c02e365711cab47ed9abe9f091c25651a3efd74

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

LOG: [lldb] fix dangling reference in `ClangHost.cpp`

The lifetime of clang_resource_path should be same as
kResourceDirSuffixes, because kResourceDirSuffixes doesn't own
clang_resource_path.

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

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
index 3bb303c0e1931..6064c02c7fd67 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -52,7 +52,7 @@ static bool DefaultComputeClangResourceDirectory(FileSpec 
&lldb_shlib_spec,
   Log *log = GetLog(LLDBLog::Host);
   std::string raw_path = lldb_shlib_spec.GetPath();
   llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
-  const std::string clang_resource_path =
+  static const std::string clang_resource_path =
   clang::driver::Driver::GetResourcesPath("bin/lldb", CLANG_RESOURCE_DIR);
 
   static const llvm::StringRef kResourceDirSuffixes[] = {



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

[Lldb-commits] [PATCH] D152225: [lldb] fix dangling reference in `ClangHost.cpp`

2023-06-06 Thread Adrian Prantl via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c02e365711c: [lldb] fix dangling reference in 
`ClangHost.cpp` (authored by paperchalice, committed by aprantl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152225

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


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -52,7 +52,7 @@
   Log *log = GetLog(LLDBLog::Host);
   std::string raw_path = lldb_shlib_spec.GetPath();
   llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
-  const std::string clang_resource_path =
+  static const std::string clang_resource_path =
   clang::driver::Driver::GetResourcesPath("bin/lldb", CLANG_RESOURCE_DIR);
 
   static const llvm::StringRef kResourceDirSuffixes[] = {


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -52,7 +52,7 @@
   Log *log = GetLog(LLDBLog::Host);
   std::string raw_path = lldb_shlib_spec.GetPath();
   llvm::StringRef parent_dir = llvm::sys::path::parent_path(raw_path);
-  const std::string clang_resource_path =
+  static const std::string clang_resource_path =
   clang::driver::Driver::GetResourcesPath("bin/lldb", CLANG_RESOURCE_DIR);
 
   static const llvm::StringRef kResourceDirSuffixes[] = {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152225: [lldb] fix dangling reference in `ClangHost.cpp`

2023-06-06 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added a comment.

Thanks!

FWIW, it seems like an unnecessary micro optimization to make both these 
variables static.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152225

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

[Lldb-commits] [PATCH] D152189: [LLDB][PDB] Fix age field in UUID in PDB file.

2023-06-06 Thread Zequan Wu via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcb9a7c22ee67: [LLDB][PDB] Fix age field in UUID in PDB file. 
(authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152189

Files:
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/test/Shell/ObjectFile/PDB/object.test


Index: lldb/test/Shell/ObjectFile/PDB/object.test
===
--- lldb/test/Shell/ObjectFile/PDB/object.test
+++ lldb/test/Shell/ObjectFile/PDB/object.test
@@ -3,7 +3,7 @@
 
 # CHECK: Plugin name: pdb
 # CHECK: Architecture: x86_64-pc-windows-msvc
-# CHECK: UUID: 61AF583F-29A8-7A6C-4C4C-44205044422E-0001
+# CHECK: UUID: 61AF583F-29A8-7A6C-4C4C-44205044422E-0003
 # CHECK: Executable: false
 # CHECK: Stripped: false
 # CHECK: Type: debug info
@@ -52,7 +52,7 @@
   Version: VC70
 DbiStream:
   VerHeader:   V70
-  Age: 1
+  Age: 3
   BuildNumber: 36363
   PdbDllVersion:   0
   PdbDllRbld:  0
Index: lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
===
--- lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
+++ lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
@@ -27,10 +27,10 @@
 
 LLDB_PLUGIN_DEFINE(ObjectFilePDB)
 
-static UUID GetPDBUUID(InfoStream &IS) {
+static UUID GetPDBUUID(InfoStream &IS, DbiStream &DS) {
   UUID::CvRecordPdb70 debug_info;
   memcpy(&debug_info.Uuid, IS.getGuid().Guid, sizeof(debug_info.Uuid));
-  debug_info.Age = IS.getAge();
+  debug_info.Age = DS.getAge();
   return UUID(debug_info);
 }
 
@@ -82,7 +82,12 @@
 llvm::consumeError(info_stream.takeError());
 return false;
   }
-  m_uuid = GetPDBUUID(*info_stream);
+  auto dbi_stream = m_file_up->getPDBDbiStream();
+  if (!dbi_stream) {
+llvm::consumeError(dbi_stream.takeError());
+return false;
+  }
+  m_uuid = GetPDBUUID(*info_stream, *dbi_stream);
   return true;
 }
 
@@ -126,7 +131,7 @@
   }
 
   lldb_private::UUID &uuid = module_spec.GetUUID();
-  uuid = GetPDBUUID(*info_stream);
+  uuid = GetPDBUUID(*info_stream, *dbi_stream);
 
   ArchSpec &module_arch = module_spec.GetArchitecture();
   switch (dbi_stream->getMachineType()) {


Index: lldb/test/Shell/ObjectFile/PDB/object.test
===
--- lldb/test/Shell/ObjectFile/PDB/object.test
+++ lldb/test/Shell/ObjectFile/PDB/object.test
@@ -3,7 +3,7 @@
 
 # CHECK: Plugin name: pdb
 # CHECK: Architecture: x86_64-pc-windows-msvc
-# CHECK: UUID: 61AF583F-29A8-7A6C-4C4C-44205044422E-0001
+# CHECK: UUID: 61AF583F-29A8-7A6C-4C4C-44205044422E-0003
 # CHECK: Executable: false
 # CHECK: Stripped: false
 # CHECK: Type: debug info
@@ -52,7 +52,7 @@
   Version: VC70
 DbiStream:
   VerHeader:   V70
-  Age: 1
+  Age: 3
   BuildNumber: 36363
   PdbDllVersion:   0
   PdbDllRbld:  0
Index: lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
===
--- lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
+++ lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
@@ -27,10 +27,10 @@
 
 LLDB_PLUGIN_DEFINE(ObjectFilePDB)
 
-static UUID GetPDBUUID(InfoStream &IS) {
+static UUID GetPDBUUID(InfoStream &IS, DbiStream &DS) {
   UUID::CvRecordPdb70 debug_info;
   memcpy(&debug_info.Uuid, IS.getGuid().Guid, sizeof(debug_info.Uuid));
-  debug_info.Age = IS.getAge();
+  debug_info.Age = DS.getAge();
   return UUID(debug_info);
 }
 
@@ -82,7 +82,12 @@
 llvm::consumeError(info_stream.takeError());
 return false;
   }
-  m_uuid = GetPDBUUID(*info_stream);
+  auto dbi_stream = m_file_up->getPDBDbiStream();
+  if (!dbi_stream) {
+llvm::consumeError(dbi_stream.takeError());
+return false;
+  }
+  m_uuid = GetPDBUUID(*info_stream, *dbi_stream);
   return true;
 }
 
@@ -126,7 +131,7 @@
   }
 
   lldb_private::UUID &uuid = module_spec.GetUUID();
-  uuid = GetPDBUUID(*info_stream);
+  uuid = GetPDBUUID(*info_stream, *dbi_stream);
 
   ArchSpec &module_arch = module_spec.GetArchitecture();
   switch (dbi_stream->getMachineType()) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] cb9a7c2 - [LLDB][PDB] Fix age field in UUID in PDB file.

2023-06-06 Thread Zequan Wu via lldb-commits


Author: Zequan Wu
Date: 2023-06-06T11:24:50-04:00
New Revision: cb9a7c22ee6789569e28dde073e6827761b4d003

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

LOG: [LLDB][PDB] Fix age field in UUID in PDB file.

There are two age fields in a PDB file. One from the PDB Stream and another one
from the DBI stream.

According to 
https://randomascii.wordpress.com/2011/11/11/source-indexing-is-underused-awesomeness/#comment-34328,
The age in DBI stream is used to against the binary's age. `Pdbstr.exe` is used
to only increment the age from PDB stream without changing the DBI age. I
also verified this by manually changing the DBI age of a PDB file and let
`windbg.exe` to load it. It shows the following logs before and after changing:

Before:
```
SYMSRV:  BYINDEX: 0xA
 c:\symbols*https://msdl.microsoft.com/download/symbols
 nlaapi.pdb
 D72AA69CD5ABE5D28C74FADB17DE3F8C1
SYMSRV:  PATH: 
c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
SYMSRV:  RESULT: 0x
*** WARNING: Unable to verify checksum for NLAapi.dll
DBGHELP: NLAapi - public symbols
c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
...
```

After:
```
SYMSRV:  BYINDEX: 0xA
 c:\symbols*https://msdl.microsoft.com/download/symbols
 nlaapi.pdb
 D72AA69CD5ABE5D28C74FADB17DE3F8C1
SYMSRV:  PATH: 
c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
SYMSRV:  RESULT: 0x
DBGHELP: c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb - 
mismatched pdb
SYMSRV:  BYINDEX: 0xB
 
c:\symbols*https://chromium-browser-symsrv.commondatastorage.googleapis.com
 nlaapi.pdb
 D72AA69CD5ABE5D28C74FADB17DE3F8C1
SYMSRV:  PATH: 
c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
SYMSRV:  RESULT: 0x
DBGHELP: c:\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb - 
mismatched pdb
SYMSRV:  BYINDEX: 0xC
 c:\src\symbols*https://msdl.microsoft.com/download/symbols
 nlaapi.pdb
 D72AA69CD5ABE5D28C74FADB17DE3F8C1
SYMSRV:  PATH: 
c:\src\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
SYMSRV:  RESULT: 0x
*** WARNING: Unable to verify checksum for NLAapi.dll
DBGHELP: NLAapi - public symbols
c:\src\symbols\nlaapi.pdb\D72AA69CD5ABE5D28C74FADB17DE3F8C1\nlaapi.pdb
```

So, `windbg.exe` uses the DBI age to detect mismatched pdb, but it still loads
the pdb even if the age mismatched. Probably lldb should do the same and give
some warnings.

This fixes a bug that lldb can't load some windows system pdbs due to mismatched
uuid.

Reviewed By: rnk

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

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
lldb/test/Shell/ObjectFile/PDB/object.test

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp 
b/lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
index c62a67fa29f97..a3b91fc37dac7 100644
--- a/lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
+++ b/lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
@@ -27,10 +27,10 @@ using namespace llvm::codeview;
 
 LLDB_PLUGIN_DEFINE(ObjectFilePDB)
 
-static UUID GetPDBUUID(InfoStream &IS) {
+static UUID GetPDBUUID(InfoStream &IS, DbiStream &DS) {
   UUID::CvRecordPdb70 debug_info;
   memcpy(&debug_info.Uuid, IS.getGuid().Guid, sizeof(debug_info.Uuid));
-  debug_info.Age = IS.getAge();
+  debug_info.Age = DS.getAge();
   return UUID(debug_info);
 }
 
@@ -82,7 +82,12 @@ bool ObjectFilePDB::initPDBFile() {
 llvm::consumeError(info_stream.takeError());
 return false;
   }
-  m_uuid = GetPDBUUID(*info_stream);
+  auto dbi_stream = m_file_up->getPDBDbiStream();
+  if (!dbi_stream) {
+llvm::consumeError(dbi_stream.takeError());
+return false;
+  }
+  m_uuid = GetPDBUUID(*info_stream, *dbi_stream);
   return true;
 }
 
@@ -126,7 +131,7 @@ size_t ObjectFilePDB::GetModuleSpecifications(
   }
 
   lldb_private::UUID &uuid = module_spec.GetUUID();
-  uuid = GetPDBUUID(*info_stream);
+  uuid = GetPDBUUID(*info_stream, *dbi_stream);
 
   ArchSpec &module_arch = module_spec.GetArchitecture();
   switch (dbi_stream->getMachineType()) {

diff  --git a/lldb/test/Shell/ObjectFile/PDB/object.test 
b/lldb/test/Shell/ObjectFile/PDB/object.test
index 35373d52a3931..62564243f054b 100644
--- a/lldb/test/Shell/ObjectFile/PDB/object.test
+++ b/lldb/test/Shell/ObjectFile/PDB/object.test
@@ -3,7 +3,7 @@
 
 # CHECK: Plugin name: pdb
 # CHECK: Architecture: x86_64-pc-windows-msvc
-# CHECK: UUID: 61AF583F-29A8-7A6C-4C4C-44205044422E-0001
+# CHECK: UUID: 61AF583F-29A8-7A6C-4C4C-44205044422E-0003
 # CHECK: Executable: false
 # CHECK: Stripped: false
 # CHECK: Type: debug info
@@ -52,7 +52,7

[Lldb-commits] [PATCH] D150805: Rate limit progress reporting

2023-06-06 Thread Jordan Rupprecht via Phabricator via lldb-commits

rupprecht added a subscriber: labath.
rupprecht added a comment.

The other tricky part I missed before is this bit in between pulling the event 
off the listener queue and deciding to show it:

  auto *data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
  if (!data)
return;
  
  // Do some bookkeeping for the current event, regardless of whether we're
  // going to show the progress.
  const uint64_t id = data->GetID();
  if (m_current_event_id) {
...
if (id != *m_current_event_id)
  return;
if (data->GetCompleted() == data->GetTotal())
  m_current_event_id.reset();
  } else {
m_current_event_id = id;
  }

When we pull the event off listener off the queue, we need to do 
post-processing ourselves to decide if it's even a progress event at all, and 
if it's meant for us. If we put the listener in charge of rate limiting and 
returning only the most recent event, it needs to know that the event it's 
returning is interesting. Otherwise the rate limiting might hide all the 
interesting events. On top of that, there are events that are *interesting* 
even if we don't want to show them.

Another option is to provide `Listener::GetRateLimitedEvents` (name TBD) 
instead. It (potentially) blocks for the rate limiting period and then returns 
a *list* of all the events that have happened within that time frame. Then we 
let the caller process it as it pleases and display only the most recent 
relevant one. It feels a little weird at first but might actually make sense 
compared to alternatives?

I think it would be nice to abstract the rate limiting somewhere, although I'm 
not sure anymore if having it directly in the broadcast/listen classes makes 
sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150805

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

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

2023-06-06 Thread Med Ismail Bennani via Phabricator via lldb-commits

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



Comment at: lldb/include/lldb/lldb-enumerations.h:1279
+  // if you & in this bit the base code will not process the option.
+  eCustomCompletion = (1u << 24)
+};

JDevlieghere wrote:
> Should this be `<< 25`? 
I've just copied the enum as-is, but now that you mention it, I also think this 
should be `<< 25`.


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

https://reviews.llvm.org/D152011

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

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

2023-06-06 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 528928.
mib marked an inline comment as done.
mib added a comment.

Address @JDevlieghere comment


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

https://reviews.llvm.org/D152011

Files:
  lldb/docs/python_api_enums.rst
  lldb/examples/python/crashlog.py
  lldb/include/lldb/Interpreter/CommandCompletions.h
  lldb/include/lldb/Interpreter/CommandObject.h
  lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
  lldb/include/lldb/Interpreter/OptionValueFileColonLine.h
  lldb/include/lldb/Interpreter/OptionValueFileSpec.h
  lldb/include/lldb/Utility/OptionDefinition.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectPlugin.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectRegexCommand.cpp
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Commands/CommandObjectSession.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/IOHandler.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/OptionValueArch.cpp
  lldb/source/Interpreter/OptionValueFileColonLine.cpp
  lldb/source/Interpreter/OptionValueFileSpec.cpp
  lldb/source/Interpreter/Options.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py
  lldb/test/API/functionalities/completion/my_test_cmd.py
  lldb/utils/TableGen/LLDBOptionDefEmitter.cpp

Index: lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
===
--- lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
+++ lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
@@ -120,12 +120,11 @@
   if (!O.Completions.empty()) {
 std::vector CompletionArgs;
 for (llvm::StringRef Completion : O.Completions)
-  CompletionArgs.push_back("CommandCompletions::e" + Completion.str() +
-   "Completion");
+  CompletionArgs.push_back("e" + Completion.str() + "Completion");
 
 OS << llvm::join(CompletionArgs.begin(), CompletionArgs.end(), " | ");
   } else
-OS << "CommandCompletions::eNoCompletion";
+OS << "CompletionType::eNoCompletion";
 
   // Add the argument type.
   OS << ", eArgType";
Index: lldb/test/API/functionalities/completion/my_test_cmd.py
===
--- /dev/null
+++ lldb/test/API/functionalities/completion/my_test_cmd.py
@@ -0,0 +1,2 @@
+def my_test_cmd(debugger, command, exe_ctx, result, dict):
+result.Print(command)
Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -436,6 +436,47 @@
 self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
 self.complete_from_to("target modules dump line-table main.cp", ["main.cpp"])
 
+def test_custom_command_completion(self):
+"""Tests completion in custom user provided commands."""
+completion_types = [
+"none",
+"source-file",
+"disk-file",
+"disk-directory",
+"symbol",
+"module",
+"settings-name",
+"platform-plugin",
+"architecture",
+"variable-path",
+"register",
+"breakpoint",
+"process-plugin",
+"disassembly-flavor",
+"type-language",
+"frame-index",
+"module-uuid",
+"stophook-id",
+"thread-index",
+"watchpoint-id",
+"breakpoint-name",
+"process-id",
+"process-name",
+"remote-disk-file",
+"remote-disk-directory",
+"type-category-name",
+"custom",
+]
+self.completions_contain("command script add -C ", completion_types)
+
+source_path = os.path.join(self.getSourceDir(), "my_test_cmd.py")
+self.runCmd("command script import '%s'" % (source_path))
+self.runCmd(
+"command script add -C disk-file -f my_test_cmd.my_test_cmd my_test_cmd"
+)
+self.complete_from_to("my_test_cmd main.cp", ["main.cpp"])
+self.expect("my_test_cmd main.cpp", substrs=["main.cpp"])
+
 def test_target_modules_load_aout(self):

[Lldb-commits] [PATCH] D152220: [lldb][NFCI] Change type of Broadcaster's name

2023-06-06 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere requested changes to this revision.
JDevlieghere added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/include/lldb/Utility/Broadcaster.h:157
+  ///   A StringRef of the name that this broadcaster will have.
+  Broadcaster(lldb::BroadcasterManagerSP manager_sp, llvm::StringRef name);
 

If we're going to store this as a `std::string` you should take it it by value 
and move it into the member. 



Comment at: lldb/source/Utility/Broadcaster.cpp:26-28
+Broadcaster::Broadcaster(BroadcasterManagerSP manager_sp, llvm::StringRef name)
 : m_broadcaster_sp(std::make_shared(*this)),
+  m_manager_sp(std::move(manager_sp)), m_broadcaster_name(name.str()) {




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152220

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

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

2023-06-06 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM. This is really cool.


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

https://reviews.llvm.org/D152011

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

[Lldb-commits] [lldb] 1e82b20 - [lldb/Target] Add ability to set a label to targets

2023-06-06 Thread Med Ismail Bennani via lldb-commits


Author: Med Ismail Bennani
Date: 2023-06-06T10:58:34-07:00
New Revision: 1e82b20118e31bd6c3844a84e03f701997a9b7ed

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

LOG: [lldb/Target] Add ability to set a label to targets

This patch add the ability for the user to set a label for a target.

This can be very useful when debugging targets with the same executables
in the same session.

Labels can be set either at the target creation in the command
interpreter or at any time using the SBAPI.

Target labels show up in the `target list` output, following the target
index, and they also allow the user to switch targets using them.

rdar://105016191

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

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/test/Shell/Target/target-label.test

Modified: 
lldb/include/lldb/API/SBTarget.h
lldb/include/lldb/Target/Target.h
lldb/include/lldb/Target/TargetList.h
lldb/source/API/SBTarget.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetList.cpp

Removed: 




diff  --git a/lldb/include/lldb/API/SBTarget.h 
b/lldb/include/lldb/API/SBTarget.h
index e7dc76396fb68..ea5212ea56d93 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/lldb/include/lldb/API/SBTarget.h
@@ -328,6 +328,10 @@ class LLDB_API SBTarget {
   
   const char *GetABIName();
 
+  const char *GetLabel() const;
+
+  SBError SetLabel(const char *label);
+
   /// Architecture data byte width accessor
   ///
   /// \return

diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index d15f0705630ce..eb5e87dcacb39 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -554,6 +554,17 @@ class Target : public std::enable_shared_from_this,
 
   bool IsDummyTarget() const { return m_is_dummy_target; }
 
+  const std::string &GetLabel() const { return m_label; }
+
+  /// Set a label for a target.
+  ///
+  /// The label cannot be used by another target or be only integral.
+  ///
+  /// \return
+  /// The label for this target or an error if the label didn't match the
+  /// requirements.
+  llvm::Error SetLabel(llvm::StringRef label);
+
   /// Find a binary on the system and return its Module,
   /// or return an existing Module that is already in the Target.
   ///
@@ -1520,6 +1531,7 @@ class Target : public 
std::enable_shared_from_this,
   /// detect that code is running on the private state thread.
   std::recursive_mutex m_private_mutex;
   Arch m_arch;
+  std::string m_label;
   ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
   SectionLoadHistory m_section_load_history;

diff  --git a/lldb/include/lldb/Target/TargetList.h 
b/lldb/include/lldb/Target/TargetList.h
index 65781a4811fd8..a1d5a84511e02 100644
--- a/lldb/include/lldb/Target/TargetList.h
+++ b/lldb/include/lldb/Target/TargetList.h
@@ -115,7 +115,7 @@ class TargetList : public Broadcaster {
   /// in \a target_sp which can then be properly released.
   bool DeleteTarget(lldb::TargetSP &target_sp);
 
-  int GetNumTargets() const;
+  size_t GetNumTargets() const;
 
   lldb::TargetSP GetTargetAtIndex(uint32_t index) const;
 

diff  --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 53af5b1d7a477..53b1bd6a535d6 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1601,6 +1601,26 @@ const char *SBTarget::GetABIName() {
   return const_name.GetCString();
 }
 
+const char *SBTarget::GetLabel() const {
+  LLDB_INSTRUMENT_VA(this);
+
+  TargetSP target_sp(GetSP());
+  if (!target_sp)
+return nullptr;
+
+  return ConstString(target_sp->GetLabel().data()).AsCString();
+}
+
+SBError SBTarget::SetLabel(const char *label) {
+  LLDB_INSTRUMENT_VA(this, label);
+
+  TargetSP target_sp(GetSP());
+  if (!target_sp)
+return Status("Couldn't get internal target object.");
+
+  return Status(target_sp->SetLabel(label));
+}
+
 uint32_t SBTarget::GetDataByteSize() {
   LLDB_INSTRUMENT_VA(this);
 

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index e3bb7fc06ed1a..0088211c49e37 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -82,8 +82,14 @@ static void DumpTargetInfo(uint32_t target_idx, Target 
*target,
   if (!exe_valid)
 ::strcpy(exe_path, "");
 
-  strm.Printf("%starget #%u: %s", prefix_cstr ? prefix_cstr : "", target_idx,
-  exe_path);
+  std::string formatted_label = "";
+  const std::string &label = target->GetLabel();
+  if (!label.empty()) {
+formatted_label = " (" + label + ")";
+  }
+
+  strm.Pr

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

2023-06-06 Thread Med Ismail Bennani via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
mib marked 2 inline comments as done.
Closed by commit rG1e82b20118e3: [lldb/Target] Add ability to set a label to 
targets (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151859

Files:
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/TargetList.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetList.cpp
  lldb/test/Shell/Target/target-label.test

Index: lldb/test/Shell/Target/target-label.test
===
--- /dev/null
+++ lldb/test/Shell/Target/target-label.test
@@ -0,0 +1,38 @@
+# RUN: %lldb -b -o 'settings set interpreter.stop-command-source-on-error false' -s %s 2>&1 | FileCheck %s
+
+target create -l "ls" /bin/ls
+target list
+# CHECK: * target #0 (ls): /bin/ls
+
+script lldb.target.SetLabel("")
+target list
+# CHECK: * target #0: /bin/ls
+
+target create -l "cat" /bin/cat
+target list
+# CHECK: target #0: /bin/ls
+# CHECK-NEXT: * target #1 (cat): /bin/cat
+
+target create -l "cat" /bin/cat
+# CHECK: Cannot use label 'cat' since it's set in target #1.
+
+target create -l 42 /bin/cat
+# CHECK: error: Cannot use integer as target label.
+
+target select 0
+# CHECK: * target #0: /bin/ls
+# CHECK-NEXT: target #1 (cat): /bin/cat
+
+target select cat
+# CHECK: target #0: /bin/ls
+# CHECK-NEXT: * target #1 (cat): /bin/cat
+
+script lldb.target.GetLabel()
+# CHECK: 'cat'
+
+script lldb.debugger.GetTargetAtIndex(0).SetLabel('Not cat')
+# CHECK: success
+
+target list
+# CHECK: target #0 (Not cat): /bin/ls
+# CHECK-NEXT: * target #1 (cat): /bin/cat
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -489,7 +489,7 @@
   return num_signals_sent;
 }
 
-int TargetList::GetNumTargets() const {
+size_t TargetList::GetNumTargets() const {
   std::lock_guard guard(m_target_list_mutex);
   return m_target_list.size();
 }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -69,6 +69,7 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -2536,6 +2537,27 @@
   Target::GetGlobalProperties().SetDefaultArchitecture(arch);
 }
 
+llvm::Error Target::SetLabel(llvm::StringRef label) {
+  size_t n = LLDB_INVALID_INDEX32;
+  if (llvm::to_integer(label, n))
+return llvm::make_error(
+"Cannot use integer as target label.", llvm::inconvertibleErrorCode());
+  TargetList &targets = GetDebugger().GetTargetList();
+  for (size_t i = 0; i < targets.GetNumTargets(); i++) {
+TargetSP target_sp = targets.GetTargetAtIndex(i);
+if (target_sp && target_sp->GetLabel() == label) {
+return llvm::make_error(
+llvm::formatv(
+"Cannot use label '{0}' since it's set in target #{1}.", label,
+i),
+llvm::inconvertibleErrorCode());
+}
+  }
+
+  m_label = label.str();
+  return llvm::Error::success();
+}
+
 Target *Target::GetTargetFromContexts(const ExecutionContext *exe_ctx_ptr,
   const SymbolContext *sc_ptr) {
   // The target can either exist in the "process" of ExecutionContext, or in
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -82,8 +82,14 @@
   if (!exe_valid)
 ::strcpy(exe_path, "");
 
-  strm.Printf("%starget #%u: %s", prefix_cstr ? prefix_cstr : "", target_idx,
-  exe_path);
+  std::string formatted_label = "";
+  const std::string &label = target->GetLabel();
+  if (!label.empty()) {
+formatted_label = " (" + label + ")";
+  }
+
+  strm.Printf("%starget #%u%s: %s", prefix_cstr ? prefix_cstr : "", target_idx,
+  formatted_label.data(), exe_path);
 
   uint32_t properties = 0;
   if (target_arch.IsValid()) {
@@ -209,6 +215,8 @@
 m_platform_options(true), // Include the --platform option.
 m_core_file(LLDB_OPT_SET_1, false, "core", 'c', 0, eArgTypeFilename,
 "Fullpath to a core file to use for this target."),
+m_label(LLDB_OPT_SET_1, false, "label", 'l', 0, eArgTypeName,
+"Optional name for this target.", nullptr),
 m_symbol_file(LLDB_OPT_SET_1, false, "symfile", 's', 0,
   eArgTypeFilename,
   "Fullpath to a stand alone debug "
@@ -234,6 +242,7 @@
 m_option_group.Append(&m_arch_option, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
 m_option_g

[Lldb-commits] [lldb] e896612 - [lldb/Commands] Fix disk completion from root directory

2023-06-06 Thread Med Ismail Bennani via lldb-commits


Author: Med Ismail Bennani
Date: 2023-06-06T10:58:34-07:00
New Revision: e8966125e2812f08bf0f548bf576981025d44cbd

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

LOG: [lldb/Commands] Fix disk completion from root directory

This patch should fix path completion starting from the root directory.

To do so, this patch adds a special case when setting the search
directory when the completion buffer points to the root directory.

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/Commands/CommandCompletions.cpp
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/source/Commands/CommandCompletions.cpp 
b/lldb/source/Commands/CommandCompletions.cpp
index 1fe25d9655dc9..35237d419d469 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -381,6 +381,8 @@ static void DiskFilesOrDirectories(const llvm::Twine 
&partial_name,
   Storage.append(RemainderDir);
 }
 SearchDir = Storage;
+  } else if (CompletionBuffer == path::root_directory(CompletionBuffer)) {
+SearchDir = CompletionBuffer;
   } else {
 SearchDir = path::parent_path(CompletionBuffer);
   }
@@ -390,9 +392,11 @@ static void DiskFilesOrDirectories(const llvm::Twine 
&partial_name,
   PartialItem = path::filename(CompletionBuffer);
 
   // path::filename() will return "." when the passed path ends with a
-  // directory separator. We have to filter those out, but only when the
-  // "." doesn't come from the completion request itself.
-  if (PartialItem == "." && path::is_separator(CompletionBuffer.back()))
+  // directory separator or the separator when passed the disk root directory.
+  // We have to filter those out, but only when the "." doesn't come from the
+  // completion request itself.
+  if ((PartialItem == "." || PartialItem == path::get_separator()) &&
+  path::is_separator(CompletionBuffer.back()))
 PartialItem = llvm::StringRef();
 
   if (SearchDir.empty()) {

diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py 
b/lldb/test/API/functionalities/completion/TestCompletion.py
index f25e1b408bbce..0cfd213177d6a 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -477,6 +477,19 @@ def test_custom_command_completion(self):
 self.complete_from_to("my_test_cmd main.cp", ["main.cpp"])
 self.expect("my_test_cmd main.cpp", substrs=["main.cpp"])
 
+def test_completion_target_create_from_root_dir(self):
+"""Tests source file completion by completing ."""
+root_dir = os.path.abspath(os.sep)
+self.completions_contain(
+"target create " + root_dir,
+list(
+filter(
+lambda x: os.path.exists(x),
+map(lambda x: root_dir + x + os.sep, os.listdir(root_dir)),
+)
+),
+)
+
 def test_target_modules_load_aout(self):
 """Tests modules completion by completing the target modules load 
argument."""
 self.build()



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

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

2023-06-06 Thread Med Ismail Bennani via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a9c3e611505: [lldb/Commands] Add support to auto-completion 
for user commands (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152011

Files:
  lldb/docs/python_api_enums.rst
  lldb/examples/python/crashlog.py
  lldb/include/lldb/Interpreter/CommandCompletions.h
  lldb/include/lldb/Interpreter/CommandObject.h
  lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
  lldb/include/lldb/Interpreter/OptionValueFileColonLine.h
  lldb/include/lldb/Interpreter/OptionValueFileSpec.h
  lldb/include/lldb/Utility/OptionDefinition.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectPlugin.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectRegexCommand.cpp
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Commands/CommandObjectSession.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/IOHandler.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/OptionValueArch.cpp
  lldb/source/Interpreter/OptionValueFileColonLine.cpp
  lldb/source/Interpreter/OptionValueFileSpec.cpp
  lldb/source/Interpreter/Options.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py
  lldb/test/API/functionalities/completion/my_test_cmd.py
  lldb/utils/TableGen/LLDBOptionDefEmitter.cpp

Index: lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
===
--- lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
+++ lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
@@ -120,12 +120,11 @@
   if (!O.Completions.empty()) {
 std::vector CompletionArgs;
 for (llvm::StringRef Completion : O.Completions)
-  CompletionArgs.push_back("CommandCompletions::e" + Completion.str() +
-   "Completion");
+  CompletionArgs.push_back("e" + Completion.str() + "Completion");
 
 OS << llvm::join(CompletionArgs.begin(), CompletionArgs.end(), " | ");
   } else
-OS << "CommandCompletions::eNoCompletion";
+OS << "CompletionType::eNoCompletion";
 
   // Add the argument type.
   OS << ", eArgType";
Index: lldb/test/API/functionalities/completion/my_test_cmd.py
===
--- /dev/null
+++ lldb/test/API/functionalities/completion/my_test_cmd.py
@@ -0,0 +1,2 @@
+def my_test_cmd(debugger, command, exe_ctx, result, dict):
+result.Print(command)
Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -436,6 +436,47 @@
 self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
 self.complete_from_to("target modules dump line-table main.cp", ["main.cpp"])
 
+def test_custom_command_completion(self):
+"""Tests completion in custom user provided commands."""
+completion_types = [
+"none",
+"source-file",
+"disk-file",
+"disk-directory",
+"symbol",
+"module",
+"settings-name",
+"platform-plugin",
+"architecture",
+"variable-path",
+"register",
+"breakpoint",
+"process-plugin",
+"disassembly-flavor",
+"type-language",
+"frame-index",
+"module-uuid",
+"stophook-id",
+"thread-index",
+"watchpoint-id",
+"breakpoint-name",
+"process-id",
+"process-name",
+"remote-disk-file",
+"remote-disk-directory",
+"type-category-name",
+"custom",
+]
+self.completions_contain("command script add -C ", completion_types)
+
+source_path = os.path.join(self.getSourceDir(), "my_test_cmd.py")
+self.runCmd("command script import '%s'" % (source_path))
+self.runCmd(
+"command script add -C disk-file -f my_test_cmd.my_test_cmd my_test_cmd"
+)
+self.complete_from_to("my_test_cmd main.cp", ["main.cpp"])
+self.expec

[Lldb-commits] [PATCH] D152012: [lldb/crashlog] Expand crash report file path before parsing

2023-06-06 Thread Med Ismail Bennani via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3bc0baf9d439: [lldb/crashlog] Expand crash report file path 
before parsing (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152012

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1346,13 +1346,7 @@
 print(error)
 
 
-def load_crashlog_in_scripted_process(debugger, crash_log_file, options, 
result):
-crashlog_path = os.path.expanduser(crash_log_file)
-if not os.path.exists(crashlog_path):
-raise InteractiveCrashLogException(
-"crashlog file %s does not exist" % crashlog_path
-)
-
+def load_crashlog_in_scripted_process(debugger, crashlog_path, options, 
result):
 crashlog = CrashLogParser.create(debugger, crashlog_path, False).parse()
 
 target = lldb.SBTarget()
@@ -1641,17 +1635,22 @@
 ci = debugger.GetCommandInterpreter()
 
 if args:
-for crash_log_file in args:
+for crashlog_file in args:
+crashlog_path = os.path.expanduser(crashlog_file)
+if not os.path.exists(crashlog_path):
+raise FileNotFoundError(
+"crashlog file %s does not exist" % crashlog_path
+)
 if should_run_in_interactive_mode(options, ci):
 try:
 load_crashlog_in_scripted_process(
-debugger, crash_log_file, options, result
+debugger, crashlog_path, options, result
 )
 except InteractiveCrashLogException as e:
 result.SetError(str(e))
 else:
 crash_log = CrashLogParser.create(
-debugger, crash_log_file, options.verbose
+debugger, crashlog_path, options.verbose
 ).parse()
 SymbolicateCrashLog(crash_log, options)
 


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1346,13 +1346,7 @@
 print(error)
 
 
-def load_crashlog_in_scripted_process(debugger, crash_log_file, options, result):
-crashlog_path = os.path.expanduser(crash_log_file)
-if not os.path.exists(crashlog_path):
-raise InteractiveCrashLogException(
-"crashlog file %s does not exist" % crashlog_path
-)
-
+def load_crashlog_in_scripted_process(debugger, crashlog_path, options, result):
 crashlog = CrashLogParser.create(debugger, crashlog_path, False).parse()
 
 target = lldb.SBTarget()
@@ -1641,17 +1635,22 @@
 ci = debugger.GetCommandInterpreter()
 
 if args:
-for crash_log_file in args:
+for crashlog_file in args:
+crashlog_path = os.path.expanduser(crashlog_file)
+if not os.path.exists(crashlog_path):
+raise FileNotFoundError(
+"crashlog file %s does not exist" % crashlog_path
+)
 if should_run_in_interactive_mode(options, ci):
 try:
 load_crashlog_in_scripted_process(
-debugger, crash_log_file, options, result
+debugger, crashlog_path, options, result
 )
 except InteractiveCrashLogException as e:
 result.SetError(str(e))
 else:
 crash_log = CrashLogParser.create(
-debugger, crash_log_file, options.verbose
+debugger, crashlog_path, options.verbose
 ).parse()
 SymbolicateCrashLog(crash_log, options)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3bc0baf - [lldb/crashlog] Expand crash report file path before parsing

2023-06-06 Thread Med Ismail Bennani via lldb-commits


Author: Med Ismail Bennani
Date: 2023-06-06T10:58:34-07:00
New Revision: 3bc0baf9d43967d828e2d311f6c0863e79158f07

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

LOG: [lldb/crashlog] Expand crash report file path before parsing

This patch should fix a crash in the opening a crash report that was
passed with a relative path.

This patch expands the crash report path before parsing it and raises a
`FileNotFoundError` exception if the file doesn't exist.

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/examples/python/crashlog.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 4a14e99bea10e..36825b077c6a0 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -1346,13 +1346,7 @@ def add_module(image, target, obj_dir):
 print(error)
 
 
-def load_crashlog_in_scripted_process(debugger, crash_log_file, options, 
result):
-crashlog_path = os.path.expanduser(crash_log_file)
-if not os.path.exists(crashlog_path):
-raise InteractiveCrashLogException(
-"crashlog file %s does not exist" % crashlog_path
-)
-
+def load_crashlog_in_scripted_process(debugger, crashlog_path, options, 
result):
 crashlog = CrashLogParser.create(debugger, crashlog_path, False).parse()
 
 target = lldb.SBTarget()
@@ -1641,17 +1635,22 @@ def should_run_in_interactive_mode(options, ci):
 ci = debugger.GetCommandInterpreter()
 
 if args:
-for crash_log_file in args:
+for crashlog_file in args:
+crashlog_path = os.path.expanduser(crashlog_file)
+if not os.path.exists(crashlog_path):
+raise FileNotFoundError(
+"crashlog file %s does not exist" % crashlog_path
+)
 if should_run_in_interactive_mode(options, ci):
 try:
 load_crashlog_in_scripted_process(
-debugger, crash_log_file, options, result
+debugger, crashlog_path, options, result
 )
 except InteractiveCrashLogException as e:
 result.SetError(str(e))
 else:
 crash_log = CrashLogParser.create(
-debugger, crash_log_file, options.verbose
+debugger, crashlog_path, options.verbose
 ).parse()
 SymbolicateCrashLog(crash_log, options)
 



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

[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

2023-06-06 Thread 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 rGe8966125e281: [lldb/Commands] Fix disk completion from root 
directory (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152013

Files:
  lldb/source/Commands/CommandCompletions.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -477,6 +477,19 @@
 self.complete_from_to("my_test_cmd main.cp", ["main.cpp"])
 self.expect("my_test_cmd main.cpp", substrs=["main.cpp"])
 
+def test_completion_target_create_from_root_dir(self):
+"""Tests source file completion by completing ."""
+root_dir = os.path.abspath(os.sep)
+self.completions_contain(
+"target create " + root_dir,
+list(
+filter(
+lambda x: os.path.exists(x),
+map(lambda x: root_dir + x + os.sep, os.listdir(root_dir)),
+)
+),
+)
+
 def test_target_modules_load_aout(self):
 """Tests modules completion by completing the target modules load 
argument."""
 self.build()
Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -381,6 +381,8 @@
   Storage.append(RemainderDir);
 }
 SearchDir = Storage;
+  } else if (CompletionBuffer == path::root_directory(CompletionBuffer)) {
+SearchDir = CompletionBuffer;
   } else {
 SearchDir = path::parent_path(CompletionBuffer);
   }
@@ -390,9 +392,11 @@
   PartialItem = path::filename(CompletionBuffer);
 
   // path::filename() will return "." when the passed path ends with a
-  // directory separator. We have to filter those out, but only when the
-  // "." doesn't come from the completion request itself.
-  if (PartialItem == "." && path::is_separator(CompletionBuffer.back()))
+  // directory separator or the separator when passed the disk root directory.
+  // We have to filter those out, but only when the "." doesn't come from the
+  // completion request itself.
+  if ((PartialItem == "." || PartialItem == path::get_separator()) &&
+  path::is_separator(CompletionBuffer.back()))
 PartialItem = llvm::StringRef();
 
   if (SearchDir.empty()) {


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -477,6 +477,19 @@
 self.complete_from_to("my_test_cmd main.cp", ["main.cpp"])
 self.expect("my_test_cmd main.cpp", substrs=["main.cpp"])
 
+def test_completion_target_create_from_root_dir(self):
+"""Tests source file completion by completing ."""
+root_dir = os.path.abspath(os.sep)
+self.completions_contain(
+"target create " + root_dir,
+list(
+filter(
+lambda x: os.path.exists(x),
+map(lambda x: root_dir + x + os.sep, os.listdir(root_dir)),
+)
+),
+)
+
 def test_target_modules_load_aout(self):
 """Tests modules completion by completing the target modules load argument."""
 self.build()
Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -381,6 +381,8 @@
   Storage.append(RemainderDir);
 }
 SearchDir = Storage;
+  } else if (CompletionBuffer == path::root_directory(CompletionBuffer)) {
+SearchDir = CompletionBuffer;
   } else {
 SearchDir = path::parent_path(CompletionBuffer);
   }
@@ -390,9 +392,11 @@
   PartialItem = path::filename(CompletionBuffer);
 
   // path::filename() will return "." when the passed path ends with a
-  // directory separator. We have to filter those out, but only when the
-  // "." doesn't come from the completion request itself.
-  if (PartialItem == "." && path::is_separator(CompletionBuffer.back()))
+  // directory separator or the separator when passed the disk root directory.
+  // We have to filter those out, but only when the "." doesn't come from the
+  // completion request itself.
+  if ((PartialItem == "." || PartialItem == path::get_separator()) &&
+  path::is_separator(CompletionBuffer.back()))
 PartialItem = llvm::StringRef();
 
   if

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

2023-06-06 Thread Andrey Ali Khan Bolshakov via Phabricator via lldb-commits

bolshakov-a added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4397
+// argument.
+// As proposed in https://github.com/itanium-cxx-abi/cxx-abi/issues/111.
+auto *SNTTPE = cast(E);

bolshakov-a wrote:
> efriedma wrote:
> > bolshakov-a wrote:
> > > erichkeane wrote:
> > > > erichkeane wrote:
> > > > > aaron.ballman wrote:
> > > > > > We should get this nailed down. It was proposed in Nov 2020 and the 
> > > > > > issue is still open. CC @rjmccall 
> > > > > This definitely needs to happen.  @rjmccall or @eli.friedman ^^ Any 
> > > > > idea what the actual mangling should be?
> > > > This is still an open, and we need @rjmccall @eli.friedman or @asl to 
> > > > help out here.
> > > Ping @efriedma, @rjmccall, @asl.
> > I'm not really familiar with the mangling implications for this particular 
> > construct, nor am I actively involved with the Itanium ABI specification, 
> > so I'm not sure how I can help you directly.
> > 
> > That said, as a general opinion, I don't think it's worth waiting for 
> > updates to the Itanuim ABI  document to be merged; such updates are 
> > happening slowly at the moment, and having a consistent mangling is clearly 
> > an improvement even if it's not specified.  My suggested plan of action:
> > 
> > - Make sure you're satisfied the proposed mangling doesn't have any holes 
> > you're concerned about (i.e. it produces a unique mangling for all the 
> > relevant cases).  If you're not sure, I can try to spend some time 
> > understanding this, but it doesn't sound like you have any concerns about 
> > this.
> > - Put a note on the issue in the Itanium ABI repo that you're planning to 
> > go ahead with using this mangling in clang.  Also send an email directly to 
> > @rjmccall and @rsmith in case they miss the notifications.
> > - Go ahead with this.
> > Put a note on the issue in the Itanium ABI repo that you're planning to go 
> > ahead with using this mangling in clang. Also send an email directly to 
> > @rjmccall and @rsmith in case they miss the notifications.
> 
> I'm sorry for noting one more time that Richard already pushed these changes 
> in clang upstream, but they had been just reverted.
> 
> Maybe, I should make a PR into Itanium API repository, but I probably need 
> some time to dig into the theory and all the discussions. But yes, even NTTP 
> argument mangling rules are not still merged: 
> https://github.com/itanium-cxx-abi/cxx-abi/pull/140
@aaron.ballman, @erichkeane, seems like it is already fixed in the ABI document:
> Typically, only references to function template parameters occurring within 
> the dependent signature of the template are mangled this way. In other 
> contexts, template instantiation replaces references to template parameters 
> with the actual template arguments, and mangling should mangle such 
> references exactly as if they were that template argument.

https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.template-param

See also [the discussion in the 
issue](https://github.com/itanium-cxx-abi/cxx-abi/issues/111#issuecomment-1567486892).


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

https://reviews.llvm.org/D140996

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

[Lldb-commits] [lldb] 6099d51 - [lldb] Support file and function names in LLDB_LOGF macro

2023-06-06 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2023-06-06T12:24:38-07:00
New Revision: 6099d519bb83cc0b97ce8b529743f832de144110

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

LOG: [lldb] Support file and function names in LLDB_LOGF macro

LLDB's logging machinery supports prepending log messages with the name
of the file and function that generates the log. However, currently this
functionality is limited to the LLDB_LOG macro. I meant to do this as a
follow up to D65128 but never got around to it.

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

Added: 


Modified: 
lldb/include/lldb/Utility/Log.h
lldb/source/Utility/Log.cpp
lldb/unittests/Utility/LogTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 2984a4bd43bb7..1fe28d61b9dac 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -232,6 +232,9 @@ class Log final {
  std::forward(args)...));
   }
 
+  void Formatf(llvm::StringRef file, llvm::StringRef function,
+   const char *format, ...) __attribute__((format(printf, 4, 5)));
+
   /// Prefer using LLDB_LOGF whenever possible.
   void Printf(const char *format, ...) __attribute__((format(printf, 2, 3)));
 
@@ -249,6 +252,8 @@ class Log final {
 
   void VAPrintf(const char *format, va_list args);
   void VAError(const char *format, va_list args);
+  void VAFormatf(llvm::StringRef file, llvm::StringRef function,
+ const char *format, va_list args);
 
 private:
   Channel &m_channel;
@@ -345,7 +350,7 @@ template  Log *GetLog(Cat mask) {
   do { 
\
 ::lldb_private::Log *log_private = (log);  
\
 if (log_private)   
\
-  log_private->Printf(__VA_ARGS__);
\
+  log_private->Formatf(__FILE__, __func__, __VA_ARGS__);   
\
   } while (0)
 
 #define LLDB_LOGV(log, ...)
\

diff  --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp
index da8569efd444b..75912683e2334 100644
--- a/lldb/source/Utility/Log.cpp
+++ b/lldb/source/Utility/Log.cpp
@@ -155,6 +155,21 @@ void Log::VAPrintf(const char *format, va_list args) {
   PutString(Content);
 }
 
+void Log::Formatf(llvm::StringRef file, llvm::StringRef function,
+  const char *format, ...) {
+  va_list args;
+  va_start(args, format);
+  VAFormatf(file, function, format, args);
+  va_end(args);
+}
+
+void Log::VAFormatf(llvm::StringRef file, llvm::StringRef function,
+const char *format, va_list args) {
+  llvm::SmallString<64> Content;
+  lldb_private::VASprintf(Content, format, args);
+  Format(file, function, llvm::formatv("{0}", Content));
+}
+
 // Printing of errors that are not fatal.
 void Log::Error(const char *format, ...) {
   va_list args;

diff  --git a/lldb/unittests/Utility/LogTest.cpp 
b/lldb/unittests/Utility/LogTest.cpp
index 275bd0dda3349..d3c878d3ea718 100644
--- a/lldb/unittests/Utility/LogTest.cpp
+++ b/lldb/unittests/Utility/LogTest.cpp
@@ -100,6 +100,7 @@ class LogChannelEnabledTest : public LogChannelTest {
   Log *getLog() { return m_log; }
   llvm::StringRef takeOutput();
   llvm::StringRef logAndTakeOutput(llvm::StringRef Message);
+  llvm::StringRef logAndTakeOutputf(llvm::StringRef Message);
 
 public:
   void SetUp() override;
@@ -136,6 +137,12 @@ LogChannelEnabledTest::logAndTakeOutput(llvm::StringRef 
Message) {
   return takeOutput();
 }
 
+llvm::StringRef
+LogChannelEnabledTest::logAndTakeOutputf(llvm::StringRef Message) {
+  LLDB_LOGF(m_log, "%s", Message.str().c_str());
+  return takeOutput();
+}
+
 TEST(LogTest, LLDB_LOG_nullptr) {
   Log *log = nullptr;
   LLDB_LOG(log, "{0}", 0); // Shouldn't crash
@@ -296,6 +303,21 @@ TEST_F(LogChannelEnabledTest, log_options) {
 EXPECT_STREQ("logAndTakeOutput", Function);
   }
 
+  {
+EXPECT_TRUE(EnableChannel(getLogHandler(),
+  LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION, "chan", 
{},
+  Err));
+llvm::StringRef Msg = logAndTakeOutputf("Hello World");
+char File[12];
+char Function[17];
+
+sscanf(Msg.str().c_str(),
+   "%[^:]:%s Hello World", File,
+   Function);
+EXPECT_STRCASEEQ("LogTest.cpp", File);
+EXPECT_STREQ("logAndTakeOutputf", Function);
+  }
+
   EXPECT_TRUE(EnableChannel(getLogHandler(),
 LLDB_LOG_OPTION_PREPEND_PROC_AND_THREAD, "chan", 
{},
 Err));



_

[Lldb-commits] [PATCH] D151764: [lldb] Support file and function names in LLDB_LOGF macro

2023-06-06 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6099d519bb83: [lldb] Support file and function names in 
LLDB_LOGF macro (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151764

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -100,6 +100,7 @@
   Log *getLog() { return m_log; }
   llvm::StringRef takeOutput();
   llvm::StringRef logAndTakeOutput(llvm::StringRef Message);
+  llvm::StringRef logAndTakeOutputf(llvm::StringRef Message);
 
 public:
   void SetUp() override;
@@ -136,6 +137,12 @@
   return takeOutput();
 }
 
+llvm::StringRef
+LogChannelEnabledTest::logAndTakeOutputf(llvm::StringRef Message) {
+  LLDB_LOGF(m_log, "%s", Message.str().c_str());
+  return takeOutput();
+}
+
 TEST(LogTest, LLDB_LOG_nullptr) {
   Log *log = nullptr;
   LLDB_LOG(log, "{0}", 0); // Shouldn't crash
@@ -296,6 +303,21 @@
 EXPECT_STREQ("logAndTakeOutput", Function);
   }
 
+  {
+EXPECT_TRUE(EnableChannel(getLogHandler(),
+  LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION, "chan", {},
+  Err));
+llvm::StringRef Msg = logAndTakeOutputf("Hello World");
+char File[12];
+char Function[17];
+
+sscanf(Msg.str().c_str(),
+   "%[^:]:%s Hello World", File,
+   Function);
+EXPECT_STRCASEEQ("LogTest.cpp", File);
+EXPECT_STREQ("logAndTakeOutputf", Function);
+  }
+
   EXPECT_TRUE(EnableChannel(getLogHandler(),
 LLDB_LOG_OPTION_PREPEND_PROC_AND_THREAD, "chan", {},
 Err));
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -155,6 +155,21 @@
   PutString(Content);
 }
 
+void Log::Formatf(llvm::StringRef file, llvm::StringRef function,
+  const char *format, ...) {
+  va_list args;
+  va_start(args, format);
+  VAFormatf(file, function, format, args);
+  va_end(args);
+}
+
+void Log::VAFormatf(llvm::StringRef file, llvm::StringRef function,
+const char *format, va_list args) {
+  llvm::SmallString<64> Content;
+  lldb_private::VASprintf(Content, format, args);
+  Format(file, function, llvm::formatv("{0}", Content));
+}
+
 // Printing of errors that are not fatal.
 void Log::Error(const char *format, ...) {
   va_list args;
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -232,6 +232,9 @@
  std::forward(args)...));
   }
 
+  void Formatf(llvm::StringRef file, llvm::StringRef function,
+   const char *format, ...) __attribute__((format(printf, 4, 5)));
+
   /// Prefer using LLDB_LOGF whenever possible.
   void Printf(const char *format, ...) __attribute__((format(printf, 2, 3)));
 
@@ -249,6 +252,8 @@
 
   void VAPrintf(const char *format, va_list args);
   void VAError(const char *format, va_list args);
+  void VAFormatf(llvm::StringRef file, llvm::StringRef function,
+ const char *format, va_list args);
 
 private:
   Channel &m_channel;
@@ -345,7 +350,7 @@
   do { \
 ::lldb_private::Log *log_private = (log);  \
 if (log_private)   \
-  log_private->Printf(__VA_ARGS__);\
+  log_private->Formatf(__FILE__, __func__, __VA_ARGS__);   \
   } while (0)
 
 #define LLDB_LOGV(log, ...)\
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 7cd1d42 - [lldb] Remove FUNCTION from log messages in lldbHost (NFC)

2023-06-06 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2023-06-06T13:02:43-07:00
New Revision: 7cd1d4231a7c83e719b83c6abbe2d479baa03808

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

LOG: [lldb] Remove __FUNCTION__ from log messages in lldbHost (NFC)

LLDB's logging infrastructure supports prepending log messages with the
name of the file and function that generates the log (see help log
enable). Therefore it's unnecessary to include the current __FUNCTION__
in the log message itself. This patch removes __FUNCTION__ from log
messages in the Host library.

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

Added: 


Modified: 
lldb/source/Host/common/HostInfoBase.cpp
lldb/source/Host/common/NativeRegisterContext.cpp
lldb/source/Host/common/TCPSocket.cpp
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

Removed: 




diff  --git a/lldb/source/Host/common/HostInfoBase.cpp 
b/lldb/source/Host/common/HostInfoBase.cpp
index 0c0d786c44f7c..5c44c2f38b287 100644
--- a/lldb/source/Host/common/HostInfoBase.cpp
+++ b/lldb/source/Host/common/HostInfoBase.cpp
@@ -225,24 +225,20 @@ bool HostInfoBase::ComputePathRelativeToLibrary(FileSpec 
&file_spec,
 return false;
 
   std::string raw_path = lldb_file_spec.GetPath();
-  LLDB_LOGF(log,
-"HostInfo::%s() attempting to "
-"derive the path %s relative to liblldb install path: %s",
-__FUNCTION__, dir.data(), raw_path.c_str());
+  LLDB_LOG(
+  log,
+  "Attempting to derive the path {0} relative to liblldb install path: 
{1}",
+  dir, raw_path);
 
   // Drop bin (windows) or lib
   llvm::StringRef parent_path = llvm::sys::path::parent_path(raw_path);
   if (parent_path.empty()) {
-LLDB_LOGF(log,
-  "HostInfo::%s() failed to find liblldb within the shared "
-  "lib path",
-  __FUNCTION__);
+LLDB_LOG(log, "Failed to find liblldb within the shared lib path");
 return false;
   }
 
   raw_path = (parent_path + dir).str();
-  LLDB_LOGF(log, "HostInfo::%s() derived the path as: %s", __FUNCTION__,
-raw_path.c_str());
+  LLDB_LOG(log, "Derived the path as: {0}", raw_path);
   file_spec.SetDirectory(raw_path);
   return (bool)file_spec.GetDirectory();
 }

diff  --git a/lldb/source/Host/common/NativeRegisterContext.cpp 
b/lldb/source/Host/common/NativeRegisterContext.cpp
index 1be519d129eef..411f5f5c92ed0 100644
--- a/lldb/source/Host/common/NativeRegisterContext.cpp
+++ b/lldb/source/Host/common/NativeRegisterContext.cpp
@@ -125,15 +125,12 @@ lldb::addr_t NativeRegisterContext::GetPC(lldb::addr_t 
fail_value) {
 
   uint32_t reg = ConvertRegisterKindToRegisterNumber(eRegisterKindGeneric,
  LLDB_REGNUM_GENERIC_PC);
-  LLDB_LOGF(log,
-"NativeRegisterContext::%s using reg index %" PRIu32
-" (default %" PRIu64 ")",
-__FUNCTION__, reg, fail_value);
+  LLDB_LOGF(log, "Using reg index %" PRIu32 " (default %" PRIu64 ")", reg,
+fail_value);
 
   const uint64_t retval = ReadRegisterAsUnsigned(reg, fail_value);
 
-  LLDB_LOGF(log, "NativeRegisterContext::%s " PRIu32 " retval %" PRIu64,
-__FUNCTION__, retval);
+  LLDB_LOGF(log, PRIu32 " retval %" PRIu64, retval);
 
   return retval;
 }
@@ -203,18 +200,15 @@ NativeRegisterContext::ReadRegisterAsUnsigned(const 
RegisterInfo *reg_info,
 Status error = ReadRegister(reg_info, value);
 if (error.Success()) {
   LLDB_LOGF(log,
-"NativeRegisterContext::%s ReadRegister() succeeded, value "
+"Read register succeeded: value "
 "%" PRIu64,
-__FUNCTION__, value.GetAsUInt64());
+value.GetAsUInt64());
   return value.GetAsUInt64();
 } else {
-  LLDB_LOGF(log,
-"NativeRegisterContext::%s ReadRegister() failed, error %s",
-__FUNCTION__, error.AsCString());
+  LLDB_LOGF(log, "Read register failed: error %s", error.AsCString());
 }
   } else {
-LLDB_LOGF(log, "NativeRegisterContext::%s ReadRegister() null reg_info",
-  __FUNCTION__);
+LLDB_LOGF(log, "Read register failed: null reg_info");
   }
   return fail_value;
 }
@@ -222,7 +216,7 @@ NativeRegisterContext::ReadRegisterAsUnsigned(const 
RegisterInfo *reg_info,
 Status NativeRegisterContext::WriteRegisterFromUnsigned(uint32_t reg,
 uint64_t uval) {
   if (reg == LLDB_INVALID_REGNUM)
-return Status("NativeRegisterContext::%s (): reg is invalid", 
__FUNCTION__);
+return Status("Write register failed: reg is invalid");
   return WriteRegisterFromUnsigned(GetRegisterInfoAtIndex(reg), uval);
 }
 

diff  --git a/lldb/source/Host/common/TCPSocket.cpp

[Lldb-commits] [PATCH] D151762: [lldb] Remove FUNCTION from log messages in lldbHost (NFC)

2023-06-06 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7cd1d4231a7c: [lldb] Remove __FUNCTION__ from log messages 
in lldbHost (NFC) (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151762

Files:
  lldb/source/Host/common/HostInfoBase.cpp
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -152,8 +152,7 @@
 FileSystem::Instance().Resolve(support_dir_spec);
 if (!FileSystem::Instance().IsDirectory(support_dir_spec)) {
   Log *log = GetLog(LLDBLog::Host);
-  LLDB_LOGF(log, "HostInfoMacOSX::%s(): failed to find support directory",
-__FUNCTION__);
+  LLDB_LOG(log, "failed to find support directory");
   return false;
 }
 
@@ -342,8 +341,8 @@
 HostInfo::GetSDKRoot(SDKOptions{XcodeSDK::GetAnyMacOS()});
 if (!sdk_path_or_err) {
   Log *log = GetLog(LLDBLog::Host);
-  LLDB_LOGF(log, "Error while searching for Xcode SDK: %s",
-toString(sdk_path_or_err.takeError()).c_str());
+  LLDB_LOG_ERROR(log, sdk_path_or_err.takeError(),
+ "Error while searching for Xcode SDK: {0}");
   return;
 }
 FileSpec fspec(*sdk_path_or_err);
@@ -496,7 +495,7 @@
   if (!path.empty())
 break;
 }
-LLDB_LOGF(log, "Couldn't find SDK %s on host", sdk_name.c_str());
+LLDB_LOG(log, "Couldn't find SDK {0} on host", sdk_name);
 
 // Try without the version.
 if (!info.version.empty()) {
@@ -510,13 +509,13 @@
 break;
 }
 
-LLDB_LOGF(log, "Couldn't find any matching SDK on host");
+LLDB_LOG(log, "Couldn't find any matching SDK on host");
 return "";
   }
 
   // Whatever is left in output should be a valid path.
   if (!FileSystem::Instance().Exists(path)) {
-LLDB_LOGF(log, "SDK returned by xcrun doesn't exist");
+LLDB_LOG(log, "SDK returned by xcrun doesn't exist");
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"SDK returned by xcrun doesn't exist");
   }
Index: lldb/source/Host/common/TCPSocket.cpp
===
--- lldb/source/Host/common/TCPSocket.cpp
+++ lldb/source/Host/common/TCPSocket.cpp
@@ -150,7 +150,7 @@
 Status TCPSocket::Connect(llvm::StringRef name) {
 
   Log *log = GetLog(LLDBLog::Communication);
-  LLDB_LOGF(log, "TCPSocket::%s (host/port = %s)", __FUNCTION__, name.data());
+  LLDB_LOG(log, "Connect to host/port {0}", name);
 
   Status error;
   llvm::Expected host_port = DecodeHostAndPort(name);
@@ -189,7 +189,7 @@
 
 Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
   Log *log = GetLog(LLDBLog::Connection);
-  LLDB_LOGF(log, "TCPSocket::%s (%s)", __FUNCTION__, name.data());
+  LLDB_LOG(log, "Listen to {0}", name);
 
   Status error;
   llvm::Expected host_port = DecodeHostAndPort(name);
Index: lldb/source/Host/common/NativeRegisterContext.cpp
===
--- lldb/source/Host/common/NativeRegisterContext.cpp
+++ lldb/source/Host/common/NativeRegisterContext.cpp
@@ -125,15 +125,12 @@
 
   uint32_t reg = ConvertRegisterKindToRegisterNumber(eRegisterKindGeneric,
  LLDB_REGNUM_GENERIC_PC);
-  LLDB_LOGF(log,
-"NativeRegisterContext::%s using reg index %" PRIu32
-" (default %" PRIu64 ")",
-__FUNCTION__, reg, fail_value);
+  LLDB_LOGF(log, "Using reg index %" PRIu32 " (default %" PRIu64 ")", reg,
+fail_value);
 
   const uint64_t retval = ReadRegisterAsUnsigned(reg, fail_value);
 
-  LLDB_LOGF(log, "NativeRegisterContext::%s " PRIu32 " retval %" PRIu64,
-__FUNCTION__, retval);
+  LLDB_LOGF(log, PRIu32 " retval %" PRIu64, retval);
 
   return retval;
 }
@@ -203,18 +200,15 @@
 Status error = ReadRegister(reg_info, value);
 if (error.Success()) {
   LLDB_LOGF(log,
-"NativeRegisterContext::%s ReadRegister() succeeded, value "
+"Read register succeeded: value "
 "%" PRIu64,
-__FUNCTION__, value.GetAsUInt64());
+value.GetAsUInt64());
   return value.GetAsUInt64();
 } else {
-  LLDB_LOGF(log,
-"NativeRegisterContext::%s ReadRegister() failed, error %s",
-__FUNCTION__, error.AsCString());
+  LLDB_LOGF(log, "Read register failed: error %s", error.AsCString());
 }
   } else {
-LLDB_LOGF(log, "NativeRegisterContext::%s ReadRegister() null reg

[Lldb-commits] [PATCH] D152220: [lldb][NFCI] Change type of Broadcaster's name

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

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

Address @JDevlieghere's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152220

Files:
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/source/API/SBBroadcaster.cpp
  lldb/source/Core/ThreadedCommunication.cpp
  lldb/source/Utility/Broadcaster.cpp
  lldb/source/Utility/Event.cpp
  lldb/source/Utility/Listener.cpp

Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -234,7 +234,7 @@
 
 if (m_broadcaster_names) {
   bool found_source = false;
-  ConstString event_broadcaster_name =
+  const llvm::StringRef event_broadcaster_name =
   event_sp->GetBroadcaster()->GetBroadcasterName();
   for (uint32_t i = 0; i < m_num_broadcaster_names; ++i) {
 if (m_broadcaster_names[i] == event_broadcaster_name) {
Index: lldb/source/Utility/Event.cpp
===
--- lldb/source/Utility/Event.cpp
+++ lldb/source/Utility/Event.cpp
@@ -58,13 +58,13 @@
   s->Printf("%p Event: broadcaster = %p (%s), type = 0x%8.8x (%s), data = ",
 static_cast(this),
 static_cast(broadcaster),
-broadcaster->GetBroadcasterName().GetCString(), m_type,
+broadcaster->GetBroadcasterName().c_str(), m_type,
 event_name.GetData());
 else
   s->Printf("%p Event: broadcaster = %p (%s), type = 0x%8.8x, data = ",
 static_cast(this),
 static_cast(broadcaster),
-broadcaster->GetBroadcasterName().GetCString(), m_type);
+broadcaster->GetBroadcasterName().c_str(), m_type);
   } else
 s->Printf("%p Event: broadcaster = NULL, type = 0x%8.8x, data = ",
   static_cast(this), m_type);
Index: lldb/source/Utility/Broadcaster.cpp
===
--- lldb/source/Utility/Broadcaster.cpp
+++ lldb/source/Utility/Broadcaster.cpp
@@ -23,9 +23,9 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Broadcaster::Broadcaster(BroadcasterManagerSP manager_sp, const char *name)
+Broadcaster::Broadcaster(BroadcasterManagerSP manager_sp, std::string name)
 : m_broadcaster_sp(std::make_shared(*this)),
-  m_manager_sp(std::move(manager_sp)), m_broadcaster_name(name) {
+  m_manager_sp(std::move(manager_sp)), m_broadcaster_name(std::move(name)) {
   Log *log = GetLog(LLDBLog::Object);
   LLDB_LOG(log, "{0} Broadcaster::Broadcaster(\"{1}\")",
static_cast(this), GetBroadcasterName());
@@ -216,8 +216,8 @@
 StreamString event_description;
 event_sp->Dump(&event_description);
 LLDB_LOGF(log,
-  "%p Broadcaster(\"%s\")::BroadcastEvent (event_sp = {%s}, "
-  "unique =%i) hijack = %p",
+  "{0:x} Broadcaster(\"{1}\")::BroadcastEvent (event_sp = {2}, "
+  "unique={3}) hijack = {4:x}",
   static_cast(this), GetBroadcasterName(),
   event_description.GetData(), unique,
   static_cast(hijacking_listener_sp.get()));
Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -57,7 +57,7 @@
 ThreadedCommunication::~ThreadedCommunication() {
   LLDB_LOG(GetLog(LLDBLog::Object | LLDBLog::Communication),
"{0} ThreadedCommunication::~ThreadedCommunication (name = {1})",
-   this, GetBroadcasterName().AsCString());
+   this, GetBroadcasterName());
 }
 
 void ThreadedCommunication::Clear() {
Index: lldb/source/API/SBBroadcaster.cpp
===
--- lldb/source/API/SBBroadcaster.cpp
+++ lldb/source/API/SBBroadcaster.cpp
@@ -92,7 +92,7 @@
   LLDB_INSTRUMENT_VA(this);
 
   if (m_opaque_ptr)
-return m_opaque_ptr->GetBroadcasterName().GetCString();
+return ConstString(m_opaque_ptr->GetBroadcasterName()).GetCString();
   return nullptr;
 }
 
Index: lldb/include/lldb/Utility/Broadcaster.h
===
--- lldb/include/lldb/Utility/Broadcaster.h
+++ lldb/include/lldb/Utility/Broadcaster.h
@@ -149,10 +149,12 @@
 public:
   /// Construct with a broadcaster with a name.
   ///
+  /// \param[in] manager_sp
+  ///   A shared pointer to the BroadcasterManager that will manage this
+  ///   broadcaster.
   /// \param[in] name
-  /// A NULL terminated C string that contains the name of the
-  /// broadcaster object.
-  Broadcaster(lldb::BroadcasterManagerSP manager_sp, const char *name);
+  ///   A std::string of the name that this broadcaster will have.
+  Broadcaster(lldb::BroadcasterManag

[Lldb-commits] [PATCH] D152220: [lldb][NFCI] Change type of Broadcaster's name

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

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

LLDB_LOGF -> LLDB_LOG where needed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152220

Files:
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/source/API/SBBroadcaster.cpp
  lldb/source/Core/ThreadedCommunication.cpp
  lldb/source/Utility/Broadcaster.cpp
  lldb/source/Utility/Event.cpp
  lldb/source/Utility/Listener.cpp

Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -234,7 +234,7 @@
 
 if (m_broadcaster_names) {
   bool found_source = false;
-  ConstString event_broadcaster_name =
+  const llvm::StringRef event_broadcaster_name =
   event_sp->GetBroadcaster()->GetBroadcasterName();
   for (uint32_t i = 0; i < m_num_broadcaster_names; ++i) {
 if (m_broadcaster_names[i] == event_broadcaster_name) {
Index: lldb/source/Utility/Event.cpp
===
--- lldb/source/Utility/Event.cpp
+++ lldb/source/Utility/Event.cpp
@@ -58,13 +58,13 @@
   s->Printf("%p Event: broadcaster = %p (%s), type = 0x%8.8x (%s), data = ",
 static_cast(this),
 static_cast(broadcaster),
-broadcaster->GetBroadcasterName().GetCString(), m_type,
+broadcaster->GetBroadcasterName().c_str(), m_type,
 event_name.GetData());
 else
   s->Printf("%p Event: broadcaster = %p (%s), type = 0x%8.8x, data = ",
 static_cast(this),
 static_cast(broadcaster),
-broadcaster->GetBroadcasterName().GetCString(), m_type);
+broadcaster->GetBroadcasterName().c_str(), m_type);
   } else
 s->Printf("%p Event: broadcaster = NULL, type = 0x%8.8x, data = ",
   static_cast(this), m_type);
Index: lldb/source/Utility/Broadcaster.cpp
===
--- lldb/source/Utility/Broadcaster.cpp
+++ lldb/source/Utility/Broadcaster.cpp
@@ -23,9 +23,9 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Broadcaster::Broadcaster(BroadcasterManagerSP manager_sp, const char *name)
+Broadcaster::Broadcaster(BroadcasterManagerSP manager_sp, std::string name)
 : m_broadcaster_sp(std::make_shared(*this)),
-  m_manager_sp(std::move(manager_sp)), m_broadcaster_name(name) {
+  m_manager_sp(std::move(manager_sp)), m_broadcaster_name(std::move(name)) {
   Log *log = GetLog(LLDBLog::Object);
   LLDB_LOG(log, "{0} Broadcaster::Broadcaster(\"{1}\")",
static_cast(this), GetBroadcasterName());
@@ -215,12 +215,12 @@
   if (log) {
 StreamString event_description;
 event_sp->Dump(&event_description);
-LLDB_LOGF(log,
-  "%p Broadcaster(\"%s\")::BroadcastEvent (event_sp = {%s}, "
-  "unique =%i) hijack = %p",
-  static_cast(this), GetBroadcasterName(),
-  event_description.GetData(), unique,
-  static_cast(hijacking_listener_sp.get()));
+LLDB_LOG(log,
+ "{0:x} Broadcaster(\"{1}\")::BroadcastEvent (event_sp = {2}, "
+ "unique={3}) hijack = {4:x}",
+ static_cast(this), GetBroadcasterName(),
+ event_description.GetData(), unique,
+ static_cast(hijacking_listener_sp.get()));
   }
 
   if (hijacking_listener_sp) {
Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -57,7 +57,7 @@
 ThreadedCommunication::~ThreadedCommunication() {
   LLDB_LOG(GetLog(LLDBLog::Object | LLDBLog::Communication),
"{0} ThreadedCommunication::~ThreadedCommunication (name = {1})",
-   this, GetBroadcasterName().AsCString());
+   this, GetBroadcasterName());
 }
 
 void ThreadedCommunication::Clear() {
Index: lldb/source/API/SBBroadcaster.cpp
===
--- lldb/source/API/SBBroadcaster.cpp
+++ lldb/source/API/SBBroadcaster.cpp
@@ -92,7 +92,7 @@
   LLDB_INSTRUMENT_VA(this);
 
   if (m_opaque_ptr)
-return m_opaque_ptr->GetBroadcasterName().GetCString();
+return ConstString(m_opaque_ptr->GetBroadcasterName()).GetCString();
   return nullptr;
 }
 
Index: lldb/include/lldb/Utility/Broadcaster.h
===
--- lldb/include/lldb/Utility/Broadcaster.h
+++ lldb/include/lldb/Utility/Broadcaster.h
@@ -149,10 +149,12 @@
 public:
   /// Construct with a broadcaster with a name.
   ///
+  /// \param[in] manager_sp
+  ///   A shared pointer to the BroadcasterManager that will manage this
+  ///   broadcaster.
   /// \param[in] name
-  /// A NULL terminated C string th

[Lldb-commits] [PATCH] D152310: [lldb][NFCI] Remove use of ConstString from OptionValueLanguage

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

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

ConstString is simply not needed here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152310

Files:
  lldb/source/Interpreter/OptionValueLanguage.cpp


Index: lldb/source/Interpreter/OptionValueLanguage.cpp
===
--- lldb/source/Interpreter/OptionValueLanguage.cpp
+++ lldb/source/Interpreter/OptionValueLanguage.cpp
@@ -43,10 +43,8 @@
 
   case eVarSetOperationReplace:
   case eVarSetOperationAssign: {
-ConstString lang_name(value.trim());
 LanguageSet languages_for_types = 
Language::GetLanguagesSupportingTypeSystems();
-LanguageType new_type =
-Language::GetLanguageTypeFromString(lang_name.GetStringRef());
+LanguageType new_type = Language::GetLanguageTypeFromString(value.trim());
 if (new_type && languages_for_types[new_type]) {
   m_value_was_set = true;
   m_current_value = new_type;


Index: lldb/source/Interpreter/OptionValueLanguage.cpp
===
--- lldb/source/Interpreter/OptionValueLanguage.cpp
+++ lldb/source/Interpreter/OptionValueLanguage.cpp
@@ -43,10 +43,8 @@
 
   case eVarSetOperationReplace:
   case eVarSetOperationAssign: {
-ConstString lang_name(value.trim());
 LanguageSet languages_for_types = Language::GetLanguagesSupportingTypeSystems();
-LanguageType new_type =
-Language::GetLanguageTypeFromString(lang_name.GetStringRef());
+LanguageType new_type = Language::GetLanguageTypeFromString(value.trim());
 if (new_type && languages_for_types[new_type]) {
   m_value_was_set = true;
   m_current_value = new_type;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] a10019a - [lldb] Fix "NameError: name 'self' is not defined" when using crashlog -c

2023-06-06 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2023-06-06T15:20:53-07:00
New Revision: a10019a496d420769d7aae94f234ab6cf9fd778d

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

LOG: [lldb] Fix "NameError: name 'self' is not defined" when using crashlog -c

This fixes a regression introduced by 27f27d15f6c9 that results in a
NameError: (name 'self' is not defined) when using crashlog with the -c
option.

rdar://110007391

Added: 


Modified: 
lldb/examples/python/crashlog.py
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 36825b077c6a0..ea197e4f13a4a 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -1307,7 +1307,7 @@ def SymbolicateCrashLog(crash_log, options):
 for thread in crash_log.threads:
 if thread.did_crash():
 for ident in thread.idents:
-for image in 
self.crashlog.find_images_with_identifier(ident):
+for image in crash_log.find_images_with_identifier(ident):
 image.resolve = True
 
 futures = []

diff  --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
index ef440fb772b48..c2e23e82be7f5 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
@@ -3,6 +3,7 @@
 # RUN: cp %S/Inputs/a.out.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash 
--offsets '{"main":20, "bar":9, "foo":16}' --json
 # RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 
'crashlog %t.crash' 2>&1 | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 
'crashlog -c %t.crash' 2>&1 | FileCheck %s
 
 # RUN: cp %S/Inputs/a.out.ips %t.nometadata.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog 
%t.nometadata.crash --offsets '{"main":20, "bar":9, "foo":16}' --json 
--no-metadata



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

[Lldb-commits] [PATCH] D152220: [lldb][NFCI] Change type of Broadcaster's name

2023-06-06 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM modulo inline question.




Comment at: lldb/include/lldb/Utility/Broadcaster.h:358-360
+llvm::StringRef GetBroadcasterName() const {
+  return m_broadcaster.GetBroadcasterName();
 }

Why not return a `const std::string &` here or, alternatively, why not return a 
StringRef above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152220

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

[Lldb-commits] [PATCH] D152310: [lldb][NFCI] Remove use of ConstString from OptionValueLanguage

2023-06-06 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152310

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

[Lldb-commits] [PATCH] D152220: [lldb][NFCI] Change type of Broadcaster's name

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/include/lldb/Utility/Broadcaster.h:358-360
+llvm::StringRef GetBroadcasterName() const {
+  return m_broadcaster.GetBroadcasterName();
 }

JDevlieghere wrote:
> Why not return a `const std::string &` here or, alternatively, why not return 
> a StringRef above?
Good question. IMO it'd be nice to return a StringRef here but it's probably 
more practical to return a `const std::string &` here since we often pass it to 
printf-style formatters for logging (which, for StringRef, you'd need to do 
`ref.str().c_str()` to do it safely).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152220

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

[Lldb-commits] [PATCH] D152315: [lldb][NFCI] Refactor TypeSystemClang::GetBasicTypeEnumeration

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, aprantl, fdeazeve, Michael137, 
augusto2112, kastiglione.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

`TypeSystemClang::GetBasicTypeEnumeration(ConstString)` builds up a
giant `UniqueCStringMap` which is effectively a vector whose 
elements
are (ConstString, lldb::BasicType). Once sorted, we get to have fairly
quick lookups (O(log n) on average). This is fine, but we can do better
on average with an llvm::StringMap.

This also allows us to simplify the initialization code for the lookup,
no more `once_flag`!

Additionally, I removed unused declarations of related functions from
CompilerType and TypeSystemClang


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152315

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  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
@@ -207,7 +207,7 @@
 
   CompilerType GetBasicType(lldb::BasicType type);
 
-  static lldb::BasicType GetBasicTypeEnumeration(ConstString name);
+  static lldb::BasicType GetBasicTypeEnumeration(llvm::StringRef name);
 
   CompilerType
   GetBuiltinTypeForDWARFEncodingAndBitSize(llvm::StringRef type_name,
@@ -834,10 +834,6 @@
   lldb::BasicType
   GetBasicTypeEnumeration(lldb::opaque_compiler_type_t type) override;
 
-  static lldb::BasicType
-  GetBasicTypeEnumeration(lldb::opaque_compiler_type_t type,
-  ConstString name);
-
   void ForEachEnumerator(
   lldb::opaque_compiler_type_t type,
   std::function TypeNameToBasicTypeMap;
-static TypeNameToBasicTypeMap g_type_map;
-static llvm::once_flag g_once_flag;
-llvm::call_once(g_once_flag, []() {
+lldb::BasicType TypeSystemClang::GetBasicTypeEnumeration(llvm::StringRef name) {
+  if (name.empty())
+return eBasicTypeInvalid;
+
+  static const llvm::StringMap g_type_map = {
   // "void"
-  g_type_map.Append(ConstString("void"), eBasicTypeVoid);
+  {"void", eBasicTypeVoid},
 
   // "char"
-  g_type_map.Append(ConstString("char"), eBasicTypeChar);
-  g_type_map.Append(ConstString("signed char"), eBasicTypeSignedChar);
-  g_type_map.Append(ConstString("unsigned char"), eBasicTypeUnsignedChar);
-  g_type_map.Append(ConstString("wchar_t"), eBasicTypeWChar);
-  g_type_map.Append(ConstString("signed wchar_t"), eBasicTypeSignedWChar);
-  g_type_map.Append(ConstString("unsigned wchar_t"),
-eBasicTypeUnsignedWChar);
+  {"char", eBasicTypeChar},
+  {"signed char", eBasicTypeSignedChar},
+  {"unsigned char", eBasicTypeUnsignedChar},
+  {"wchar_t", eBasicTypeWChar},
+  {"signed wchar_t", eBasicTypeSignedWChar},
+  {"unsigned wchar_t", eBasicTypeUnsignedWChar},
+
   // "short"
-  g_type_map.Append(ConstString("short"), eBasicTypeShort);
-  g_type_map.Append(ConstString("short int"), eBasicTypeShort);
-  g_type_map.Append(ConstString("unsigned short"), eBasicTypeUnsignedShort);
-  g_type_map.Append(ConstString("unsigned short int"),
-eBasicTypeUnsignedShort);
+  {"short", eBasicTypeShort},
+  {"short int", eBasicTypeShort},
+  {"unsigned short", eBasicTypeUnsignedShort},
+  {"unsigned short int", eBasicTypeUnsignedShort},
 
   // "int"
-  g_type_map.Append(ConstString("int"), eBasicTypeInt);
-  g_type_map.Append(ConstString("signed int"), eBasicTypeInt);
-  g_type_map.Append(ConstString("unsigned int"), eBasicTypeUnsignedInt);
-  g_type_map.Append(ConstString("unsigned"), eBasicTypeUnsignedInt);
+  {"int", eBasicTypeInt},
+  {"signed int", eBasicTypeInt},
+  {"unsigned int", eBasicTypeUnsignedInt},
+  {"unsigned", eBasicTypeUnsignedInt},
 
   // "long"
-  g_type_map.Append(ConstString("long"), eBasicTypeLong);
-  g_type_map.Append(ConstString("long int"), eBasicTypeLong);
-  g_type_map.Append(ConstString("unsigned long"), eBasicTypeUnsignedLong);
-  g_type_map.Append(ConstString("unsigned long int"),
-eBasicTypeUnsignedLong);
+  {"long", eBasicTypeLong},
+  {"long int", eBasicTypeLong},
+  {"unsigned long", eBasicTypeUnsignedLong},
+  {"unsigned long int", eBasicTypeUnsignedLong},
 
   // "long long"
-  g_type_map.Append(ConstString("long long"), eBasicTypeLongLong);
-  g_type_map.Append(ConstString("long long int"), eBasicTypeLongLong);
-  g_type_map.Append(ConstString("unsigned long long"),
-eBasicTypeUnsignedLongLong);
-  g_type_map.Append(ConstString("unsigned long long int"),
-

[Lldb-commits] [PATCH] D152220: [lldb][NFCI] Change type of Broadcaster's name

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

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

`llvm::StringRef` -> `const std::string &`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152220

Files:
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/source/API/SBBroadcaster.cpp
  lldb/source/Core/ThreadedCommunication.cpp
  lldb/source/Utility/Broadcaster.cpp
  lldb/source/Utility/Event.cpp
  lldb/source/Utility/Listener.cpp

Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -234,7 +234,7 @@
 
 if (m_broadcaster_names) {
   bool found_source = false;
-  ConstString event_broadcaster_name =
+  const llvm::StringRef event_broadcaster_name =
   event_sp->GetBroadcaster()->GetBroadcasterName();
   for (uint32_t i = 0; i < m_num_broadcaster_names; ++i) {
 if (m_broadcaster_names[i] == event_broadcaster_name) {
Index: lldb/source/Utility/Event.cpp
===
--- lldb/source/Utility/Event.cpp
+++ lldb/source/Utility/Event.cpp
@@ -58,13 +58,13 @@
   s->Printf("%p Event: broadcaster = %p (%s), type = 0x%8.8x (%s), data = ",
 static_cast(this),
 static_cast(broadcaster),
-broadcaster->GetBroadcasterName().GetCString(), m_type,
+broadcaster->GetBroadcasterName().c_str(), m_type,
 event_name.GetData());
 else
   s->Printf("%p Event: broadcaster = %p (%s), type = 0x%8.8x, data = ",
 static_cast(this),
 static_cast(broadcaster),
-broadcaster->GetBroadcasterName().GetCString(), m_type);
+broadcaster->GetBroadcasterName().c_str(), m_type);
   } else
 s->Printf("%p Event: broadcaster = NULL, type = 0x%8.8x, data = ",
   static_cast(this), m_type);
Index: lldb/source/Utility/Broadcaster.cpp
===
--- lldb/source/Utility/Broadcaster.cpp
+++ lldb/source/Utility/Broadcaster.cpp
@@ -23,9 +23,9 @@
 using namespace lldb;
 using namespace lldb_private;
 
-Broadcaster::Broadcaster(BroadcasterManagerSP manager_sp, const char *name)
+Broadcaster::Broadcaster(BroadcasterManagerSP manager_sp, std::string name)
 : m_broadcaster_sp(std::make_shared(*this)),
-  m_manager_sp(std::move(manager_sp)), m_broadcaster_name(name) {
+  m_manager_sp(std::move(manager_sp)), m_broadcaster_name(std::move(name)) {
   Log *log = GetLog(LLDBLog::Object);
   LLDB_LOG(log, "{0} Broadcaster::Broadcaster(\"{1}\")",
static_cast(this), GetBroadcasterName());
@@ -215,12 +215,12 @@
   if (log) {
 StreamString event_description;
 event_sp->Dump(&event_description);
-LLDB_LOGF(log,
-  "%p Broadcaster(\"%s\")::BroadcastEvent (event_sp = {%s}, "
-  "unique =%i) hijack = %p",
-  static_cast(this), GetBroadcasterName(),
-  event_description.GetData(), unique,
-  static_cast(hijacking_listener_sp.get()));
+LLDB_LOG(log,
+ "{0:x} Broadcaster(\"{1}\")::BroadcastEvent (event_sp = {2}, "
+ "unique={3}) hijack = {4:x}",
+ static_cast(this), GetBroadcasterName(),
+ event_description.GetData(), unique,
+ static_cast(hijacking_listener_sp.get()));
   }
 
   if (hijacking_listener_sp) {
Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -57,7 +57,7 @@
 ThreadedCommunication::~ThreadedCommunication() {
   LLDB_LOG(GetLog(LLDBLog::Object | LLDBLog::Communication),
"{0} ThreadedCommunication::~ThreadedCommunication (name = {1})",
-   this, GetBroadcasterName().AsCString());
+   this, GetBroadcasterName());
 }
 
 void ThreadedCommunication::Clear() {
Index: lldb/source/API/SBBroadcaster.cpp
===
--- lldb/source/API/SBBroadcaster.cpp
+++ lldb/source/API/SBBroadcaster.cpp
@@ -92,7 +92,7 @@
   LLDB_INSTRUMENT_VA(this);
 
   if (m_opaque_ptr)
-return m_opaque_ptr->GetBroadcasterName().GetCString();
+return ConstString(m_opaque_ptr->GetBroadcasterName()).GetCString();
   return nullptr;
 }
 
Index: lldb/include/lldb/Utility/Broadcaster.h
===
--- lldb/include/lldb/Utility/Broadcaster.h
+++ lldb/include/lldb/Utility/Broadcaster.h
@@ -149,10 +149,12 @@
 public:
   /// Construct with a broadcaster with a name.
   ///
+  /// \param[in] manager_sp
+  ///   A shared pointer to the BroadcasterManager that will manage this
+  ///   broadcaster.
   /// \param[in] name
-  /// A NULL terminated C s

[Lldb-commits] [PATCH] D152315: [lldb][NFCI] Refactor TypeSystemClang::GetBasicTypeEnumeration

2023-06-06 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:868
+lldb::BasicType TypeSystemClang::GetBasicTypeEnumeration(llvm::StringRef name) 
{
+  if (name.empty())
+return eBasicTypeInvalid;

Isn't this redundant?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152315

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

[Lldb-commits] [PATCH] D152315: [lldb][NFCI] Refactor TypeSystemClang::GetBasicTypeEnumeration

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:868
+lldb::BasicType TypeSystemClang::GetBasicTypeEnumeration(llvm::StringRef name) 
{
+  if (name.empty())
+return eBasicTypeInvalid;

aprantl wrote:
> Isn't this redundant?
Technically yes. My reasoning is that it's faster to see if a StringRef is 
empty than to perform a lookup in a hash map. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152315

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

[Lldb-commits] [lldb] 431f082 - [lldb] Disable some tests on windows

2023-06-06 Thread Med Ismail Bennani via lldb-commits


Author: Med Ismail Bennani
Date: 2023-06-06T16:10:05-07:00
New Revision: 431f082a55488b12ae12d4330b6c6c61dc3e61be

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

LOG: [lldb] Disable some tests on windows

This patch skips both `test_completion_target_create_from_root_dir`
introduced in `e896612` and  `target-label.test` introduced in `1e82b20`
since I don't have a windows machine to try to accomodate the filesystem
path style differences for these tests to pass.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/test/API/functionalities/completion/TestCompletion.py
lldb/test/Shell/Target/target-label.test

Removed: 




diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py 
b/lldb/test/API/functionalities/completion/TestCompletion.py
index 0cfd213177d6a..bc39f18fb225b 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -477,6 +477,7 @@ def test_custom_command_completion(self):
 self.complete_from_to("my_test_cmd main.cp", ["main.cpp"])
 self.expect("my_test_cmd main.cpp", substrs=["main.cpp"])
 
+@skipIfWindows
 def test_completion_target_create_from_root_dir(self):
 """Tests source file completion by completing ."""
 root_dir = os.path.abspath(os.sep)

diff  --git a/lldb/test/Shell/Target/target-label.test 
b/lldb/test/Shell/Target/target-label.test
index 7194f4b558f34..e550e863dbe1b 100644
--- a/lldb/test/Shell/Target/target-label.test
+++ b/lldb/test/Shell/Target/target-label.test
@@ -1,3 +1,4 @@
+# UNSUPPORTED: system-windows
 # RUN: %lldb -b -o 'settings set interpreter.stop-command-source-on-error 
false' -s %s 2>&1 | FileCheck %s
 
 target create -l "ls" /bin/ls



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

[Lldb-commits] [PATCH] D152315: [lldb][NFCI] Refactor TypeSystemClang::GetBasicTypeEnumeration

2023-06-06 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:868
+lldb::BasicType TypeSystemClang::GetBasicTypeEnumeration(llvm::StringRef name) 
{
+  if (name.empty())
+return eBasicTypeInvalid;

bulbazord wrote:
> aprantl wrote:
> > Isn't this redundant?
> Technically yes. My reasoning is that it's faster to see if a StringRef is 
> empty than to perform a lookup in a hash map. 
True, but this adds extra code to the binary that in practice will 
(presumably?) never get executed. Or are we often looking up empty types? (It's 
not a big deal either way, but since we are being pedantic :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152315

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

[Lldb-commits] [PATCH] D152315: [lldb][NFCI] Refactor TypeSystemClang::GetBasicTypeEnumeration

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

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

Minor pedantry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152315

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  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
@@ -207,7 +207,7 @@
 
   CompilerType GetBasicType(lldb::BasicType type);
 
-  static lldb::BasicType GetBasicTypeEnumeration(ConstString name);
+  static lldb::BasicType GetBasicTypeEnumeration(llvm::StringRef name);
 
   CompilerType
   GetBuiltinTypeForDWARFEncodingAndBitSize(llvm::StringRef type_name,
@@ -834,10 +834,6 @@
   lldb::BasicType
   GetBasicTypeEnumeration(lldb::opaque_compiler_type_t type) override;
 
-  static lldb::BasicType
-  GetBasicTypeEnumeration(lldb::opaque_compiler_type_t type,
-  ConstString name);
-
   void ForEachEnumerator(
   lldb::opaque_compiler_type_t type,
   std::function TypeNameToBasicTypeMap;
-static TypeNameToBasicTypeMap g_type_map;
-static llvm::once_flag g_once_flag;
-llvm::call_once(g_once_flag, []() {
+lldb::BasicType TypeSystemClang::GetBasicTypeEnumeration(llvm::StringRef name) {
+  static const llvm::StringMap g_type_map = {
   // "void"
-  g_type_map.Append(ConstString("void"), eBasicTypeVoid);
+  {"void", eBasicTypeVoid},
 
   // "char"
-  g_type_map.Append(ConstString("char"), eBasicTypeChar);
-  g_type_map.Append(ConstString("signed char"), eBasicTypeSignedChar);
-  g_type_map.Append(ConstString("unsigned char"), eBasicTypeUnsignedChar);
-  g_type_map.Append(ConstString("wchar_t"), eBasicTypeWChar);
-  g_type_map.Append(ConstString("signed wchar_t"), eBasicTypeSignedWChar);
-  g_type_map.Append(ConstString("unsigned wchar_t"),
-eBasicTypeUnsignedWChar);
+  {"char", eBasicTypeChar},
+  {"signed char", eBasicTypeSignedChar},
+  {"unsigned char", eBasicTypeUnsignedChar},
+  {"wchar_t", eBasicTypeWChar},
+  {"signed wchar_t", eBasicTypeSignedWChar},
+  {"unsigned wchar_t", eBasicTypeUnsignedWChar},
+
   // "short"
-  g_type_map.Append(ConstString("short"), eBasicTypeShort);
-  g_type_map.Append(ConstString("short int"), eBasicTypeShort);
-  g_type_map.Append(ConstString("unsigned short"), eBasicTypeUnsignedShort);
-  g_type_map.Append(ConstString("unsigned short int"),
-eBasicTypeUnsignedShort);
+  {"short", eBasicTypeShort},
+  {"short int", eBasicTypeShort},
+  {"unsigned short", eBasicTypeUnsignedShort},
+  {"unsigned short int", eBasicTypeUnsignedShort},
 
   // "int"
-  g_type_map.Append(ConstString("int"), eBasicTypeInt);
-  g_type_map.Append(ConstString("signed int"), eBasicTypeInt);
-  g_type_map.Append(ConstString("unsigned int"), eBasicTypeUnsignedInt);
-  g_type_map.Append(ConstString("unsigned"), eBasicTypeUnsignedInt);
+  {"int", eBasicTypeInt},
+  {"signed int", eBasicTypeInt},
+  {"unsigned int", eBasicTypeUnsignedInt},
+  {"unsigned", eBasicTypeUnsignedInt},
 
   // "long"
-  g_type_map.Append(ConstString("long"), eBasicTypeLong);
-  g_type_map.Append(ConstString("long int"), eBasicTypeLong);
-  g_type_map.Append(ConstString("unsigned long"), eBasicTypeUnsignedLong);
-  g_type_map.Append(ConstString("unsigned long int"),
-eBasicTypeUnsignedLong);
+  {"long", eBasicTypeLong},
+  {"long int", eBasicTypeLong},
+  {"unsigned long", eBasicTypeUnsignedLong},
+  {"unsigned long int", eBasicTypeUnsignedLong},
 
   // "long long"
-  g_type_map.Append(ConstString("long long"), eBasicTypeLongLong);
-  g_type_map.Append(ConstString("long long int"), eBasicTypeLongLong);
-  g_type_map.Append(ConstString("unsigned long long"),
-eBasicTypeUnsignedLongLong);
-  g_type_map.Append(ConstString("unsigned long long int"),
-eBasicTypeUnsignedLongLong);
+  {"long long", eBasicTypeLongLong},
+  {"long long int", eBasicTypeLongLong},
+  {"unsigned long long", eBasicTypeUnsignedLongLong},
+  {"unsigned long long int", eBasicTypeUnsignedLongLong},
 
   // "int128"
-  g_type_map.Append(ConstString("__int128_t"), eBasicTypeInt128);
-  g_type_map.Append(ConstString("__uint128_t"), eBasicTypeUnsignedInt128);
+  {"__int128_t", eBasicTypeInt128},
+  {"__uint128_t", eBasicTypeUnsignedInt128},
 
   // Miscellaneous
-  g_type_map.Append(ConstString("bool"), eBasicTypeBool);
-  g_type_map.Append(ConstString("float"), eBasicTypeFloat);
-  g_type_

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

2023-06-06 Thread Haowei Wu via Phabricator via lldb-commits

haowei added a comment.

Hi

The `lldb-shell :: Target/target-label.test` is broken on our LLVM builder 
after this change was landed. Error message we saw:

  Script:
  --
  : 'RUN: at line 1';   /b/s/w/ir/x/w/staging/llvm_build/bin/lldb --no-lldbinit 
-S /b/s/w/ir/x/w/staging/llvm_build/tools/lldb/test/Shell/lit-lldb-init-quiet 
-b -o 'settings set interpreter.stop-command-source-on-error false' -s 
/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/Shell/Target/target-label.test 2>&1 | 
/b/s/w/ir/x/w/staging/llvm_build/bin/FileCheck 
/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/Shell/Target/target-label.test
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  
/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/Shell/Target/target-label.test:9:10: 
error: CHECK: expected string not found in input
  # CHECK: * target #0: /bin/ls
   ^
  :10:26: note: scanning from here
  * target #0 (ls): /bin/ls ( arch=x86_64-*-linux, platform=host )
   ^
  :29:2: note: possible intended match here
   target #2: /bin/cat ( arch=x86_64-*-linux, platform=host )
   ^
  
  Input file: 
  Check file: 
/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/Shell/Target/target-label.test
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
 .
 .
 .
 5: (lldb) command source -s 0 
'/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/Shell/Target/target-label.test' 
 6: Executing commands in 
'/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/Shell/Target/target-label.test'. 
 7: (lldb) target create -l "ls" /bin/ls 
 8: (lldb) target list 
 9: Current targets: 
10: * target #0 (ls): /bin/ls ( arch=x86_64-*-linux, platform=host 
) 
  check:9'0  
X~~~ error: no match found
11: (lldb) script lldb.target.SetLabel("") 
  check:9'0 ~~~
12: error: there is no embedded script interpreter in this mode. 
  check:9'0 ~
13: (lldb) target list 
  check:9'0 ~~~
14: Current targets: 
  check:9'0 ~
15: * target #0 (ls): /bin/ls ( arch=x86_64-*-linux, platform=host 
) 
  check:9'0 
~
 .
 .
 .
24: error: Cannot use integer as target label. 
  check:9'0 ~~~
25: (lldb) target select 0 
  check:9'0 ~~~
26: Current targets: 
  check:9'0 ~
27: * target #0 (ls): /bin/ls ( arch=x86_64-*-linux, platform=host 
) 
  check:9'0 
~
28:  target #1 (cat): /bin/cat ( arch=x86_64-*-linux, platform=host 
) 
  check:9'0 
~~
29:  target #2: /bin/cat ( arch=x86_64-*-linux, platform=host ) 
  check:9'0 
  check:9'1  ?   
possible intended match
30:  target #3: /bin/cat ( arch=x86_64-*-linux, platform=host ) 
  check:9'0 
31: (lldb) target select cat 
  check:9'0 ~
32: Current targets: 
  check:9'0 ~
33:  target #0 (ls): /bin/ls ( arch=x86_64-*-linux, platform=host ) 
  check:9'0 
34: * target #1 (cat): /bin/cat ( arch=x86_64-*-linux, 
platform=host ) 
  check:9'0 
~~~
 .
 .
 .
  >>
  
  --

Link to the builder: 
https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8779016248689544129/overview

Could you take a look? If it takes some time to fix, could you revert this 
change please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151859

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

[Lldb-commits] [PATCH] D152319: [lldb] Run crashlog inside lldb when invoked in interactive mode from the command line

2023-06-06 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: mib, aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

Run crashlog inside lldb when invoked in interactive mode from the command 
line. Currently, when passing -i to crashlog from the command line, we 
symbolicate in LLDB and immediately exit right after which pretty much defeats 
the purpose of interactive mode. On the other hand, we wouldn't want to 
reimplement the driver from the crashlog script. Re-invoking ourself from 
inside LLDB solves the issue.

rdar://97801509


https://reviews.llvm.org/D152319

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1271,7 +1271,7 @@
 pass
 
 def __call__(self, debugger, command, exe_ctx, result):
-SymbolicateCrashLogs(debugger, shlex.split(command), result)
+SymbolicateCrashLogs(debugger, shlex.split(command), result, True)
 
 def get_short_help(self):
 return "Symbolicate one or more darwin crash log files."
@@ -1596,7 +1596,7 @@
 return CreateSymbolicateCrashLogOptions("crashlog", description, True)
 
 
-def SymbolicateCrashLogs(debugger, command_args, result):
+def SymbolicateCrashLogs(debugger, command_args, result, is_command):
 option_parser = CrashLogOptionParser()
 
 if not len(command_args):
@@ -1608,6 +1608,26 @@
 except:
 return
 
+# Interactive mode requires running the crashlog command from inside lldb.
+if options.interactive and not is_command:
+lldb_exec = (
+subprocess.check_output(["/usr/bin/xcrun", "-f", "lldb"])
+.decode("utf-8")
+.strip()
+)
+sys.exit(
+os.execv(
+lldb_exec,
+[
+lldb_exec,
+"-o",
+"command script import lldb.macosx",
+"-o",
+"crashlog {}".format(shlex.join(command_args)),
+],
+)
+)
+
 if options.version:
 print(debugger.GetVersionString())
 return
@@ -1659,7 +1679,7 @@
 # Create a new debugger instance
 debugger = lldb.SBDebugger.Create()
 result = lldb.SBCommandReturnObject()
-SymbolicateCrashLogs(debugger, sys.argv[1:], result)
+SymbolicateCrashLogs(debugger, sys.argv[1:], result, False)
 lldb.SBDebugger.Destroy(debugger)
 
 


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1271,7 +1271,7 @@
 pass
 
 def __call__(self, debugger, command, exe_ctx, result):
-SymbolicateCrashLogs(debugger, shlex.split(command), result)
+SymbolicateCrashLogs(debugger, shlex.split(command), result, True)
 
 def get_short_help(self):
 return "Symbolicate one or more darwin crash log files."
@@ -1596,7 +1596,7 @@
 return CreateSymbolicateCrashLogOptions("crashlog", description, True)
 
 
-def SymbolicateCrashLogs(debugger, command_args, result):
+def SymbolicateCrashLogs(debugger, command_args, result, is_command):
 option_parser = CrashLogOptionParser()
 
 if not len(command_args):
@@ -1608,6 +1608,26 @@
 except:
 return
 
+# Interactive mode requires running the crashlog command from inside lldb.
+if options.interactive and not is_command:
+lldb_exec = (
+subprocess.check_output(["/usr/bin/xcrun", "-f", "lldb"])
+.decode("utf-8")
+.strip()
+)
+sys.exit(
+os.execv(
+lldb_exec,
+[
+lldb_exec,
+"-o",
+"command script import lldb.macosx",
+"-o",
+"crashlog {}".format(shlex.join(command_args)),
+],
+)
+)
+
 if options.version:
 print(debugger.GetVersionString())
 return
@@ -1659,7 +1679,7 @@
 # Create a new debugger instance
 debugger = lldb.SBDebugger.Create()
 result = lldb.SBCommandReturnObject()
-SymbolicateCrashLogs(debugger, sys.argv[1:], result)
+SymbolicateCrashLogs(debugger, sys.argv[1:], result, False)
 lldb.SBDebugger.Destroy(debugger)
 
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152319: [lldb] Run crashlog inside lldb when invoked in interactive mode from the command line

2023-06-06 Thread Adrian Prantl via Phabricator via lldb-commits

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

This is super convenient! Thanks!


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

https://reviews.llvm.org/D152319

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

[Lldb-commits] [PATCH] D152324: [lldb][NFCI] Change return type of PersistentExpressionState::GetNextPersistentVariableName

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

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

These don't really need to be in the ConstString StringPool. Note that
they still end up there because we need to wrap them in ConstStrings to
use them with certain APIs, but we can stop creating them as
ConstStrings to start.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152324

Files:
  lldb/include/lldb/Expression/ExpressionVariable.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Target/ABI.cpp

Index: lldb/source/Target/ABI.cpp
===
--- lldb/source/Target/ABI.cpp
+++ lldb/source/Target/ABI.cpp
@@ -93,8 +93,8 @@
 if (!persistent_expression_state)
   return {};
 
-ConstString persistent_variable_name =
-persistent_expression_state->GetNextPersistentVariableName();
+ConstString persistent_variable_name = ConstString(
+persistent_expression_state->GetNextPersistentVariableName());
 
 lldb::ValueObjectSP const_valobj_sp;
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -1015,7 +1015,7 @@
 }
 
 ConstString ClangUserExpression::ResultDelegate::GetName() {
-  return m_persistent_state->GetNextPersistentVariableName(false);
+  return ConstString(m_persistent_state->GetNextPersistentVariableName(false));
 }
 
 void ClangUserExpression::ResultDelegate::DidDematerialize(
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.h
@@ -55,7 +55,7 @@
 
   void RemovePersistentVariable(lldb::ExpressionVariableSP variable) override;
 
-  ConstString GetNextPersistentVariableName(bool is_error = false) override;
+  std::string GetNextPersistentVariableName(bool is_error = false) override;
 
   /// Returns the next file name that should be used for user expressions.
   std::string GetNextExprFileName() {
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangPersistentVariables.cpp
@@ -124,7 +124,7 @@
   return m_modules_decl_vendor_sp;
 }
 
-ConstString
+std::string
 ClangPersistentVariables::GetNextPersistentVariableName(bool is_error) {
   llvm::SmallString<64> name;
   {
@@ -132,5 +132,5 @@
 os << GetPersistentVariablePrefix(is_error)
<< m_next_persistent_variable_id++;
   }
-  return ConstString(name);
+  return std::string(name);
 }
Index: lldb/source/Expression/Materializer.cpp
===
--- lldb/source/Expression/Materializer.cpp
+++ lldb/source/Expression/Materializer.cpp
@@ -1041,9 +1041,10 @@
   return;
 }
 
-ConstString name = m_delegate
-   ? m_delegate->GetName()
-   : persistent_state->GetNextPersistentVariableName();
+ConstString name =
+m_delegate
+? m_delegate->GetName()
+: ConstString(persistent_state->GetNextPersistentVariableName());
 
 lldb::ExpressionVariableSP ret = persistent_state->CreatePersistentVariable(
 exe_scope, name, m_type, map.GetByteOrder(), map.GetAddressByteSize());
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -3143,10 +3143,10 @@
   if (!persistent_state)
 return nullptr;
 
-  ConstString name = persistent_state->GetNextPersistentVariableName();
+  const std::string name = persistent_state->GetNextPersistentVariableName();
 
-  ValueObjectSP const_result_sp =
-  ValueObjectConstResult::Create(target_sp.get(), GetValue(), name);
+  ValueObjectSP const_result_sp = ValueObjectConstResult::Create(
+  target_sp.get(), GetValue(), ConstString(name));
 
   ExpressionVariableSP persistent_var_sp =
   persistent_state->CreatePersistentVariable(const_result_sp);
Index: lldb/include/lldb/Expression/ExpressionVariable.h

[Lldb-commits] [lldb] bcfd85a - [lldb/test] Fix target-label.test on Fuchsia

2023-06-06 Thread Med Ismail Bennani via lldb-commits


Author: Med Ismail Bennani
Date: 2023-06-06T17:42:09-07:00
New Revision: bcfd85a25857294cb4a5cfa3f616f57a6911cda6

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

LOG: [lldb/test] Fix target-label.test on Fuchsia

This shell test also checks some SBAPI functionalities and thus requires
python support.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/test/Shell/Target/target-label.test

Removed: 




diff  --git a/lldb/test/Shell/Target/target-label.test 
b/lldb/test/Shell/Target/target-label.test
index e550e863dbe1b..5ac430601e29a 100644
--- a/lldb/test/Shell/Target/target-label.test
+++ b/lldb/test/Shell/Target/target-label.test
@@ -1,3 +1,4 @@
+# REQUIRES: python
 # UNSUPPORTED: system-windows
 # RUN: %lldb -b -o 'settings set interpreter.stop-command-source-on-error 
false' -s %s 2>&1 | FileCheck %s
 



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

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

2023-06-06 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

> Could you take a look? If it takes some time to fix, could you revert this 
> change please?

@haowei bcfd85a25857294cb4a5cfa3f616f57a6911cda6 
 should 
fix the test failure. Let me know if there is another on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151859

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

[Lldb-commits] [PATCH] D152319: [lldb] Run crashlog inside lldb when invoked in interactive mode from the command line

2023-06-06 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib accepted this revision.
mib added a comment.

LGTM! Thanks for taking care of this :)


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

https://reviews.llvm.org/D152319

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

[Lldb-commits] [PATCH] D152326: [lldb][NFCI] DecodedThread::TraceItemStorage::error should own its own data

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

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

The way it works now, it stores a `const char *` that it does not
explicitly own. It's owned by the ConstString StringPool. This is purely
to manage its lifetime, we don't really benefit from deduplication (nor
should we try to, they are errors). We also don't really benefit from
quick comparisons.

This may make the size of TraceItemStorage larger, but you have to pay
the cost of owning the data somewhere. The ConstString StringPool is an
attractive choice but ultimately a poor one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152326

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h


Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -28,7 +28,7 @@
 
   bool HasValue() const override;
 
-  const char *GetError() const override;
+  llvm::StringRef GetError() const override;
 
   lldb::addr_t GetLoadAddress() const override;
 
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -95,7 +95,7 @@
   return m_decoded_thread_sp->GetItemKindByIndex(m_pos);
 }
 
-const char *TraceCursorIntelPT::GetError() const {
+llvm::StringRef TraceCursorIntelPT::GetError() const {
   return m_decoded_thread_sp->GetErrorByIndex(m_pos);
 }
 
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -159,7 +159,7 @@
 
   /// \return
   ///   The error associated with a given trace item.
-  const char *GetErrorByIndex(uint64_t item_index) const;
+  llvm::StringRef GetErrorByIndex(uint64_t item_index) const;
 
   /// \return
   ///   The trace item kind given an item index.
@@ -275,7 +275,7 @@
 lldb::TraceEvent event;
 
 /// The string message of this item if it's an error
-const char *error;
+std::string error;
   };
 
   /// Create a new trace item.
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -186,14 +186,12 @@
 }
 
 void DecodedThread::AppendError(const IntelPTError &error) {
-  CreateNewTraceItem(lldb::eTraceItemKindError).error =
-  ConstString(error.message()).AsCString();
+  CreateNewTraceItem(lldb::eTraceItemKindError).error = error.message();
   m_error_stats.RecordError(/*fatal=*/false);
 }
 
 void DecodedThread::AppendCustomError(StringRef err, bool fatal) {
-  CreateNewTraceItem(lldb::eTraceItemKindError).error =
-  ConstString(err).AsCString();
+  CreateNewTraceItem(lldb::eTraceItemKindError).error = err.str();
   m_error_stats.RecordError(fatal);
 }
 
@@ -238,7 +236,9 @@
   return static_cast(m_item_kinds[item_index]);
 }
 
-const char *DecodedThread::GetErrorByIndex(uint64_t item_index) const {
+llvm::StringRef DecodedThread::GetErrorByIndex(uint64_t item_index) const {
+  if (item_index >= m_item_data.size())
+return llvm::StringRef();
   return m_item_data[item_index].error;
 }
 
Index: lldb/include/lldb/Target/TraceCursor.h
===
--- lldb/include/lldb/Target/TraceCursor.h
+++ lldb/include/lldb/Target/TraceCursor.h
@@ -217,7 +217,7 @@
 
   /// \return
   /// The error message the cursor is pointing at.
-  virtual const char *GetError() const = 0;
+  virtual llvm::StringRef GetError() const = 0;
 
   /// \return
   /// Whether the cursor points to an event or not.


Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -28,7 +28,7 @@
 
   bool HasValue() const override;
 
-  const char *GetError() const override;
+  llvm::StringRef GetError() const override;
 
   lldb::addr_t GetLoadAddress() const override;
 
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/i

[Lldb-commits] [PATCH] D152326: [lldb][NFCI] DecodedThread::TraceItemStorage::error should own its own data

2023-06-06 Thread walter erquinigo via Phabricator via lldb-commits

wallace accepted this revision.
wallace added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: JDevlieghere.

lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152326

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

[Lldb-commits] [lldb] d4c5b45 - [NFC] Remove unneeded semicolon after function definition

2023-06-06 Thread Jim Lin via lldb-commits


Author: Jim Lin
Date: 2023-06-07T09:29:49+08:00
New Revision: d4c5b452934a31f9b3685cf58bd682104b686d1a

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

LOG: [NFC] Remove unneeded semicolon after function definition

Added: 


Modified: 
clang/lib/Tooling/Transformer/Stencil.cpp
lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
llvm/include/llvm/CodeGen/GlobalISel/GISelKnownBits.h
llvm/include/llvm/Object/GOFFObjectFile.h
llvm/lib/Target/CSKY/CSKYAsmPrinter.h
llvm/lib/Target/SPIRV/MCTargetDesc/SPIRVTargetStreamer.h
llvm/lib/Target/X86/MCTargetDesc/X86MCExpr.h
llvm/unittests/Transforms/Vectorize/VPlanTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Transformer/Stencil.cpp 
b/clang/lib/Tooling/Transformer/Stencil.cpp
index 2198aefddc9f1..f2c1b6f8520a8 100644
--- a/clang/lib/Tooling/Transformer/Stencil.cpp
+++ b/clang/lib/Tooling/Transformer/Stencil.cpp
@@ -327,7 +327,7 @@ class SelectBoundStencil : public 
clang::transformer::StencilInterface {
 assert(containsNoNullStencils(CaseStencils) &&
"cases of selectBound may not be null");
   }
-  ~SelectBoundStencil() override{};
+  ~SelectBoundStencil() override {}
 
   llvm::Error eval(const MatchFinder::MatchResult &match,
std::string *result) const override {

diff  --git a/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h 
b/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
index e1b8558e1cda0..da0b867fb1e9b 100644
--- a/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
+++ b/lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
@@ -22,7 +22,7 @@ class ArchitectureAArch64 : public Architecture {
 
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 
-  void OverrideStopInfo(Thread &thread) const override{};
+  void OverrideStopInfo(Thread &thread) const override {}
 
   const MemoryTagManager *GetMemoryTagManager() const override {
 return &m_memory_tag_manager;

diff  --git a/llvm/include/llvm/CodeGen/GlobalISel/GISelKnownBits.h 
b/llvm/include/llvm/CodeGen/GlobalISel/GISelKnownBits.h
index 035c5a08feefd..eff87c5617d99 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GISelKnownBits.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GISelKnownBits.h
@@ -93,10 +93,10 @@ class GISelKnownBits : public GISelChangeObserver {
   Align computeKnownAlignment(Register R, unsigned Depth = 0);
 
   // Observer API. No-op for non-caching implementation.
-  void erasingInstr(MachineInstr &MI) override{};
-  void createdInstr(MachineInstr &MI) override{};
-  void changingInstr(MachineInstr &MI) override{};
-  void changedInstr(MachineInstr &MI) override{};
+  void erasingInstr(MachineInstr &MI) override {}
+  void createdInstr(MachineInstr &MI) override {}
+  void changingInstr(MachineInstr &MI) override {}
+  void changedInstr(MachineInstr &MI) override {}
 
 protected:
   unsigned getMaxDepth() const { return MaxDepth; }

diff  --git a/llvm/include/llvm/Object/GOFFObjectFile.h 
b/llvm/include/llvm/Object/GOFFObjectFile.h
index 37b6b1ec659a0..c39a9dee98d14 100644
--- a/llvm/include/llvm/Object/GOFFObjectFile.h
+++ b/llvm/include/llvm/Object/GOFFObjectFile.h
@@ -82,7 +82,7 @@ class GOFFObjectFile : public ObjectFile {
   bool isSymbolIndirect(DataRefImpl Symb) const;
 
   // SectionRef.
-  void moveSectionNext(DataRefImpl &Sec) const override{};
+  void moveSectionNext(DataRefImpl &Sec) const override {}
   virtual Expected getSectionName(DataRefImpl Sec) const override {
 return StringRef();
   }
@@ -112,7 +112,7 @@ class GOFFObjectFile : public ObjectFile {
   const uint8_t *getSectionPrEsdRecord(uint32_t SectionIndex) const;
 
   // RelocationRef.
-  void moveRelocationNext(DataRefImpl &Rel) const override{};
+  void moveRelocationNext(DataRefImpl &Rel) const override {}
   uint64_t getRelocationOffset(DataRefImpl Rel) const override { return 0; }
   symbol_iterator getRelocationSymbol(DataRefImpl Rel) const override {
 DataRefImpl Temp;
@@ -120,7 +120,7 @@ class GOFFObjectFile : public ObjectFile {
   }
   uint64_t getRelocationType(DataRefImpl Rel) const override { return 0; }
   void getRelocationTypeName(DataRefImpl Rel,
- SmallVectorImpl &Result) const override{};
+ SmallVectorImpl &Result) const override {}
 };
 
 } // namespace object

diff  --git a/llvm/lib/Target/CSKY/CSKYAsmPrinter.h 
b/llvm/lib/Target/CSKY/CSKYAsmPrinter.h
index 5e87594e4fdf1..379189512405a 100644
--- a/llvm/lib/Target/CSKY/CSKYAsmPrinter.h
+++ b/llvm/lib/Target/CSKY/CSKYAsmPrinter.h
@@ -57,7 +57,7 @@ class LLVM_LIBRARY_VISIBILITY CSKYAsmPrinter : public 
AsmPrinter {
   bool runOnMachineFunction(MachineFunction &MF) override;
 
   // we emit cons

[Lldb-commits] [PATCH] D152331: [lldb][NFCI] Platforms should own their SDKBuild and SDKRootDirectory strings

2023-06-06 Thread Alex Langford via Phabricator via lldb-commits

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

These don't need to be ConstStrings. They don't really benefit much from
deduplication and comparing them isn't on a hot path, so they don't
really benefit much from quick comparisons.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152331

Files:
  lldb/include/lldb/Interpreter/OptionGroupPlatform.h
  lldb/include/lldb/Target/Platform.h
  lldb/source/Interpreter/OptionGroupPlatform.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
  lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
  lldb/source/Target/Platform.cpp

Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -210,11 +210,10 @@
 Status error(eErrorTypeGeneric);
 ModuleSpec resolved_spec;
 // Check if we have sysroot set.
-if (m_sdk_sysroot) {
+if (!m_sdk_sysroot.empty()) {
   // Prepend sysroot to module spec.
   resolved_spec = spec;
-  resolved_spec.GetFileSpec().PrependPathComponent(
-  m_sdk_sysroot.GetStringRef());
+  resolved_spec.GetFileSpec().PrependPathComponent(m_sdk_sysroot);
   // Try to get shared module with resolved spec.
   error = ModuleList::GetSharedModule(resolved_spec, module_sp,
   module_search_paths_ptr, old_modules,
@@ -312,9 +311,9 @@
 strm.Printf(" Connected: %s\n", is_connected ? "yes" : "no");
   }
 
-  if (GetSDKRootDirectory()) {
-strm.Format("   Sysroot: {0}\n", GetSDKRootDirectory());
-  }
+  if (const std::string &sdk_root = GetSDKRootDirectory(); !sdk_root.empty())
+strm.Format("   Sysroot: {0}\n", sdk_root);
+
   if (GetWorkingDirectory()) {
 strm.Printf("WorkingDir: %s\n", GetWorkingDirectory().GetPath().c_str());
   }
Index: lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
===
--- lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -191,8 +191,8 @@
   launch_info.SetArguments(args, true);
 
   Environment emulator_env = Host::GetEnvironment();
-  if (ConstString sysroot = GetSDKRootDirectory())
-emulator_env["QEMU_LD_PREFIX"] = sysroot.GetStringRef().str();
+  if (const std::string &sysroot = GetSDKRootDirectory(); !sysroot.empty())
+emulator_env["QEMU_LD_PREFIX"] = sysroot;
   for (const auto &KV : GetGlobalProperties().GetEmulatorEnvVars())
 emulator_env[KV.first()] = KV.second;
   launch_info.GetEnvironment() = ComputeLaunchEnvironment(
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
@@ -34,8 +34,8 @@
   std::lock_guard guard(m_sdk_dir_mutex);
   if (m_sdk_directory_infos.empty()) {
 // A --sysroot option was supplied - add it to our list of SDKs to check
-if (m_sdk_sysroot) {
-  FileSpec sdk_sysroot_fspec(m_sdk_sysroot.GetCString());
+if (!m_sdk_sysroot.empty()) {
+  FileSpec sdk_sysroot_fspec(m_sdk_sysroot.c_str());
   FileSystem::Instance().Resolve(sdk_sysroot_fspec);
   const SDKDirectoryInfo sdk_sysroot_directory_info(sdk_sysroot_fspec);
   m_sdk_directory_infos.push_back(sdk_sysroot_directory_info);
@@ -43,7 +43,7 @@
 LLDB_LOGF(log,
   "PlatformDarwinDevice::UpdateSDKDirectoryInfosIfNeeded added "
   "--sysroot SDK directory %s",
-  m_sdk_sysroot.GetCString());
+  m_sdk_sysroot.c_str());
   }
   return true;
 }
@@ -152,20 +152,20 @@
 std::vector check_sdk_info(num_sdk_infos, true);
 
 // Prefer the user SDK build string.
-ConstString build = GetSDKBuild();
+std::string build = GetSDKBuild();
 
 // Fall back to the platform's build string.
-if (!build) {
-  if (std::optional os_build_str = GetOSBuildString()) {
-build = ConstString(*os_build_str);
-  }
+if (build.empty()) {
+  if (std::optional os_build_str = GetOSBuildString())
+build.assign(*os_build_str);
 }
 
 // If we have a build string, only check platforms for which the build
 // string matches.
-if (build) {
+if (!build.empty()) {
   for (i = 0; i < num_sdk_infos; ++i)
-check_sdk_info[i] = m_sdk_directory_infos[i].build == build;
+check_sdk_info[i] = m_sdk_directory_infos[i].build.GetStringRef() ==
+llvm::StringRef(build);
 }
 
 // If we are connected we can find the version of the OS the platform us
@@ -201,7 +201,7 @@

49 matches

Mail list logo