[Lldb-commits] [PATCH] D125218: [lldb][NFC] Make cmd a reference in GenerateOptionUsage

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe473e79cd194: [lldb][NFC] Make cmd a reference in 
GenerateOptionUsage (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125218

Files:
  lldb/include/lldb/Interpreter/Options.h
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/Options.cpp

Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -388,21 +388,15 @@
   return true;
 }
 
-void Options::GenerateOptionUsage(Stream &strm, CommandObject *cmd,
+void Options::GenerateOptionUsage(Stream &strm, CommandObject &cmd,
   uint32_t screen_width) {
-  const bool only_print_args = cmd->IsDashDashCommand();
+  const bool only_print_args = cmd.IsDashDashCommand();
 
   auto opt_defs = GetDefinitions();
   const uint32_t save_indent_level = strm.GetIndentLevel();
-  llvm::StringRef name;
-
+  llvm::StringRef name = cmd.GetCommandName();
   StreamString arguments_str;
-
-  if (cmd) {
-name = cmd->GetCommandName();
-cmd->GetFormattedCommandArguments(arguments_str);
-  } else
-name = "";
+  cmd.GetFormattedCommandArguments(arguments_str);
 
   const uint32_t num_options = NumCommandOptions();
   if (num_options == 0)
@@ -432,8 +426,7 @@
 
   // Different option sets may require different args.
   StreamString args_str;
-  if (cmd)
-cmd->GetFormattedCommandArguments(args_str, opt_set_mask);
+  cmd.GetFormattedCommandArguments(args_str, opt_set_mask);
 
   // First go through and print all options that take no arguments as a
   // single string. If a command has "-a" "-b" and "-c", this will show up
@@ -482,7 +475,7 @@
   }
 
   if (args_str.GetSize() > 0) {
-if (cmd->WantsRawCommandString() && !only_print_args)
+if (cmd.WantsRawCommandString() && !only_print_args)
   strm.Printf(" --");
 
 strm << " " << args_str.GetString();
@@ -492,7 +485,7 @@
 }
   }
 
-  if (cmd && (only_print_args || cmd->WantsRawCommandString()) &&
+  if ((only_print_args || cmd.WantsRawCommandString()) &&
   arguments_str.GetSize() > 0) {
 if (!only_print_args)
   strm.PutChar('\n');
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -132,7 +132,7 @@
   } else {
 // No error string, output the usage information into result
 options->GenerateOptionUsage(
-result.GetErrorStream(), this,
+result.GetErrorStream(), *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
   }
 }
@@ -326,7 +326,7 @@
   if (!found_word && search_options && GetOptions() != nullptr) {
 StreamString usage_help;
 GetOptions()->GenerateOptionUsage(
-usage_help, this,
+usage_help, *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
 if (!usage_help.Empty()) {
   llvm::StringRef usage_text = usage_help.GetString();
@@ -863,7 +863,7 @@
   Options *options = GetOptions();
   if (options != nullptr) {
 options->GenerateOptionUsage(
-output_strm, this,
+output_strm, *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
   }
   llvm::StringRef long_help = GetHelpLong();
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -3815,7 +3815,7 @@
 
 default:
   m_options.GenerateOptionUsage(
-  result.GetErrorStream(), this,
+  result.GetErrorStream(), *this,
   GetCommandInterpreter().GetDebugger().GetTerminalWidth());
   syntax_error = true;
   break;
Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -348,7 +348,7 @@
 "too many arguments; expected frame-index, saw '%s'.\n",
 command[0].c_str());
 m_options.GenerateOptionUsage(
-result.GetErrorStream(), this,
+result.GetErrorStream(), *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
 return false;
   }
Index: lldb/source/Commands/CommandObjectDisassemble.cpp
===
--- lldb/source/Commands/CommandObjectDi

[Lldb-commits] [lldb] e473e79 - [lldb][NFC] Make cmd a reference in GenerateOptionUsage

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-05-16T10:46:56Z
New Revision: e473e79cd194d7a74158ff7e8b97076a7f71c511

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

LOG: [lldb][NFC] Make cmd a reference in GenerateOptionUsage

Nowhere in lldb do we call this with a null pointer.
If we did, the first line of the function would fault anyway.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/Options.h
lldb/source/Commands/CommandObjectDisassemble.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/source/Interpreter/Options.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/Options.h 
b/lldb/include/lldb/Interpreter/Options.h
index 5899a9edc47f9..a952d5f78df9e 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -86,7 +86,7 @@ class Options {
 const OptionDefinition &option_def,
 uint32_t output_max_columns);
 
-  void GenerateOptionUsage(Stream &strm, CommandObject *cmd,
+  void GenerateOptionUsage(Stream &strm, CommandObject &cmd,
uint32_t screen_width);
 
   bool SupportsLongOption(const char *long_option);

diff  --git a/lldb/source/Commands/CommandObjectDisassemble.cpp 
b/lldb/source/Commands/CommandObjectDisassemble.cpp
index b02e071e7ac4f..9d081c83c0fb7 100644
--- a/lldb/source/Commands/CommandObjectDisassemble.cpp
+++ b/lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -474,7 +474,7 @@ bool CommandObjectDisassemble::DoExecute(Args &command,
 "\"disassemble\" arguments are specified as options.\n");
 const int terminal_width =
 GetCommandInterpreter().GetDebugger().GetTerminalWidth();
-GetOptions()->GenerateOptionUsage(result.GetErrorStream(), this,
+GetOptions()->GenerateOptionUsage(result.GetErrorStream(), *this,
   terminal_width);
 return false;
   }

diff  --git a/lldb/source/Commands/CommandObjectFrame.cpp 
b/lldb/source/Commands/CommandObjectFrame.cpp
index 2a1cef7264ea7..0e6ddfa156763 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -348,7 +348,7 @@ class CommandObjectFrameSelect : public CommandObjectParsed 
{
 "too many arguments; expected frame-index, saw '%s'.\n",
 command[0].c_str());
 m_options.GenerateOptionUsage(
-result.GetErrorStream(), this,
+result.GetErrorStream(), *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
 return false;
   }

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index c3376d04854e1..f42588b673a46 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3815,7 +3815,7 @@ class CommandObjectTargetModulesLookup : public 
CommandObjectParsed {
 
 default:
   m_options.GenerateOptionUsage(
-  result.GetErrorStream(), this,
+  result.GetErrorStream(), *this,
   GetCommandInterpreter().GetDebugger().GetTerminalWidth());
   syntax_error = true;
   break;

diff  --git a/lldb/source/Interpreter/CommandObject.cpp 
b/lldb/source/Interpreter/CommandObject.cpp
index dcae27ff54790..b38bf86dbb870 100644
--- a/lldb/source/Interpreter/CommandObject.cpp
+++ b/lldb/source/Interpreter/CommandObject.cpp
@@ -132,7 +132,7 @@ bool CommandObject::ParseOptions(Args &args, 
CommandReturnObject &result) {
   } else {
 // No error string, output the usage information into result
 options->GenerateOptionUsage(
-result.GetErrorStream(), this,
+result.GetErrorStream(), *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
   }
 }
@@ -326,7 +326,7 @@ bool CommandObject::HelpTextContainsWord(llvm::StringRef 
search_word,
   if (!found_word && search_options && GetOptions() != nullptr) {
 StreamString usage_help;
 GetOptions()->GenerateOptionUsage(
-usage_help, this,
+usage_help, *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
 if (!usage_help.Empty()) {
   llvm::StringRef usage_text = usage_help.GetString();
@@ -863,7 +863,7 @@ void CommandObject::GenerateHelpText(Stream &output_strm) {
   Options *options = GetOptions();
   if (options != nullptr) {
 options->GenerateOptionUsage(
-output_strm, this,
+output_strm, *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
   }
   llvm::String

[Lldb-commits] [PATCH] D125219: [lldb][NFC] Simplify GenerateOptionUsage

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4a94e3801dd7: [lldb][NFC] Simplify GenerateOptionUsage 
(authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125219

Files:
  lldb/source/Interpreter/Options.cpp


Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -390,8 +390,6 @@
 
 void Options::GenerateOptionUsage(Stream &strm, CommandObject &cmd,
   uint32_t screen_width) {
-  const bool only_print_args = cmd.IsDashDashCommand();
-
   auto opt_defs = GetDefinitions();
   const uint32_t save_indent_level = strm.GetIndentLevel();
   llvm::StringRef name = cmd.GetCommandName();
@@ -402,6 +400,7 @@
   if (num_options == 0)
 return;
 
+  const bool only_print_args = cmd.IsDashDashCommand();
   if (!only_print_args)
 strm.PutCString("\nCommand Options Usage:\n");
 
@@ -413,19 +412,16 @@
   //   [options-for-level-1]
   //   etc.
 
-  uint32_t num_option_sets = GetRequiredOptions().size();
-
   if (!only_print_args) {
+uint32_t num_option_sets = GetRequiredOptions().size();
 for (uint32_t opt_set = 0; opt_set < num_option_sets; ++opt_set) {
-  uint32_t opt_set_mask;
-
-  opt_set_mask = 1 << opt_set;
   if (opt_set > 0)
 strm.Printf("\n");
   strm.Indent(name);
 
   // Different option sets may require different args.
   StreamString args_str;
+  uint32_t opt_set_mask = 1 << opt_set;
   cmd.GetFormattedCommandArguments(args_str, opt_set_mask);
 
   // First go through and print all options that take no arguments as a
@@ -475,12 +471,9 @@
   }
 
   if (args_str.GetSize() > 0) {
-if (cmd.WantsRawCommandString() && !only_print_args)
+if (cmd.WantsRawCommandString())
   strm.Printf(" --");
-
 strm << " " << args_str.GetString();
-if (only_print_args)
-  break;
   }
 }
   }


Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -390,8 +390,6 @@
 
 void Options::GenerateOptionUsage(Stream &strm, CommandObject &cmd,
   uint32_t screen_width) {
-  const bool only_print_args = cmd.IsDashDashCommand();
-
   auto opt_defs = GetDefinitions();
   const uint32_t save_indent_level = strm.GetIndentLevel();
   llvm::StringRef name = cmd.GetCommandName();
@@ -402,6 +400,7 @@
   if (num_options == 0)
 return;
 
+  const bool only_print_args = cmd.IsDashDashCommand();
   if (!only_print_args)
 strm.PutCString("\nCommand Options Usage:\n");
 
@@ -413,19 +412,16 @@
   //   [options-for-level-1]
   //   etc.
 
-  uint32_t num_option_sets = GetRequiredOptions().size();
-
   if (!only_print_args) {
+uint32_t num_option_sets = GetRequiredOptions().size();
 for (uint32_t opt_set = 0; opt_set < num_option_sets; ++opt_set) {
-  uint32_t opt_set_mask;
-
-  opt_set_mask = 1 << opt_set;
   if (opt_set > 0)
 strm.Printf("\n");
   strm.Indent(name);
 
   // Different option sets may require different args.
   StreamString args_str;
+  uint32_t opt_set_mask = 1 << opt_set;
   cmd.GetFormattedCommandArguments(args_str, opt_set_mask);
 
   // First go through and print all options that take no arguments as a
@@ -475,12 +471,9 @@
   }
 
   if (args_str.GetSize() > 0) {
-if (cmd.WantsRawCommandString() && !only_print_args)
+if (cmd.WantsRawCommandString())
   strm.Printf(" --");
-
 strm << " " << args_str.GetString();
-if (only_print_args)
-  break;
   }
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 4a94e38 - [lldb][NFC] Simplify GenerateOptionUsage

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-05-16T10:57:34Z
New Revision: 4a94e3801dd721c0083c1259d019a1540388d9d3

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

LOG: [lldb][NFC] Simplify GenerateOptionUsage

Once we get into the if block we know the value of only_print_args.
Move some variables closer to point of use.

Depends on D125218

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/Interpreter/Options.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/Options.cpp 
b/lldb/source/Interpreter/Options.cpp
index 5957f46f3442..1278f2ad5d7b 100644
--- a/lldb/source/Interpreter/Options.cpp
+++ b/lldb/source/Interpreter/Options.cpp
@@ -390,8 +390,6 @@ static bool PrintOption(const OptionDefinition &opt_def,
 
 void Options::GenerateOptionUsage(Stream &strm, CommandObject &cmd,
   uint32_t screen_width) {
-  const bool only_print_args = cmd.IsDashDashCommand();
-
   auto opt_defs = GetDefinitions();
   const uint32_t save_indent_level = strm.GetIndentLevel();
   llvm::StringRef name = cmd.GetCommandName();
@@ -402,6 +400,7 @@ void Options::GenerateOptionUsage(Stream &strm, 
CommandObject &cmd,
   if (num_options == 0)
 return;
 
+  const bool only_print_args = cmd.IsDashDashCommand();
   if (!only_print_args)
 strm.PutCString("\nCommand Options Usage:\n");
 
@@ -413,19 +412,16 @@ void Options::GenerateOptionUsage(Stream &strm, 
CommandObject &cmd,
   //   [options-for-level-1]
   //   etc.
 
-  uint32_t num_option_sets = GetRequiredOptions().size();
-
   if (!only_print_args) {
+uint32_t num_option_sets = GetRequiredOptions().size();
 for (uint32_t opt_set = 0; opt_set < num_option_sets; ++opt_set) {
-  uint32_t opt_set_mask;
-
-  opt_set_mask = 1 << opt_set;
   if (opt_set > 0)
 strm.Printf("\n");
   strm.Indent(name);
 
   // Different option sets may require 
diff erent args.
   StreamString args_str;
+  uint32_t opt_set_mask = 1 << opt_set;
   cmd.GetFormattedCommandArguments(args_str, opt_set_mask);
 
   // First go through and print all options that take no arguments as a
@@ -475,12 +471,9 @@ void Options::GenerateOptionUsage(Stream &strm, 
CommandObject &cmd,
   }
 
   if (args_str.GetSize() > 0) {
-if (cmd.WantsRawCommandString() && !only_print_args)
+if (cmd.WantsRawCommandString())
   strm.Printf(" --");
-
 strm << " " << args_str.GetString();
-if (only_print_args)
-  break;
   }
 }
   }



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

[Lldb-commits] [PATCH] D115277: [lldb] Workaround for type-like entities defined in a lexical block

[Kristina Bessonova via Phabricator via lldb-commits]

krisb abandoned this revision.
krisb added a comment.
Herald added a project: All.

Abandon for now.
Issue on gitlab: https://github.com/llvm/llvm-project/issues/54475


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115277

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

[Lldb-commits] [PATCH] D125616: [NFC] two small perf fixes for when using a DebugSymbols DBGShellCommands to find a dSYM

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp:155
+  // add the symbol ObjectFile to this Module.
+  if (dsym_objfile_sp.get() && !module_sp->GetSymbolFileFileSpec()) {
+module_sp->SetSymbolFileFileSpec(dsym_fspec);





Comment at: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp:154-155
+  // Check to see if we have the file on the local filesystem.
+  if (!module_spec.GetFileSpec().GetPath().empty() &&
+  FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+ModuleSpec exe_spec;

I assume an empty file spec wouldn't exist. Can this be simplified?



Comment at: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp:165-169
+  if (log) {
+LLDB_LOGF(log, "using original binary filepath %s for UUID %s",
+  module_spec.GetFileSpec().GetPath().c_str(),
+  uuid->GetAsString().c_str());
+  }

The macro checks if log is NULL.



Comment at: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp:185-191
+  if (log) {
+LLDB_LOGF(log,
+  "using binary from shared cache for filepath %s for "
+  "UUID %s",
+  module_spec.GetFileSpec().GetPath().c_str(),
+  uuid->GetAsString().c_str());
+  }





Comment at: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp:203-206
   if (log) {
 LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
   path, uuid->GetAsString().c_str());
   }




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125616

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

[Lldb-commits] [PATCH] D124499: Rename conflict testcase

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan abandoned this revision.
yinghuitan added a comment.

Seems that this is a duplication to Pavel Labath's commit 
f513b5fc47df5c040a257ab6f4643942828fa610 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124499

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

[Lldb-commits] [PATCH] D125616: [NFC] two small perf fixes for when using a DebugSymbols DBGShellCommands to find a dSYM

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp:154-155
+  // Check to see if we have the file on the local filesystem.
+  if (!module_spec.GetFileSpec().GetPath().empty() &&
+  FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+ModuleSpec exe_spec;

JDevlieghere wrote:
> I assume an empty file spec wouldn't exist. Can this be simplified?
I was worried that FileSystem::Exists might return true for an empty string 
filepath; it looks like this devolves down to a `access()` call, it's probably 
fine.



Comment at: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp:165-169
+  if (log) {
+LLDB_LOGF(log, "using original binary filepath %s for UUID %s",
+  module_spec.GetFileSpec().GetPath().c_str(),
+  uuid->GetAsString().c_str());
+  }

JDevlieghere wrote:
> The macro checks if log is NULL.
Ah good catch.  I was copy & pasting from nearby code that did the same, I'll 
fix both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125616

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

[Lldb-commits] [PATCH] D125616: [NFC] two small perf fixes for when using a DebugSymbols DBGShellCommands to find a dSYM

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda updated this revision to Diff 429782.
jasonmolenda added a comment.

Update to address Jonas' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125616

Files:
  lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -18,9 +18,11 @@
 #include "Host/macosx/cfcpp/CFCData.h"
 #include "Host/macosx/cfcpp/CFCReleaser.h"
 #include "Host/macosx/cfcpp/CFCString.h"
+#include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/Host.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataBuffer.h"
@@ -108,12 +110,10 @@
 if (dsym_url.get()) {
   if (::CFURLGetFileSystemRepresentation(
   dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
-if (log) {
-  LLDB_LOGF(log,
-"DebugSymbols framework returned dSYM path of %s for "
-"UUID %s -- looking for the dSYM",
-path, uuid->GetAsString().c_str());
-}
+LLDB_LOGF(log,
+  "DebugSymbols framework returned dSYM path of %s for "
+  "UUID %s -- looking for the dSYM",
+  path, uuid->GetAsString().c_str());
 FileSpec dsym_filespec(path);
 if (path[0] == '~')
   FileSystem::Instance().Resolve(dsym_filespec);
@@ -147,16 +147,54 @@
 uuid_dict = static_cast(
 ::CFDictionaryGetValue(dict.get(), uuid_cfstr.get()));
   }
-  if (uuid_dict) {
+
+  // Check to see if we have the file on the local filesystem.
+  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+ModuleSpec exe_spec;
+exe_spec.GetFileSpec() = module_spec.GetFileSpec();
+exe_spec.GetUUID() = module_spec.GetUUID();
+ModuleSP module_sp;
+module_sp.reset(new Module(exe_spec));
+if (module_sp && module_sp->GetObjectFile() &&
+module_sp->MatchesModuleSpec(exe_spec)) {
+  success = true;
+  return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
+  LLDB_LOGF(log, "using original binary filepath %s for UUID %s",
+module_spec.GetFileSpec().GetPath().c_str(),
+uuid->GetAsString().c_str());
+  ++items_found;
+}
+  }
+
+  // Check if the requested image is in our shared cache.
+  if (!success) {
+SharedCacheImageInfo image_info = HostInfo::GetSharedCacheImageInfo(
+module_spec.GetFileSpec().GetPath());
+
+// If we found it and it has the correct UUID, let's proceed with
+// creating a module from the memory contents.
+if (image_info.uuid && (!module_spec.GetUUID() ||
+module_spec.GetUUID() == image_info.uuid)) {
+  success = true;
+  return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
+  LLDB_LOGF(log,
+"using binary from shared cache for filepath %s for "
+"UUID %s",
+module_spec.GetFileSpec().GetPath().c_str(),
+uuid->GetAsString().c_str());
+  ++items_found;
+}
+  }
+
+  // Use the DBGSymbolRichExecutable filepath if present
+  if (!success && uuid_dict) {
 CFStringRef exec_cf_path =
 static_cast(::CFDictionaryGetValue(
 uuid_dict, CFSTR("DBGSymbolRichExecutable")));
 if (exec_cf_path && ::CFStringGetFileSystemRepresentation(
 exec_cf_path, path, sizeof(path))) {
-  if (log) {
-LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
-  path, uuid->GetAsString().c_str());
-  }
+  LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
+path, uuid->GetAsString().c_str());
   ++items_found;
   FileSpec exec_filespec(path);
   if (path[0] == '~')
@@ -168,20 +206,17 @@
 }
   }
 
+  // Look next to the dSYM for the binary file.
   if (!success) {
-// No dictionary, check near the dSYM bundle for an executable that
-// matches...
 if (::CFURLGetFileSystemRepresentation(
 

[Lldb-commits] [PATCH] D125616: [NFC] two small perf fixes for when using a DebugSymbols DBGShellCommands to find a dSYM

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

https://reviews.llvm.org/D125616

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

[Lldb-commits] [PATCH] D125716: [lldb] Prevent Overflow (Underflow) error in crashlog.py

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

Avoid a OverflowError when the pc is zero. This can happen for "unknown frames" 
where the crashlog generator reports a zero pc. We could omit them altogether, 
but if they're part of the crashlog it seems fair to display them in lldb as 
well.

rdar://9268


https://reviews.llvm.org/D125716

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -114,14 +114,12 @@
 for frame_idx, frame in enumerate(self.frames):
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) 
and frame_idx < options.disassemble_depth
-if frame_idx == 0:
-symbolicated_frame_addresses = crash_log.symbolicate(
-frame.pc & crash_log.addr_mask, options.verbose)
-else:
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry
-symbolicated_frame_addresses = crash_log.symbolicate(
-(frame.pc & crash_log.addr_mask) - 1, options.verbose)
+
+# Any frame above frame zero and we have to subtract one to
+# get the previous line entry.
+pc = frame.pc & crash_log.addr_mask
+pc = pc if frame_idx == 0 or pc == 0 else pc - 1
+symbolicated_frame_addresses = crash_log.symbolicate(pc, 
options.verbose)
 
 if symbolicated_frame_addresses:
 symbolicated_frame_address_idx = 0


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -114,14 +114,12 @@
 for frame_idx, frame in enumerate(self.frames):
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) and frame_idx < options.disassemble_depth
-if frame_idx == 0:
-symbolicated_frame_addresses = crash_log.symbolicate(
-frame.pc & crash_log.addr_mask, options.verbose)
-else:
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry
-symbolicated_frame_addresses = crash_log.symbolicate(
-(frame.pc & crash_log.addr_mask) - 1, options.verbose)
+
+# Any frame above frame zero and we have to subtract one to
+# get the previous line entry.
+pc = frame.pc & crash_log.addr_mask
+pc = pc if frame_idx == 0 or pc == 0 else pc - 1
+symbolicated_frame_addresses = crash_log.symbolicate(pc, options.verbose)
 
 if symbolicated_frame_addresses:
 symbolicated_frame_address_idx = 0
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D125716: [lldb] Prevent Overflow (Underflow) error in crashlog.py

[Med Ismail Bennani via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D125716

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

[Lldb-commits] [PATCH] D125716: [lldb] Prevent Overflow (Underflow) error in crashlog.py

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9defb3b4b4a3: [lldb] Prevent underflow in crashlog.py 
(authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125716

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -114,14 +114,12 @@
 for frame_idx, frame in enumerate(self.frames):
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) 
and frame_idx < options.disassemble_depth
-if frame_idx == 0:
-symbolicated_frame_addresses = crash_log.symbolicate(
-frame.pc & crash_log.addr_mask, options.verbose)
-else:
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry
-symbolicated_frame_addresses = crash_log.symbolicate(
-(frame.pc & crash_log.addr_mask) - 1, options.verbose)
+
+# Any frame above frame zero and we have to subtract one to
+# get the previous line entry.
+pc = frame.pc & crash_log.addr_mask
+pc = pc if frame_idx == 0 or pc == 0 else pc - 1
+symbolicated_frame_addresses = crash_log.symbolicate(pc, 
options.verbose)
 
 if symbolicated_frame_addresses:
 symbolicated_frame_address_idx = 0


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -114,14 +114,12 @@
 for frame_idx, frame in enumerate(self.frames):
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) and frame_idx < options.disassemble_depth
-if frame_idx == 0:
-symbolicated_frame_addresses = crash_log.symbolicate(
-frame.pc & crash_log.addr_mask, options.verbose)
-else:
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry
-symbolicated_frame_addresses = crash_log.symbolicate(
-(frame.pc & crash_log.addr_mask) - 1, options.verbose)
+
+# Any frame above frame zero and we have to subtract one to
+# get the previous line entry.
+pc = frame.pc & crash_log.addr_mask
+pc = pc if frame_idx == 0 or pc == 0 else pc - 1
+symbolicated_frame_addresses = crash_log.symbolicate(pc, options.verbose)
 
 if symbolicated_frame_addresses:
 symbolicated_frame_address_idx = 0
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 9defb3b - [lldb] Prevent underflow in crashlog.py

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-05-16T15:00:36-07:00
New Revision: 9defb3b4b4a3ab5a95c449471aaa930cf63a7106

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

LOG: [lldb] Prevent underflow in crashlog.py

Avoid a OverflowError (an underflow really) when the pc is zero. This
can happen for "unknown frames" where the crashlog generator reports a
zero pc. We could omit them altogether, but if they're part of the
crashlog it seems fair to display them in lldb as well.

rdar://9268

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

Added: 


Modified: 
lldb/examples/python/crashlog.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 49c9a92497eb1..a20798ab11940 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -114,14 +114,12 @@ def dump_symbolicated(self, crash_log, options):
 for frame_idx, frame in enumerate(self.frames):
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) 
and frame_idx < options.disassemble_depth
-if frame_idx == 0:
-symbolicated_frame_addresses = crash_log.symbolicate(
-frame.pc & crash_log.addr_mask, options.verbose)
-else:
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry
-symbolicated_frame_addresses = crash_log.symbolicate(
-(frame.pc & crash_log.addr_mask) - 1, options.verbose)
+
+# Any frame above frame zero and we have to subtract one to
+# get the previous line entry.
+pc = frame.pc & crash_log.addr_mask
+pc = pc if frame_idx == 0 or pc == 0 else pc - 1
+symbolicated_frame_addresses = crash_log.symbolicate(pc, 
options.verbose)
 
 if symbolicated_frame_addresses:
 symbolicated_frame_address_idx = 0



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

[Lldb-commits] [lldb] d2f3b60 - [NFC] Don't bother with unstripped binary w/ dSYM, don't DebugSymbols twice

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2022-05-16T15:30:39-07:00
New Revision: d2f3b6020fbfa2dd56ebd03c942acda961c421e2

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

LOG: [NFC] Don't bother with unstripped binary w/ dSYM, don't DebugSymbols twice

This patch addresses two perf issues when we find a dSYM on macOS
after calling into the DebugSymbols framework.  First, when we have
a local (probably stripped) binaary, we find the dSYM and we may
be told about the location of the symbol rich binary (probably
unstripped) which may be on a remote filesystem.  We don't need the
unstripped binary, use the local binary we already have.
Second, after we've found the path to the dSYM, save that in the Module
so we don't call into DebugSymbols a second time later on to
rediscover it.  If the user has a DBGShellCommands set, we need to
exec that process twice, serially, which can add up.

Differential Revision: https://reviews.llvm.org/D125616
rdar://84576917

Added: 


Modified: 
lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp 
b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
index d9f4174b19a3..b99ade50d857 100644
--- a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -149,6 +149,12 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP 
&module_sp,
   ObjectFile::FindPlugin(module_sp, &dsym_fspec, 0,
  
FileSystem::Instance().GetByteSize(dsym_fspec),
  dsym_file_data_sp, dsym_file_data_offset);
+  // Important to save the dSYM FileSpec so we don't call
+  // Symbols::LocateExecutableSymbolFile a second time while trying to
+  // add the symbol ObjectFile to this Module.
+  if (dsym_objfile_sp && !module_sp->GetSymbolFileFileSpec()) {
+module_sp->SetSymbolFileFileSpec(dsym_fspec);
+  }
   if (UUIDsMatch(module_sp.get(), dsym_objfile_sp.get(), feedback_strm)) {
 // We need a XML parser if we hope to parse a plist...
 if (XMLDocument::XMLEnabled()) {

diff  --git a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp 
b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
index d905a8ed88e3..4dc42cf01716 100644
--- a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -18,9 +18,11 @@
 #include "Host/macosx/cfcpp/CFCData.h"
 #include "Host/macosx/cfcpp/CFCReleaser.h"
 #include "Host/macosx/cfcpp/CFCString.h"
+#include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/Host.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataBuffer.h"
@@ -108,12 +110,10 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec 
&module_spec,
 if (dsym_url.get()) {
   if (::CFURLGetFileSystemRepresentation(
   dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
-if (log) {
-  LLDB_LOGF(log,
-"DebugSymbols framework returned dSYM path of %s for "
-"UUID %s -- looking for the dSYM",
-path, uuid->GetAsString().c_str());
-}
+LLDB_LOGF(log,
+  "DebugSymbols framework returned dSYM path of %s for "
+  "UUID %s -- looking for the dSYM",
+  path, uuid->GetAsString().c_str());
 FileSpec dsym_filespec(path);
 if (path[0] == '~')
   FileSystem::Instance().Resolve(dsym_filespec);
@@ -147,16 +147,54 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec 
&module_spec,
 uuid_dict = static_cast(
 ::CFDictionaryGetValue(dict.get(), uuid_cfstr.get()));
   }
-  if (uuid_dict) {
+
+  // Check to see if we have the file on the local filesystem.
+  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+ModuleSpec exe_spec;
+exe_spec.GetFileSpec() = module_spec.GetFileSpec();
+exe_spec.GetUUID() = module_spec.GetUUID();
+ModuleSP module_sp;
+module_sp.reset(new Module(exe_spec));
+if (module_sp && module_sp->GetObjectFile() &&
+module_sp->MatchesModuleSpec(exe_spec)) {
+  success = true;
+  return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
+  LLDB_LOGF(log, "using original binary filepath %s for UUID %s",
+   

[Lldb-commits] [PATCH] D125616: [NFC] two small perf fixes for when using a DebugSymbols DBGShellCommands to find a dSYM

[Jason Molenda via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd2f3b6020fbf: [NFC] Don't bother with unstripped binary 
w/ dSYM, don't DebugSymbols twice (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125616

Files:
  lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -18,9 +18,11 @@
 #include "Host/macosx/cfcpp/CFCData.h"
 #include "Host/macosx/cfcpp/CFCReleaser.h"
 #include "Host/macosx/cfcpp/CFCString.h"
+#include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/Host.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataBuffer.h"
@@ -108,12 +110,10 @@
 if (dsym_url.get()) {
   if (::CFURLGetFileSystemRepresentation(
   dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
-if (log) {
-  LLDB_LOGF(log,
-"DebugSymbols framework returned dSYM path of %s for "
-"UUID %s -- looking for the dSYM",
-path, uuid->GetAsString().c_str());
-}
+LLDB_LOGF(log,
+  "DebugSymbols framework returned dSYM path of %s for "
+  "UUID %s -- looking for the dSYM",
+  path, uuid->GetAsString().c_str());
 FileSpec dsym_filespec(path);
 if (path[0] == '~')
   FileSystem::Instance().Resolve(dsym_filespec);
@@ -147,16 +147,54 @@
 uuid_dict = static_cast(
 ::CFDictionaryGetValue(dict.get(), uuid_cfstr.get()));
   }
-  if (uuid_dict) {
+
+  // Check to see if we have the file on the local filesystem.
+  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+ModuleSpec exe_spec;
+exe_spec.GetFileSpec() = module_spec.GetFileSpec();
+exe_spec.GetUUID() = module_spec.GetUUID();
+ModuleSP module_sp;
+module_sp.reset(new Module(exe_spec));
+if (module_sp && module_sp->GetObjectFile() &&
+module_sp->MatchesModuleSpec(exe_spec)) {
+  success = true;
+  return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
+  LLDB_LOGF(log, "using original binary filepath %s for UUID %s",
+module_spec.GetFileSpec().GetPath().c_str(),
+uuid->GetAsString().c_str());
+  ++items_found;
+}
+  }
+
+  // Check if the requested image is in our shared cache.
+  if (!success) {
+SharedCacheImageInfo image_info = HostInfo::GetSharedCacheImageInfo(
+module_spec.GetFileSpec().GetPath());
+
+// If we found it and it has the correct UUID, let's proceed with
+// creating a module from the memory contents.
+if (image_info.uuid && (!module_spec.GetUUID() ||
+module_spec.GetUUID() == image_info.uuid)) {
+  success = true;
+  return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
+  LLDB_LOGF(log,
+"using binary from shared cache for filepath %s for "
+"UUID %s",
+module_spec.GetFileSpec().GetPath().c_str(),
+uuid->GetAsString().c_str());
+  ++items_found;
+}
+  }
+
+  // Use the DBGSymbolRichExecutable filepath if present
+  if (!success && uuid_dict) {
 CFStringRef exec_cf_path =
 static_cast(::CFDictionaryGetValue(
 uuid_dict, CFSTR("DBGSymbolRichExecutable")));
 if (exec_cf_path && ::CFStringGetFileSystemRepresentation(
 exec_cf_path, path, sizeof(path))) {
-  if (log) {
-LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
-  path, uuid->GetAsString().c_str());
-  }
+  LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
+path, uuid->GetAsString().c_str());
   ++items_found;
   FileSpec exec_filespec(path);
   if (path[0] == '~')
@@ -168,20 +206,17 @@
 }
   }
 
+  // Look next to the dSYM for the binary file.
   if (!success) {
-// No dictionary, check near the dSYM bundle for an executable that