[Lldb-commits] [PATCH] D126668: LLDB: Fix resolving nested template parameters

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat created this revision.
PatriosTheGreat added a reviewer: teemperor.
Herald added a reviewer: shafik.
Herald added a project: All.
PatriosTheGreat requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Right now LLDB ignores nested template type:

echo "
template 
struct A {};
int main() {

  A> s;

}
" > sample.cc

clang++ sample.cc -g -O0
lldb-15 a.out -o "breakpoint set -l 6 -f sample.cc" -o "run" -o "frame variable"

The result:
(A**>) s = {}

This issue was introduced in LLDB 12 due to this CL: 
https://reviews.llvm.org/D92425
Before LLDB 12 this type was resolving correctly:
(A** >) s = {}

I discussed this issue with Raphael in discord:
https://discord.com/channels/636084430946959380/636732809708306432/980825811714191371

Apparently in this case Clang emits A as a forward declaration:
0x05b4:   DW_TAG_base_type

  DW_AT_name  ("int")
  DW_AT_encoding  (DW_ATE_signed)
  DW_AT_byte_size (0x04)

0x05bb:   DW_TAG_structure_type

  DW_AT_calling_convention(DW_CC_pass_by_value)
  DW_AT_name  ("A >")
  DW_AT_byte_size (0x01)
  DW_AT_decl_file ("/home/teemperor/test/args.cpp")
  DW_AT_decl_line (2)

0x05c4: DW_TAG_template_type_parameter

  DW_AT_type(0x05ce "A")
  DW_AT_name("T")

0x05cd: NULL

0x05ce:   DW_TAG_structure_type

  DW_AT_name  ("A")
  DW_AT_declaration   (true)

0x05d3:   NULL

So for LLDB it looks the same as template with empty arguments.
Turning back the old logic is fixing this issue. Other tests from LLVM test 
suite on Linux seems to be green.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126668

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py


Index: lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
===
--- lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -47,7 +47,7 @@
 # Record types without a defining declaration are not complete.
 self.assertPointeeIncomplete("FwdClass *", "fwd_class")
 self.assertPointeeIncomplete("FwdClassTypedef *", "fwd_class_typedef")
-self.assertPointeeIncomplete("FwdTemplateClass<> *", 
"fwd_template_class")
+self.assertPointeeIncomplete("FwdTemplateClass *", 
"fwd_template_class")
 
 # A pointer type is complete even when it points to an incomplete type.
 fwd_class_ptr = self.expect_expr("fwd_class", result_type="FwdClass *")
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2050,6 +2050,8 @@
   break;
 }
   }
+  if (template_param_infos.args.empty() && template_param_infos.names.empty())
+return false;
   return template_param_infos.args.size() == template_param_infos.names.size();
 }
 


Index: lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
===
--- lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -47,7 +47,7 @@
 # Record types without a defining declaration are not complete.
 self.assertPointeeIncomplete("FwdClass *", "fwd_class")
 self.assertPointeeIncomplete("FwdClassTypedef *", "fwd_class_typedef")
-self.assertPointeeIncomplete("FwdTemplateClass<> *", "fwd_template_class")
+self.assertPointeeIncomplete("FwdTemplateClass *", "fwd_template_class")
 
 # A pointer type is complete even when it points to an incomplete type.
 fwd_class_ptr = self.expect_expr("fwd_class", result_type="FwdClass *")
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2050,6 +2050,8 @@
   break;
 }
   }
+  if (template_param_infos.args.empty() && template_param_infos.names.empty())
+return false;
   return template_param_infos.args.size() == template_param_infos.names.size();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Thanks Greg,
Can you or someone please take this commit to main branch since I don't have a 
commit permission?


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

https://reviews.llvm.org/D125325

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

[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Hi Greg,
Sorry for a delay had an issue with tests local run.
I fixed the previous feedback.


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

https://reviews.llvm.org/D125325

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

[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 430357.
PatriosTheGreat marked 5 inline comments as done.

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

https://reviews.llvm.org/D125325

Files:
  lldb/bindings/interface/SBProcess.i
  lldb/include/lldb/API/SBProcess.h
  lldb/source/API/SBProcess.cpp
  
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -21,6 +21,7 @@
 self.build()
 exe = self.getBuildArtifact("a.out")
 core = self.getBuildArtifact("core.dmp")
+core_sb = self.getBuildArtifact("core_sb.dmp")
 try:
 target = self.dbg.CreateTarget(exe)
 process = target.LaunchSimple(
@@ -43,6 +44,17 @@
 # save core and, kill process and verify corefile existence
 self.runCmd("process save-core --plugin-name=minidump --style=stack " + core)
 self.assertTrue(os.path.isfile(core))
+
+# validate savinig via SBProcess
+error = process.SaveCore(core_sb, "minidump", lldb.eSaveCoreStackOnly)
+self.assertTrue(error.Success())
+self.assertTrue(os.path.isfile(core_sb))
+
+error = process.SaveCore(core_sb, "minidump", lldb.eSaveCoreFull)
+self.assertTrue(error.Fail())
+error = process.SaveCore(core_sb, "minidump", lldb.eSaveCoreDirtyOnly)
+self.assertTrue(error.Fail())
+
 self.assertSuccess(process.Kill())
 
 # To verify, we'll launch with the mini dump
@@ -77,3 +89,5 @@
 self.assertTrue(self.dbg.DeleteTarget(target))
 if (os.path.isfile(core)):
 os.unlink(core)
+if (os.path.isfile(core_sb)):
+os.unlink(core_sb)
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1138,6 +1138,13 @@
 
 lldb::SBError SBProcess::SaveCore(const char *file_name) {
   LLDB_INSTRUMENT_VA(this, file_name);
+  return SaveCore(file_name, "", SaveCoreStyle::eSaveCoreFull);
+}
+
+lldb::SBError SBProcess::SaveCore(const char *file_name,
+  const char *flavor,
+  SaveCoreStyle core_style) {
+  LLDB_INSTRUMENT_VA(this, file_name, flavor, core_style);
 
   lldb::SBError error;
   ProcessSP process_sp(GetSP());
@@ -1155,8 +1162,9 @@
   }
 
   FileSpec core_file(file_name);
-  SaveCoreStyle core_style = SaveCoreStyle::eSaveCoreFull;
-  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style, "");
+  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
+flavor);
+
   return error;
 }
 
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -337,7 +337,21 @@
 
   bool IsInstrumentationRuntimePresent(InstrumentationRuntimeType type);
 
-  /// Save the state of the process in a core file (or mini dump on Windows).
+  /// Save the state of the process in a core file.
+  ///
+  /// \param[in] file_name - The name of the file to save the core file to.
+  ///
+  /// \param[in] flavor - Specify the flavor of a core file plug-in to save.
+  /// Currently supported flavors include "mach-o" and "minidump"
+  ///
+  /// \param[in] core_style - Specify the style of a core file to save.
+  lldb::SBError SaveCore(const char *file_name, const char *flavor,
+ SaveCoreStyle core_style);
+
+  /// Save the state of the process with the a flavor that matches the
+  /// current process' main executable (if supported).
+  ///
+  /// \param[in] file_name - The name of the file to save the core file to.
   lldb::SBError SaveCore(const char *file_name);
 
   /// Query the address load_addr and store the details of the memory
Index: lldb/bindings/interface/SBProcess.i
===
--- lldb/bindings/interface/SBProcess.i
+++ lldb/bindings/interface/SBProcess.i
@@ -397,6 +397,9 @@
 bool
 IsInstrumentationRuntimePresent(lldb::InstrumentationRuntimeType type);
 
+lldb::SBError
+SaveCore(const char *file_name, const char *flavor, lldb::SaveCoreStyle core_style);
+
 lldb::SBError
 SaveCore(const char *file_name);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 429160.

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

https://reviews.llvm.org/D125325

Files:
  lldb/bindings/interface/SBProcess.i
  lldb/include/lldb/API/SBProcess.h
  lldb/source/API/SBProcess.cpp
  
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py


Index: 
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- 
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ 
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -21,6 +21,7 @@
 self.build()
 exe = self.getBuildArtifact("a.out")
 core = self.getBuildArtifact("core.dmp")
+core_sb = self.getBuildArtifact("core_sb.dmp")
 try:
 target = self.dbg.CreateTarget(exe)
 process = target.LaunchSimple(
@@ -42,7 +43,10 @@
 
 # save core and, kill process and verify corefile existence
 self.runCmd("process save-core --plugin-name=minidump 
--style=stack " + core)
+# validate savinig via SBProcess
+lldb.SBProcess().SaveCore(core_sb, "minidump", "stack")
 self.assertTrue(os.path.isfile(core))
+self.assertTrue(os.path.isfile(core_sb))
 self.assertSuccess(process.Kill())
 
 # To verify, we'll launch with the mini dump
@@ -77,3 +81,5 @@
 self.assertTrue(self.dbg.DeleteTarget(target))
 if (os.path.isfile(core)):
 os.unlink(core)
+if (os.path.isfile(core_sb)):
+os.unlink(core_sb)
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1138,6 +1138,13 @@
 
 lldb::SBError SBProcess::SaveCore(const char *file_name) {
   LLDB_INSTRUMENT_VA(this, file_name);
+  return SaveCore(file_name, "", SaveCoreStyle::eSaveCoreFull);
+}
+
+lldb::SBError SBProcess::SaveCore(const char *file_name,
+  const char *flavor,
+  SaveCoreStyle core_style) {
+  LLDB_INSTRUMENT_VA(this, file_name, flavor);
 
   lldb::SBError error;
   ProcessSP process_sp(GetSP());
@@ -1155,8 +1162,8 @@
   }
 
   FileSpec core_file(file_name);
-  SaveCoreStyle core_style = SaveCoreStyle::eSaveCoreFull;
-  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style, "");
+  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
+flavor);
   return error;
 }
 
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -337,7 +337,22 @@
 
   bool IsInstrumentationRuntimePresent(InstrumentationRuntimeType type);
 
-  /// Save the state of the process in a core file (or mini dump on Windows).
+  /// Save the state of the process in a core file.
+  ///
+  /// \param[in] file_name - The name of the file to save the core file to.
+  ///
+  /// \param[in] flavor - Specify the flavor of a core file plug-in to save.
+  /// Currently supported flavors include "mach-o" and "minidump"
+  ///
+  /// \param[in] core_style - Specify the style of a core file to save.
+  /// "minidump" plug-in supports only "eSaveCoreStackOnly".
+  lldb::SBError SaveCore(const char *file_name, const char *flavor,
+ SaveCoreStyle core_style);
+
+  /// Save the state of the process with the a flavor that matches the
+  /// current process' main executable (if supported).
+  ///
+  /// \param[in] file_name - The name of the file to save the core file to.
   lldb::SBError SaveCore(const char *file_name);
 
   /// Query the address load_addr and store the details of the memory
Index: lldb/bindings/interface/SBProcess.i
===
--- lldb/bindings/interface/SBProcess.i
+++ lldb/bindings/interface/SBProcess.i
@@ -397,6 +397,9 @@
 bool
 IsInstrumentationRuntimePresent(lldb::InstrumentationRuntimeType type);
 
+lldb::SBError
+SaveCore(const char *file_name, const char *flavor, lldb::SaveCoreStyle 
core_style);
+
 lldb::SBError
 SaveCore(const char *file_name);
 


Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -21,6 +21,7 @@
 self.build()
 exe = self.getBuildArtifact("a.out")
 core = self.getBuildArtifact("core.dmp")
+core_sb = 

[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat marked 3 inline comments as done.
PatriosTheGreat added a comment.

Thanks for clarifications.
Fixed header doc and parameter name.


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

https://reviews.llvm.org/D125325

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

[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 428880.

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

https://reviews.llvm.org/D125325

Files:
  lldb/bindings/interface/SBProcess.i
  lldb/include/lldb/API/SBProcess.h
  lldb/source/API/SBProcess.cpp


Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1138,6 +1138,12 @@
 
 lldb::SBError SBProcess::SaveCore(const char *file_name) {
   LLDB_INSTRUMENT_VA(this, file_name);
+  return SaveCore(file_name, "");
+}
+
+lldb::SBError SBProcess::SaveCore(const char *file_name,
+  const char *flavor) {
+  LLDB_INSTRUMENT_VA(this, file_name, flavor);
 
   lldb::SBError error;
   ProcessSP process_sp(GetSP());
@@ -1156,7 +1162,8 @@
 
   FileSpec core_file(file_name);
   SaveCoreStyle core_style = SaveCoreStyle::eSaveCoreFull;
-  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style, "");
+  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
+flavor);
   return error;
 }
 
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -337,7 +337,18 @@
 
   bool IsInstrumentationRuntimePresent(InstrumentationRuntimeType type);
 
-  /// Save the state of the process in a core file (or mini dump on Windows).
+  /// Save the state of the process in a core file.
+  ///
+  /// \param[in] file_name - The name of the file to save the core file to.
+  ///
+  /// \param[in] flavor - Specify the flavor of a core file plug-in to save.
+  /// Currently supported flavors include "mach-o" and "minidump"
+  lldb::SBError SaveCore(const char *file_name, const char *flavor);
+
+  /// Save the state of the process with the a flavor that matches the
+  /// current process' main executable (if supported).
+  ///
+  /// \param[in] file_name - The name of the file to save the core file to.
   lldb::SBError SaveCore(const char *file_name);
 
   /// Query the address load_addr and store the details of the memory
Index: lldb/bindings/interface/SBProcess.i
===
--- lldb/bindings/interface/SBProcess.i
+++ lldb/bindings/interface/SBProcess.i
@@ -397,6 +397,9 @@
 bool
 IsInstrumentationRuntimePresent(lldb::InstrumentationRuntimeType type);
 
+lldb::SBError
+SaveCore(const char *file_name, const char *flavor);
+
 lldb::SBError
 SaveCore(const char *file_name);
 


Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1138,6 +1138,12 @@
 
 lldb::SBError SBProcess::SaveCore(const char *file_name) {
   LLDB_INSTRUMENT_VA(this, file_name);
+  return SaveCore(file_name, "");
+}
+
+lldb::SBError SBProcess::SaveCore(const char *file_name,
+  const char *flavor) {
+  LLDB_INSTRUMENT_VA(this, file_name, flavor);
 
   lldb::SBError error;
   ProcessSP process_sp(GetSP());
@@ -1156,7 +1162,8 @@
 
   FileSpec core_file(file_name);
   SaveCoreStyle core_style = SaveCoreStyle::eSaveCoreFull;
-  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style, "");
+  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
+flavor);
   return error;
 }
 
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -337,7 +337,18 @@
 
   bool IsInstrumentationRuntimePresent(InstrumentationRuntimeType type);
 
-  /// Save the state of the process in a core file (or mini dump on Windows).
+  /// Save the state of the process in a core file.
+  ///
+  /// \param[in] file_name - The name of the file to save the core file to.
+  ///
+  /// \param[in] flavor - Specify the flavor of a core file plug-in to save.
+  /// Currently supported flavors include "mach-o" and "minidump"
+  lldb::SBError SaveCore(const char *file_name, const char *flavor);
+
+  /// Save the state of the process with the a flavor that matches the
+  /// current process' main executable (if supported).
+  ///
+  /// \param[in] file_name - The name of the file to save the core file to.
   lldb::SBError SaveCore(const char *file_name);
 
   /// Query the address load_addr and store the details of the memory
Index: lldb/bindings/interface/SBProcess.i
===
--- lldb/bindings/interface/SBProcess.i
+++ lldb/bindings/interface/SBProcess.i
@@ -397,6 +397,9 @@
 bool
 IsInstrumentationRuntimePresent(lldb::InstrumentationRuntimeType type);
 
+lldb::SBError
+SaveCore(const char 

[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat marked 3 inline comments as done.
PatriosTheGreat added a comment.

Hi Greg.

Thanks for the review.
I fixed the feedback.
I forgot the default initialization of plugin_name parameter in SBProcess.i in 
previous version, but I assume it's still better to explicitly create a 
separate method.


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

https://reviews.llvm.org/D125325

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

[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 428577.
PatriosTheGreat edited the summary of this revision.

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

https://reviews.llvm.org/D125325

Files:
  lldb/bindings/interface/SBProcess.i
  lldb/include/lldb/API/SBProcess.h
  lldb/source/API/SBProcess.cpp


Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1138,6 +1138,12 @@
 
 lldb::SBError SBProcess::SaveCore(const char *file_name) {
   LLDB_INSTRUMENT_VA(this, file_name);
+  return SaveCore(file_name, "");
+}
+
+lldb::SBError SBProcess::SaveCore(const char *file_name,
+  const char *plugin_name) {
+  LLDB_INSTRUMENT_VA(this, file_name, plugin_name);
 
   lldb::SBError error;
   ProcessSP process_sp(GetSP());
@@ -1156,7 +1162,8 @@
 
   FileSpec core_file(file_name);
   SaveCoreStyle core_style = SaveCoreStyle::eSaveCoreFull;
-  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style, "");
+  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
+plugin_name);
   return error;
 }
 
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -338,6 +338,9 @@
   bool IsInstrumentationRuntimePresent(InstrumentationRuntimeType type);
 
   /// Save the state of the process in a core file (or mini dump on Windows).
+  lldb::SBError SaveCore(const char *file_name, const char *plugin_name);
+
+  // Save the state of the process with the default plugin.
   lldb::SBError SaveCore(const char *file_name);
 
   /// Query the address load_addr and store the details of the memory
Index: lldb/bindings/interface/SBProcess.i
===
--- lldb/bindings/interface/SBProcess.i
+++ lldb/bindings/interface/SBProcess.i
@@ -397,6 +397,9 @@
 bool
 IsInstrumentationRuntimePresent(lldb::InstrumentationRuntimeType type);
 
+lldb::SBError
+SaveCore(const char *file_name, const char *plugin_name);
+
 lldb::SBError
 SaveCore(const char *file_name);
 


Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1138,6 +1138,12 @@
 
 lldb::SBError SBProcess::SaveCore(const char *file_name) {
   LLDB_INSTRUMENT_VA(this, file_name);
+  return SaveCore(file_name, "");
+}
+
+lldb::SBError SBProcess::SaveCore(const char *file_name,
+  const char *plugin_name) {
+  LLDB_INSTRUMENT_VA(this, file_name, plugin_name);
 
   lldb::SBError error;
   ProcessSP process_sp(GetSP());
@@ -1156,7 +1162,8 @@
 
   FileSpec core_file(file_name);
   SaveCoreStyle core_style = SaveCoreStyle::eSaveCoreFull;
-  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style, "");
+  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
+plugin_name);
   return error;
 }
 
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -338,6 +338,9 @@
   bool IsInstrumentationRuntimePresent(InstrumentationRuntimeType type);
 
   /// Save the state of the process in a core file (or mini dump on Windows).
+  lldb::SBError SaveCore(const char *file_name, const char *plugin_name);
+
+  // Save the state of the process with the default plugin.
   lldb::SBError SaveCore(const char *file_name);
 
   /// Query the address load_addr and store the details of the memory
Index: lldb/bindings/interface/SBProcess.i
===
--- lldb/bindings/interface/SBProcess.i
+++ lldb/bindings/interface/SBProcess.i
@@ -397,6 +397,9 @@
 bool
 IsInstrumentationRuntimePresent(lldb::InstrumentationRuntimeType type);
 
+lldb::SBError
+SaveCore(const char *file_name, const char *plugin_name);
+
 lldb::SBError
 SaveCore(const char *file_name);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat created this revision.
PatriosTheGreat added reviewers: clayborg, labath.
Herald added a project: All.
PatriosTheGreat requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This CL allows to use minidump save-core functionality 
(https://reviews.llvm.org/D108233) via SBProcess interface.
After adding a support from gdb-remote client 
(https://reviews.llvm.org/D101329) if the plugin name is empty the plugin 
manager will try to save the core directly from the process plugin.
See 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Core/PluginManager.cpp#L696

To have an ability to save the core with minidump plugin I added plugin name as 
a parameter in SBProcess::SaveCore.
See the usage in TestProcessSaveCoreMinidump.py


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125325

Files:
  lldb/bindings/interface/SBProcess.i
  lldb/include/lldb/API/SBProcess.h
  lldb/source/API/SBProcess.cpp
  
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py


Index: 
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- 
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ 
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -21,6 +21,7 @@
 self.build()
 exe = self.getBuildArtifact("a.out")
 core = self.getBuildArtifact("core.dmp")
+core_sb = self.getBuildArtifact("core_sb.dmp")
 try:
 target = self.dbg.CreateTarget(exe)
 process = target.LaunchSimple(
@@ -42,7 +43,10 @@
 
 # save core and, kill process and verify corefile existence
 self.runCmd("process save-core --plugin-name=minidump 
--style=stack " + core)
+# validate savinig via SBProcess
+lldb.SBProcess().SaveCore(core_sb, "minidump")
 self.assertTrue(os.path.isfile(core))
+self.assertTrue(os.path.isfile(core_sb))
 self.assertSuccess(process.Kill())
 
 # To verify, we'll launch with the mini dump
@@ -77,3 +81,5 @@
 self.assertTrue(self.dbg.DeleteTarget(target))
 if (os.path.isfile(core)):
 os.unlink(core)
+if (os.path.isfile(core_sb)):
+os.unlink(core_sb)
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1136,8 +1136,9 @@
   return runtime_sp->IsActive();
 }
 
-lldb::SBError SBProcess::SaveCore(const char *file_name) {
-  LLDB_INSTRUMENT_VA(this, file_name);
+lldb::SBError SBProcess::SaveCore(const char *file_name,
+  const char *plugin_name) {
+  LLDB_INSTRUMENT_VA(this, file_name, plugin_name);
 
   lldb::SBError error;
   ProcessSP process_sp(GetSP());
@@ -1156,7 +1157,8 @@
 
   FileSpec core_file(file_name);
   SaveCoreStyle core_style = SaveCoreStyle::eSaveCoreFull;
-  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style, "");
+  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
+plugin_name);
   return error;
 }
 
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -338,7 +338,8 @@
   bool IsInstrumentationRuntimePresent(InstrumentationRuntimeType type);
 
   /// Save the state of the process in a core file (or mini dump on Windows).
-  lldb::SBError SaveCore(const char *file_name);
+  lldb::SBError SaveCore(const char *file_name,
+ const char *plugin_name = "");
 
   /// Query the address load_addr and store the details of the memory
   /// region that contains it in the supplied SBMemoryRegionInfo object.
Index: lldb/bindings/interface/SBProcess.i
===
--- lldb/bindings/interface/SBProcess.i
+++ lldb/bindings/interface/SBProcess.i
@@ -398,7 +398,7 @@
 IsInstrumentationRuntimePresent(lldb::InstrumentationRuntimeType type);
 
 lldb::SBError
-SaveCore(const char *file_name);
+SaveCore(const char *file_name, const char *plugin_name);
 
 lldb::SBError
 GetMemoryRegionInfo(lldb::addr_t load_addr, lldb::SBMemoryRegionInfo 
_info);


Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -21,6 +21,7 @@
 self.build()
 exe = 

[Lldb-commits] [PATCH] D114554: Fix TestFileHandle.py. Remove redundant skipIfReproducer annotation

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat abandoned this revision.
PatriosTheGreat added a comment.

Closing this revision since Pavel already fixed this in  rGc0e3bb4d4ba306 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114554

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

[Lldb-commits] [PATCH] D114554: Fix TestFileHandle.py. Remove redundant skipIfReproducer annotation

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Hi Pavel.

Got it,
Thanks for fixing and clarification.

In D114554#3152953 , @labath wrote:

> I've fixed this in rGc0e3bb4d4ba306 
> .
>
>> Not sure if it's better to revert original commit 
>> (https://reviews.llvm.org/rGf23b829a2635a6d833bdc81a12d6217b52ae9e45) and 
>> then re-commit the change with this fix.
>
> For a simple fix like this, it doesn't really matter what you do. Just don't 
> leave the build broken for extended periods of time. :)
>
> That said, it's pretty hard to do anything when you don't have commit access 
> to the repository, so part of the responsibility also lies with the person 
> who commits the patch for you. I think that, for a while, we had the problem 
> that the blame emails were not going to the person who committed the patch 
> (only the author), but I was under the impression that was now fixed. David, 
> did you get any breakage emails about that commit ?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114554

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

[Lldb-commits] [PATCH] D114554: Fix TestFileHandle.py. Remove redundant skipIfReproducer annotation

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat created this revision.
PatriosTheGreat added reviewers: DavidSpickett, JDevlieghere, shafik.
PatriosTheGreat requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

My recent change (https://reviews.llvm.org/D104413)  broke the lldb tests 
(https://lab.llvm.org/buildbot#builders/17/builds/13800).
The problem is that skipIfReproducer annotation  was removed in this change: 
https://github.com/llvm/llvm-project/commit/b505ed9d313653782b81bbc97979c98edb205558

In this change I remove unnecessary annotation. I checked tests on my local 
Linux machine they are passing on it.
Not sure if it's better to revert original commit 
(https://reviews.llvm.org/rGf23b829a2635a6d833bdc81a12d6217b52ae9e45) and then 
re-commit the change with this fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114554

Files:
  lldb/test/API/python_api/file_handle/TestFileHandle.py


Index: lldb/test/API/python_api/file_handle/TestFileHandle.py
===
--- lldb/test/API/python_api/file_handle/TestFileHandle.py
+++ lldb/test/API/python_api/file_handle/TestFileHandle.py
@@ -853,7 +853,6 @@
 with open(self.out_filename, 'r') as f:
 self.assertEqual(f.read().strip(), "Frobozz")
 
-@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_set_sbstream(self):
 with open(self.out_filename, 'w') as outf:
 outsbf = lldb.SBFile(outf.fileno(), "w", False)


Index: lldb/test/API/python_api/file_handle/TestFileHandle.py
===
--- lldb/test/API/python_api/file_handle/TestFileHandle.py
+++ lldb/test/API/python_api/file_handle/TestFileHandle.py
@@ -853,7 +853,6 @@
 with open(self.out_filename, 'r') as f:
 self.assertEqual(f.read().strip(), "Frobozz")
 
-@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_set_sbstream(self):
 with open(self.out_filename, 'w') as outf:
 outsbf = lldb.SBFile(outf.fileno(), "w", False)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Thanks for review!

Could you or someone else take this commit to master?
I don't have a commit permissions.


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

https://reviews.llvm.org/D104413

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

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Friendly ping =)


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

https://reviews.llvm.org/D104413

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

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat marked 2 inline comments as done.
PatriosTheGreat added a comment.

Sorry for delay from my side also


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

https://reviews.llvm.org/D104413

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

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 371002.

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

https://reviews.llvm.org/D104413

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Core/Debugger.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/test/API/python_api/default-constructor/sb_debugger.py
  lldb/test/API/python_api/file_handle/TestFileHandle.py
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -23,7 +23,6 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
@@ -43,14 +42,6 @@
 #include 
 #include 
 
-// Includes for pipe()
-#if defined(_WIN32)
-#include 
-#include 
-#else
-#include 
-#endif
-
 #if !defined(__APPLE__)
 #include "llvm/Support/DataTypes.h"
 #endif
@@ -401,60 +392,6 @@
   return error;
 }
 
-static inline int OpenPipe(int fds[2], std::size_t size) {
-#ifdef _WIN32
-  return _pipe(fds, size, O_BINARY);
-#else
-  (void)size;
-  return pipe(fds);
-#endif
-}
-
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
-  size_t commands_size) {
-  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
-  int fds[2] = {-1, -1};
-
-  if (OpenPipe(fds, commands_size) != 0) {
-WithColor::error()
-<< "can't create pipe file descriptors for LLDB commands\n";
-return nullptr;
-  }
-
-  ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
-  if (size_t(nrwr) != commands_size) {
-WithColor::error()
-<< format(
-   "write(%i, %p, %" PRIu64
-   ") failed (errno = %i) when trying to open LLDB commands pipe",
-   fds[WRITE], static_cast(commands_data),
-   static_cast(commands_size), errno)
-<< '\n';
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-return nullptr;
-  }
-
-  // Close the write end of the pipe, so that the command interpreter will exit
-  // when it consumes all the data.
-  llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-
-  // Open the read file descriptor as a FILE * that we can return as an input
-  // handle.
-  ::FILE *commands_file = fdopen(fds[READ], "rb");
-  if (commands_file == nullptr) {
-WithColor::error() << format("fdopen(%i, \"rb\") failed (errno = %i) "
- "when trying to open LLDB commands pipe",
- fds[READ], errno)
-   << '\n';
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
-return nullptr;
-  }
-
-  // 'commands_file' now owns the read descriptor.
-  return commands_file;
-}
-
 std::string EscapeString(std::string arg) {
   std::string::size_type pos = 0;
   while ((pos = arg.find_first_of("\"\\", pos)) != std::string::npos) {
@@ -584,21 +521,15 @@
   // Check if we have any data in the commands stream, and if so, save it to a
   // temp file
   // so we can then run the command interpreter using the file contents.
-  const char *commands_data = commands_stream.GetData();
-  const size_t commands_size = commands_stream.GetSize();
-
   bool go_interactive = true;
-  if ((commands_data != nullptr) && (commands_size != 0u)) {
-FILE *commands_file =
-PrepareCommandsForSourcing(commands_data, commands_size);
-
-if (commands_file == nullptr) {
-  // We should have already printed an error in PrepareCommandsForSourcing.
+  if ((commands_stream.GetData() != nullptr) &&
+  (commands_stream.GetSize() != 0u)) {
+SBError error = m_debugger.SetInputString(commands_stream.GetData());
+if (error.Fail()) {
+  WithColor::error() << error.GetCString() << '\n';
   return 1;
 }
 
-m_debugger.SetInputFileHandle(commands_file, true);
-
 // Set the debugger into Sync mode when running the command file. Otherwise
 // command files that run the target won't run in a sensible way.
 bool old_async = m_debugger.GetAsync();
@@ -630,12 +561,9 @@
   SBStream crash_commands_stream;
   WriteCommandsForSourcing(eCommandPlacementAfterCrash,
crash_commands_stream);
-  const char *crash_commands_data = crash_commands_stream.GetData();
-  const size_t crash_commands_size = crash_commands_stream.GetSize();
-  commands_file =
-  PrepareCommandsForSourcing(crash_commands_data, crash_commands_size);
-  if (commands_file != nullptr) {
-m_debugger.SetInputFileHandle(commands_file, true);
+  SBError error =
+  m_debugger.SetInputString(crash_commands_stream.GetData());

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Should I send this somehow back to review?


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

https://reviews.llvm.org/D104413

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

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 353875.
PatriosTheGreat added a comment.

Hi everyone.
Thanks for review.
In order to move new method code to Debugger class I also had to move there 
SetInputFile(lldb::FileSP file) method.
After I did that -- the reproducer tests started to fail. 
If I understand correctly there is a problem to deserialize SBStream object 
during the reply session and in previous patch it the replay tests was working 
since I was calling SetInputFile API method from SetStreamInput.
To fix that in this patch I replaced SetStreamInput method with SetInputData 
(const char* data, size_t size); which takes a raw chat array instead of 
SBStream.


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

https://reviews.llvm.org/D104413

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Core/Debugger.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/test/API/python_api/default-constructor/sb_debugger.py
  lldb/test/API/python_api/file_handle/TestFileHandle.py
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -23,7 +23,6 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
@@ -42,14 +41,6 @@
 #include 
 #include 
 
-// Includes for pipe()
-#if defined(_WIN32)
-#include 
-#include 
-#else
-#include 
-#endif
-
 #if !defined(__APPLE__)
 #include "llvm/Support/DataTypes.h"
 #endif
@@ -400,60 +391,6 @@
   return error;
 }
 
-static inline int OpenPipe(int fds[2], std::size_t size) {
-#ifdef _WIN32
-  return _pipe(fds, size, O_BINARY);
-#else
-  (void)size;
-  return pipe(fds);
-#endif
-}
-
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
-  size_t commands_size) {
-  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
-  int fds[2] = {-1, -1};
-
-  if (OpenPipe(fds, commands_size) != 0) {
-WithColor::error()
-<< "can't create pipe file descriptors for LLDB commands\n";
-return nullptr;
-  }
-
-  ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
-  if (size_t(nrwr) != commands_size) {
-WithColor::error()
-<< format(
-   "write(%i, %p, %" PRIu64
-   ") failed (errno = %i) when trying to open LLDB commands pipe",
-   fds[WRITE], static_cast(commands_data),
-   static_cast(commands_size), errno)
-<< '\n';
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-return nullptr;
-  }
-
-  // Close the write end of the pipe, so that the command interpreter will exit
-  // when it consumes all the data.
-  llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-
-  // Open the read file descriptor as a FILE * that we can return as an input
-  // handle.
-  ::FILE *commands_file = fdopen(fds[READ], "rb");
-  if (commands_file == nullptr) {
-WithColor::error() << format("fdopen(%i, \"rb\") failed (errno = %i) "
- "when trying to open LLDB commands pipe",
- fds[READ], errno)
-   << '\n';
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
-return nullptr;
-  }
-
-  // 'commands_file' now owns the read descriptor.
-  return commands_file;
-}
-
 std::string EscapeString(std::string arg) {
   std::string::size_type pos = 0;
   while ((pos = arg.find_first_of("\"\\", pos)) != std::string::npos) {
@@ -583,21 +520,16 @@
   // Check if we have any data in the commands stream, and if so, save it to a
   // temp file
   // so we can then run the command interpreter using the file contents.
-  const char *commands_data = commands_stream.GetData();
-  const size_t commands_size = commands_stream.GetSize();
-
   bool go_interactive = true;
-  if ((commands_data != nullptr) && (commands_size != 0u)) {
-FILE *commands_file =
-PrepareCommandsForSourcing(commands_data, commands_size);
-
-if (commands_file == nullptr) {
-  // We should have already printed an error in PrepareCommandsForSourcing.
+  if ((commands_stream.GetData() != nullptr) &&
+  (commands_stream.GetSize() != 0u)) {
+SBError error = m_debugger.SetInputData(commands_stream.GetData(),
+commands_stream.GetSize());
+if (error.Fail()) {
+  WithColor::error() << error.GetCString() << '\n';
   return 1;
 }
 
-m_debugger.SetInputFileHandle(commands_file, true);
-
 // Set the debugger into Sync mode when running the command file. Otherwise
 // command files that run the target 

[Lldb-commits] [PATCH] D103271: [lldb/Target] Select most relevant frame only in case of signal

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 352966.
PatriosTheGreat added a comment.

Thanks for the review feedback.
Looks like moving frame recognizer call to StackFrameList::GetFramesUpTo also 
fixes infinity recursion issue.

Regarding the zeroth frame issue: I'm actually using the lldb-server compiled 
with followed cmake command:
cmake -G Ninja -DLLVM_ENABLE_PROJECTS='lldb;clang;libcxx'  ../llvm

I see a performance improvement even on a local run, however it's not as huge:
In the sample I provided previously with this patch the execution time will be 
around 4 seconds instead of 6 seconds without it during the local run.

I see at the profiler that asking 0th frame triggers 
UnwindLLDB::DoGetFrameInfoAtIndex -> UnwindLLDB::AddFirstFrame -> 
UnwindLLDB::UpdateUnwindPlanForFirstFrameIfInvalid -> 
UnwindLLDB::AddOneMoreFrame -> UnwindLLDB::GetOneMoreFrame -> 
RegisterContextUnwind::RegisterContextUnwind -> 
RegisterContextUnwind::InitializeNonZerothFrame

- which is costly.

If I understand correctly the LLDB won't do any of this for background threads 
by default.

Could it be that accelerated stop-reply packets you said previously is turned 
off by default in lldb-server and I need to set a special setting to turn them 
on?


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

https://reviews.llvm.org/D103271

Files:
  lldb/include/lldb/Target/Thread.h
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/Thread.cpp


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -578,15 +578,9 @@
   return raw_stop_description;
 }
 
-void Thread::SelectMostRelevantFrame() {
+void Thread::SelectMostRelevantFrame(lldb::StackFrameSP first_frame) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD);
-
-  auto frames_list_sp = GetStackFrameList();
-
-  // Only the top frame should be recognized.
-  auto frame_sp = frames_list_sp->GetFrameAtIndex(0);
-
-  auto recognized_frame_sp = frame_sp->GetRecognizedFrame();
+  auto recognized_frame_sp = first_frame->GetRecognizedFrame();
 
   if (!recognized_frame_sp) {
 LLDB_LOG(log, "Frame #0 not recognized");
@@ -606,8 +600,6 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
-
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.
 
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -547,6 +547,9 @@
 curr_frame_address = next_frame_address;
   }
 }
+
+if (idx == 0)
+  m_thread.SelectMostRelevantFrame(unwind_frame_sp);
   } while (m_frames.size() - 1 < end_idx);
 
   // Don't try to merge till you've calculated all the frames in this stack.
Index: lldb/include/lldb/Target/Thread.h
===
--- lldb/include/lldb/Target/Thread.h
+++ lldb/include/lldb/Target/Thread.h
@@ -218,7 +218,7 @@
 
   virtual void RefreshStateAfterStop() = 0;
 
-  void SelectMostRelevantFrame();
+  void SelectMostRelevantFrame(lldb::StackFrameSP first_frame);
 
   std::string GetStopDescription();
 


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -578,15 +578,9 @@
   return raw_stop_description;
 }
 
-void Thread::SelectMostRelevantFrame() {
+void Thread::SelectMostRelevantFrame(lldb::StackFrameSP first_frame) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD);
-
-  auto frames_list_sp = GetStackFrameList();
-
-  // Only the top frame should be recognized.
-  auto frame_sp = frames_list_sp->GetFrameAtIndex(0);
-
-  auto recognized_frame_sp = frame_sp->GetRecognizedFrame();
+  auto recognized_frame_sp = first_frame->GetRecognizedFrame();
 
   if (!recognized_frame_sp) {
 LLDB_LOG(log, "Frame #0 not recognized");
@@ -606,8 +600,6 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
-
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.
 
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -547,6 +547,9 @@
 curr_frame_address = next_frame_address;
   }
 }
+
+if (idx == 0)
+  m_thread.SelectMostRelevantFrame(unwind_frame_sp);
   } while (m_frames.size() - 1 < end_idx);
 
   // Don't try to merge till you've calculated all the frames in this stack.
Index: lldb/include/lldb/Target/Thread.h
===
--- 

[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat created this revision.
PatriosTheGreat added reviewers: teemperor, jarin.
PatriosTheGreat requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Right now if the LLDB is compiled under the windows with static vcruntime 
library, the -o and -k commands will not work.

The problem is that the LLDB create FILE* in lldb.exe and pass it to 
liblldb.dll which is an object from CRT.
Since the CRT is statically linked each of these module has its own copy of the 
CRT with it's own global state and the LLDB should not share CRT objects 
between them.

In this change I moved the logic of creating FILE* out of commands stream from 
Driver class to SBDebugger.
To do this I added new method: SBError SBDebugger::SetInputStream(SBStream 
)

Command to build the LLDB:
cmake -G Ninja -DLLVM_ENABLE_PROJECTS="clang;lldb;libcxx"  
-DLLVM_USE_CRT_RELEASE="MT" -DLLVM_USE_CRT_MINSIZEREL="MT" 
-DLLVM_USE_CRT_RELWITHDEBINFO="MT" -DP
YTHON_HOME:FILEPATH=C:/Python38 -DCMAKE_C_COMPILER:STRING=cl.exe 
-DCMAKE_CXX_COMPILER:STRING=cl.exe ../llvm

Command which will fail:
lldb.exe -o help

See discord discussion for more details: 
https://discord.com/channels/636084430946959380/636732809708306432/854629125398724628
This revision is for the further discussion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104413

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

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -23,7 +23,6 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
@@ -42,14 +41,6 @@
 #include 
 #include 
 
-// Includes for pipe()
-#if defined(_WIN32)
-#include 
-#include 
-#else
-#include 
-#endif
-
 #if !defined(__APPLE__)
 #include "llvm/Support/DataTypes.h"
 #endif
@@ -400,60 +391,6 @@
   return error;
 }
 
-static inline int OpenPipe(int fds[2], std::size_t size) {
-#ifdef _WIN32
-  return _pipe(fds, size, O_BINARY);
-#else
-  (void)size;
-  return pipe(fds);
-#endif
-}
-
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
-  size_t commands_size) {
-  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
-  int fds[2] = {-1, -1};
-
-  if (OpenPipe(fds, commands_size) != 0) {
-WithColor::error()
-<< "can't create pipe file descriptors for LLDB commands\n";
-return nullptr;
-  }
-
-  ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
-  if (size_t(nrwr) != commands_size) {
-WithColor::error()
-<< format(
-   "write(%i, %p, %" PRIu64
-   ") failed (errno = %i) when trying to open LLDB commands pipe",
-   fds[WRITE], static_cast(commands_data),
-   static_cast(commands_size), errno)
-<< '\n';
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-return nullptr;
-  }
-
-  // Close the write end of the pipe, so that the command interpreter will exit
-  // when it consumes all the data.
-  llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-
-  // Open the read file descriptor as a FILE * that we can return as an input
-  // handle.
-  ::FILE *commands_file = fdopen(fds[READ], "rb");
-  if (commands_file == nullptr) {
-WithColor::error() << format("fdopen(%i, \"rb\") failed (errno = %i) "
- "when trying to open LLDB commands pipe",
- fds[READ], errno)
-   << '\n';
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
-return nullptr;
-  }
-
-  // 'commands_file' now owns the read descriptor.
-  return commands_file;
-}
-
 std::string EscapeString(std::string arg) {
   std::string::size_type pos = 0;
   while ((pos = arg.find_first_of("\"\\", pos)) != std::string::npos) {
@@ -583,21 +520,15 @@
   // Check if we have any data in the commands stream, and if so, save it to a
   // temp file
   // so we can then run the command interpreter using the file contents.
-  const char *commands_data = commands_stream.GetData();
-  const size_t commands_size = commands_stream.GetSize();
-
   bool go_interactive = true;
-  if ((commands_data != nullptr) && (commands_size != 0u)) {
-FILE *commands_file =
-PrepareCommandsForSourcing(commands_data, commands_size);
-
-if (commands_file == nullptr) {
-  // We should have already printed an error in PrepareCommandsForSourcing.
+  if ((commands_stream.GetData() != nullptr) &&
+  (commands_stream.GetSize() != 0u)) {
+SBError error = 

[Lldb-commits] [PATCH] D103271: [lldb/Target] Select most relevant frame only in case of signal

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 350596.
PatriosTheGreat added a comment.

Thanks for your suggestions.
I tried both of them, however the solution to patch assert recognizer doesn’t 
solve the original performance issue. Since the performance degradation doesn’t 
come from the recognizer. It appears since the LLDB needs to unwind at least 
one stack frame for each thread in order to determine does the LLDB even need 
to run recognizers (see this line: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Target/Thread.cpp#L587)

The solution to select the most relevant frame at the moment when the user 
actually needs a stack frame seems to fork fine. If I understand correctly it 
can be done at the Thread::GetStackFrameAtIndex method, since to provide stack 
trace view to the user all clients need to call it for all stack frames.

One problem I see in this solution is that recognizers should be aware to not 
call Thread::GetStackFrameAtIndex for the zeroth frame (which they actually got 
as a parameter), since otherwise it will bring to the infinity recursion. See 
changes to AssertFrameRecognizer.cpp

I’ve attached this solution to the current revision for further discussion.


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

https://reviews.llvm.org/D103271

Files:
  lldb/include/lldb/Target/Thread.h
  lldb/source/Target/AssertFrameRecognizer.cpp
  lldb/source/Target/Thread.cpp


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -578,14 +578,8 @@
   return raw_stop_description;
 }
 
-void Thread::SelectMostRelevantFrame() {
+void Thread::SelectMostRelevantFrame(lldb::StackFrameSP _sp) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD);
-
-  auto frames_list_sp = GetStackFrameList();
-
-  // Only the top frame should be recognized.
-  auto frame_sp = frames_list_sp->GetFrameAtIndex(0);
-
   auto recognized_frame_sp = frame_sp->GetRecognizedFrame();
 
   if (!recognized_frame_sp) {
@@ -606,8 +600,6 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
-
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.
 
@@ -1405,6 +1397,15 @@
   exe_ctx.SetContext(shared_from_this());
 }
 
+lldb::StackFrameSP Thread::GetStackFrameAtIndex(uint32_t idx) {
+  auto frame = GetStackFrameList()->GetFrameAtIndex(idx);
+
+  if (frame && idx == 0)
+SelectMostRelevantFrame(frame);
+
+  return frame;
+}
+
 StackFrameListSP Thread::GetStackFrameList() {
   std::lock_guard guard(m_frame_mutex);
 
Index: lldb/source/Target/AssertFrameRecognizer.cpp
===
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -118,8 +118,9 @@
 
   // Fetch most relevant frame
   for (uint32_t frame_index = 0; frame_index < frames_to_fetch; frame_index++) 
{
-prev_frame_sp = thread_sp->GetStackFrameAtIndex(frame_index);
-
+prev_frame_sp = frame_index == 0
+? frame_sp
+: thread_sp->GetStackFrameAtIndex(frame_index);
 if (!prev_frame_sp) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
   LLDB_LOG(log, "Abort Recognizer: Hit unwinding bound ({1} frames)!",
Index: lldb/include/lldb/Target/Thread.h
===
--- lldb/include/lldb/Target/Thread.h
+++ lldb/include/lldb/Target/Thread.h
@@ -218,7 +218,7 @@
 
   virtual void RefreshStateAfterStop() = 0;
 
-  void SelectMostRelevantFrame();
+  void SelectMostRelevantFrame(lldb::StackFrameSP _sp);
 
   std::string GetStopDescription();
 
@@ -396,9 +396,7 @@
 return GetStackFrameList()->GetNumFrames();
   }
 
-  virtual lldb::StackFrameSP GetStackFrameAtIndex(uint32_t idx) {
-return GetStackFrameList()->GetFrameAtIndex(idx);
-  }
+  virtual lldb::StackFrameSP GetStackFrameAtIndex(uint32_t idx);
 
   virtual lldb::StackFrameSP
   GetFrameWithConcreteFrameIndex(uint32_t unwind_idx);


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -578,14 +578,8 @@
   return raw_stop_description;
 }
 
-void Thread::SelectMostRelevantFrame() {
+void Thread::SelectMostRelevantFrame(lldb::StackFrameSP _sp) {
   Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD);
-
-  auto frames_list_sp = GetStackFrameList();
-
-  // Only the top frame should be recognized.
-  auto frame_sp = frames_list_sp->GetFrameAtIndex(0);
-
   auto recognized_frame_sp = frame_sp->GetRecognizedFrame();
 
   if (!recognized_frame_sp) {
@@ -606,8 +600,6 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  

[Lldb-commits] [PATCH] D103271: [lldb/Target] Select most relevant frame only in case of signal

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 348919.
PatriosTheGreat added a comment.

Select most related frame only in threads which were stopped with reason.
This diff is for further discussion.


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

https://reviews.llvm.org/D103271

Files:
  lldb/source/Target/Thread.cpp


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -606,7 +606,9 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
+  lldb::StopReason stop_reason = GetStopReason();
+  if (stop_reason != lldb::StopReason::eStopReasonNone)
+SelectMostRelevantFrame();
 
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -606,7 +606,9 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
+  lldb::StopReason stop_reason = GetStopReason();
+  if (stop_reason != lldb::StopReason::eStopReasonNone)
+SelectMostRelevantFrame();
 
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D103271: [lldb/Target] Select most relevant frame only in case of signal

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat requested review of this revision.
PatriosTheGreat added a comment.

Thanks for feedback.

I made a small sample: https://pastebin.com/F8nde1zi
Compile command: clang++ -O0 -g -pthread sample.cpp 
LLDB command: breakpoint set -f sample.cpp -l 24 --condition 'i == 100' 
This break should not be hit due to the condition.

I got followed results for local and remote run:
LLDB 11:
Local:
Time difference = 10 330 [ms]
Remote:
Time difference = 189 707 [ms]

Patched (turned off select of the most relevant frame for threads which were 
stopped without a reason):
Local:
Time difference = 7 865 [ms]
Remote:
Time difference = 13 630 [ms]

Looks like there is a performance improvement even with local run, but with 
remote run it becomes much more significant.

In D103271#2785450 , @jingham wrote:

> It is incorrect to say that SelectMostRelevantFrame is only relevant for 
> asserts.  The FrameRecognizer is a generic mechanism, and you have no a 
> priori way to know what kinds of thread stops it wants to operate on.  So 
> this patch is not correct.
>
> Other than calling the Frame Recognizers, SelectMostRelevantFrame does NOT 
> trigger a stack walk itself.
>
> So if there is a frame recognizer that is doing a frame walk on threads that 
> aren't relevant to it, then changing the frame recognizer to check that the 
> stop reason of its thread makes sense before doing any work seems a better 
> solution to your problem.

From performance analyzer I see that SelectMostRelevantFrame calls 
StackFrameList::GetFrameAtIndex -> StackFrameList::GetFramesUpTo -> 
UnwindLLDB::DoGetFrameInfoAtIndex -> UnwindLLDB::AddFirstFrame
UnwindLLDB::AddFirstFrame calls two expensive methods:  
UnwindLLDB::UpdateUnwindPlanForFirstFrameIfInvalid (takes around 2/3 of 
execution time) and UnwindLLDB::RegisterContextUnwind (takes around 1/3 of 
execution time)

Does frame recognition relevant for threads which were stopped without any 
reason?
May I filter this logic like this:

  lldb::StopReason stop_reason = GetStopReason();
  if (stop_reason != lldb::StopReason::eStopReasonNone)
SelectMostRelevantFrame();

If this is also an incorrect solution may we somehow select most relevant frame 
without calling of  UnwindLLDB::DoGetFrameInfoAtIndex?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103271

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

[Lldb-commits] [PATCH] D103271: [lldb/Target] Select most relevant frame only in case of signal

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

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

In order to select most relevant frame the debugger needs to unwind current 
frames for each thread which could be slow.
This is a problem in case of executing with remote debugger attached and 
conditional breakpoints set.
In that case the debugger will stop the execution on a breakpoint hit to run 
conditional expression and will resume the execution after.
If there are a lot of threads (90+) the simple "for" loop with 50 iterations 
and conditional breakpoint could take more than 2 minutes to execute.
>From my observation most of this time will be spent on SelectMostRelevantFrame 
>method (since it need to unwind stack for all threads).

Since the most relevant frame is needed only for assert failure it looks like 
we can collect it only for signals.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103271

Files:
  lldb/source/Target/Thread.cpp


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -606,7 +606,9 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
+  lldb::StopReason stop_reason = GetStopReason();
+  if (stop_reason == lldb::StopReason::eStopReasonSignal)
+SelectMostRelevantFrame();
 
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -606,7 +606,9 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
+  lldb::StopReason stop_reason = GetStopReason();
+  if (stop_reason == lldb::StopReason::eStopReasonSignal)
+SelectMostRelevantFrame();
 
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Hi Muhammad,
Thank you for review.
Could you or Pavel commit this to master, since I don't have commit access?


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

https://reviews.llvm.org/D74217



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

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 245167.
PatriosTheGreat added a comment.

In this revision I fix value_regnums and invalidate_regnums serialization in 
target.xml.
However I'm not so sure that this is the cause of problem with ARM tests.

>From tests log it seems like there are some unexpected value in register.
Is it possible to get logs output from this test run 
http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/1713/steps/test/logs/stdio
 that would help us a lot to fix it.


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

https://reviews.llvm.org/D74217

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/main.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -199,6 +199,8 @@
   static std::string XMLEncodeAttributeValue(llvm::StringRef value);
 
 private:
+  llvm::Expected> BuildTargetXml();
+
   void HandleInferiorState_Exited(NativeProcessProtocol *process);
 
   void HandleInferiorState_Stopped(NativeProcessProtocol *process);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -377,6 +377,99 @@
   }
 }
 
+static llvm::StringRef GetEncodingNameOrEmpty(const RegisterInfo _info) {
+  switch (reg_info.encoding) {
+  case eEncodingUint:
+return "uint";
+  case eEncodingSint:
+return "sint";
+  case eEncodingIEEE754:
+return "ieee754";
+  case eEncodingVector:
+return "vector";
+  default:
+return "";
+  }
+}
+
+static llvm::StringRef GetFormatNameOrEmpty(const RegisterInfo _info) {
+  switch (reg_info.format) {
+  case eFormatBinary:
+return "binary";
+  case eFormatDecimal:
+return "decimal";
+  case eFormatHex:
+return "hex";
+  case eFormatFloat:
+return "float";
+  case eFormatVectorOfSInt8:
+return "vector-sint8";
+  case eFormatVectorOfUInt8:
+return "vector-uint8";
+  case eFormatVectorOfSInt16:
+return "vector-sint16";
+  case eFormatVectorOfUInt16:
+return "vector-uint16";
+  case eFormatVectorOfSInt32:
+return "vector-sint32";
+  case eFormatVectorOfUInt32:
+return "vector-uint32";
+  case eFormatVectorOfFloat32:
+return "vector-float32";
+  case eFormatVectorOfUInt64:
+return "vector-uint64";
+  case eFormatVectorOfUInt128:
+return "vector-uint128";
+  default:
+return "";
+  };
+}
+
+static llvm::StringRef GetKindGenericOrEmpty(const RegisterInfo _info) {
+  switch (reg_info.kinds[RegisterKind::eRegisterKindGeneric]) {
+  case LLDB_REGNUM_GENERIC_PC:
+return "pc";
+  case LLDB_REGNUM_GENERIC_SP:
+return "sp";
+  case LLDB_REGNUM_GENERIC_FP:
+return "fp";
+  case LLDB_REGNUM_GENERIC_RA:
+return "ra";
+  case LLDB_REGNUM_GENERIC_FLAGS:
+return "flags";
+  case LLDB_REGNUM_GENERIC_ARG1:
+return "arg1";
+  case LLDB_REGNUM_GENERIC_ARG2:
+return "arg2";
+  case LLDB_REGNUM_GENERIC_ARG3:
+return "arg3";
+  case LLDB_REGNUM_GENERIC_ARG4:
+return "arg4";
+  case LLDB_REGNUM_GENERIC_ARG5:
+return "arg5";
+  case LLDB_REGNUM_GENERIC_ARG6:
+return "arg6";
+  case LLDB_REGNUM_GENERIC_ARG7:
+return "arg7";
+  case LLDB_REGNUM_GENERIC_ARG8:
+return "arg8";
+  default:
+return "";
+  }
+}
+
+static void CollectRegNums(const uint32_t *reg_num, StreamString ,
+   bool usehex) {
+  for (int i = 0; *reg_num != LLDB_INVALID_REGNUM; ++reg_num, ++i) {
+if (i > 0)
+  response.PutChar(',');
+if (usehex)
+  response.Printf("%" PRIx32, *reg_num);
+else
+  response.Printf("%" PRIu32, *reg_num);
+  }
+}
+
 static void WriteRegisterValueInHexFixedWidth(
 StreamString , NativeRegisterContext _ctx,
 const RegisterInfo _info, const RegisterValue *reg_value_p,
@@ -1699,74 +1792,18 @@
   response.Printf("bitsize:%" PRIu32 ";offset:%" PRIu32 ";",
   reg_info->byte_size * 8, reg_info->byte_offset);
 
-  switch (reg_info->encoding) {
-  case eEncodingUint:
-response.PutCString("encoding:uint;");
-break;
-  case eEncodingSint:
-response.PutCString("encoding:sint;");
-break;
- 

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Hi Muhammad,

Thank you for finding this issue.
After I fix that, could I update current review or should I create new one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74217



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

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Hi Pavel,
Thank you for review.
Could you also commit this to master, since I don't have commit access?


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

https://reviews.llvm.org/D74217



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

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 244152.

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

https://reviews.llvm.org/D74217

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/main.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -199,6 +199,8 @@
   static std::string XMLEncodeAttributeValue(llvm::StringRef value);
 
 private:
+  llvm::Expected> BuildTargetXml();
+
   void HandleInferiorState_Exited(NativeProcessProtocol *process);
 
   void HandleInferiorState_Stopped(NativeProcessProtocol *process);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -377,6 +377,96 @@
   }
 }
 
+static llvm::StringRef GetEncodingNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->encoding) {
+  case eEncodingUint:
+return "uint";
+  case eEncodingSint:
+return "sint";
+  case eEncodingIEEE754:
+return "ieee754";
+  case eEncodingVector:
+return "vector";
+  default:
+return "";
+  }
+}
+
+static llvm::StringRef GetFormatNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->format) {
+  case eFormatBinary:
+return "binary";
+  case eFormatDecimal:
+return "decimal";
+  case eFormatHex:
+return "hex";
+  case eFormatFloat:
+return "float";
+  case eFormatVectorOfSInt8:
+return "vector-sint8";
+  case eFormatVectorOfUInt8:
+return "vector-uint8";
+  case eFormatVectorOfSInt16:
+return "vector-sint16";
+  case eFormatVectorOfUInt16:
+return "vector-uint16";
+  case eFormatVectorOfSInt32:
+return "vector-sint32";
+  case eFormatVectorOfUInt32:
+return "vector-uint32";
+  case eFormatVectorOfFloat32:
+return "vector-float32";
+  case eFormatVectorOfUInt64:
+return "vector-uint64";
+  case eFormatVectorOfUInt128:
+return "vector-uint128";
+  default:
+return "";
+  };
+}
+
+static llvm::StringRef GetKindGenericOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->kinds[RegisterKind::eRegisterKindGeneric]) {
+  case LLDB_REGNUM_GENERIC_PC:
+return "pc";
+  case LLDB_REGNUM_GENERIC_SP:
+return "sp";
+  case LLDB_REGNUM_GENERIC_FP:
+return "fp";
+  case LLDB_REGNUM_GENERIC_RA:
+return "ra";
+  case LLDB_REGNUM_GENERIC_FLAGS:
+return "flags";
+  case LLDB_REGNUM_GENERIC_ARG1:
+return "arg1";
+  case LLDB_REGNUM_GENERIC_ARG2:
+return "arg2";
+  case LLDB_REGNUM_GENERIC_ARG3:
+return "arg3";
+  case LLDB_REGNUM_GENERIC_ARG4:
+return "arg4";
+  case LLDB_REGNUM_GENERIC_ARG5:
+return "arg5";
+  case LLDB_REGNUM_GENERIC_ARG6:
+return "arg6";
+  case LLDB_REGNUM_GENERIC_ARG7:
+return "arg7";
+  case LLDB_REGNUM_GENERIC_ARG8:
+return "arg8";
+  default:
+return "";
+  }
+}
+
+static void CollectRegNums(const uint32_t *reg_num,
+   lldb_private::StreamString response) {
+  for (int i = 0; *reg_num != LLDB_INVALID_REGNUM; ++reg_num, ++i) {
+if (i > 0)
+  response.PutChar(',');
+response.Printf("%" PRIx32, *reg_num);
+  }
+}
+
 static void WriteRegisterValueInHexFixedWidth(
 StreamString , NativeRegisterContext _ctx,
 const RegisterInfo _info, const RegisterValue *reg_value_p,
@@ -1699,74 +1789,18 @@
   response.Printf("bitsize:%" PRIu32 ";offset:%" PRIu32 ";",
   reg_info->byte_size * 8, reg_info->byte_offset);
 
-  switch (reg_info->encoding) {
-  case eEncodingUint:
-response.PutCString("encoding:uint;");
-break;
-  case eEncodingSint:
-response.PutCString("encoding:sint;");
-break;
-  case eEncodingIEEE754:
-response.PutCString("encoding:ieee754;");
-break;
-  case eEncodingVector:
-response.PutCString("encoding:vector;");
-break;
-  default:
-break;
-  }
+  llvm::StringRef encoding = GetEncodingNameOrEmpty(reg_info);
+  if (!encoding.empty())
+response << "encoding:" << encoding << ';';
 
-  switch (reg_info->format) {
-  case eFormatBinary:
-response.PutCString("format:binary;");
-break;
-  case eFormatDecimal:
-

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 244130.

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

https://reviews.llvm.org/D74217

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/main.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -199,6 +199,8 @@
   static std::string XMLEncodeAttributeValue(llvm::StringRef value);
 
 private:
+  llvm::Expected> BuildTargetXml();
+
   void HandleInferiorState_Exited(NativeProcessProtocol *process);
 
   void HandleInferiorState_Stopped(NativeProcessProtocol *process);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -377,6 +377,109 @@
   }
 }
 
+static llvm::StringRef GetEncodingNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->encoding) {
+  case eEncodingUint:
+return "uint";
+  case eEncodingSint:
+return "sint";
+  case eEncodingIEEE754:
+return "ieee754";
+  case eEncodingVector:
+return "vector";
+  default:
+return "";
+  }
+}
+
+static llvm::StringRef GetFormatNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->format) {
+  case eFormatBinary:
+return "binary";
+  case eFormatDecimal:
+return "decimal";
+  case eFormatHex:
+return "hex";
+  case eFormatFloat:
+return "float";
+  case eFormatVectorOfSInt8:
+return "vector-sint8";
+  case eFormatVectorOfUInt8:
+return "vector-uint8";
+  case eFormatVectorOfSInt16:
+return "vector-sint16";
+  case eFormatVectorOfUInt16:
+return "vector-uint16";
+  case eFormatVectorOfSInt32:
+return "vector-sint32";
+  case eFormatVectorOfUInt32:
+return "vector-uint32";
+  case eFormatVectorOfFloat32:
+return "vector-float32";
+  case eFormatVectorOfUInt64:
+return "vector-uint64";
+  case eFormatVectorOfUInt128:
+return "vector-uint128";
+  default:
+return "";
+  };
+}
+
+static llvm::StringRef GetKindGenericOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->kinds[RegisterKind::eRegisterKindGeneric]) {
+  case LLDB_REGNUM_GENERIC_PC:
+return "pc";
+  case LLDB_REGNUM_GENERIC_SP:
+return "sp";
+  case LLDB_REGNUM_GENERIC_FP:
+return "fp";
+  case LLDB_REGNUM_GENERIC_RA:
+return "ra";
+  case LLDB_REGNUM_GENERIC_FLAGS:
+return "flags";
+  case LLDB_REGNUM_GENERIC_ARG1:
+return "arg1";
+  case LLDB_REGNUM_GENERIC_ARG2:
+return "arg2";
+  case LLDB_REGNUM_GENERIC_ARG3:
+return "arg3";
+  case LLDB_REGNUM_GENERIC_ARG4:
+return "arg4";
+  case LLDB_REGNUM_GENERIC_ARG5:
+return "arg5";
+  case LLDB_REGNUM_GENERIC_ARG6:
+return "arg6";
+  case LLDB_REGNUM_GENERIC_ARG7:
+return "arg7";
+  case LLDB_REGNUM_GENERIC_ARG8:
+return "arg8";
+  default:
+return "";
+  }
+}
+
+static void CollectRegNums(const RegisterInfo *reg_info,
+   lldb_private::StreamString response) {
+  int i = 0;
+  for (const uint32_t *reg_num = reg_info->value_regs;
+   *reg_num != LLDB_INVALID_REGNUM; ++reg_num, ++i) {
+if (i > 0)
+  response.PutChar(',');
+response.Printf("%" PRIx32, *reg_num);
+  }
+}
+
+static void CollectInvalidateRegNums(const RegisterInfo *reg_info,
+ lldb_private::StreamString response) {
+  int i = 0;
+  for (const uint32_t *reg_num = reg_info->invalidate_regs;
+   *reg_num != LLDB_INVALID_REGNUM; ++reg_num, ++i) {
+if (i > 0)
+  response.PutChar(',');
+response.Printf("%" PRIx32, *reg_num);
+  }
+}
+
 static void WriteRegisterValueInHexFixedWidth(
 StreamString , NativeRegisterContext _ctx,
 const RegisterInfo _info, const RegisterValue *reg_value_p,
@@ -1699,74 +1802,18 @@
   response.Printf("bitsize:%" PRIu32 ";offset:%" PRIu32 ";",
   reg_info->byte_size * 8, reg_info->byte_offset);
 
-  switch (reg_info->encoding) {
-  case eEncodingUint:
-response.PutCString("encoding:uint;");
-break;
-  case eEncodingSint:
-response.PutCString("encoding:sint;");
-break;
-  case eEncodingIEEE754:
-

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 243932.

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

https://reviews.llvm.org/D74217

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/main.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -199,6 +199,8 @@
   static std::string XMLEncodeAttributeValue(llvm::StringRef value);
 
 private:
+  llvm::Expected> BuildTargetXml();
+
   void HandleInferiorState_Exited(NativeProcessProtocol *process);
 
   void HandleInferiorState_Stopped(NativeProcessProtocol *process);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -377,6 +377,109 @@
   }
 }
 
+static llvm::StringRef GetEncodingNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->encoding) {
+  case eEncodingUint:
+return "uint";
+  case eEncodingSint:
+return "sint";
+  case eEncodingIEEE754:
+return "ieee754";
+  case eEncodingVector:
+return "vector";
+  default:
+return "";
+  }
+}
+
+static llvm::StringRef GetFormatNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->format) {
+  case eFormatBinary:
+return "binary";
+  case eFormatDecimal:
+return "decimal";
+  case eFormatHex:
+return "hex";
+  case eFormatFloat:
+return "float";
+  case eFormatVectorOfSInt8:
+return "vector-sint8";
+  case eFormatVectorOfUInt8:
+return "vector-uint8";
+  case eFormatVectorOfSInt16:
+return "vector-sint16";
+  case eFormatVectorOfUInt16:
+return "vector-uint16";
+  case eFormatVectorOfSInt32:
+return "vector-sint32";
+  case eFormatVectorOfUInt32:
+return "vector-uint32";
+  case eFormatVectorOfFloat32:
+return "vector-float32";
+  case eFormatVectorOfUInt64:
+return "vector-uint64";
+  case eFormatVectorOfUInt128:
+return "vector-uint128";
+  default:
+return "";
+  };
+}
+
+static llvm::StringRef GetKindGenericOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->kinds[RegisterKind::eRegisterKindGeneric]) {
+  case LLDB_REGNUM_GENERIC_PC:
+return "pc";
+  case LLDB_REGNUM_GENERIC_SP:
+return "sp";
+  case LLDB_REGNUM_GENERIC_FP:
+return "fp";
+  case LLDB_REGNUM_GENERIC_RA:
+return "ra";
+  case LLDB_REGNUM_GENERIC_FLAGS:
+return "flags";
+  case LLDB_REGNUM_GENERIC_ARG1:
+return "arg1";
+  case LLDB_REGNUM_GENERIC_ARG2:
+return "arg2";
+  case LLDB_REGNUM_GENERIC_ARG3:
+return "arg3";
+  case LLDB_REGNUM_GENERIC_ARG4:
+return "arg4";
+  case LLDB_REGNUM_GENERIC_ARG5:
+return "arg5";
+  case LLDB_REGNUM_GENERIC_ARG6:
+return "arg6";
+  case LLDB_REGNUM_GENERIC_ARG7:
+return "arg7";
+  case LLDB_REGNUM_GENERIC_ARG8:
+return "arg8";
+  default:
+return "";
+  }
+}
+
+static void CollectRegNums(const RegisterInfo *reg_info,
+   lldb_private::StreamString response) {
+  int i = 0;
+  for (const uint32_t *reg_num = reg_info->value_regs;
+   *reg_num != LLDB_INVALID_REGNUM; ++reg_num, ++i) {
+if (i > 0)
+  response.PutChar(',');
+response.Printf("%" PRIx32, *reg_num);
+  }
+}
+
+static void CollectInvalidateRegNums(const RegisterInfo *reg_info,
+ lldb_private::StreamString response) {
+  int i = 0;
+  for (const uint32_t *reg_num = reg_info->invalidate_regs;
+   *reg_num != LLDB_INVALID_REGNUM; ++reg_num, ++i) {
+if (i > 0)
+  response.PutChar(',');
+response.Printf("%" PRIx32, *reg_num);
+  }
+}
+
 static void WriteRegisterValueInHexFixedWidth(
 StreamString , NativeRegisterContext _ctx,
 const RegisterInfo _info, const RegisterValue *reg_value_p,
@@ -1699,66 +1802,19 @@
   response.Printf("bitsize:%" PRIu32 ";offset:%" PRIu32 ";",
   reg_info->byte_size * 8, reg_info->byte_offset);
 
-  switch (reg_info->encoding) {
-  case eEncodingUint:
-response.PutCString("encoding:uint;");
-break;
-  case eEncodingSint:
-response.PutCString("encoding:sint;");
-break;
-  case eEncodingIEEE754:
-

[Lldb-commits] [PATCH] D74217: Add target.xml support for qXfer request.

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 243579.

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

https://reviews.llvm.org/D74217

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/Makefile
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/registers-target-xml-reading/main.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -377,6 +377,87 @@
   }
 }
 
+static std::string GetEncodingNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->encoding) {
+  case eEncodingUint:
+return "uint";
+  case eEncodingSint:
+return "sint";
+  case eEncodingIEEE754:
+return "ieee754";
+  case eEncodingVector:
+return "vector";
+  default:
+return "";
+  }
+}
+
+static std::string GetFormatNameOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->format) {
+  case eFormatBinary:
+return "binary";
+  case eFormatDecimal:
+return "decimal";
+  case eFormatHex:
+return "hex";
+  case eFormatFloat:
+return "float";
+  case eFormatVectorOfSInt8:
+return "vector-sint8";
+  case eFormatVectorOfUInt8:
+return "vector-uint8";
+  case eFormatVectorOfSInt16:
+return "vector-sint16";
+  case eFormatVectorOfUInt16:
+return "vector-uint16";
+  case eFormatVectorOfSInt32:
+return "vector-sint32";
+  case eFormatVectorOfUInt32:
+return "vector-uint32";
+  case eFormatVectorOfFloat32:
+return "vector-float32";
+  case eFormatVectorOfUInt64:
+return "vector-uint64";
+  case eFormatVectorOfUInt128:
+return "vector-uint128";
+  default:
+return "";
+  };
+}
+
+static std::string GetKindGenericOrEmpty(const RegisterInfo *reg_info) {
+  switch (reg_info->kinds[RegisterKind::eRegisterKindGeneric]) {
+  case LLDB_REGNUM_GENERIC_PC:
+return "pc";
+  case LLDB_REGNUM_GENERIC_SP:
+return "sp";
+  case LLDB_REGNUM_GENERIC_FP:
+return "fp";
+  case LLDB_REGNUM_GENERIC_RA:
+return "ra";
+  case LLDB_REGNUM_GENERIC_FLAGS:
+return "flags";
+  case LLDB_REGNUM_GENERIC_ARG1:
+return "arg1";
+  case LLDB_REGNUM_GENERIC_ARG2:
+return "arg2";
+  case LLDB_REGNUM_GENERIC_ARG3:
+return "arg3";
+  case LLDB_REGNUM_GENERIC_ARG4:
+return "arg4";
+  case LLDB_REGNUM_GENERIC_ARG5:
+return "arg5";
+  case LLDB_REGNUM_GENERIC_ARG6:
+return "arg6";
+  case LLDB_REGNUM_GENERIC_ARG7:
+return "arg7";
+  case LLDB_REGNUM_GENERIC_ARG8:
+return "arg8";
+  default:
+return "";
+  }
+}
+
 static void WriteRegisterValueInHexFixedWidth(
 StreamString , NativeRegisterContext _ctx,
 const RegisterInfo _info, const RegisterValue *reg_value_p,
@@ -1699,66 +1780,19 @@
   response.Printf("bitsize:%" PRIu32 ";offset:%" PRIu32 ";",
   reg_info->byte_size * 8, reg_info->byte_offset);
 
-  switch (reg_info->encoding) {
-  case eEncodingUint:
-response.PutCString("encoding:uint;");
-break;
-  case eEncodingSint:
-response.PutCString("encoding:sint;");
-break;
-  case eEncodingIEEE754:
-response.PutCString("encoding:ieee754;");
-break;
-  case eEncodingVector:
-response.PutCString("encoding:vector;");
-break;
-  default:
-break;
+  std::string encoding = GetEncodingNameOrEmpty(reg_info);
+  if (!encoding.empty()) {
+response.PutCString("encoding:");
+response.PutCString(encoding.c_str());
+response.PutChar(';');
   }
 
-  switch (reg_info->format) {
-  case eFormatBinary:
-response.PutCString("format:binary;");
-break;
-  case eFormatDecimal:
-response.PutCString("format:decimal;");
-break;
-  case eFormatHex:
-response.PutCString("format:hex;");
-break;
-  case eFormatFloat:
-response.PutCString("format:float;");
-break;
-  case eFormatVectorOfSInt8:
-response.PutCString("format:vector-sint8;");
-break;
-  case eFormatVectorOfUInt8:
-response.PutCString("format:vector-uint8;");
-break;
-  case eFormatVectorOfSInt16:
-response.PutCString("format:vector-sint16;");
-break;
-  case eFormatVectorOfUInt16:
-response.PutCString("format:vector-uint16;");
-break;
-  case eFormatVectorOfSInt32:
-response.PutCString("format:vector-sint32;");
-break;
-  case eFormatVectorOfUInt32:
-response.PutCString("format:vector-uint32;");
-break;
-  case eFormatVectorOfFloat32:
-response.PutCString("format:vector-float32;");
-break;
-  case eFormatVectorOfUInt64:
-

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 237604.

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

https://reviews.llvm.org/D70846

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp


Index: lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
@@ -21,7 +21,7 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck --check-prefix=FULL %s
+// RUN:   FileCheck --check-prefix=FULL-APPLE %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
@@ -55,14 +55,16 @@
 // METHOD-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
 // METHOD-DAG: name = "ffbar()::sbaz::foo()", mangled = 
"_ZZ5ffbarvEN4sbaz3fooEv"
 
-// FULL: Found 7 functions:
-// FULL-DAG: name = "foo()", mangled = "_Z3foov"
-// FULL-DAG: name = "foo(int)", mangled = "_Z3fooi"
-// FULL-DAG: name = "bar::foo()", mangled = "_ZN3bar3fooEv"
-// FULL-DAG: name = "bar::baz::foo()", mangled = "_ZN3bar3baz3fooEv"
-// FULL-DAG: name = "sbar::foo()", mangled = "_ZN4sbar3fooEv"
-// FULL-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
-// FULL-DAG: name = "ffbar()::sbaz::foo()", mangled = "_ZZ5ffbarvEN4sbaz3fooEv"
+// FULL-APPLE: Found 7 functions:
+// FULL-APPLE-DAG: name = "foo()", mangled = "_Z3foov"
+// FULL-APPLE-DAG: name = "foo(int)", mangled = "_Z3fooi"
+// FULL-APPLE-DAG: name = "bar::foo()", mangled = "_ZN3bar3fooEv"
+// FULL-APPLE-DAG: name = "bar::baz::foo()", mangled = "_ZN3bar3baz3fooEv"
+// FULL-APPLE-DAG: name = "sbar::foo()", mangled = "_ZN4sbar3fooEv"
+// FULL-APPLE-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
+// FULL-APPLE-DAG: name = "ffbar()::sbaz::foo()", mangled = 
"_ZZ5ffbarvEN4sbaz3fooEv"
+
+// FULL: Found 0 functions:
 
 // FULL-MANGLED: Found 1 functions:
 // FULL-MANGLED-DAG: name = "foo(int)", mangled = "_Z3fooi"
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -401,8 +401,6 @@
 
   if (name_type_mask & eFunctionNameTypeFull) {
 DIEArray offsets;
-m_set.function_basenames.Find(name, offsets);
-m_set.function_methods.Find(name, offsets);
 m_set.function_fullnames.Find(name, offsets);
 for (const DIERef _ref: offsets) {
   DWARFDIE die = dwarf.GetDIE(die_ref);
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1254,9 +1254,9 @@
 // TODO Fix FindFunctions so that it doesn't return
 //   instance methods for eFunctionNameTypeBase.
 
-target->GetImages().FindFunctions(name, eFunctionNameTypeFull,
-  include_symbols, include_inlines,
-  sc_list);
+target->GetImages().FindFunctions(
+name, eFunctionNameTypeFull | eFunctionNameTypeBase, include_symbols,
+include_inlines, sc_list);
   }
 
   // If we found more than one function, see if we can use the frame's decl


Index: lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
@@ -21,7 +21,7 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \
-// RUN:   FileCheck --check-prefix=FULL %s
+// RUN:   FileCheck --check-prefix=FULL-APPLE %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \
@@ -55,14 +55,16 @@
 // METHOD-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
 // METHOD-DAG: name = "ffbar()::sbaz::foo()", mangled = "_ZZ5ffbarvEN4sbaz3fooEv"
 
-// FULL: Found 7 functions:
-// FULL-DAG: name = "foo()", mangled = "_Z3foov"

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

Made clang-format with last update of diff.


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

https://reviews.llvm.org/D70846



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

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

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



Comment at: lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp:67
+
+// FULL: Found 0 functions:
 

shafik wrote:
> You don't actually run a `FULL` test, is this purposeful? 
FULL tests runs for target=x86_64-pc-linux. 
It runs in lines 10 and 39 in this file, If I'm not mistaken.


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

https://reviews.llvm.org/D70846



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

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat updated this revision to Diff 234899.

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

https://reviews.llvm.org/D70846

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp


Index: lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
@@ -21,7 +21,7 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck --check-prefix=FULL %s
+// RUN:   FileCheck --check-prefix=FULL-APPLE %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
@@ -55,14 +55,16 @@
 // METHOD-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
 // METHOD-DAG: name = "ffbar()::sbaz::foo()", mangled = 
"_ZZ5ffbarvEN4sbaz3fooEv"
 
-// FULL: Found 7 functions:
-// FULL-DAG: name = "foo()", mangled = "_Z3foov"
-// FULL-DAG: name = "foo(int)", mangled = "_Z3fooi"
-// FULL-DAG: name = "bar::foo()", mangled = "_ZN3bar3fooEv"
-// FULL-DAG: name = "bar::baz::foo()", mangled = "_ZN3bar3baz3fooEv"
-// FULL-DAG: name = "sbar::foo()", mangled = "_ZN4sbar3fooEv"
-// FULL-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
-// FULL-DAG: name = "ffbar()::sbaz::foo()", mangled = "_ZZ5ffbarvEN4sbaz3fooEv"
+// FULL-APPLE: Found 7 functions:
+// FULL-APPLE-DAG: name = "foo()", mangled = "_Z3foov"
+// FULL-APPLE-DAG: name = "foo(int)", mangled = "_Z3fooi"
+// FULL-APPLE-DAG: name = "bar::foo()", mangled = "_ZN3bar3fooEv"
+// FULL-APPLE-DAG: name = "bar::baz::foo()", mangled = "_ZN3bar3baz3fooEv"
+// FULL-APPLE-DAG: name = "sbar::foo()", mangled = "_ZN4sbar3fooEv"
+// FULL-APPLE-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
+// FULL-APPLE-DAG: name = "ffbar()::sbaz::foo()", mangled = 
"_ZZ5ffbarvEN4sbaz3fooEv"
+
+// FULL: Found 0 functions:
 
 // FULL-MANGLED: Found 1 functions:
 // FULL-MANGLED-DAG: name = "foo(int)", mangled = "_Z3fooi"
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -401,8 +401,6 @@
 
   if (name_type_mask & eFunctionNameTypeFull) {
 DIEArray offsets;
-m_set.function_basenames.Find(name, offsets);
-m_set.function_methods.Find(name, offsets);
 m_set.function_fullnames.Find(name, offsets);
 for (const DIERef _ref: offsets) {
   DWARFDIE die = dwarf.GetDIE(die_ref);
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1273,7 +1273,7 @@
 // TODO Fix FindFunctions so that it doesn't return
 //   instance methods for eFunctionNameTypeBase.
 
-target->GetImages().FindFunctions(name, eFunctionNameTypeFull,
+target->GetImages().FindFunctions(name, eFunctionNameTypeFull | 
eFunctionNameTypeBase,
   include_symbols, include_inlines,
   sc_list);
   }


Index: lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
@@ -21,7 +21,7 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method %t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t | \
-// RUN:   FileCheck --check-prefix=FULL %s
+// RUN:   FileCheck --check-prefix=FULL-APPLE %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full %t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function --function-flags=base %t | \
@@ -55,14 +55,16 @@
 // METHOD-DAG: name = "sbar::foo(int)", mangled = "_ZN4sbar3fooEi"
 // METHOD-DAG: name = "ffbar()::sbaz::foo()", mangled = "_ZZ5ffbarvEN4sbaz3fooEv"
 
-// FULL: Found 7 functions:
-// FULL-DAG: name = "foo()", mangled = "_Z3foov"
-// FULL-DAG: name = "foo(int)", mangled = "_Z3fooi"
-// FULL-DAG: name = "bar::foo()", mangled = "_ZN3bar3fooEv"
-// FULL-DAG: name = 

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat added a comment.

It seems like with this CL find-basic-functions logic became inconsistent for 
macos target and pc-linux.
In cases of pc-linux it returns no function with --function-flags=full flag and 
in macos it returns 7 functions.

I think there are 3 ways to fix this:

1. Fix the find-basic-functions by splitting checks for mac and pc.
2. Make mac logic same as linux. Fix this on mac GetFunctions method as well.
3. Revert to old logic for eFunctionNameTypeFull and add one more enum value 
like "eFunctionNameTypeFullMangled" which will return only function_fullnames 
and will be supported only in pc version.

Which are the most consistent way for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70846



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

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat created this revision.
PatriosTheGreat added reviewers: labath, jarin, aprantl.
PatriosTheGreat added a project: LLDB.
Herald added a subscriber: arphaman.

This change is connected with
https://reviews.llvm.org/D69843

In large codebases, we sometimes see Module::FindFunctions (when called from
ClangExpressionDeclMap::FindExternalVisibleDecls) returning huge amounts of
functions.

In current fix I trying to return only function_fullnames from 
ManualDWARFIndex::GetFunctions when eFunctionNameTypeFull is passed as argument.

I run check-lldb on this changes on linux machine and it seems like there are 
no additional failing tests compared with master. It seems a bit weird that 
there are no other places in code were it is expected that 
eFunctionNameTypeFull on GetFunctions will return fullnames + basenames + 
function_methods.
Is there any additional tests which I can run to be sure that I didn't break 
anything?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70846

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -401,8 +401,6 @@
 
   if (name_type_mask & eFunctionNameTypeFull) {
 DIEArray offsets;
-m_set.function_basenames.Find(name, offsets);
-m_set.function_methods.Find(name, offsets);
 m_set.function_fullnames.Find(name, offsets);
 for (const DIERef _ref: offsets) {
   DWARFDIE die = dwarf.GetDIE(die_ref);
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1210,7 +1210,7 @@
 // TODO Fix FindFunctions so that it doesn't return
 //   instance methods for eFunctionNameTypeBase.
 
-target->GetImages().FindFunctions(name, eFunctionNameTypeFull,
+target->GetImages().FindFunctions(name, eFunctionNameTypeFull | 
eFunctionNameTypeBase,
   include_symbols, include_inlines,
   sc_list);
   }


Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -401,8 +401,6 @@
 
   if (name_type_mask & eFunctionNameTypeFull) {
 DIEArray offsets;
-m_set.function_basenames.Find(name, offsets);
-m_set.function_methods.Find(name, offsets);
 m_set.function_fullnames.Find(name, offsets);
 for (const DIERef _ref: offsets) {
   DWARFDIE die = dwarf.GetDIE(die_ref);
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1210,7 +1210,7 @@
 // TODO Fix FindFunctions so that it doesn't return
 //   instance methods for eFunctionNameTypeBase.
 
-target->GetImages().FindFunctions(name, eFunctionNameTypeFull,
+target->GetImages().FindFunctions(name, eFunctionNameTypeFull | eFunctionNameTypeBase,
   include_symbols, include_inlines,
   sc_list);
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits