date:20220505

[Lldb-commits] [lldb] f416e57 - [lldb] Fix ppc64 detection in lldb

2022-05-05 Thread via lldb-commits


Author: serge-sans-paille
Date: 2022-05-05T09:22:02+02:00
New Revision: f416e57339bd3315834e6c14d6476e63ccb363ff

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

LOG: [lldb] Fix ppc64 detection in lldb

Currently, ppc64le and ppc64 (defaulting to big endian) have the same
descriptor, thus the linear scan always return ppc64le. Handle that through
subtype.

This is a recommit of f114f009486816ed4b3bf984f0fbbb8fc80914f6 with a new test
setup that doesn't involves (unsupported) corefiles.

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

Added: 
lldb/test/Shell/Breakpoint/Inputs/ppc64le-localentry.s
lldb/test/Shell/Breakpoint/ppc64le-localentry.test

Modified: 
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Utility/ArchSpec.cpp
lldb/test/Shell/Breakpoint/ppc64-localentry.test

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 684d070c54d26..28ccfbe3d6e64 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -310,9 +310,19 @@ static uint32_t riscvVariantFromElfFlags(const 
elf::ELFHeader &header) {
   }
 }
 
+static uint32_t ppc64VariantFromElfFlags(const elf::ELFHeader &header) {
+  uint32_t endian = header.e_ident[EI_DATA];
+  if (endian == ELFDATA2LSB)
+return ArchSpec::eCore_ppc64le_generic;
+  else
+return ArchSpec::eCore_ppc64_generic;
+}
+
 static uint32_t subTypeFromElfHeader(const elf::ELFHeader &header) {
   if (header.e_machine == llvm::ELF::EM_MIPS)
 return mipsVariantFromElfFlags(header);
+  else if (header.e_machine == llvm::ELF::EM_PPC64)
+return ppc64VariantFromElfFlags(header);
   else if (header.e_machine == llvm::ELF::EM_RISCV)
 return riscvVariantFromElfFlags(header);
 

diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index 963005b5cdfa7..c6feacfc7c1e6 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -358,10 +358,10 @@ static const ArchDefinitionEntry g_elf_arch_entries[] = {
  0xu, 0xu}, // Intel MCU // FIXME: is this correct?
 {ArchSpec::eCore_ppc_generic, llvm::ELF::EM_PPC, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // PowerPC
-{ArchSpec::eCore_ppc64le_generic, llvm::ELF::EM_PPC64, 
LLDB_INVALID_CPUTYPE,
- 0xu, 0xu}, // PowerPC64le
-{ArchSpec::eCore_ppc64_generic, llvm::ELF::EM_PPC64, LLDB_INVALID_CPUTYPE,
- 0xu, 0xu}, // PowerPC64
+{ArchSpec::eCore_ppc64le_generic, llvm::ELF::EM_PPC64,
+ ArchSpec::eCore_ppc64le_generic, 0xu, 0xu}, // PowerPC64le
+{ArchSpec::eCore_ppc64_generic, llvm::ELF::EM_PPC64,
+ ArchSpec::eCore_ppc64_generic, 0xu, 0xu}, // PowerPC64
 {ArchSpec::eCore_arm_generic, llvm::ELF::EM_ARM, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARM
 {ArchSpec::eCore_arm_aarch64, llvm::ELF::EM_AARCH64, LLDB_INVALID_CPUTYPE,
@@ -400,8 +400,8 @@ static const ArchDefinitionEntry g_elf_arch_entries[] = {
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARC
-{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE,
- 0xu, 0xu}, // AVR
+{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE, 0xu,
+ 0xu}, // AVR
 {ArchSpec::eCore_riscv32, llvm::ELF::EM_RISCV,
  ArchSpec::eRISCVSubType_riscv32, 0xu, 0xu}, // riscv32
 {ArchSpec::eCore_riscv64, llvm::ELF::EM_RISCV,

diff  --git a/lldb/test/Shell/Breakpoint/Inputs/ppc64le-localentry.s 
b/lldb/test/Shell/Breakpoint/Inputs/ppc64le-localentry.s
new file mode 100644
index 0..5fe0a97d06d38
--- /dev/null
+++ b/lldb/test/Shell/Breakpoint/Inputs/ppc64le-localentry.s
@@ -0,0 +1,55 @@
+   .text
+   .abiversion 2
+
+   .globl  lfunc
+   .p2align4
+   .type   lfunc,@function
+lfunc:  # @lfunc
+.Lfunc_begin0:
+.Lfunc_gep0:
+   addis 2, 12, .TOC.-.Lfunc_gep0@ha
+   addi 2, 2, .TOC.-.Lfunc_gep0@l
+.Lfunc_lep0:
+   .localentry lfunc, .Lfunc_lep0-.Lfunc_gep0
+# BB#0:
+   mr 4, 3
+   addis 3, 2, .LC0@toc@ha
+   ld 3, .LC0@toc@l(3)
+   stw 4, -12(1)
+   lwz 4, 0(3)
+   lwz 5, -12(1)
+   mullw 4, 4, 5
+   extsw 3, 4
+   blr
+   .long   0
+   .quad   0
+.Lfunc_end0:
+   .size   lfunc, .Lfunc_end0-.Lfunc_begin0
+
+   .globl  simple
+   .p2align4
+   .type   simple,@function
+simple: # @simple
+.L

[Lldb-commits] [PATCH] D124760: [lldb] Fix ppc64 detection in lldb

2022-05-05 Thread serge via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf416e57339bd: [lldb] Fix ppc64 detection in lldb (authored 
by serge-sans-paille).

Changed prior to commit:
  https://reviews.llvm.org/D124760?vs=427119&id=427225#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124760

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/Breakpoint/Inputs/ppc64le-localentry.s
  lldb/test/Shell/Breakpoint/ppc64-localentry.test
  lldb/test/Shell/Breakpoint/ppc64le-localentry.test

Index: lldb/test/Shell/Breakpoint/ppc64le-localentry.test
===
--- lldb/test/Shell/Breakpoint/ppc64le-localentry.test
+++ lldb/test/Shell/Breakpoint/ppc64le-localentry.test
@@ -1,6 +1,6 @@
 # REQUIRES: powerpc
 #
-# RUN: llvm-mc -triple=powerpc64le -filetype=obj %p/Inputs/ppc64-localentry.s -o %t
+# RUN: llvm-mc -triple=powerpc64le -filetype=obj %p/Inputs/ppc64le-localentry.s -o %t
 # RUN: lldb-test breakpoints %t %s | FileCheck %s
 
 breakpoint set -n lfunc
Index: lldb/test/Shell/Breakpoint/ppc64-localentry.test
===
--- lldb/test/Shell/Breakpoint/ppc64-localentry.test
+++ lldb/test/Shell/Breakpoint/ppc64-localentry.test
@@ -1,6 +1,6 @@
 # REQUIRES: powerpc
 #
-# RUN: llvm-mc -triple=powerpc64le -filetype=obj %p/Inputs/ppc64-localentry.s -o %t
+# RUN: llvm-mc -triple=powerpc64 -filetype=obj %p/Inputs/ppc64-localentry.s -o %t
 # RUN: lldb-test breakpoints %t %s | FileCheck %s
 
 breakpoint set -n lfunc
Index: lldb/test/Shell/Breakpoint/Inputs/ppc64le-localentry.s
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/ppc64le-localentry.s
@@ -0,0 +1,55 @@
+	.text
+	.abiversion 2
+
+	.globl	lfunc
+	.p2align	4
+	.type	lfunc,@function
+lfunc:  # @lfunc
+.Lfunc_begin0:
+.Lfunc_gep0:
+	addis 2, 12, .TOC.-.Lfunc_gep0@ha
+	addi 2, 2, .TOC.-.Lfunc_gep0@l
+.Lfunc_lep0:
+	.localentry	lfunc, .Lfunc_lep0-.Lfunc_gep0
+# BB#0:
+	mr 4, 3
+	addis 3, 2, .LC0@toc@ha
+	ld 3, .LC0@toc@l(3)
+	stw 4, -12(1)
+	lwz 4, 0(3)
+	lwz 5, -12(1)
+	mullw 4, 4, 5
+	extsw 3, 4
+	blr
+	.long	0
+	.quad	0
+.Lfunc_end0:
+	.size	lfunc, .Lfunc_end0-.Lfunc_begin0
+
+	.globl	simple
+	.p2align	4
+	.type	simple,@function
+simple: # @simple
+.Lfunc_begin1:
+# %bb.0:# %entry
+	mr 4, 3
+	stw 4, -12(1)
+	lwz 4, -12(1)
+	mulli 4, 4, 10
+	extsw 3, 4
+	blr
+	.long	0
+	.quad	0
+.Lfunc_end1:
+	.size	simple, .Lfunc_end1-.Lfunc_begin1
+
+	.section	.toc,"aw",@progbits
+.LC0:
+	.tc g_foo[TC],g_foo
+	.type	g_foo,@object   # @g_foo
+	.data
+	.globl	g_foo
+	.p2align	2
+g_foo:
+	.long	2   # 0x2
+	.size	g_foo, 4
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -358,10 +358,10 @@
  0xu, 0xu}, // Intel MCU // FIXME: is this correct?
 {ArchSpec::eCore_ppc_generic, llvm::ELF::EM_PPC, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // PowerPC
-{ArchSpec::eCore_ppc64le_generic, llvm::ELF::EM_PPC64, LLDB_INVALID_CPUTYPE,
- 0xu, 0xu}, // PowerPC64le
-{ArchSpec::eCore_ppc64_generic, llvm::ELF::EM_PPC64, LLDB_INVALID_CPUTYPE,
- 0xu, 0xu}, // PowerPC64
+{ArchSpec::eCore_ppc64le_generic, llvm::ELF::EM_PPC64,
+ ArchSpec::eCore_ppc64le_generic, 0xu, 0xu}, // PowerPC64le
+{ArchSpec::eCore_ppc64_generic, llvm::ELF::EM_PPC64,
+ ArchSpec::eCore_ppc64_generic, 0xu, 0xu}, // PowerPC64
 {ArchSpec::eCore_arm_generic, llvm::ELF::EM_ARM, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARM
 {ArchSpec::eCore_arm_aarch64, llvm::ELF::EM_AARCH64, LLDB_INVALID_CPUTYPE,
@@ -400,8 +400,8 @@
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
 {ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
  0xu, 0xu}, // ARC
-{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE,
- 0xu, 0xu}, // AVR
+{ArchSpec::eCore_avr, llvm::ELF::EM_AVR, LLDB_INVALID_CPUTYPE, 0xu,
+ 0xu}, // AVR
 {ArchSpec::eCore_riscv32, llvm::ELF::EM_RISCV,
  ArchSpec::eRISCVSubType_riscv32, 0xu, 0xu}, // riscv32
 {ArchSpec::eCore_riscv64, llvm::ELF::EM_RISCV,
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -310,9 +310,19 @@
   }
 }
 
+static uint32_t ppc64VariantFromElfFlag

[Lldb-commits] [PATCH] D125012: [clang] createInvocationFromCommandLine -> createInvocation, delete former. NFC

2022-05-05 Thread Sam McCall via Phabricator via lldb-commits

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

(Followup from 40c13720a4b977d4347bbde53c52a4d0703823c2 
)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125012

Files:
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/tools/c-index-test/core_main.cpp
  clang/tools/diagtool/ShowEnabledWarnings.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/Frontend/ASTUnitTest.cpp
  clang/unittests/Frontend/CompilerInstanceTest.cpp
  clang/unittests/Serialization/ModuleCacheTest.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -663,9 +663,11 @@
llvm::make_range(compiler_invocation_arguments.begin(),
 compiler_invocation_arguments.end()));
 
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = diagnostics_engine;
   std::shared_ptr invocation =
-  clang::createInvocationFromCommandLine(compiler_invocation_argument_cstrs,
- diagnostics_engine);
+  clang::createInvocation(compiler_invocation_argument_cstrs,
+  std::move(CIOpts));
 
   if (!invocation)
 return nullptr;
Index: clang/unittests/Tooling/Syntax/TreeTestBase.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.cpp
+++ clang/unittests/Tooling/Syntax/TreeTestBase.cpp
@@ -134,7 +134,10 @@
 ArgsCStr.push_back(arg.c_str());
   }
 
-  Invocation = createInvocationFromCommandLine(ArgsCStr, Diags, FS);
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = Diags;
+  CIOpts.VFS = FS;
+  Invocation = createInvocation(ArgsCStr, std::move(CIOpts));
   assert(Invocation);
   Invocation->getFrontendOpts().DisableFree = false;
   Invocation->getPreprocessorOpts().addRemappedFile(
Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -125,7 +125,10 @@
   Diags->setClient(new IgnoringDiagConsumer);
 std::vector Args = {"tok-test", "-std=c++03", "-fsyntax-only",
   FileName};
-auto CI = createInvocationFromCommandLine(Args, Diags, FS);
+CreateInvocationOptions CIOpts;
+CIOpts.Diags = Diags;
+CIOpts.VFS = FS;
+auto CI = createInvocation(Args, std::move(CIOpts));
 assert(CI);
 CI->getFrontendOpts().DisableFree = false;
 CI->getPreprocessorOpts().addRemappedFile(
Index: clang/unittests/Serialization/ModuleCacheTest.cpp
===
--- clang/unittests/Serialization/ModuleCacheTest.cpp
+++ clang/unittests/Serialization/ModuleCacheTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
@@ -95,13 +96,15 @@
   MCPArg.append(ModuleCachePath);
   IntrusiveRefCntPtr Diags =
   CompilerInstance::createDiagnostics(new DiagnosticOptions());
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = Diags;
 
   // First run should pass with no errors
   const char *Args[] = {"clang","-fmodules",  "-Fframeworks",
 MCPArg.c_str(), "-working-directory", TestDir.c_str(),
 "test.m"};
   std::shared_ptr Invocation =
-  createInvocationFromCommandLine(Args, Diags);
+  createInvocation(Args, CIOpts);
   ASSERT_TRUE(Invocation);
   CompilerInstance Instance;
   Instance.setDiagnostics(Diags.get());
@@ -124,7 +127,7 @@
  "-Fframeworks",  MCPArg.c_str(), "-working-directory",
  TestDir.c_str(), "test.m"};
   std::shared_ptr Invocation2 =
-  createInvocationFromCommandLine(Args2, Diags);
+  createInvocation(Args2, CIOpts);
   ASSERT_TRUE(Invocation2);
   CompilerInstance Instance2(Instance.getPCHContainerOperations(),
  &Instance.getModuleCache());
@@ -142,13 +145,15 @@
   MCPArg.append(ModuleCachePath);

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

2022-05-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

This looks reasonable to me, but I'm not sure if there's anything special about 
the executable module that would result in us doing the wrong thing. I'm sure 
Jim or Pavel will know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124731

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

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

2022-05-05 Thread Greg Clayton via Phabricator via lldb-commits

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



Comment at: lldb/source/Utility/ConstString.cpp:177-183
   uint8_t hash(const llvm::StringRef &s) const {
-uint32_t h = llvm::djbHash(s);
+return hash(StringPool::hash(s));
+  }
+
+  uint8_t hash(uint32_t h) const {
 return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
   }

Can we rename these functions to something other than "hash"? It is a bit 
confusing to have a function called "hash" that returns a uint8_t when what 
this is really doing is getting use the string pool index. Maybe 
"GetPoolIndex"? I could see this function accidentally being used to hash a 
string instead of using StringPool::hash()



Comment at: llvm/include/llvm/ADT/StringMap.h:69
+  /// Overload that explicitly takes precomputed hash(Key).
+  unsigned LookupBucketFor(StringRef Key, unsigned FullHashValue);
 

Use "uint32_t" instead of unsigned to clarify the exact width of the integer. 
Or create a "HashType" typedef and then use that.



Comment at: llvm/include/llvm/ADT/StringMap.h:106
 
+  static unsigned hash(StringRef Key) { return llvm::djbHash(Key, 0); }
+





Comment at: llvm/lib/Support/StringMap.cpp:83
+unsigned StringMapImpl::LookupBucketFor(StringRef Name,
+unsigned FullHashValue) {
+#ifdef EXPENSIVE_CHECKS





Comment at: llvm/lib/Support/StringMap.cpp:141
 /// This does not modify the map.
-int StringMapImpl::FindKey(StringRef Key) const {
+int StringMapImpl::FindKey(StringRef Key, unsigned FullHashValue) const {
   if (NumBuckets == 0)




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

https://reviews.llvm.org/D122974

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

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

2022-05-05 Thread Jim Ingham via Phabricator via lldb-commits

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

This seems like a reasonable fallback, and the implementation looks fine.
You need to add a test case setting an address breakpoint in the executable and 
making sure that works.  Should be easy, set a line breakpoint in the main 
executable, get the resolved address and set an address breakpoint with that 
and make sure that breakpoint got a location.
You should also add some words to the "break set" command help saying that this 
is the fallback behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124731

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

[Lldb-commits] [lldb] eb3136f - Fix debugserver translation check

2022-05-05 Thread Alexandre Perez via lldb-commits


Author: Alexandre Perez
Date: 2022-05-05T11:31:23-07:00
New Revision: eb3136f022b3e5061fe790e7886f4bb592d8a1d1

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

LOG: Fix debugserver translation check

Currently, debugserver has a test to check if it was launched in
translation. The intent was to cover the case where an x86_64
debugserver attempts to control an arm64/arm64e process, returning
an error. However, this check also covers the case where users
are attaching to an x86_64 process, exiting out before attempting
to hand off control to the translated debugserver at
`/Library/Apple/usr/libexec/oah/debugserver`.

This diff delays the debugserver translation check until after
determining whether to hand off control to
`/Library/Apple/usr/libexec/oah/debugserver`. Only when the
process is not translated and thus has not been handed off do we
check if the debugserver is translated, erroring out in that case.

Reviewed By: jasonmolenda

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

Added: 


Modified: 
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/DNBDefs.h
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/tools/debugserver/source/DNB.cpp 
b/lldb/tools/debugserver/source/DNB.cpp
index 59bb91b82c351..c037c48c02868 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -477,6 +477,10 @@ nub_process_t DNBProcessAttach(nub_process_t attach_pid,
 }
   }
 
+  if (DNBDebugserverIsTranslated()) {
+return INVALID_NUB_PROCESS_ARCH;
+  }
+
   pid_t pid = INVALID_NUB_PROCESS;
   MachProcessSP processSP(new MachProcess);
   if (processSP.get()) {

diff  --git a/lldb/tools/debugserver/source/DNBDefs.h 
b/lldb/tools/debugserver/source/DNBDefs.h
index 657964215d0f3..ee31f1c7a4ba0 100644
--- a/lldb/tools/debugserver/source/DNBDefs.h
+++ b/lldb/tools/debugserver/source/DNBDefs.h
@@ -54,6 +54,7 @@ typedef uint32_t nub_event_t;
 typedef uint32_t nub_bool_t;
 
 #define INVALID_NUB_PROCESS ((nub_process_t)0)
+#define INVALID_NUB_PROCESS_ARCH ((nub_process_t)-1)
 #define INVALID_NUB_THREAD ((nub_thread_t)0)
 #define INVALID_NUB_WATCH_ID ((nub_watch_t)0)
 #define INVALID_NUB_HW_INDEX UINT32_MAX

diff  --git a/lldb/tools/debugserver/source/RNBRemote.cpp 
b/lldb/tools/debugserver/source/RNBRemote.cpp
index c909cba872f7c..7c68f85225a24 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3753,17 +3753,6 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
 char err_str[1024] = {'\0'};
 std::string attach_name;
 
-if (DNBDebugserverIsTranslated()) {
-  DNBLogError("debugserver is x86_64 binary running in translation, attach 
"
-  "failed.");
-  std::string return_message = "E96;";
-  return_message +=
-  cstring_to_asciihex_string("debugserver is x86_64 binary running in "
- "translation, attached failed.");
-  SendPacket(return_message);
-  return rnb_err;
-}
-
 if (strstr(p, "vAttachWait;") == p) {
   p += strlen("vAttachWait;");
   if (!GetProcessNameFrom_vAttach(p, attach_name)) {
@@ -3823,6 +3812,17 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
   return HandlePacket_UNIMPLEMENTED(p);
 }
 
+if (attach_pid == INVALID_NUB_PROCESS_ARCH) {
+  DNBLogError("debugserver is x86_64 binary running in translation, attach 
"
+  "failed.");
+  std::string return_message = "E96;";
+  return_message +=
+  cstring_to_asciihex_string("debugserver is x86_64 binary running in "
+ "translation, attach failed.");
+  SendPacket(return_message.c_str());
+  return rnb_err;
+}
+
 if (attach_pid != INVALID_NUB_PROCESS) {
   if (m_ctx.ProcessID() != attach_pid)
 m_ctx.SetProcessID(attach_pid);



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

[Lldb-commits] [PATCH] D124814: Fix debugserver translation check

2022-05-05 Thread Alexandre Perez via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeb3136f022b3: Fix debugserver translation check (authored by 
aperez).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124814

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNBDefs.h
  lldb/tools/debugserver/source/RNBRemote.cpp


Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3753,17 +3753,6 @@
 char err_str[1024] = {'\0'};
 std::string attach_name;
 
-if (DNBDebugserverIsTranslated()) {
-  DNBLogError("debugserver is x86_64 binary running in translation, attach 
"
-  "failed.");
-  std::string return_message = "E96;";
-  return_message +=
-  cstring_to_asciihex_string("debugserver is x86_64 binary running in "
- "translation, attached failed.");
-  SendPacket(return_message);
-  return rnb_err;
-}
-
 if (strstr(p, "vAttachWait;") == p) {
   p += strlen("vAttachWait;");
   if (!GetProcessNameFrom_vAttach(p, attach_name)) {
@@ -3823,6 +3812,17 @@
   return HandlePacket_UNIMPLEMENTED(p);
 }
 
+if (attach_pid == INVALID_NUB_PROCESS_ARCH) {
+  DNBLogError("debugserver is x86_64 binary running in translation, attach 
"
+  "failed.");
+  std::string return_message = "E96;";
+  return_message +=
+  cstring_to_asciihex_string("debugserver is x86_64 binary running in "
+ "translation, attach failed.");
+  SendPacket(return_message.c_str());
+  return rnb_err;
+}
+
 if (attach_pid != INVALID_NUB_PROCESS) {
   if (m_ctx.ProcessID() != attach_pid)
 m_ctx.SetProcessID(attach_pid);
Index: lldb/tools/debugserver/source/DNBDefs.h
===
--- lldb/tools/debugserver/source/DNBDefs.h
+++ lldb/tools/debugserver/source/DNBDefs.h
@@ -54,6 +54,7 @@
 typedef uint32_t nub_bool_t;
 
 #define INVALID_NUB_PROCESS ((nub_process_t)0)
+#define INVALID_NUB_PROCESS_ARCH ((nub_process_t)-1)
 #define INVALID_NUB_THREAD ((nub_thread_t)0)
 #define INVALID_NUB_WATCH_ID ((nub_watch_t)0)
 #define INVALID_NUB_HW_INDEX UINT32_MAX
Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -477,6 +477,10 @@
 }
   }
 
+  if (DNBDebugserverIsTranslated()) {
+return INVALID_NUB_PROCESS_ARCH;
+  }
+
   pid_t pid = INVALID_NUB_PROCESS;
   MachProcessSP processSP(new MachProcess);
   if (processSP.get()) {


Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3753,17 +3753,6 @@
 char err_str[1024] = {'\0'};
 std::string attach_name;
 
-if (DNBDebugserverIsTranslated()) {
-  DNBLogError("debugserver is x86_64 binary running in translation, attach "
-  "failed.");
-  std::string return_message = "E96;";
-  return_message +=
-  cstring_to_asciihex_string("debugserver is x86_64 binary running in "
- "translation, attached failed.");
-  SendPacket(return_message);
-  return rnb_err;
-}
-
 if (strstr(p, "vAttachWait;") == p) {
   p += strlen("vAttachWait;");
   if (!GetProcessNameFrom_vAttach(p, attach_name)) {
@@ -3823,6 +3812,17 @@
   return HandlePacket_UNIMPLEMENTED(p);
 }
 
+if (attach_pid == INVALID_NUB_PROCESS_ARCH) {
+  DNBLogError("debugserver is x86_64 binary running in translation, attach "
+  "failed.");
+  std::string return_message = "E96;";
+  return_message +=
+  cstring_to_asciihex_string("debugserver is x86_64 binary running in "
+ "translation, attach failed.");
+  SendPacket(return_message.c_str());
+  return rnb_err;
+}
+
 if (attach_pid != INVALID_NUB_PROCESS) {
   if (m_ctx.ProcessID() != attach_pid)
 m_ctx.SetProcessID(attach_pid);
Index: lldb/tools/debugserver/source/DNBDefs.h
===
--- lldb/tools/debugserver/source/DNBDefs.h
+++ lldb/tools/debugserver/source/DNBDefs.h
@@ -54,6 +54,7 @@
 typedef uint32_t nub_bool_t;
 
 #define INVALID_NUB_PROCESS ((nub_process_t)0)
+#define INVALID_NUB_PROCESS_ARCH ((nub_process_t)-1)
 #define INVALID_NUB_THREAD ((nub_thread_t)0)
 #define INVALID_NUB_WATCH_ID ((nub_watch_t)0)
 #define INVALID_NUB_HW_INDEX UINT32_MAX
Index: lldb/tools/debugserver/source/DNB.cpp
=

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

2022-05-05 Thread Greg Clayton via Phabricator via lldb-commits

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

Inline comments point out where can can get rid of the second parameter to 
IsUnwindPlanValidForCurrentPC() as no one uses it anymore.




Comment at: lldb/source/Target/RegisterContextUnwind.cpp:519-522
+if (m_behaves_like_zeroth_frame)
+  decr_pc_and_recompute_addr_range = false;
+else
+  decr_pc_and_recompute_addr_range = true;

This can easily just be:
```
m_behaves_like_zeroth_frame = !decr_pc_and_recompute_addr_range;
```



Comment at: lldb/source/Target/RegisterContextUnwind.cpp:642-643
 m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
 int valid_offset = -1;
 if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
+  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(

remove "valid_offset" variable and also remove it from the second argument to 
IsUnwindPlanValidForCurrentPC as it isn't used anywhere now and is just dead 
code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124957

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

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

2022-05-05 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added inline comments.



Comment at: lldb/source/Target/RegisterContextUnwind.cpp:642-643
 m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
 int valid_offset = -1;
 if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
+  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(

clayborg wrote:
> remove "valid_offset" variable and also remove it from the second argument to 
> IsUnwindPlanValidForCurrentPC as it isn't used anywhere now and is just dead 
> code.
hm, this merits consideration more widely; I looked at all callers to 
IsUnwindPlanValidForCurrentPC and none of them use the valid_offset that it 
provides; they are all merely checking that the offset is covered by the 
UnwindPlan's byte size.  I suspect none of these are actually necessary; we 
picked the unwind plan based on symbol name so you'd need a "symbol" that has a 
large byte size, but an UnwindPlan that was sourced from some input that 
limited the byte size range covered.  idk, it might be possible tho, especially 
in a stripped binary.

I'll change all of the callers to not use the returned valid_offset for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124957

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

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

2022-05-05 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: lldb/source/Target/RegisterContextUnwind.cpp:642-643
 m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
 int valid_offset = -1;
 if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
+  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(

jasonmolenda wrote:
> clayborg wrote:
> > remove "valid_offset" variable and also remove it from the second argument 
> > to IsUnwindPlanValidForCurrentPC as it isn't used anywhere now and is just 
> > dead code.
> hm, this merits consideration more widely; I looked at all callers to 
> IsUnwindPlanValidForCurrentPC and none of them use the valid_offset that it 
> provides; they are all merely checking that the offset is covered by the 
> UnwindPlan's byte size.  I suspect none of these are actually necessary; we 
> picked the unwind plan based on symbol name so you'd need a "symbol" that has 
> a large byte size, but an UnwindPlan that was sourced from some input that 
> limited the byte size range covered.  idk, it might be possible tho, 
> especially in a stripped binary.
> 
> I'll change all of the callers to not use the returned valid_offset for now.
If it is actually being used somewhere, then that is fine. My search of the 
code showed 3 locations, and this location was the only one that was using it, 
but now it isn't. Are there other locations I missed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124957

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

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

2022-05-05 Thread Jason Molenda via Phabricator via lldb-commits

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

Update to address Greg's feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124957

Files:
  lldb/examples/python/crashlog.py
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -82,14 +82,13 @@
 }
 
 bool RegisterContextUnwind::IsUnwindPlanValidForCurrentPC(
-lldb::UnwindPlanSP unwind_plan_sp, int &valid_pc_offset) {
+lldb::UnwindPlanSP unwind_plan_sp) {
   if (!unwind_plan_sp)
 return false;
 
   // check if m_current_pc is valid
   if (unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
 // yes - current offset can be used as is
-valid_pc_offset = m_current_offset;
 return true;
   }
 
@@ -101,8 +100,6 @@
   Address pc_minus_one(m_current_pc);
   pc_minus_one.SetOffset(m_current_pc.GetOffset() - 1);
   if (unwind_plan_sp->PlanValidAtAddress(pc_minus_one)) {
-// *valid_pc_offset = m_current_offset - 1;
-valid_pc_offset = m_current_pc.GetOffset() - 1;
 return true;
   }
 
@@ -514,9 +511,12 @@
   } else if (!addr_range.GetBaseAddress().IsValid() ||
  addr_range.GetBaseAddress().GetSection() != m_current_pc.GetSection() ||
  addr_range.GetBaseAddress().GetOffset() != m_current_pc.GetOffset()) {
-// If our "current" pc isn't the start of a function, no need
-// to decrement and recompute.
-decr_pc_and_recompute_addr_range = false;
+// If our "current" pc isn't the start of a function, decrement the pc
+// if we're up the stack.
+if (m_behaves_like_zeroth_frame)
+  decr_pc_and_recompute_addr_range = false;
+else
+  decr_pc_and_recompute_addr_range = true;
   } else if (IsTrapHandlerSymbol(process, m_sym_ctx)) {
 // Signal dispatch may set the return address of the handler it calls to
 // point to the first byte of a return trampoline (like __kernel_rt_sigreturn),
@@ -636,9 +636,8 @@
 }
   } else {
 m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
-int valid_offset = -1;
-if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
-  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(valid_offset);
+if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp)) {
+  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset_backed_up_one);
   row_register_kind = m_full_unwind_plan_sp->GetRegisterKind();
   PropagateTrapHandlerFlagFromUnwindPlan(m_full_unwind_plan_sp);
   if (active_row.get() && log) {
@@ -1007,8 +1006,7 @@
 unwind_plan_sp = func_unwinders_sp->GetUnwindPlanAtCallSite(
 process->GetTarget(), m_thread);
   }
-  int valid_offset = -1;
-  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp, valid_offset)) {
+  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
 UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because this "
 "is the call-site unwind plan",
 unwind_plan_sp->GetSourceName().GetCString());
@@ -1047,7 +1045,7 @@
 }
   }
 
-  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp, valid_offset)) {
+  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
 UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because we "
 "failed to find a call-site unwind plan that would work",
 unwind_plan_sp->GetSourceName().GetCString());
@@ -1313,7 +1311,7 @@
LLDB_REGNUM_GENERIC_PC);
 
   UnwindPlan::RowSP active_row =
-  m_full_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
+  m_full_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset_backed_up_one);
   unwindplan_registerkind = m_full_unwind_plan_sp->GetRegisterKind();
 
   if (got_new_full_unwindplan && active_row.get() && log) {
@@ -1770,7 +1768,7 @@
   m_full_unwind_plan_sp = m_fallback_unwind_plan_sp;
 
   UnwindPlan::RowSP active_row =
-  m_fallback_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
+  m_fallback_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset_backed_up_one);
 
   if (active_row &&
   active_row->GetCFAValue().GetValueType() !=
Index: lldb/include/lldb/Target/RegisterContextUnwind.h
===
--- lldb/include/lldb/Target/RegisterContextUnwind.h
+++ lldb/include/lldb/Target/RegisterContextUnwind.h
@@ -203,8 +203,7 @@
   void UnwindLogMsgVerbose(const char *fmt, ...)
   __attribute__((format(printf, 2, 3)));
 
-  bool IsUnwindPlanValidForCurrentPC(lldb::UnwindPlanSP unwind_plan_sp,
- int &valid_p

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

2022-05-05 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added inline comments.



Comment at: lldb/source/Target/RegisterContextUnwind.cpp:642-643
 m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
 int valid_offset = -1;
 if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
+  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(

clayborg wrote:
> jasonmolenda wrote:
> > clayborg wrote:
> > > remove "valid_offset" variable and also remove it from the second 
> > > argument to IsUnwindPlanValidForCurrentPC as it isn't used anywhere now 
> > > and is just dead code.
> > hm, this merits consideration more widely; I looked at all callers to 
> > IsUnwindPlanValidForCurrentPC and none of them use the valid_offset that it 
> > provides; they are all merely checking that the offset is covered by the 
> > UnwindPlan's byte size.  I suspect none of these are actually necessary; we 
> > picked the unwind plan based on symbol name so you'd need a "symbol" that 
> > has a large byte size, but an UnwindPlan that was sourced from some input 
> > that limited the byte size range covered.  idk, it might be possible tho, 
> > especially in a stripped binary.
> > 
> > I'll change all of the callers to not use the returned valid_offset for now.
> If it is actually being used somewhere, then that is fine. My search of the 
> code showed 3 locations, and this location was the only one that was using 
> it, but now it isn't. Are there other locations I missed?
I wrote that poorly.  I meant to say that no one was using the valid_offset 
returned value, so I would change the method to not return it.  I uploaded an 
updated patch showing what I mean here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124957

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

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

2022-05-05 Thread Jason Molenda via Phabricator via lldb-commits

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

Accidentally included an unrelated change in the last update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124957

Files:
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -82,14 +82,13 @@
 }
 
 bool RegisterContextUnwind::IsUnwindPlanValidForCurrentPC(
-lldb::UnwindPlanSP unwind_plan_sp, int &valid_pc_offset) {
+lldb::UnwindPlanSP unwind_plan_sp) {
   if (!unwind_plan_sp)
 return false;
 
   // check if m_current_pc is valid
   if (unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
 // yes - current offset can be used as is
-valid_pc_offset = m_current_offset;
 return true;
   }
 
@@ -101,8 +100,6 @@
   Address pc_minus_one(m_current_pc);
   pc_minus_one.SetOffset(m_current_pc.GetOffset() - 1);
   if (unwind_plan_sp->PlanValidAtAddress(pc_minus_one)) {
-// *valid_pc_offset = m_current_offset - 1;
-valid_pc_offset = m_current_pc.GetOffset() - 1;
 return true;
   }
 
@@ -514,9 +511,12 @@
   } else if (!addr_range.GetBaseAddress().IsValid() ||
  addr_range.GetBaseAddress().GetSection() != m_current_pc.GetSection() ||
  addr_range.GetBaseAddress().GetOffset() != m_current_pc.GetOffset()) {
-// If our "current" pc isn't the start of a function, no need
-// to decrement and recompute.
-decr_pc_and_recompute_addr_range = false;
+// If our "current" pc isn't the start of a function, decrement the pc
+// if we're up the stack.
+if (m_behaves_like_zeroth_frame)
+  decr_pc_and_recompute_addr_range = false;
+else
+  decr_pc_and_recompute_addr_range = true;
   } else if (IsTrapHandlerSymbol(process, m_sym_ctx)) {
 // Signal dispatch may set the return address of the handler it calls to
 // point to the first byte of a return trampoline (like __kernel_rt_sigreturn),
@@ -636,9 +636,9 @@
 }
   } else {
 m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
-int valid_offset = -1;
-if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
-  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(valid_offset);
+if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp)) {
+  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(
+  m_current_offset_backed_up_one);
   row_register_kind = m_full_unwind_plan_sp->GetRegisterKind();
   PropagateTrapHandlerFlagFromUnwindPlan(m_full_unwind_plan_sp);
   if (active_row.get() && log) {
@@ -1007,8 +1007,7 @@
 unwind_plan_sp = func_unwinders_sp->GetUnwindPlanAtCallSite(
 process->GetTarget(), m_thread);
   }
-  int valid_offset = -1;
-  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp, valid_offset)) {
+  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
 UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because this "
 "is the call-site unwind plan",
 unwind_plan_sp->GetSourceName().GetCString());
@@ -1047,7 +1046,7 @@
 }
   }
 
-  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp, valid_offset)) {
+  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
 UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because we "
 "failed to find a call-site unwind plan that would work",
 unwind_plan_sp->GetSourceName().GetCString());
@@ -1313,7 +1312,8 @@
LLDB_REGNUM_GENERIC_PC);
 
   UnwindPlan::RowSP active_row =
-  m_full_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
+  m_full_unwind_plan_sp->GetRowForFunctionOffset(
+  m_current_offset_backed_up_one);
   unwindplan_registerkind = m_full_unwind_plan_sp->GetRegisterKind();
 
   if (got_new_full_unwindplan && active_row.get() && log) {
@@ -1770,7 +1770,8 @@
   m_full_unwind_plan_sp = m_fallback_unwind_plan_sp;
 
   UnwindPlan::RowSP active_row =
-  m_fallback_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
+  m_fallback_unwind_plan_sp->GetRowForFunctionOffset(
+  m_current_offset_backed_up_one);
 
   if (active_row &&
   active_row->GetCFAValue().GetValueType() !=
Index: lldb/include/lldb/Target/RegisterContextUnwind.h
===
--- lldb/include/lldb/Target/RegisterContextUnwind.h
+++ lldb/include/lldb/Target/RegisterContextUnwind.h
@@ -203,8 +203,7 @@
   void UnwindLogMsgVerbose(const char *fmt, ...)
   __attribute__((format(printf, 2, 3)));
 
-  bool IsUnwindPlanValidForCurrentPC(lldb::UnwindPlanSP unwind_plan_sp,
-

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

2022-05-05 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda created this revision.
jasonmolenda added a reviewer: kastiglione.
jasonmolenda added a project: LLDB.
Herald added subscribers: JDevlieghere, kristof.beyls.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

A common crash is calling through a null function pointer and trying to execute 
an instruction at address 0.  lldb's backtracer special cases this, but the 
macOS crash tracer algorithm does not; crash reports will have a stack frame at 
0 and then skip the calling stack frame, assuming a stack frame was set up.  On 
arm64, the caller function is likely available in $lr, so let's insert a stack 
frame with that return pc value.

Also fix a little bug where register names without a "prefix" would get "None" 
prepended, so "Nonepc", "Nonelr" etc.

rdar://92631787


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125042

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -516,6 +516,22 @@
 image_addr = self.get_used_image(image_id)['base']
 pc = image_addr + frame_offset
 thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
+
+# on arm64 systems, if jump through a null function pointer,
+# we end up at address 0 and the crash reporter unwinder 
+# misses the frame that actually faulted.  
+# But $lr can tell us where the last BL/BLR instruction used 
+# was at, so insert that address as the caller stack frame.  
+if idx == 0 and pc == 0 and "lr" in thread.registers:
+pc = thread.registers["lr"]
+for image in self.data['usedImages']:
+text_lo = image['base']
+text_hi = text_lo + image['size']
+if text_lo <= pc < text_hi:
+  idx += 1
+  frame_offset = pc - text_lo
+  thread.frames.append(self.crashlog.Frame(idx, pc, 
frame_offset))
+
 idx += 1
 
 def parse_threads(self, json_threads):
@@ -551,7 +567,7 @@
 continue
 try:
 value = int(state['value'])
-registers["{}{}".format(prefix,key)] = value
+registers["{}{}".format(prefix or '',key)] = value
 except (KeyError, ValueError, TypeError):
 pass
 return registers


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -516,6 +516,22 @@
 image_addr = self.get_used_image(image_id)['base']
 pc = image_addr + frame_offset
 thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
+
+# on arm64 systems, if jump through a null function pointer,
+# we end up at address 0 and the crash reporter unwinder 
+# misses the frame that actually faulted.  
+# But $lr can tell us where the last BL/BLR instruction used 
+# was at, so insert that address as the caller stack frame.  
+if idx == 0 and pc == 0 and "lr" in thread.registers:
+pc = thread.registers["lr"]
+for image in self.data['usedImages']:
+text_lo = image['base']
+text_hi = text_lo + image['size']
+if text_lo <= pc < text_hi:
+  idx += 1
+  frame_offset = pc - text_lo
+  thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
+
 idx += 1
 
 def parse_threads(self, json_threads):
@@ -551,7 +567,7 @@
 continue
 try:
 value = int(state['value'])
-registers["{}{}".format(prefix,key)] = value
+registers["{}{}".format(prefix or '',key)] = value
 except (KeyError, ValueError, TypeError):
 pass
 return registers
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] b6388e4 - Decr return pc mid-stack when picking UnwindPlan row

2022-05-05 Thread Jason Molenda via lldb-commits


Author: Jason Molenda
Date: 2022-05-05T14:18:21-07:00
New Revision: b6388e4a0050fa248f774f5dacfa3e566e8daef1

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

LOG: Decr return pc mid-stack when picking UnwindPlan row

When picking the UnwindPlan row to use to backtrace,
off of the zeroth frame, decrement the return pc so
we're in the address range of the call instruction.
If this is a noretrun function call, the instruction
at the "return address" is likely an entirely different
basic block with possibly very different unwind rules,
and this can cause the backtrace to be incorrect.

Differential Revision: https://reviews.llvm.org/D124957
rdar://84651805

Added: 


Modified: 
lldb/include/lldb/Target/RegisterContextUnwind.h
lldb/source/Target/RegisterContextUnwind.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/RegisterContextUnwind.h 
b/lldb/include/lldb/Target/RegisterContextUnwind.h
index ef03a6746d723..ef8ae88403866 100644
--- a/lldb/include/lldb/Target/RegisterContextUnwind.h
+++ b/lldb/include/lldb/Target/RegisterContextUnwind.h
@@ -203,8 +203,7 @@ class RegisterContextUnwind : public 
lldb_private::RegisterContext {
   void UnwindLogMsgVerbose(const char *fmt, ...)
   __attribute__((format(printf, 2, 3)));
 
-  bool IsUnwindPlanValidForCurrentPC(lldb::UnwindPlanSP unwind_plan_sp,
- int &valid_pc_offset);
+  bool IsUnwindPlanValidForCurrentPC(lldb::UnwindPlanSP unwind_plan_sp);
 
   lldb::addr_t GetReturnAddressHint(int32_t plan_offset);
 

diff  --git a/lldb/source/Target/RegisterContextUnwind.cpp 
b/lldb/source/Target/RegisterContextUnwind.cpp
index 339e4c6ad0fde..e98aed7e15552 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -82,14 +82,13 @@ RegisterContextUnwind::RegisterContextUnwind(Thread &thread,
 }
 
 bool RegisterContextUnwind::IsUnwindPlanValidForCurrentPC(
-lldb::UnwindPlanSP unwind_plan_sp, int &valid_pc_offset) {
+lldb::UnwindPlanSP unwind_plan_sp) {
   if (!unwind_plan_sp)
 return false;
 
   // check if m_current_pc is valid
   if (unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
 // yes - current offset can be used as is
-valid_pc_offset = m_current_offset;
 return true;
   }
 
@@ -101,8 +100,6 @@ bool RegisterContextUnwind::IsUnwindPlanValidForCurrentPC(
   Address pc_minus_one(m_current_pc);
   pc_minus_one.SetOffset(m_current_pc.GetOffset() - 1);
   if (unwind_plan_sp->PlanValidAtAddress(pc_minus_one)) {
-// *valid_pc_offset = m_current_offset - 1;
-valid_pc_offset = m_current_pc.GetOffset() - 1;
 return true;
   }
 
@@ -514,9 +511,12 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
   } else if (!addr_range.GetBaseAddress().IsValid() ||
  addr_range.GetBaseAddress().GetSection() != 
m_current_pc.GetSection() ||
  addr_range.GetBaseAddress().GetOffset() != 
m_current_pc.GetOffset()) {
-// If our "current" pc isn't the start of a function, no need
-// to decrement and recompute.
-decr_pc_and_recompute_addr_range = false;
+// If our "current" pc isn't the start of a function, decrement the pc
+// if we're up the stack.
+if (m_behaves_like_zeroth_frame)
+  decr_pc_and_recompute_addr_range = false;
+else
+  decr_pc_and_recompute_addr_range = true;
   } else if (IsTrapHandlerSymbol(process, m_sym_ctx)) {
 // Signal dispatch may set the return address of the handler it calls to
 // point to the first byte of a return trampoline (like 
__kernel_rt_sigreturn),
@@ -636,9 +636,9 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
 }
   } else {
 m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
-int valid_offset = -1;
-if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
-  active_row = 
m_full_unwind_plan_sp->GetRowForFunctionOffset(valid_offset);
+if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp)) {
+  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(
+  m_current_offset_backed_up_one);
   row_register_kind = m_full_unwind_plan_sp->GetRegisterKind();
   PropagateTrapHandlerFlagFromUnwindPlan(m_full_unwind_plan_sp);
   if (active_row.get() && log) {
@@ -1007,8 +1007,7 @@ UnwindPlanSP 
RegisterContextUnwind::GetFullUnwindPlanForFrame() {
 unwind_plan_sp = func_unwinders_sp->GetUnwindPlanAtCallSite(
 process->GetTarget(), m_thread);
   }
-  int valid_offset = -1;
-  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp, valid_offset)) {
+  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
 UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because this "
 "is the call-site unwind plan",

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

2022-05-05 Thread Jason Molenda via Phabricator via lldb-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6388e4a0050: Decr return pc mid-stack when picking 
UnwindPlan row (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124957

Files:
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/source/Target/RegisterContextUnwind.cpp

Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -82,14 +82,13 @@
 }
 
 bool RegisterContextUnwind::IsUnwindPlanValidForCurrentPC(
-lldb::UnwindPlanSP unwind_plan_sp, int &valid_pc_offset) {
+lldb::UnwindPlanSP unwind_plan_sp) {
   if (!unwind_plan_sp)
 return false;
 
   // check if m_current_pc is valid
   if (unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
 // yes - current offset can be used as is
-valid_pc_offset = m_current_offset;
 return true;
   }
 
@@ -101,8 +100,6 @@
   Address pc_minus_one(m_current_pc);
   pc_minus_one.SetOffset(m_current_pc.GetOffset() - 1);
   if (unwind_plan_sp->PlanValidAtAddress(pc_minus_one)) {
-// *valid_pc_offset = m_current_offset - 1;
-valid_pc_offset = m_current_pc.GetOffset() - 1;
 return true;
   }
 
@@ -514,9 +511,12 @@
   } else if (!addr_range.GetBaseAddress().IsValid() ||
  addr_range.GetBaseAddress().GetSection() != m_current_pc.GetSection() ||
  addr_range.GetBaseAddress().GetOffset() != m_current_pc.GetOffset()) {
-// If our "current" pc isn't the start of a function, no need
-// to decrement and recompute.
-decr_pc_and_recompute_addr_range = false;
+// If our "current" pc isn't the start of a function, decrement the pc
+// if we're up the stack.
+if (m_behaves_like_zeroth_frame)
+  decr_pc_and_recompute_addr_range = false;
+else
+  decr_pc_and_recompute_addr_range = true;
   } else if (IsTrapHandlerSymbol(process, m_sym_ctx)) {
 // Signal dispatch may set the return address of the handler it calls to
 // point to the first byte of a return trampoline (like __kernel_rt_sigreturn),
@@ -636,9 +636,9 @@
 }
   } else {
 m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
-int valid_offset = -1;
-if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
-  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(valid_offset);
+if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp)) {
+  active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(
+  m_current_offset_backed_up_one);
   row_register_kind = m_full_unwind_plan_sp->GetRegisterKind();
   PropagateTrapHandlerFlagFromUnwindPlan(m_full_unwind_plan_sp);
   if (active_row.get() && log) {
@@ -1007,8 +1007,7 @@
 unwind_plan_sp = func_unwinders_sp->GetUnwindPlanAtCallSite(
 process->GetTarget(), m_thread);
   }
-  int valid_offset = -1;
-  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp, valid_offset)) {
+  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
 UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because this "
 "is the call-site unwind plan",
 unwind_plan_sp->GetSourceName().GetCString());
@@ -1047,7 +1046,7 @@
 }
   }
 
-  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp, valid_offset)) {
+  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
 UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because we "
 "failed to find a call-site unwind plan that would work",
 unwind_plan_sp->GetSourceName().GetCString());
@@ -1313,7 +1312,8 @@
LLDB_REGNUM_GENERIC_PC);
 
   UnwindPlan::RowSP active_row =
-  m_full_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
+  m_full_unwind_plan_sp->GetRowForFunctionOffset(
+  m_current_offset_backed_up_one);
   unwindplan_registerkind = m_full_unwind_plan_sp->GetRegisterKind();
 
   if (got_new_full_unwindplan && active_row.get() && log) {
@@ -1770,7 +1770,8 @@
   m_full_unwind_plan_sp = m_fallback_unwind_plan_sp;
 
   UnwindPlan::RowSP active_row =
-  m_fallback_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
+  m_fallback_unwind_plan_sp->GetRowForFunctionOffset(
+  m_current_offset_backed_up_one);
 
   if (active_row &&
   active_row->GetCFAValue().GetValueType() !=
Index: lldb/include/lldb/Target/RegisterContextUnwind.h
===
--- lldb/include/lldb/Target/RegisterContextUnwind.h
+++ lldb/include/lldb/Target/RegisterContextUnwind.h
@@ -203,8 +203,7 @@
   void UnwindLogMsgVerbose(const char *fmt, ...)
   __att

[Lldb-commits] [PATCH] D124962: [trace][intelpt] Support system-wide tracing [5] - Disable/enable per-core tracing based on the process state

2022-05-05 Thread walter erquinigo via Phabricator via lldb-commits

wallace updated this revision to Diff 427448.
wallace added a comment.

add better error handling


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124962

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h

Index: lldb/source/Plugins/Process/Linux/Perf.h
===
--- lldb/source/Plugins/Process/Linux/Perf.h
+++ lldb/source/Plugins/Process/Linux/Perf.h
@@ -208,6 +208,19 @@
   ///   \a ArrayRef extending \a aux_size bytes from \a aux_offset.
   llvm::ArrayRef GetAuxBuffer() const;
 
+  /// Use the ioctl API to disable the perf event. This doesn't terminate the
+  /// perf event.
+  ///
+  /// \return
+  ///   An Error if the perf event couldn't be disabled.
+  llvm::Error DisableWithIoctl() const;
+
+  /// Use the ioctl API to enable the perf event.
+  ///
+  /// \return
+  ///   An Error if the perf event couldn't be enabled.
+  llvm::Error EnableWithIoctl() const;
+
 private:
   /// Create new \a PerfEvent.
   ///
Index: lldb/source/Plugins/Process/Linux/Perf.cpp
===
--- lldb/source/Plugins/Process/Linux/Perf.cpp
+++ lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 
+#include 
 #include 
 #include 
 #include 
@@ -218,3 +219,19 @@
   return {reinterpret_cast(m_aux_base.get()),
static_cast(mmap_metadata.aux_size)};
 }
+
+Error PerfEvent::DisableWithIoctl() const {
+  if (ioctl(*m_fd, PERF_EVENT_IOC_DISABLE) < 0)
+return createStringError(inconvertibleErrorCode(),
+ "Can't disable perf event. %s",
+ std::strerror(errno));
+  return Error::success();
+}
+
+Error PerfEvent::EnableWithIoctl() const {
+  if (ioctl(*m_fd, PERF_EVENT_IOC_ENABLE) < 0)
+return createStringError(inconvertibleErrorCode(),
+ "Can't disable perf event. %s",
+ std::strerror(errno));
+  return Error::success();
+}
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -209,6 +209,8 @@
   /// stopping for threads being destroyed.
   Status NotifyTracersOfThreadDestroyed(lldb::tid_t tid);
 
+  void NotifyTracersProcessStateChanged(lldb::StateType state) override;
+
   /// Writes the raw event message code (vis-a-vis PTRACE_GETEVENTMSG)
   /// corresponding to the given thread ID to the memory pointed to by @p
   /// message.
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1665,6 +1665,11 @@
   SignalIfAllThreadsStopped();
 }
 
+void NativeProcessLinux::NotifyTracersProcessStateChanged(
+lldb::StateType state) {
+  m_intel_pt_collector.OnProcessStateChanged(state);
+}
+
 Status NativeProcessLinux::NotifyTracersOfNewThread(lldb::tid_t tid) {
   Log *log = GetLog(POSIXLog::Thread);
   Status error(m_intel_pt_collector.OnThreadCreated(tid));
Index: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
===
--- lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
+++ lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
@@ -27,6 +27,11 @@
 
 using IntelPTSingleBufferTraceUP = std::unique_ptr;
 
+enum class TraceCollectionState {
+  Running,
+  Paused,
+};
+
 /// This class wraps a single perf event collecting intel pt data in a single
 /// buffer.
 class IntelPTSingleBufferTrace {
@@ -43,13 +48,17 @@
   /// \param[in] core_id
   /// The CPU core id where to trace. If \b None, then this traces all CPUs.
   ///
+  /// \param[in] initial_state
+  /// The initial trace collection state.
+  ///
   /// \return
   ///   A \a IntelPTSingleBufferTrace instance if tracing was successful, or
   ///   an \a llvm::Error otherwise.
   static llvm::Expected
   Start(const TraceIntelPTStartRequest &request,
 llvm::Option

[Lldb-commits] [PATCH] D124858: [trace][intelpt] Support system-wide tracing [4] - Support per core tracing on lldb-server

2022-05-05 Thread walter erquinigo via Phabricator via lldb-commits

wallace updated this revision to Diff 427449.
wallace added a comment.

nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124858

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceGDBRemotePackets.h
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/include/lldb/lldb-types.h
  lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h
  lldb/source/Plugins/Process/Linux/Procfs.cpp
  lldb/source/Plugins/Process/Linux/Procfs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Target/Trace.cpp
  lldb/source/Utility/TraceGDBRemotePackets.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py

Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -30,6 +30,8 @@
 self.expect("continue")
 self.expect("thread trace dump instructions", substrs=['main.cpp:4'])
 
+self.traceStopProcess()
+
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
 @testSBAPIAndCommands
 def testStartMultipleLiveThreadsWithStops(self):
@@ -65,6 +67,8 @@
 
 self.expect("thread trace dump instructions 2", substrs=['not traced'])
 
+self.traceStopProcess()
+
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
 @testSBAPIAndCommands
 def testStartMultipleLiveThreadsWithStops(self):
@@ -100,6 +104,8 @@
 self.expect("thread trace dump instructions 1", substrs=['not traced'])
 self.expect("thread trace dump instructions 2", substrs=['not traced'])
 
+self.traceStopProcess()
+
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
 def testStartMultipleLiveThreadsWithThreadStartAll(self):
 self.build()
@@ -156,6 +162,8 @@
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
 @testSBAPIAndCommands
 def testStartPerCoreSession(self):
+self.skipIfPerCoreTracingIsNotSupported()
+
 self.build()
 exe = self.getBuildArtifact("a.out")
 self.dbg.CreateTarget(exe)
@@ -163,6 +171,32 @@
 self.expect("b main")
 self.expect("r")
 
-self.traceStartProcess(
-error=True, perCoreTracing=True,
-substrs=["Per-core tracing is not supported"])
+# We should fail if we hit the total buffer limit. Useful if the number
+# of cores is huge.
+self.traceStartProcess(error="True", processBufferSizeLimit=100,
+perCoreTracing=True,
+substrs=["The process can't be traced because the process trace size "
+"limit has been reached. Consider retracing with a higher limit."])
+
+self.traceStartProcess(perCoreTracing=True)
+self.traceStopProcess()
+
+self.traceStartProcess(perCoreTracing=True)
+# We can't support multiple per-core tracing sessions.
+self.traceStartProcess(error=True, perCoreTracing=True,
+substrs=["Process currently traced. Stop process tracing first"])
+
+# We can't support tracing per thread is per core is enabled.
+self.traceStartThread(
+error="True",
+substrs=["Process currently traced with per-core tracing. Stop process tracing first"])
+
+# We can't stop individual thread when per core is enabled.
+self.traceStopThread(error="True",
+substrs=["Can't stop tracing an individual thread when per-core process tracing is enabled"])
+
+# The GetState packet should return trace buffers per core
+self.expect("""process plugin packet send 'jLLDBTraceGetState:{"type":"intel-pt"}]'""",
+substrs=['''[{"kind":"traceBuffer","size":4096}],"coreId":'''])
+
+self.traceStopProcess()
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -16,6 +16,

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

2022-05-05 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: lldb/source/Target/RegisterContextUnwind.cpp:90
   // check if m_current_pc is valid
   if (unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
 // yes - current offset can be used as is

Don't we want to subtract 1 here? Otherwise this function might return true 
incorrectly if we actually need to subtract at least 1 from the PC


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124957

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

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

2022-05-05 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added inline comments.



Comment at: lldb/source/Target/RegisterContextUnwind.cpp:90
   // check if m_current_pc is valid
   if (unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
 // yes - current offset can be used as is

clayborg wrote:
> Don't we want to subtract 1 here? Otherwise this function might return true 
> incorrectly if we actually need to subtract at least 1 from the PC
honestly this method is a bit suspect, when we look at it today.  It's first 
"is $pc within ByteSize", then "ok is pc-1 within ByteSize", with a bonus check 
if somehow we have a negative offset.  Maybe it became suspect with revision 
over time.  I don't think it's wrong, but it's not written with much clarity 
and I should look into how it's changed over time to see if there was a cleaner 
vision of its goals originally...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124957

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

[Lldb-commits] [PATCH] D125047: [trace][intelpt] Support system-wide tracing [6] - Break IntelPTCollector into smaller files and minor refactor

2022-05-05 Thread walter erquinigo via Phabricator via lldb-commits

wallace created this revision.
wallace added a reviewer: jj10306.
Herald added a subscriber: mgorny.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

IntelPTCollector is very big and has 3 classes in it. It's actually cleaner if 
each one has its own file. This also gives more visibility to the developer 
about the different kinds of "tracers" that we have.

Besides that, I'm now restricting the creation of the BinaryData chunks to 
GetState() instead of having it in different places, which is not very clean, 
because the gdb-remote protocol should be as restricted as possible.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125047

Files:
  lldb/include/lldb/Utility/TraceGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp
  lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.h
  lldb/source/Utility/TraceGDBRemotePackets.cpp

Index: lldb/source/Utility/TraceGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceGDBRemotePackets.cpp
@@ -133,15 +133,20 @@
 {"kind", packet.kind},
 {"offset", packet.offset},
 {"tid", packet.tid},
-{"size", packet.size}});
+{"size", static_cast(packet.size)}});
 }
 
 bool fromJSON(const json::Value &value, TraceGetBinaryDataRequest &packet,
   Path path) {
   ObjectMapper o(value, path);
-  return o && o.map("type", packet.type) && o.map("kind", packet.kind) &&
- o.map("tid", packet.tid) && o.map("offset", packet.offset) &&
- o.map("size", packet.size);
+  int64_t size;
+  if (!o || !o.map("type", packet.type) || !o.map("kind", packet.kind) ||
+  !o.map("tid", packet.tid) || !o.map("offset", packet.offset) ||
+  !o.map("size", size)) {
+return false;
+  }
+  packet.size = static_cast(size);
+  return true;
 }
 /// \}
 
Index: lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.h
@@ -0,0 +1,71 @@
+//===-- IntelPTThreadTraceCollection.h  -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_IntelPTPerThreadTraceCollection_H_
+#define liblldb_IntelPTPerThreadTraceCollection_H_
+
+#include "IntelPTSingleBufferTrace.h"
+
+namespace lldb_private {
+namespace process_linux {
+
+/// Manages a list of thread traces.
+class IntelPTThreadTraceCollection {
+public:
+  IntelPTThreadTraceCollection() {}
+
+  /// Dispose of all traces
+  void Clear();
+
+  /// \return
+  ///   \b true if and only if this instance of tracing the provided \p tid.
+  bool TracesThread(lldb::tid_t tid) const;
+
+  /// \return
+  ///   The total sum of the trace buffer sizes used by this collection.
+  size_t GetTotalBufferSize() const;
+
+  /// Execute the provided callback on each thread that is being traced.
+  ///
+  /// \param[in] callback.tid
+  ///   The id of the thread that is being traced.
+  ///
+  /// \param[in] callback.core_trace
+  ///   The single-buffer trace instance for the given core.
+  void ForEachThread(std::function
+ callback);
+
+  llvm::Expected GetTracedThread(lldb::tid_t tid);
+
+  /// Start tracing the thread given by its \p tid.
+  ///
+  /// \return
+  ///   An error if the operation failed.
+  llvm::Error TraceStart(lldb::tid_t tid,
+ const TraceIntelPTStartRequest &request);
+
+  /// Stop tracing the thread given by its \p tid.
+  ///
+  /// \return
+  ///   An error if the given thread is not being traced or tracing couldn't be
+  ///   stopped.
+  llvm::Error TraceStop(lldb::tid_t tid);
+
+  size_t GetTracedThreadsCount() const;
+
+private:
+  llvm::DenseMap m_thread_traces;
+  /// Total actual thread buffer size in bytes
+  size_t m_total_buffer_size = 0;
+};
+
+} // namespace process_linux
+} // namespace lldb_private
+
+#endif // liblldb_IntelPTPerThreadTraceCollection_H_
Index: lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp
===
--- /dev/null
+++ lldb/sou

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

2022-05-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Does that mean that the interactive crashlogs (`crashlog -i`) do this correctly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125042

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

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

2022-05-05 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

In D125042#3495475 , @JDevlieghere 
wrote:

> Does that mean that the interactive crashlogs (`crashlog -i`) do this 
> correctly?

I didn't look at that when I wrote the patch; asked @mib and it sounded like he 
was going to do that separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125042

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

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

2022-05-05 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

In D125042#3495479 , @jasonmolenda 
wrote:

> In D125042#3495475 , @JDevlieghere 
> wrote:
>
>> Does that mean that the interactive crashlogs (`crashlog -i`) do this 
>> correctly?
>
> I didn't look at that when I wrote the patch; asked @mib and it sounded like 
> he was going to do that separately.

Interactive crashlogs use the same crashlog parser to fetch the register 
context, so I think they should work out of the box with this patch applied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125042

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

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

2022-05-05 Thread Dave Lee via Phabricator via lldb-commits

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

thanks for fixing these, especially the `lr` technique!




Comment at: lldb/examples/python/crashlog.py:520
+
+# on arm64 systems, if jump through a null function pointer,
+# we end up at address 0 and the crash reporter unwinder 

(minor) I think a word is missing between "if" and "jump".



Comment at: lldb/examples/python/crashlog.py:533
+  frame_offset = pc - text_lo
+  thread.frames.append(self.crashlog.Frame(idx, pc, 
frame_offset))
+

What do you think of adding a `break` here? Not that it would change the 
outcome, but it might make the intention of the loop slightly better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125042

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

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

2022-05-05 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

In D125042#3495488 , @mib wrote:

> In D125042#3495479 , @jasonmolenda 
> wrote:
>
>> In D125042#3495475 , @JDevlieghere 
>> wrote:
>>
>>> Does that mean that the interactive crashlogs (`crashlog -i`) do this 
>>> correctly?
>>
>> I didn't look at that when I wrote the patch; asked @mib and it sounded like 
>> he was going to do that separately.
>
> Interactive crashlogs use the same crashlog parser to fetch the register 
> context, so I think they should work out of the box with this patch applied.

I applied @jasonmolenda's patch to test it with interactive crashlog and I can 
see the added stack frame as expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125042

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

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

2022-05-05 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib accepted this revision.
mib added a comment.

LGTM! Thanks J!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125042

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

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

2022-05-05 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added inline comments.



Comment at: lldb/examples/python/crashlog.py:520
+
+# on arm64 systems, if jump through a null function pointer,
+# we end up at address 0 and the crash reporter unwinder 

kastiglione wrote:
> (minor) I think a word is missing between "if" and "jump".
yep good catch.



Comment at: lldb/examples/python/crashlog.py:533
+  frame_offset = pc - text_lo
+  thread.frames.append(self.crashlog.Frame(idx, pc, 
frame_offset))
+

kastiglione wrote:
> What do you think of adding a `break` here? Not that it would change the 
> outcome, but it might make the intention of the loop slightly better.
ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125042

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

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

2022-05-05 Thread Will Hawkins via Phabricator via lldb-commits

hawkinsw added a comment.

In D124731#3494609 , @jingham wrote:

> This seems like a reasonable fallback, and the implementation looks fine.
> You need to add a test case setting an address breakpoint in the executable 
> and making sure that works.  Should be easy, set a line breakpoint in the 
> main executable, get the resolved address and set an address breakpoint with 
> that and make sure that breakpoint got a location.
> You should also add some words to the "break set" command help saying that 
> this is the fallback behavior.

Thank you for the feedback! I will make those changes!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124731

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

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

2022-05-05 Thread Jason Molenda via Phabricator via lldb-commits

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

update to incorporate dave's suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125042

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -516,6 +516,23 @@
 image_addr = self.get_used_image(image_id)['base']
 pc = image_addr + frame_offset
 thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
+
+# on arm64 systems, if it jump through a null function pointer,
+# we end up at address 0 and the crash reporter unwinder 
+# misses the frame that actually faulted.  
+# But $lr can tell us where the last BL/BLR instruction used 
+# was at, so insert that address as the caller stack frame.  
+if idx == 0 and pc == 0 and "lr" in thread.registers:
+pc = thread.registers["lr"]
+for image in self.data['usedImages']:
+text_lo = image['base']
+text_hi = text_lo + image['size']
+if text_lo <= pc < text_hi:
+  idx += 1
+  frame_offset = pc - text_lo
+  thread.frames.append(self.crashlog.Frame(idx, pc, 
frame_offset))
+  break
+
 idx += 1
 
 def parse_threads(self, json_threads):
@@ -551,7 +568,7 @@
 continue
 try:
 value = int(state['value'])
-registers["{}{}".format(prefix,key)] = value
+registers["{}{}".format(prefix or '',key)] = value
 except (KeyError, ValueError, TypeError):
 pass
 return registers


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -516,6 +516,23 @@
 image_addr = self.get_used_image(image_id)['base']
 pc = image_addr + frame_offset
 thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
+
+# on arm64 systems, if it jump through a null function pointer,
+# we end up at address 0 and the crash reporter unwinder 
+# misses the frame that actually faulted.  
+# But $lr can tell us where the last BL/BLR instruction used 
+# was at, so insert that address as the caller stack frame.  
+if idx == 0 and pc == 0 and "lr" in thread.registers:
+pc = thread.registers["lr"]
+for image in self.data['usedImages']:
+text_lo = image['base']
+text_hi = text_lo + image['size']
+if text_lo <= pc < text_hi:
+  idx += 1
+  frame_offset = pc - text_lo
+  thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
+  break
+
 idx += 1
 
 def parse_threads(self, json_threads):
@@ -551,7 +568,7 @@
 continue
 try:
 value = int(state['value'])
-registers["{}{}".format(prefix,key)] = value
+registers["{}{}".format(prefix or '',key)] = value
 except (KeyError, ValueError, TypeError):
 pass
 return registers
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 2be791e - Insert crashing stack frame when call to null func ptr

2022-05-05 Thread Jason Molenda via lldb-commits


Author: Jason Molenda
Date: 2022-05-05T17:55:22-07:00
New Revision: 2be791e32af33b7ef735bb180a8a91ee501d0560

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

LOG: Insert crashing stack frame when call to null func ptr

On arm64 targets, when the crashing pc is 0, the caller
frame can be found by looking at $lr, but the crash
reports don't use that trick to show the actual crashing
frame.  This patch adds that stack frame that lldb shows.

Also fix an issue where some register names were printed
as having a prefix of 'None'.

Differential Revision: https://reviews.llvm.org/D125042
rdar://92631787

Added: 


Modified: 
lldb/examples/python/crashlog.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index e0bd52d8711ef..bc34bf75f8b19 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -516,6 +516,23 @@ def parse_frames(self, thread, json_frames):
 image_addr = self.get_used_image(image_id)['base']
 pc = image_addr + frame_offset
 thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
+
+# on arm64 systems, if it jump through a null function pointer,
+# we end up at address 0 and the crash reporter unwinder 
+# misses the frame that actually faulted.  
+# But $lr can tell us where the last BL/BLR instruction used 
+# was at, so insert that address as the caller stack frame.  
+if idx == 0 and pc == 0 and "lr" in thread.registers:
+pc = thread.registers["lr"]
+for image in self.data['usedImages']:
+text_lo = image['base']
+text_hi = text_lo + image['size']
+if text_lo <= pc < text_hi:
+  idx += 1
+  frame_offset = pc - text_lo
+  thread.frames.append(self.crashlog.Frame(idx, pc, 
frame_offset))
+  break
+
 idx += 1
 
 def parse_threads(self, json_threads):
@@ -551,7 +568,7 @@ def parse_thread_registers(self, json_thread_state, 
prefix=None):
 continue
 try:
 value = int(state['value'])
-registers["{}{}".format(prefix,key)] = value
+registers["{}{}".format(prefix or '',key)] = value
 except (KeyError, ValueError, TypeError):
 pass
 return registers



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

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

2022-05-05 Thread Jason Molenda 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 rG2be791e32af3: Insert crashing stack frame when call to null 
func ptr (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125042

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -516,6 +516,23 @@
 image_addr = self.get_used_image(image_id)['base']
 pc = image_addr + frame_offset
 thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
+
+# on arm64 systems, if it jump through a null function pointer,
+# we end up at address 0 and the crash reporter unwinder 
+# misses the frame that actually faulted.  
+# But $lr can tell us where the last BL/BLR instruction used 
+# was at, so insert that address as the caller stack frame.  
+if idx == 0 and pc == 0 and "lr" in thread.registers:
+pc = thread.registers["lr"]
+for image in self.data['usedImages']:
+text_lo = image['base']
+text_hi = text_lo + image['size']
+if text_lo <= pc < text_hi:
+  idx += 1
+  frame_offset = pc - text_lo
+  thread.frames.append(self.crashlog.Frame(idx, pc, 
frame_offset))
+  break
+
 idx += 1
 
 def parse_threads(self, json_threads):
@@ -551,7 +568,7 @@
 continue
 try:
 value = int(state['value'])
-registers["{}{}".format(prefix,key)] = value
+registers["{}{}".format(prefix or '',key)] = value
 except (KeyError, ValueError, TypeError):
 pass
 return registers


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -516,6 +516,23 @@
 image_addr = self.get_used_image(image_id)['base']
 pc = image_addr + frame_offset
 thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
+
+# on arm64 systems, if it jump through a null function pointer,
+# we end up at address 0 and the crash reporter unwinder 
+# misses the frame that actually faulted.  
+# But $lr can tell us where the last BL/BLR instruction used 
+# was at, so insert that address as the caller stack frame.  
+if idx == 0 and pc == 0 and "lr" in thread.registers:
+pc = thread.registers["lr"]
+for image in self.data['usedImages']:
+text_lo = image['base']
+text_hi = text_lo + image['size']
+if text_lo <= pc < text_hi:
+  idx += 1
+  frame_offset = pc - text_lo
+  thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
+  break
+
 idx += 1
 
 def parse_threads(self, json_threads):
@@ -551,7 +568,7 @@
 continue
 try:
 value = int(state['value'])
-registers["{}{}".format(prefix,key)] = value
+registers["{}{}".format(prefix or '',key)] = value
 except (KeyError, ValueError, TypeError):
 pass
 return registers
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] f416e57 - [lldb] Fix ppc64 detection in lldb

[Lldb-commits] [PATCH] D124760: [lldb] Fix ppc64 detection in lldb

[Lldb-commits] [PATCH] D125012: [clang] createInvocationFromCommandLine -> createInvocation, delete former. NFC

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

[Lldb-commits] [lldb] eb3136f - Fix debugserver translation check

[Lldb-commits] [PATCH] D124814: Fix debugserver translation check

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

[Lldb-commits] [lldb] b6388e4 - Decr return pc mid-stack when picking UnwindPlan row

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

[Lldb-commits] [PATCH] D124962: [trace][intelpt] Support system-wide tracing [5] - Disable/enable per-core tracing based on the process state

[Lldb-commits] [PATCH] D124858: [trace][intelpt] Support system-wide tracing [4] - Support per core tracing on lldb-server

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

[Lldb-commits] [PATCH] D125047: [trace][intelpt] Support system-wide tracing [6] - Break IntelPTCollector into smaller files and minor refactor

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

[Lldb-commits] [lldb] 2be791e - Insert crashing stack frame when call to null func ptr

[Lldb-commits] [PATCH] D125042: have crashlog.py insert a stack frame with $lr when stack frame 0 is address 0

33 matches

Site Navigation

Mail list logo

Footer information