date:20220411

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

2022-04-11 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D123020#3437246 , @llunak wrote:

> In D123020#3426867 , @JDevlieghere 
> wrote:
>
>> FWIW the official policy is outlined here: 
>> https://llvm.org/docs/CodeReview.html
>
> I'm aware of it, but as far as I can judge I was following it. Even reading 
> it now again I see nothing that I would understand as mandating review for 
> everything.

It does say "patches that meet likely-community-consensus requirements can be 
committed prior to an explicit review" and "where there is any uncertainty, a 
patch should be reviewed prior to being committed".
It can be hard to judge what is a likely-community-consensus without being an 
active member of the community, which is why it's safer to go down the 
pre-commit review path.

Also note that when I said that "all patches are expected to be reviewed", that 
included both pre-commit and post-commit review. I deliberately used passive 
voice because in the latter case, there's nothing for you (as the patch author) 
to do. It's generally up to the owners of individual components to ensure that 
all patches going in get reviewed by someone. Since there's no paper trail, 
this is very hard to verify, but I can tell you that people do that, and that 
it's not a good way to introduce yourself to someone.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D123128: don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

2022-04-11 Thread Pavel Labath via Phabricator via lldb-commits

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

Cool. Thanks.


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

https://reviews.llvm.org/D123128

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

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

2022-04-11 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I'm looking at the SymbolFileOnDemand friendship and the forwarding of 
protected methods (mostly dealing with compile unit lists). Does this by any 
chance have something to do with the fact that there are now two compile unit 
lists (one in the actual symbol file, and one in the wrapping ondemand class? 
Would it be possible to avoid that by making `SymbolFile` a stateless interface 
(just methods, no member variables), and having a putting the member variables 
and other utility functions into a separate class that the real symbol files 
would inherit from? Then only SymbolFileOnDemand would inherit from SymbolFile 
directly, and it would only add the state that it needs (the bool flag, and the 
actual symbol file pointer).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

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

[Lldb-commits] [PATCH] D123500: [lldb][NFC] Add more tests for GenerateOptionsUsage

2022-04-11 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This adds a few targeted tests to make sure that when refactoring
this function later I don't break these properties.

Some are tested in passing elsewhere but this makes it more
obvious what went wrong when it fails.

This doesn't cover everything the function does, I couldn't
find any examples that would exercise some of the code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123500

Files:
  lldb/test/API/commands/help/TestHelp.py


Index: lldb/test/API/commands/help/TestHelp.py
===
--- lldb/test/API/commands/help/TestHelp.py
+++ lldb/test/API/commands/help/TestHelp.py
@@ -243,3 +243,30 @@
 "-f  ( --format  )", "The format to use for 
each of the value to be written.",
 "-s  ( --size  )", "The size in bytes to 
write from input file or each value."])
 
+@no_debug_info_test
+def test_help_shows_optional_short_options(self):
+"""Test that optional short options are printed and that they are in
+   alphabetical order with upper case options first."""
+self.expect("help memory read",
+substrs=["memory read [-br]", "memory read [-AFLORTr]"])
+
+@no_debug_info_test
+def test_help_shows_command_options_usage(self):
+"""Test that we start the usage section with a specific line."""
+self.expect("help memory read", substrs=["Command Options Usage:\n  
memory read"])
+
+@no_debug_info_test
+def test_help_detailed_information_spacing(self):
+"""Test that we put a break between the usage and the options help 
lines,
+   and between the options themselves."""
+self.expect("help memory read", substrs=[
+"[]\n\n   --show-tags",
+# Starts with the end of the show-tags line
+"output).\n\n   -A"])
+
+@no_debug_info_test
+def test_help_detailed_information_ordering(self):
+"""Test that we order options alphabetically, upper case first."""
+self.expect("help memory read", patterns=[
+"(?s)\-L \( \-\-location \).*\-T \( \-\-show\-types \).*"
+"\-b \( \-\-binary \).*\-r \( \-\-force \)"])
\ No newline at end of file


Index: lldb/test/API/commands/help/TestHelp.py
===
--- lldb/test/API/commands/help/TestHelp.py
+++ lldb/test/API/commands/help/TestHelp.py
@@ -243,3 +243,30 @@
 "-f  ( --format  )", "The format to use for each of the value to be written.",
 "-s  ( --size  )", "The size in bytes to write from input file or each value."])
 
+@no_debug_info_test
+def test_help_shows_optional_short_options(self):
+"""Test that optional short options are printed and that they are in
+   alphabetical order with upper case options first."""
+self.expect("help memory read",
+substrs=["memory read [-br]", "memory read [-AFLORTr]"])
+
+@no_debug_info_test
+def test_help_shows_command_options_usage(self):
+"""Test that we start the usage section with a specific line."""
+self.expect("help memory read", substrs=["Command Options Usage:\n  memory read"])
+
+@no_debug_info_test
+def test_help_detailed_information_spacing(self):
+"""Test that we put a break between the usage and the options help lines,
+   and between the options themselves."""
+self.expect("help memory read", substrs=[
+"[]\n\n   --show-tags",
+# Starts with the end of the show-tags line
+"output).\n\n   -A"])
+
+@no_debug_info_test
+def test_help_detailed_information_ordering(self):
+"""Test that we order options alphabetically, upper case first."""
+self.expect("help memory read", patterns=[
+"(?s)\-L \( \-\-location \).*\-T \( \-\-show\-types \).*"
+"\-b \( \-\-binary \).*\-r \( \-\-force \)"])
\ No newline at end of file
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123501: [lldb][NFC] Simplify part of Options::GenerateOptionUsage

2022-04-11 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Use llvm::enumerate, remove an unused arg name stream and
replace repeated uses of indexing to get the option def.

We could use map instead of multimap but I'm not 100% that
would be NFC. All short options should be unique in theory.

Depends on D123500 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123501

Files:
  lldb/source/Interpreter/Options.cpp


Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/STLExtras.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -540,63 +541,49 @@
 //   -short  ( --long_name  )
 //   help text
 
-// This variable is used to keep track of which options' info we've printed
-// out, because some options can be in more than one usage level, but we
-// only want to print the long form of its information once.
-
-std::multimap options_seen;
 strm.IndentMore(5);
 
-// Put the unique command options in a vector & sort it, so we can output
-// them alphabetically (by short_option) when writing out detailed help for
-// each option.
-
-i = 0;
-for (auto &def : opt_defs)
-  options_seen.insert(std::make_pair(def.short_option, i++));
+// Put the command options in a sorted container, so we can output
+// them alphabetically by short_option.
+std::multimap options_ordered;
+for (auto def : llvm::enumerate(opt_defs))
+  options_ordered.insert(
+  std::make_pair(def.value().short_option, def.index()));
 
-// Go through the unique'd and alphabetically sorted vector of options,
-// find the table entry for each option and write out the detailed help
-// information for that option.
+// Go through each option, find the table entry and write out the detailed
+// help information for that option.
 
 bool first_option_printed = false;
 
-for (auto pos : options_seen) {
-  i = pos.second;
-  // Print out the help information for this option.
-
+for (auto pos : options_ordered) {
   // Put a newline separation between arguments
   if (first_option_printed)
 strm.EOL();
   else
 first_option_printed = true;
 
-  CommandArgumentType arg_type = opt_defs[i].argument_type;
-
-  StreamString arg_name_str;
-  arg_name_str.Printf("<%s>", CommandObject::GetArgumentName(arg_type));
+  OptionDefinition opt_def = opt_defs[pos.second];
 
   strm.Indent();
-  if (opt_defs[i].short_option && opt_defs[i].HasShortOption()) {
-PrintOption(opt_defs[i], eDisplayShortOption, nullptr, nullptr, false,
+  if (opt_def.short_option && opt_def.HasShortOption()) {
+PrintOption(opt_def, eDisplayShortOption, nullptr, nullptr, false,
 strm);
-PrintOption(opt_defs[i], eDisplayLongOption, " ( ", " )", false, strm);
+PrintOption(opt_def, eDisplayLongOption, " ( ", " )", false, strm);
   } else {
 // Short option is not printable, just print long option
-PrintOption(opt_defs[i], eDisplayLongOption, nullptr, nullptr, false,
-strm);
+PrintOption(opt_def, eDisplayLongOption, nullptr, nullptr, false, 
strm);
   }
   strm.EOL();
 
   strm.IndentMore(5);
 
-  if (opt_defs[i].usage_text)
-OutputFormattedUsageText(strm, opt_defs[i], screen_width);
-  if (!opt_defs[i].enum_values.empty()) {
+  if (opt_def.usage_text)
+OutputFormattedUsageText(strm, opt_def, screen_width);
+  if (!opt_def.enum_values.empty()) {
 strm.Indent();
 strm.Printf("Values: ");
 bool is_first = true;
-for (const auto &enum_value : opt_defs[i].enum_values) {
+for (const auto &enum_value : opt_def.enum_values) {
   if (is_first) {
 strm.Printf("%s", enum_value.string_value);
 is_first = false;


Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/STLExtras.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -540,63 +541,49 @@
 //   -short  ( --long_name  )
 //   help text
 
-// This variable is used to keep track of which options' info we've printed
-// out, because some options can be in more than one usage level,

[Lldb-commits] [PATCH] D123502: [lldb][NFC] Refactor printing of short options in help

2022-04-11 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Instead of building a set twice for optional and required,
build a set for each while walking the options once.

Then take advantage of set being sorted meaning we don't
have to enforce the upper/lower order ourselves.

Just cleaned up the formatting on the later loops.
Combined the if conditions and used a single line if.

Depends on D123501 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123502

Files:
  lldb/source/Interpreter/Options.cpp

Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -421,8 +421,6 @@
 
   uint32_t num_option_sets = GetRequiredOptions().size();
 
-  uint32_t i;
-
   if (!only_print_args) {
 for (uint32_t opt_set = 0; opt_set < num_option_sets; ++opt_set) {
   uint32_t opt_set_mask;
@@ -441,77 +439,46 @@
   // single string. If a command has "-a" "-b" and "-c", this will show up
   // as [-abc]
 
-  std::set options;
-  std::set::const_iterator options_pos, options_end;
-  for (auto &def : opt_defs) {
-if (def.usage_mask & opt_set_mask && def.HasShortOption()) {
-  // Add current option to the end of out_stream.
+  // We use a set here so that they will be sorted.
+  std::set required_options;
+  std::set optional_options;
 
-  if (def.required && def.option_has_arg == OptionParser::eNoArgument) {
-options.insert(def.short_option);
+  for (auto &def : opt_defs) {
+if (def.usage_mask & opt_set_mask && def.HasShortOption() &&
+def.option_has_arg == OptionParser::eNoArgument) {
+  if (def.required) {
+required_options.insert(def.short_option);
+  } else {
+optional_options.insert(def.short_option);
   }
 }
   }
 
-  if (!options.empty()) {
-// We have some required options with no arguments
+  if (required_options.size()) {
 strm.PutCString(" -");
-for (i = 0; i < 2; ++i)
-  for (options_pos = options.begin(), options_end = options.end();
-   options_pos != options_end; ++options_pos) {
-if (i == 0 && ::islower(*options_pos))
-  continue;
-if (i == 1 && ::isupper(*options_pos))
-  continue;
-strm << (char)*options_pos;
-  }
-  }
-
-  options.clear();
-  for (auto &def : opt_defs) {
-if (def.usage_mask & opt_set_mask && def.HasShortOption()) {
-  // Add current option to the end of out_stream.
-
-  if (!def.required &&
-  def.option_has_arg == OptionParser::eNoArgument) {
-options.insert(def.short_option);
-  }
-}
+for (int short_option : required_options)
+  strm.PutChar(short_option);
   }
 
-  if (!options.empty()) {
-// We have some required options with no arguments
+  if (optional_options.size()) {
 strm.PutCString(" [-");
-for (i = 0; i < 2; ++i)
-  for (options_pos = options.begin(), options_end = options.end();
-   options_pos != options_end; ++options_pos) {
-if (i == 0 && ::islower(*options_pos))
-  continue;
-if (i == 1 && ::isupper(*options_pos))
-  continue;
-strm << (char)*options_pos;
-  }
+for (int short_option : optional_options)
+  strm.PutChar(short_option);
 strm.PutChar(']');
   }
 
   // First go through and print the required options (list them up front).
-
   for (auto &def : opt_defs) {
-if (def.usage_mask & opt_set_mask && def.HasShortOption()) {
-  if (def.required && def.option_has_arg != OptionParser::eNoArgument)
-PrintOption(def, eDisplayBestOption, " ", nullptr, true, strm);
-}
+if (def.usage_mask & opt_set_mask && def.HasShortOption() &&
+def.required && def.option_has_arg != OptionParser::eNoArgument)
+  PrintOption(def, eDisplayBestOption, " ", nullptr, true, strm);
   }
 
   // Now go through again, and this time only print the optional options.
-
   for (auto &def : opt_defs) {
-if (def.usage_mask & opt_set_mask) {
-  // Add current option to the end of out_stream.
-
-  if (!def.required && def.option_has_arg != OptionParser::eNoArgument)
-PrintOption(def, eDisplayBestOption, " ", nullptr, true, strm);
-}
+if (def.usage_mask & opt_set_mask && !def.required &&
+def.option_has_arg != OptionParser::eNoArgument)
+  PrintOption(def, eDisplayBestOption, " ", nullptr, true, strm);
   }
 
   if (args_str.G

[Lldb-commits] [PATCH] D123500: [lldb][NFC] Add more tests for GenerateOptionsUsage

2022-04-11 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a reviewer: jingham.
DavidSpickett added a subscriber: jingham.
DavidSpickett added a comment.

@jingham You suggested I take a look at this function after I did a small fix a 
while back so adding you on review for this and the following changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123500

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

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

2022-04-11 Thread Luboš Luňák via Phabricator via lldb-commits

llunak added a comment.

In D123020#3442434 , @labath wrote:

> In D123020#3437246 , @llunak wrote:
>
>> In D123020#3426867 , @JDevlieghere 
>> wrote:
>>
>>> FWIW the official policy is outlined here: 
>>> https://llvm.org/docs/CodeReview.html
>>
>> I'm aware of it, but as far as I can judge I was following it. Even reading 
>> it now again I see nothing that I would understand as mandating review for 
>> everything.
>
> It does say "patches that meet likely-community-consensus requirements can be 
> committed prior to an explicit review" and "where there is any uncertainty, a 
> patch should be reviewed prior to being committed".
> It can be hard to judge what is a likely-community-consensus without being an 
> active member of the community, which is why it's safer to go down the 
> pre-commit review path.
>
> Also note that when I said that "all patches are expected to be reviewed", 
> that included both pre-commit and post-commit review. I deliberately used 
> passive voice because in the latter case, there's nothing for you (as the 
> patch author) to do. It's generally up to the owners of individual components 
> to ensure that all patches going in get reviewed by someone. Since there's no 
> paper trail, this is very hard to verify, but I can tell you that people do 
> that, and that it's not a good way to introduce yourself to someone.

Based on that 'introduce' comment I expect the part that you're not aware of is 
that all 4 of those simple commits I pushed directly changed code that had been 
written by me. So I still think I was following the guidelines, and I got an 
explicit review for all changes where I had any uncertainty, but as I said if 
it's expected that I'll get explicit review even for simple changes in code I'm 
familiar with, I can do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D123204: [lldb] Fix initialization of LazyBool/bool variables m_overwrite/m_overwrite_lazy. NFCI.

2022-04-11 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo added a comment.

Ping @jingham


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123204

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

[Lldb-commits] [lldb] 373d08a - [lldb] Silence warnings about unused static variables in RegisterInfos_arm64.h

2022-04-11 Thread Martin Storsjö via lldb-commits


Author: Martin Storsjö
Date: 2022-04-11T19:50:48+03:00
New Revision: 373d08adb4454d416bd2232525e5a6fbe45935ab

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

LOG: [lldb] Silence warnings about unused static variables in 
RegisterInfos_arm64.h

Move them to the only source file that included RegisterInfos_arm64.h
that actually used these variables.

This silences warnings like these:

In file included from 
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp:42:
lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:790:35: warning: 
‘g_register_infos_mte’ defined but not used [-Wunused-variable]
  790 | static lldb_private::RegisterInfo g_register_infos_mte[] = {
  |   ^~~~
lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:787:35: warning: 
‘g_register_infos_pauth’ defined but not used [-Wunused-variable]
  787 | static lldb_private::RegisterInfo g_register_infos_pauth[] = {
  |   ^~

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
index d6c4a8687ec5c..579ac6e36d0ba 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -72,6 +72,12 @@
 #include "RegisterInfos_arm64_sve.h"
 #undef DECLARE_REGISTER_INFOS_ARM64_STRUCT
 
+static lldb_private::RegisterInfo g_register_infos_pauth[] = {
+DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
+
+static lldb_private::RegisterInfo g_register_infos_mte[] = {
+DEFINE_EXTENSION_REG(mte_ctrl)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h 
b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
index ccfbd6afbefba..d647fcaa600ad 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -784,10 +784,5 @@ static lldb_private::RegisterInfo 
g_register_infos_arm64_le[] = {
 {DEFINE_DBG(wcr, 15)}
 };
 // clang-format on
-static lldb_private::RegisterInfo g_register_infos_pauth[] = {
-DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
-
-static lldb_private::RegisterInfo g_register_infos_mte[] = {
-DEFINE_EXTENSION_REG(mte_ctrl)};
 
 #endif // DECLARE_REGISTER_INFOS_ARM64_STRUCT



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

[Lldb-commits] [PATCH] D123206: [lldb] Silence warnings about unused static variables in RegisterInfos_arm64.h

2022-04-11 Thread Martin Storsjö via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG373d08adb445: [lldb] Silence warnings about unused static 
variables in RegisterInfos_arm64.h (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123206

Files:
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h


Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -784,10 +784,5 @@
 {DEFINE_DBG(wcr, 15)}
 };
 // clang-format on
-static lldb_private::RegisterInfo g_register_infos_pauth[] = {
-DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
-
-static lldb_private::RegisterInfo g_register_infos_mte[] = {
-DEFINE_EXTENSION_REG(mte_ctrl)};
 
 #endif // DECLARE_REGISTER_INFOS_ARM64_STRUCT
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -72,6 +72,12 @@
 #include "RegisterInfos_arm64_sve.h"
 #undef DECLARE_REGISTER_INFOS_ARM64_STRUCT
 
+static lldb_private::RegisterInfo g_register_infos_pauth[] = {
+DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
+
+static lldb_private::RegisterInfo g_register_infos_mte[] = {
+DEFINE_EXTENSION_REG(mte_ctrl)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,


Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -784,10 +784,5 @@
 {DEFINE_DBG(wcr, 15)}
 };
 // clang-format on
-static lldb_private::RegisterInfo g_register_infos_pauth[] = {
-DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
-
-static lldb_private::RegisterInfo g_register_infos_mte[] = {
-DEFINE_EXTENSION_REG(mte_ctrl)};
 
 #endif // DECLARE_REGISTER_INFOS_ARM64_STRUCT
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -72,6 +72,12 @@
 #include "RegisterInfos_arm64_sve.h"
 #undef DECLARE_REGISTER_INFOS_ARM64_STRUCT
 
+static lldb_private::RegisterInfo g_register_infos_pauth[] = {
+DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
+
+static lldb_private::RegisterInfo g_register_infos_mte[] = {
+DEFINE_EXTENSION_REG(mte_ctrl)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] cbcb3bc - [lldb] Don't report progress in the REPL

2022-04-11 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-04-11T13:06:40-07:00
New Revision: cbcb3bcee3efc8ea4e72bc36ae5cbaf946804b58

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

LOG: [lldb] Don't report progress in the REPL

Don't report progress events in the REPL. Most of the progress events
are debugger specific which are useful when you're debugging, but not so
much when you're waiting for the next line to be executed in the REPL.

This patch disables reporting of progress events when in REPL mode.

rdar://91502950

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

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/source/Core/Debugger.cpp
lldb/source/Expression/REPL.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 6b6ad22d23511..354230b7a88a3 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -301,6 +301,8 @@ class Debugger : public 
std::enable_shared_from_this,
 
   bool GetShowProgress() const;
 
+  bool SetShowProgress(bool show_progress);
+
   llvm::StringRef GetShowProgressAnsiPrefix() const;
 
   llvm::StringRef GetShowProgressAnsiSuffix() const;

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 37e54a9abefaa..a812848c7e8b1 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -385,6 +385,12 @@ bool Debugger::GetShowProgress() const {
   nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
 }
 
+bool Debugger::SetShowProgress(bool show_progress) {
+  const uint32_t idx = ePropertyShowProgress;
+  return m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx,
+  show_progress);
+}
+
 llvm::StringRef Debugger::GetShowProgressAnsiPrefix() const {
   const uint32_t idx = ePropertyShowProgressAnsiPrefix;
   return m_collection_sp->GetPropertyAtIndexAsString(nullptr, idx, "");

diff  --git a/lldb/source/Expression/REPL.cpp b/lldb/source/Expression/REPL.cpp
index be41c60ebb5ff..d7582af9b2eab 100644
--- a/lldb/source/Expression/REPL.cpp
+++ b/lldb/source/Expression/REPL.cpp
@@ -25,6 +25,7 @@ using namespace lldb_private;
 REPL::REPL(LLVMCastKind kind, Target &target) : m_target(target), m_kind(kind) 
{
   // Make sure all option values have sane defaults
   Debugger &debugger = m_target.GetDebugger();
+  debugger.SetShowProgress(false);
   auto exe_ctx = debugger.GetCommandInterpreter().GetExecutionContext();
   m_format_options.OptionParsingStarting(&exe_ctx);
   m_varobj_options.OptionParsingStarting(&exe_ctx);



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

[Lldb-commits] [PATCH] D123426: [lldb] Don't report progress in the REPL

2022-04-11 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcbcb3bcee3ef: [lldb] Don't report progress in the REPL 
(authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123426

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Expression/REPL.cpp


Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -25,6 +25,7 @@
 REPL::REPL(LLVMCastKind kind, Target &target) : m_target(target), m_kind(kind) 
{
   // Make sure all option values have sane defaults
   Debugger &debugger = m_target.GetDebugger();
+  debugger.SetShowProgress(false);
   auto exe_ctx = debugger.GetCommandInterpreter().GetExecutionContext();
   m_format_options.OptionParsingStarting(&exe_ctx);
   m_varobj_options.OptionParsingStarting(&exe_ctx);
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -385,6 +385,12 @@
   nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
 }
 
+bool Debugger::SetShowProgress(bool show_progress) {
+  const uint32_t idx = ePropertyShowProgress;
+  return m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx,
+  show_progress);
+}
+
 llvm::StringRef Debugger::GetShowProgressAnsiPrefix() const {
   const uint32_t idx = ePropertyShowProgressAnsiPrefix;
   return m_collection_sp->GetPropertyAtIndexAsString(nullptr, idx, "");
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -301,6 +301,8 @@
 
   bool GetShowProgress() const;
 
+  bool SetShowProgress(bool show_progress);
+
   llvm::StringRef GetShowProgressAnsiPrefix() const;
 
   llvm::StringRef GetShowProgressAnsiSuffix() const;


Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -25,6 +25,7 @@
 REPL::REPL(LLVMCastKind kind, Target &target) : m_target(target), m_kind(kind) {
   // Make sure all option values have sane defaults
   Debugger &debugger = m_target.GetDebugger();
+  debugger.SetShowProgress(false);
   auto exe_ctx = debugger.GetCommandInterpreter().GetExecutionContext();
   m_format_options.OptionParsingStarting(&exe_ctx);
   m_varobj_options.OptionParsingStarting(&exe_ctx);
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -385,6 +385,12 @@
   nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
 }
 
+bool Debugger::SetShowProgress(bool show_progress) {
+  const uint32_t idx = ePropertyShowProgress;
+  return m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx,
+  show_progress);
+}
+
 llvm::StringRef Debugger::GetShowProgressAnsiPrefix() const {
   const uint32_t idx = ePropertyShowProgressAnsiPrefix;
   return m_collection_sp->GetPropertyAtIndexAsString(nullptr, idx, "");
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -301,6 +301,8 @@
 
   bool GetShowProgress() const;
 
+  bool SetShowProgress(bool show_progress);
+
   llvm::StringRef GetShowProgressAnsiPrefix() const;
 
   llvm::StringRef GetShowProgressAnsiSuffix() const;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 195a8b9 - don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

2022-04-11 Thread Luboš Luňák via lldb-commits


Author: Luboš Luňák
Date: 2022-04-12T00:36:33+02:00
New Revision: 195a8b977efe7a087709692f98821cdab1826346

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

LOG: don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

Places calling LoadModuleAtAddress() already call ModulesDidLoad()
after a loop calling LoadModuleAtAddress(), so it's not necessary
to call it from there, and the batched ModulesDidLoad() may be
more efficient than this place calling it one after one.

This also makes the ModuleLoadedNotifys test pass on Linux now that
the duplicates no longer bring down the average of modules notified
per call.

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

Added: 


Modified: 
lldb/include/lldb/Target/DynamicLoader.h
lldb/source/Core/DynamicLoader.cpp

lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py

Removed: 




diff  --git a/lldb/include/lldb/Target/DynamicLoader.h 
b/lldb/include/lldb/Target/DynamicLoader.h
index 4f07045f86457..397b4aa303448 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -203,6 +203,8 @@ class DynamicLoader : public PluginInterface {
 
   /// Locates or creates a module given by \p file and updates/loads the
   /// resulting module at the virtual base address \p base_addr.
+  /// Note that this calls Target::GetOrCreateModule with notify being false,
+  /// so it is necessary to call Target::ModulesDidLoad afterwards.
   virtual lldb::ModuleSP LoadModuleAtAddress(const lldb_private::FileSpec 
&file,
  lldb::addr_t link_map_addr,
  lldb::addr_t base_addr,

diff  --git a/lldb/source/Core/DynamicLoader.cpp 
b/lldb/source/Core/DynamicLoader.cpp
index 36443228ecd1d..96e0d4ec6555d 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -152,8 +152,7 @@ ModuleSP DynamicLoader::FindModuleViaTarget(const FileSpec 
&file) {
   if (ModuleSP module_sp = target.GetImages().FindFirstModule(module_spec))
 return module_sp;
 
-  if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
-/*notify=*/true))
+  if (ModuleSP module_sp = target.GetOrCreateModule(module_spec, false))
 return module_sp;
 
   return nullptr;

diff  --git 
a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
 
b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
index fc8596db54606..925fbfff22baa 100644
--- 
a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
+++ 
b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
@@ -16,10 +16,10 @@ class ModuleLoadedNotifysTestCase(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
-# DynamicLoaderDarwin should batch up notifications about
-# newly added/removed libraries.  Other DynamicLoaders may
+# At least DynamicLoaderDarwin and DynamicLoaderPOSIXDYLD should batch up
+# notifications about newly added/removed libraries.  Other DynamicLoaders 
may
 # not be written this way.
-@skipUnlessDarwin
+@skipUnlessPlatform(["linux"]+lldbplatformutil.getDarwinOSTriples())
 
 def setUp(self):
 # Call super's setUp().
@@ -72,20 +72,24 @@ def test_launch_notifications(self):
 total_solibs_removed = 0
 total_modules_added_events = 0
 total_modules_removed_events = 0
+already_loaded_modules = []
 while listener.GetNextEvent(event):
 if lldb.SBTarget.EventIsTargetEvent(event):
 if event.GetType() == lldb.SBTarget.eBroadcastBitModulesLoaded:
 solib_count = lldb.SBTarget.GetNumModulesFromEvent(event)
 total_modules_added_events += 1
 total_solibs_added += solib_count
+added_files = []
+i = 0
+while i < solib_count:
+module = lldb.SBTarget.GetModuleAtIndexFromEvent(i, 
event)
+self.assertTrue(module not in already_loaded_modules)
+already_loaded_modules.append(module)
+if self.TraceOn():
+
added_files.append(module.GetFileSpec().GetFilename())
+i = i + 1
 if self.TraceOn():
 # print all of the binaries that have been added
-added_files = []
-i = 0
-while i < solib_count:
-

[Lldb-commits] [PATCH] D123128: don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

2022-04-11 Thread Luboš Luňák via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG195a8b977efe: don't extra notify ModulesDidLoad() from 
LoadModuleAtAddress() (authored by llunak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123128

Files:
  lldb/include/lldb/Target/DynamicLoader.h
  lldb/source/Core/DynamicLoader.cpp
  
lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py


Index: 
lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
===
--- 
lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
+++ 
lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py
@@ -16,10 +16,10 @@
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
-# DynamicLoaderDarwin should batch up notifications about
-# newly added/removed libraries.  Other DynamicLoaders may
+# At least DynamicLoaderDarwin and DynamicLoaderPOSIXDYLD should batch up
+# notifications about newly added/removed libraries.  Other DynamicLoaders 
may
 # not be written this way.
-@skipUnlessDarwin
+@skipUnlessPlatform(["linux"]+lldbplatformutil.getDarwinOSTriples())
 
 def setUp(self):
 # Call super's setUp().
@@ -72,20 +72,24 @@
 total_solibs_removed = 0
 total_modules_added_events = 0
 total_modules_removed_events = 0
+already_loaded_modules = []
 while listener.GetNextEvent(event):
 if lldb.SBTarget.EventIsTargetEvent(event):
 if event.GetType() == lldb.SBTarget.eBroadcastBitModulesLoaded:
 solib_count = lldb.SBTarget.GetNumModulesFromEvent(event)
 total_modules_added_events += 1
 total_solibs_added += solib_count
+added_files = []
+i = 0
+while i < solib_count:
+module = lldb.SBTarget.GetModuleAtIndexFromEvent(i, 
event)
+self.assertTrue(module not in already_loaded_modules)
+already_loaded_modules.append(module)
+if self.TraceOn():
+
added_files.append(module.GetFileSpec().GetFilename())
+i = i + 1
 if self.TraceOn():
 # print all of the binaries that have been added
-added_files = []
-i = 0
-while i < solib_count:
-module = 
lldb.SBTarget.GetModuleAtIndexFromEvent(i, event)
-
added_files.append(module.GetFileSpec().GetFilename())
-i = i + 1
 print("Loaded files: %s" % (', '.join(added_files)))
 
 if event.GetType() == 
lldb.SBTarget.eBroadcastBitModulesUnloaded:
@@ -107,6 +111,7 @@
 # binaries in batches.  Check that we got back more than 1 solib per 
event.  
 # In practice on Darwin today, we get back two events for a do-nothing 
c 
 # program: a.out and dyld, and then all the rest of the system 
libraries.
+# On Linux we get events for ld.so, [vdso], the binary and then all 
libraries.
 
-avg_solibs_added_per_event = int(float(total_solibs_added) / 
float(total_modules_added_events))
+avg_solibs_added_per_event = round(float(total_solibs_added) / 
float(total_modules_added_events))
 self.assertGreater(avg_solibs_added_per_event, 1)
Index: lldb/source/Core/DynamicLoader.cpp
===
--- lldb/source/Core/DynamicLoader.cpp
+++ lldb/source/Core/DynamicLoader.cpp
@@ -152,8 +152,7 @@
   if (ModuleSP module_sp = target.GetImages().FindFirstModule(module_spec))
 return module_sp;
 
-  if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
-/*notify=*/true))
+  if (ModuleSP module_sp = target.GetOrCreateModule(module_spec, false))
 return module_sp;
 
   return nullptr;
Index: lldb/include/lldb/Target/DynamicLoader.h
===
--- lldb/include/lldb/Target/DynamicLoader.h
+++ lldb/include/lldb/Target/DynamicLoader.h
@@ -203,6 +203,8 @@
 
   /// Locates or creates a module given by \p file and updates/loads the
   /// resulting module at the virtual base address \p base_addr.
+  /// Note that this calls Target::GetOrCreateModule with notify being false,
+  /// so it is necessary to call Target::ModulesDidLoad afterwards.
   virtual lldb::ModuleSP LoadModuleAtAddress(const lldb_private::FileSpec 
&file,
  lldb::addr_t link_map_addr,

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

2022-04-11 Thread jeffrey tan via Phabricator via lldb-commits

yinghuitan added a comment.

> Does this by any chance have something to do with the fact that there are now 
> two compile unit lists (one in the actual symbol file, and one in the 
> wrapping ondemand class?

Yes, that's the major reason. We also need make SymbolFileOnDemand friend in 
SymbolFile so that SymbolFileOnDemand can call/forward protected virtual 
methods of `SymbolFile` during overriding.

> Would it be possible to avoid that by making SymbolFile a stateless interface?

What part do you want to avoid? We could do that by creating a new 
`SymbolFileReal` class, but we still have to make these compile unit lists 
methods virtual in `SymbolFile` class so that, like, calling 
`SymbolFile::GetCompileUnitAtIndex()` can be overridden by 
`SymbolFileReal`(touching data fields) and `SymbolFileOnDemand` (forwarding to 
real impl), right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631

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

[Lldb-commits] [PATCH] D123375: [lldb][intelpt] Reduce trace memory usage by grouping instructions

2022-04-11 Thread walter erquinigo via Phabricator via lldb-commits

wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.

I did a first pass on this diff. I'm asking to refactor a bit the 
InstructionBlock classes so that they are smarter. Besides that, if you use IDs 
more ubiquitously and stop using instruction indices everywhere, everything 
becomes much simpler.




Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:112-135
+  /// \struct InstructionPointer
+  /// Structure that stores a 6 byte prefix a list of 2 byte suffixes for each
+  /// instruction that is appended to the trace, in order. Concatenating the
+  /// prefix and suffix for any instruction ID (which is the concatenation of
+  /// two indices) will give the instruction load address.
+  ///
+  /// Generally most consecutive instructions will be close. Unless you are

Let's rename it to InstructionsBlock or another similar name. An instruction 
pointer is actually another way to refer to an instruction address.
Let's also try to remove numbers from the comments, so that if we modify them, 
we don't need to update the comment. It's hard to update comments because the 
compiler will never complaint if the comment doesn't make sense anymore.

Let's also make the size of the suffix dynamic, so that we can easily 
experiment with it later. We can use templates asking for a suffix type that 
must be unsigned.

If we embed the type of the suffix in the template, then we can do the 
splitting inside this class instead of outside. If you use my proposal for the 
constructor, please move its implementation to the cpp file.

Also, as this struct is not just a simple bag of data, let's make it a proper 
class.

I renamed a few things to create a nicer API.

First of all, I'm renaming the Append method to TryAppend, which receives the 
full load address and returns true if the append could happen, and false 
otherwise.

Besides that, I'm asking for a new class called Instructions that contains all 
the instruction blocks and receives instruction ids, and then it's able to 
defer the job to the actual instruction block.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:173-201
   /// \return
   /// The load address of the instruction at the given index, or \a
   /// LLDB_INVALID_ADDRESS if it is an error.
-  lldb::addr_t GetInstructionLoadAddress(size_t insn_index) const;
+  lldb::addr_t GetInstructionLoadAddress(size_t insn_id) const;
 
   /// Get the \a lldb::TraceInstructionControlFlowType categories of the
   /// instruction.

all of these become simpler if you work directly with the id instead of the 
instruction index



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:260-263
   /// The size in bytes of each instruction.
   std::vector m_instruction_sizes;
   /// The libipt instruction class for each instruction.
   std::vector m_instruction_classes;

you can move these two to the new InstructionsBlock class



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:271
   /// first one. Otherwise, this map will be empty.
   std::map m_instruction_timestamps;
   /// This is the chronologically last TSC that has been added.

if you convert this from `instruction index -> TSC` to `instruction id -> TSC`, 
then everything becomes simpler



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:274-276
+  /// For each \a InstructionPointer block, this contains the net number of
+  /// instructions upto that block
+  std::vector m_ipblock_sizes;

i have the impression that you don't need this anymore



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:279
+  /// trace. It maps `instruction index -> error message`.
   llvm::DenseMap m_errors;
   /// The size in bytes of the raw buffer before decoding. It might be None if

you can also make it `instruction id -> error message`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123375

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

[Lldb-commits] [PATCH] D123358: [trace][intelpt] Remove code smell when printing the raw trace size

2022-04-11 Thread walter erquinigo via Phabricator via lldb-commits

wallace added a comment.

Well, not all theoretical trace plugins might have raw trace sizes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123358

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

[Lldb-commits] [PATCH] D123501: [lldb][NFC] Simplify part of Options::GenerateOptionUsage

2022-04-11 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/D123501/new/

https://reviews.llvm.org/D123501

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

[Lldb-commits] [PATCH] D123502: [lldb][NFC] Refactor printing of short options in help

2022-04-11 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Interpreter/Options.cpp:457
 
-  if (!options.empty()) {
-// We have some required options with no arguments
+  if (required_options.size()) {
 strm.PutCString(" -");

I'm surprised you change this from `!empty()` to `size()`. I personally think 
the former is more readable. Not an issue in practice, but I do believe the STL 
guarantees `::empty` to be `O(1)` while technically `::size` could be `O(n)`, 
though I think in practice they're both `O(1)`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123502

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

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

[Lldb-commits] [PATCH] D123128: don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

[Lldb-commits] [PATCH] D123500: [lldb][NFC] Add more tests for GenerateOptionsUsage

[Lldb-commits] [PATCH] D123501: [lldb][NFC] Simplify part of Options::GenerateOptionUsage

[Lldb-commits] [PATCH] D123502: [lldb][NFC] Refactor printing of short options in help

[Lldb-commits] [PATCH] D123500: [lldb][NFC] Add more tests for GenerateOptionsUsage

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

[Lldb-commits] [PATCH] D123204: [lldb] Fix initialization of LazyBool/bool variables m_overwrite/m_overwrite_lazy. NFCI.

[Lldb-commits] [lldb] 373d08a - [lldb] Silence warnings about unused static variables in RegisterInfos_arm64.h

[Lldb-commits] [PATCH] D123206: [lldb] Silence warnings about unused static variables in RegisterInfos_arm64.h

[Lldb-commits] [lldb] cbcb3bc - [lldb] Don't report progress in the REPL

[Lldb-commits] [PATCH] D123426: [lldb] Don't report progress in the REPL

[Lldb-commits] [lldb] 195a8b9 - don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

[Lldb-commits] [PATCH] D123128: don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

[Lldb-commits] [PATCH] D123375: [lldb][intelpt] Reduce trace memory usage by grouping instructions

[Lldb-commits] [PATCH] D123358: [trace][intelpt] Remove code smell when printing the raw trace size

[Lldb-commits] [PATCH] D123501: [lldb][NFC] Simplify part of Options::GenerateOptionUsage

[Lldb-commits] [PATCH] D123502: [lldb][NFC] Refactor printing of short options in help

20 matches

Site Navigation

Mail list logo

Footer information