date:20200927

[Lldb-commits] [PATCH] D88123: Add the ability to write 'target stop-hooks' in Python

2020-09-27 Thread David Zarzycki via Phabricator via lldb-commits

davezarzycki added a comment.

Hi Jim, this change broke my Fedora 33 Linux (x86) box. Do you think we can get 
a quick fix or revert this?

  FAIL: lldb-shell :: Commands/command-stop-hook-output.test (69796 of 70913)
   TEST 'lldb-shell :: 
Commands/command-stop-hook-output.test' FAILED 
  Script:
  --
  : 'RUN: at line 1';   /tmp/_update_lc/r/bin/clang 
--target=specify-a-target-or-use-a-_host-substitution 
--target=x86_64-unknown-linux-gnu -pthread 
-fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell
 -g /home/dave/ro_s/lp/lldb/test/Shell/Commands/Inputs/main.c -o 
/tmp/_update_lc/r/tools/lldb/test/Commands/Output/command-stop-hook-output.test.tmp
  : 'RUN: at line 2';   /tmp/_update_lc/r/bin/lldb --no-lldbinit -S 
/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init 
/tmp/_update_lc/r/tools/lldb/test/Commands/Output/command-stop-hook-output.test.tmp
 -O 'command script import 
/home/dave/ro_s/lp/lldb/test/Shell/Commands/Inputs/stop_hook.py' -s 
/home/dave/ro_s/lp/lldb/test/Shell/Commands/command-stop-hook-output.test -o 
exit | /tmp/_update_lc/r/bin/FileCheck 
/home/dave/ro_s/lp/lldb/test/Shell/Commands/command-stop-hook-output.test
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  clang-12: warning: argument unused during compilation: 
'-fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell'
 [-Wunused-command-line-argument]
  error: module importing failed: This script interpreter does not support 
importing modules.
  
/home/dave/ro_s/lp/lldb/test/Shell/Commands/command-stop-hook-output.test:5:16: 
error: CHECK-LABEL: expected string not found in input
  # CHECK-LABEL: b main
 ^
  :1:1: note: scanning from here
  (lldb) command source -s 0 
'/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init'
  ^
  :1:8: note: possible intended match here
  (lldb) command source -s 0 
'/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init'
 ^
  
  Input file: 
  Check file: 
/home/dave/ro_s/lp/lldb/test/Shell/Commands/command-stop-hook-output.test
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
 1: (lldb) command source -s 0 
'/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init'
  label:5'0 
X~
 error: no match found
  label:5'1?
   possible intended match
 2: Executing commands in 
'/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init'.
  label:5'0 
~~
 3: (lldb) # LLDB init file for the LIT tests.
  label:5'0 ~~
 4: (lldb) settings set symbols.enable-external-lookup false
  label:5'0 
 5: (lldb) settings set plugin.process.gdb-remote.packet-timeout 60
  label:5'0 ~~~
 6: (lldb) settings set interpreter.echo-comment-commands false
  label:5'0 ~~~
 .
 .
 .
  >>
  
  --
  
  
  
  Failed Tests (1):
lldb-shell :: Commands/command-stop-hook-output.test
  
  
  Testing Time: 66.80s
Unsupported  : 10853
Passed   : 59957
Expectedly Failed:   102
Failed   : 1
  FAILED: CMakeFiles/check-all


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88123

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

[Lldb-commits] [PATCH] D88375: Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

2020-09-27 Thread Tatsuo Nomura via Phabricator via lldb-commits

tatsuo created this revision.
tatsuo added reviewers: LLDB, jasonmolenda.
Herald added subscribers: lldb-commits, atanasyan, jrtc27, arichardson, sdardis.
Herald added a project: LLDB.
tatsuo requested review of this revision.
Herald added a subscriber: JDevlieghere.

RegInfoBasedABI::GetRegisterInfoByName was failing because mips/mips64 ABIs 
don't use ConstString in their register info array.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88375

Files:
  lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp


Index: lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
===
--- lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
+++ lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
@@ -75,7 +75,7 @@
   dwarf_pc
 };
 
-static const RegisterInfo g_register_infos_mips64[] = {
+static RegisterInfo g_register_infos_mips64[] = {
 //  NAME  ALTSZ OFF ENCODINGFORMAT EH_FRAME
 //  DWARF   GENERIC PROCESS PLUGIN
 //  LLDB NATIVE
@@ -542,9 +542,24 @@
 
 static const uint32_t k_num_register_infos =
 llvm::array_lengthof(g_register_infos_mips64);
+static bool g_register_info_names_constified = false;
 
 const lldb_private::RegisterInfo *
 ABISysV_mips64::GetRegisterInfoArray(uint32_t &count) {
+  // Make the C-string names and alt_names for the register infos into const
+  // C-string values by having the ConstString unique the names in the global
+  // constant C-string pool.
+  if (!g_register_info_names_constified) {
+g_register_info_names_constified = true;
+for (uint32_t i = 0; i < k_num_register_infos; ++i) {
+  if (g_register_infos_mips64[i].name)
+g_register_infos_mips64[i].name =
+ConstString(g_register_infos_mips64[i].name).GetCString();
+  if (g_register_infos_mips64[i].alt_name)
+g_register_infos_mips64[i].alt_name =
+ConstString(g_register_infos_mips64[i].alt_name).GetCString();
+}
+  }
   count = k_num_register_infos;
   return g_register_infos_mips64;
 }
Index: lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
===
--- lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
+++ lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
@@ -75,7 +75,7 @@
   dwarf_pc
 };
 
-static const RegisterInfo g_register_infos[] = {
+static RegisterInfo g_register_infos[] = {
 //  NAME  ALTSZ OFF ENCODINGFORMAT EH_FRAME
 //  DWARF   GENERIC PROCESS PLUGINS
 //  LLDB NATIVEVALUE REGS  INVALIDATE REGS
@@ -542,9 +542,24 @@
 
 static const uint32_t k_num_register_infos =
 llvm::array_lengthof(g_register_infos);
+static bool g_register_info_names_constified = false;
 
 const lldb_private::RegisterInfo *
 ABISysV_mips::GetRegisterInfoArray(uint32_t &count) {
+  // Make the C-string names and alt_names for the register infos into const
+  // C-string values by having the ConstString unique the names in the global
+  // constant C-string pool.
+  if (!g_register_info_names_constified) {
+g_register_info_names_constified = true;
+for (uint32_t i = 0; i < k_num_register_infos; ++i) {
+  if (g_register_infos[i].name)
+g_register_infos[i].name =
+ConstString(g_register_infos[i].name).GetCString();
+  if (g_register_infos[i].alt_name)
+g_register_infos[i].alt_name =
+ConstString(g_register_infos[i].alt_name).GetCString();
+}
+  }
   count = k_num_register_infos;
   return g_register_infos;
 }


Index: lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
===
--- lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
+++ lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
@@ -75,7 +75,7 @@
   dwarf_pc
 };
 
-static const RegisterInfo g_register_infos_mips64[] = {
+static RegisterInfo g_register_infos_mips64[] = {
 //  NAME  ALTSZ OFF ENCODINGFORMAT EH_FRAME
 //  DWARF   GENERIC PROCESS PLUGIN
 //  LLDB NATIVE
@@ -542,9 +542,24 @@
 
 static const uint32_t k_num_register_infos =
 llvm::array_lengthof(g_register_infos_mips64);
+static bool g_register_info_names_constified = false;
 
 const lldb_private::RegisterInfo *
 ABISysV_mips64::GetRegisterInfoArray(uint32_t &count) {
+  // Make the C-string names and alt_names for the register infos into const
+  // C-string values by having the ConstString unique the names in the global
+  // constant C-string pool.
+  if (!g_register_info_names_constified) {
+g_register_info_names_constified = true;
+for (uint32_t i = 0; i < k_num_register_infos; ++i) {
+  if (g_register_infos_mips64[i].name)
+g_register_infos_mips64[i].name =
+ConstString(g_register_infos_mips64[i].name).GetCString();
+  if (g_register_infos

[Lldb-commits] [PATCH] D88375: Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

2020-09-27 Thread Raphael Isemann via Phabricator via lldb-commits

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

To clarify the description a bit: `RegInfoBasedABI::GetRegisterInfoByName` is 
comparing the name/alt_name by doing a `const char *` pointer value comparison. 
That only works if both strings that are compared are coming from a 
ConstString. Since b0060c3a7868ef026d95d0cf8a076791ef74f474 
 
`GetRegisterInfoByName` is checking with an assert that both C strings came 
from a ConstString (which is failing as these ABI implementation changed here 
are lacking the ConstString'ification code).

Obviously copying this code around isn't ideal, so this patch is just to get 
the backend at least running again until I get around to refactor this code.

Anyway, this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88375

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

[Lldb-commits] [PATCH] D88375: Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

2020-09-27 Thread Tatsuo Nomura via Phabricator via lldb-commits

tatsuo added a comment.

@teemperor Thanks for the quick review and context!
I don't have commit access, that'd be great if you could merge it in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88375

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

[Lldb-commits] [PATCH] D88375: Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

2020-09-27 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe779427757f2: Fix MIPS and MIPS64 ABI to use ConstString in 
their register info arrays. (authored by tatsuo, committed by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88375

Files:
  lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp


Index: lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
===
--- lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
+++ lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
@@ -75,7 +75,7 @@
   dwarf_pc
 };
 
-static const RegisterInfo g_register_infos_mips64[] = {
+static RegisterInfo g_register_infos_mips64[] = {
 //  NAME  ALTSZ OFF ENCODINGFORMAT EH_FRAME
 //  DWARF   GENERIC PROCESS PLUGIN
 //  LLDB NATIVE
@@ -542,9 +542,24 @@
 
 static const uint32_t k_num_register_infos =
 llvm::array_lengthof(g_register_infos_mips64);
+static bool g_register_info_names_constified = false;
 
 const lldb_private::RegisterInfo *
 ABISysV_mips64::GetRegisterInfoArray(uint32_t &count) {
+  // Make the C-string names and alt_names for the register infos into const
+  // C-string values by having the ConstString unique the names in the global
+  // constant C-string pool.
+  if (!g_register_info_names_constified) {
+g_register_info_names_constified = true;
+for (uint32_t i = 0; i < k_num_register_infos; ++i) {
+  if (g_register_infos_mips64[i].name)
+g_register_infos_mips64[i].name =
+ConstString(g_register_infos_mips64[i].name).GetCString();
+  if (g_register_infos_mips64[i].alt_name)
+g_register_infos_mips64[i].alt_name =
+ConstString(g_register_infos_mips64[i].alt_name).GetCString();
+}
+  }
   count = k_num_register_infos;
   return g_register_infos_mips64;
 }
Index: lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
===
--- lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
+++ lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
@@ -75,7 +75,7 @@
   dwarf_pc
 };
 
-static const RegisterInfo g_register_infos[] = {
+static RegisterInfo g_register_infos[] = {
 //  NAME  ALTSZ OFF ENCODINGFORMAT EH_FRAME
 //  DWARF   GENERIC PROCESS PLUGINS
 //  LLDB NATIVEVALUE REGS  INVALIDATE REGS
@@ -542,9 +542,24 @@
 
 static const uint32_t k_num_register_infos =
 llvm::array_lengthof(g_register_infos);
+static bool g_register_info_names_constified = false;
 
 const lldb_private::RegisterInfo *
 ABISysV_mips::GetRegisterInfoArray(uint32_t &count) {
+  // Make the C-string names and alt_names for the register infos into const
+  // C-string values by having the ConstString unique the names in the global
+  // constant C-string pool.
+  if (!g_register_info_names_constified) {
+g_register_info_names_constified = true;
+for (uint32_t i = 0; i < k_num_register_infos; ++i) {
+  if (g_register_infos[i].name)
+g_register_infos[i].name =
+ConstString(g_register_infos[i].name).GetCString();
+  if (g_register_infos[i].alt_name)
+g_register_infos[i].alt_name =
+ConstString(g_register_infos[i].alt_name).GetCString();
+}
+  }
   count = k_num_register_infos;
   return g_register_infos;
 }


Index: lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
===
--- lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
+++ lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
@@ -75,7 +75,7 @@
   dwarf_pc
 };
 
-static const RegisterInfo g_register_infos_mips64[] = {
+static RegisterInfo g_register_infos_mips64[] = {
 //  NAME  ALTSZ OFF ENCODINGFORMAT EH_FRAME
 //  DWARF   GENERIC PROCESS PLUGIN
 //  LLDB NATIVE
@@ -542,9 +542,24 @@
 
 static const uint32_t k_num_register_infos =
 llvm::array_lengthof(g_register_infos_mips64);
+static bool g_register_info_names_constified = false;
 
 const lldb_private::RegisterInfo *
 ABISysV_mips64::GetRegisterInfoArray(uint32_t &count) {
+  // Make the C-string names and alt_names for the register infos into const
+  // C-string values by having the ConstString unique the names in the global
+  // constant C-string pool.
+  if (!g_register_info_names_constified) {
+g_register_info_names_constified = true;
+for (uint32_t i = 0; i < k_num_register_infos; ++i) {
+  if (g_register_infos_mips64[i].name)
+g_register_infos_mips64[i].name =
+ConstString(g_register_infos_mips64[i].name).GetCString();
+  if (g_register_infos_mips64[i].alt_name)
+g_register_infos_mi

[Lldb-commits] [PATCH] D88375: Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

2020-09-27 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

Done. Thanks for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88375

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

[Lldb-commits] [lldb] e779427 - Fix MIPS and MIPS64 ABI to use ConstString in their register info arrays.

2020-09-27 Thread Raphael Isemann via lldb-commits


Author: Tatsuo Nomura
Date: 2020-09-27T12:36:09+02:00
New Revision: e779427757f233ef7bbb381289c63f2399ecca92

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

LOG: Fix MIPS and MIPS64 ABI to use ConstString in their register info arrays.

RegInfoBasedABI::GetRegisterInfoByName was failing because mips/mips64 ABIs
don't use ConstString in their register info array.

Reviewed By: #lldb, teemperor

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

Added: 


Modified: 
lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp 
b/lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
index d66e0926ad99..a209fa760556 100644
--- a/lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
+++ b/lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
@@ -75,7 +75,7 @@ enum dwarf_regnums {
   dwarf_pc
 };
 
-static const RegisterInfo g_register_infos[] = {
+static RegisterInfo g_register_infos[] = {
 //  NAME  ALTSZ OFF ENCODINGFORMAT EH_FRAME
 //  DWARF   GENERIC PROCESS PLUGINS
 //  LLDB NATIVEVALUE REGS  INVALIDATE REGS
@@ -542,9 +542,24 @@ static const RegisterInfo g_register_infos[] = {
 
 static const uint32_t k_num_register_infos =
 llvm::array_lengthof(g_register_infos);
+static bool g_register_info_names_constified = false;
 
 const lldb_private::RegisterInfo *
 ABISysV_mips::GetRegisterInfoArray(uint32_t &count) {
+  // Make the C-string names and alt_names for the register infos into const
+  // C-string values by having the ConstString unique the names in the global
+  // constant C-string pool.
+  if (!g_register_info_names_constified) {
+g_register_info_names_constified = true;
+for (uint32_t i = 0; i < k_num_register_infos; ++i) {
+  if (g_register_infos[i].name)
+g_register_infos[i].name =
+ConstString(g_register_infos[i].name).GetCString();
+  if (g_register_infos[i].alt_name)
+g_register_infos[i].alt_name =
+ConstString(g_register_infos[i].alt_name).GetCString();
+}
+  }
   count = k_num_register_infos;
   return g_register_infos;
 }

diff  --git a/lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp 
b/lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
index 751555722dac..9a07c3398e19 100644
--- a/lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
+++ b/lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
@@ -75,7 +75,7 @@ enum dwarf_regnums {
   dwarf_pc
 };
 
-static const RegisterInfo g_register_infos_mips64[] = {
+static RegisterInfo g_register_infos_mips64[] = {
 //  NAME  ALTSZ OFF ENCODINGFORMAT EH_FRAME
 //  DWARF   GENERIC PROCESS PLUGIN
 //  LLDB NATIVE
@@ -542,9 +542,24 @@ static const RegisterInfo g_register_infos_mips64[] = {
 
 static const uint32_t k_num_register_infos =
 llvm::array_lengthof(g_register_infos_mips64);
+static bool g_register_info_names_constified = false;
 
 const lldb_private::RegisterInfo *
 ABISysV_mips64::GetRegisterInfoArray(uint32_t &count) {
+  // Make the C-string names and alt_names for the register infos into const
+  // C-string values by having the ConstString unique the names in the global
+  // constant C-string pool.
+  if (!g_register_info_names_constified) {
+g_register_info_names_constified = true;
+for (uint32_t i = 0; i < k_num_register_infos; ++i) {
+  if (g_register_infos_mips64[i].name)
+g_register_infos_mips64[i].name =
+ConstString(g_register_infos_mips64[i].name).GetCString();
+  if (g_register_infos_mips64[i].alt_name)
+g_register_infos_mips64[i].alt_name =
+ConstString(g_register_infos_mips64[i].alt_name).GetCString();
+}
+  }
   count = k_num_register_infos;
   return g_register_infos_mips64;
 }



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

[Lldb-commits] [PATCH] D88123: Add the ability to write 'target stop-hooks' in Python

2020-09-27 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

This also doesn't work on my macOS system:

  FAIL: LLDB (/Users/teemperor/1llvm/rel/bin/clang-x86_64) :: 
test_stop_hooks_scripted_return_false (TestStopHookScripted.TestStopHooks)
  ==
  FAIL: test_stop_hooks_scripted_return_false 
(TestStopHookScripted.TestStopHooks)
 Test that the returning False from a stop hook works
  --
  Traceback (most recent call last):
File 
"/Users/teemperor/1llvm/llvm-project/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py",
 line 54, in test_stop_hooks_scripted_return_false
  self.do_test_auto_continue(True)
File 
"/Users/teemperor/1llvm/llvm-project/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py",
 line 91, in do_test_auto_continue
  self.assertEqual("main", func_name, "Didn't stop at the expected 
function.")
  AssertionError: 'main' != 'step_out_of_me'
  - main+ step_out_of_me : Didn't stop at the expected function.
  Config=x86_64-/Users/teemperor/1llvm/rel/bin/clang
  --


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88123

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

[Lldb-commits] [PATCH] D88387: Create "skinny corefiles" for Mach-O with process save-core / reading

2020-09-27 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda created this revision.
jasonmolenda added reviewers: jingham, clayborg.
jasonmolenda added a project: LLDB.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a reviewer: JDevlieghere.
jasonmolenda requested review of this revision.

I implemented something that's been talked about for a while -- creating 
"skinny corefiles", or a user process corefile that only includes dirty memory 
in it.  This is the patchset to implement this with lldb's "process save-core" 
and lldb's corefile reading, for Mach-O files.

Because of how the system libraries are shared across all processes on Darwin 
systems, dumping all available memory for a process involves writing multiple 
gigabytes for even a trivial command line app.  The majority of this corefile 
are unmodified memory pages for the system libraries; it's a very expensive and 
slow operation.

This patchset does a few things:

1. Adds support in debugserver to find the list of pages in a memory region 
that are dirty.  It adds a key-value pair to qHostInfo to advertise how large a 
VM memory page is on the target system, and it includes a list of dirty pages 
for a memory region in the qMemoryRegionInfo reply to lldb's queries about 
memory regions.

2. It adds a new LC_NOTE "all image infos" in the corefile, which specifies all 
binary images that are present in the inferior process and where they are 
loaded in memory.  The filepaths, UUIDs, and segment name+load addresses are 
required because lldb will not have access to dyld's dyld_all_image_infos 
structure or the ability to read these binaries out of memory.  (normally the 
segment load addresses could be found by reading the Mach-O load commands for 
the binaries from the corefile)

3. It adds a new LC_NOTE "executing uuids" in the corefile, which is a list of 
the UUIDs of the binary images that are actually executing code at the point 
when the corefile was written.  That is, the set of binaries that appear on any 
backtrace on any thread at the point of the coredump.  A graphical app on macOS 
these days can easily have 500 different binary images loaded in the process; 
likely only a couple dozen of those are actually executing.  Knowing which 
binary images are executing allows us to do a more expensive search for these 
most-important binaries, like we do with crashlog.py.

4. Changes the Mach-O corefile creation to only include dirty pages when 
communicating with a debugserver that provides this information.

5. Read and use the "all image infos" and "executing uuids" in ProcessMachCore.

6. Finally, of course, adds a test case where we have 3 binaries (one 
executable, two dylibs), hits a breakpoint, saves a skinny corefile.  Then it 
moves the executable to a hidden location that lldb can only discover by 
calling the dsymForUUID script (to test 'executing uuids'), deletes one dylib, 
and leaves one at the expected location.  It loads the skinny corefile into 
lldb and confirms that the present libraries are present, the absent library is 
absent (and its non-dirty memory is unreadable), and we can read variables that 
are on the heap/stack correctly.

Before this change, the minimum size for a corefile on macOS 10.15 was around 
2GB and it would take around 5 minutes to dump.  With this change, a simple 
test binary user process corefile is around 500KB and takes a few seconds to 
dump.  Larger real-world applications will have larger amounts of stack & heap 
and will have correspondingly larger corefiles.

I'm not thrilled that I'm touching the same parts of lldb as David Spickett in 
https://reviews.llvm.org/D87442 - we're going to create conflicts depending on 
which of us lands first.  Both of us need to modify the qMemoryRegionInfo 
packet and lldb's MemoryRegionInfo class to store our additional informations.

The majority of this patch concerns ObjectFileMachO and ProcessMachCore, as 
well as debugserver; I expect those to be of less interest to any 
reviewer/commenters.  But of course I welcome any suggestions/comments to those 
parts as well as the other parts of lldb I touched along the way.  I did need 
to add some API/structs in the ObjectFile base class so ObjectFileMachO could 
pass information up to ProcessMachCore that are very specific to what I'm doing 
here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88387

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/test/API/macosx/skinny-corefile/Makefile
  lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
  lldb/test/API/macosx/skinny-corefile/main.c
  lldb/test/API/macosx

[Lldb-commits] [PATCH] D88123: Add the ability to write 'target stop-hooks' in Python

[Lldb-commits] [PATCH] D88375: Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

[Lldb-commits] [PATCH] D88375: Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

[Lldb-commits] [PATCH] D88375: Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

[Lldb-commits] [PATCH] D88375: Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

[Lldb-commits] [PATCH] D88375: Fix MIPS and MIPS64 ABI to use ConstString in thier register info arrays.

[Lldb-commits] [lldb] e779427 - Fix MIPS and MIPS64 ABI to use ConstString in their register info arrays.

[Lldb-commits] [PATCH] D88123: Add the ability to write 'target stop-hooks' in Python

[Lldb-commits] [PATCH] D88387: Create "skinny corefiles" for Mach-O with process save-core / reading

9 matches

Site Navigation

Mail list logo

Footer information