[Lldb-commits] [PATCH] D110569: [lldb] [Process/FreeBSD] Rework arm64 register access

[Andrew Turner via Phabricator via lldb-commits]

andrew updated this revision to Diff 48.
andrew added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110569

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h

Index: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
===
--- lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
+++ lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
@@ -63,14 +63,29 @@
   // LLDB's GPR/FPU structs.  However, all fields have matching offsets
   // and sizes, so we do not have to worry about these (and we have
   // a unittest to assert that).
-  std::array m_reg_data;
+  std::array m_reg_data;
+  std::array m_fpreg_data;
 #ifdef LLDB_HAS_FREEBSD_WATCHPOINT
   dbreg m_dbreg;
   bool m_read_dbreg;
 #endif
 
-  Status ReadRegisterSet(uint32_t set);
-  Status WriteRegisterSet(uint32_t set);
+  void *GetGPRBuffer() { return &m_reg_data; }
+  size_t GetGPRBufferSize() { return sizeof(m_reg_data); }
+
+  void *GetFPRBuffer() { return &m_fpreg_data; }
+  size_t GetFPRSize() { return sizeof(m_fpreg_data); }
+
+  bool IsGPR(unsigned reg) const;
+  bool IsFPR(unsigned reg) const;
+
+  Status ReadGPR();
+  Status WriteGPR();
+
+  Status ReadFPR();
+  Status WriteFPR();
+
+  uint32_t CalculateFprOffset(const RegisterInfo *reg_info) const;
 
   llvm::Error ReadHardwareDebugInfo() override;
   llvm::Error WriteHardwareDebugRegs(DREGType hwbType) override;
Index: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
@@ -24,6 +24,8 @@
 #include 
 // clang-format on
 
+#define REG_CONTEXT_SIZE (GetGPRSize() + GetFPRSize())
+
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_freebsd;
@@ -68,30 +70,43 @@
   return count;
 }
 
-Status NativeRegisterContextFreeBSD_arm64::ReadRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
-   m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(
-PT_GETFPREGS, m_thread.GetID(),
-m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::ReadRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsGPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
+  RegisterInfoPOSIX_arm64::GPRegSet)
+return true;
+  return false;
 }
 
-Status NativeRegisterContextFreeBSD_arm64::WriteRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(),
-   m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(
-PT_SETFPREGS, m_thread.GetID(),
-m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::WriteRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsFPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
+  RegisterInfoPOSIX_arm64::FPRegSet)
+return true;
+  return false;
+}
+
+Status NativeRegisterContextFreeBSD_arm64::ReadGPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_GETREGS, m_thread.GetID(), m_reg_data.data());
+}
+
+Status NativeRegisterContextFreeBSD_arm64::WriteGPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_SETREGS, m_thread.GetID(), m_reg_data.data());
+}
+
+Status NativeRegisterContextFreeBSD_arm64::ReadFPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_GETFPREGS, m_thread.GetID(), m_fpreg_data.data());
+}
+
+Status NativeRegisterContextFreeBSD_arm64::WriteFPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_SETFPREGS, m_thread.GetID(), m_fpreg_data.data());
+}
+
+uint32_t NativeRegisterContextFreeBSD_arm64::CalculateFprOffset(
+const RegisterInfo *reg_info) const {
+  return reg_info->byte_offset - GetGPRSize();
 }
 
 Status
@@ -111,14 +126,30 @@
? reg_info->name
: "");
 
-  uint32_t set = GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg);
-  error = ReadRegisterSet(set);
-  if (error.Fail())

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

[Andrew Turner via Phabricator via lldb-commits]

andrew created this revision.
andrew added a reviewer: mgorny.
Herald added subscribers: kristof.beyls, krytarowski, arichardson.
andrew requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Read the mask using ptrace with PT_GETREGSET with NT_ARM_ADDR_MASK
for live processes, or from the NT_ARM_ADDR_MASK note in a core
file.

As NT_ARM_ADDR_MASK is on FreeBSD is the same value as NT_ARM_PAC_MASK
and it contains a structure with the same layout and a superset of the
same data use this when reading a core file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120485

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h

Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -120,6 +120,7 @@
 };
 
 constexpr RegsetDesc AARCH64_PAC_Desc[] = {
+{llvm::Triple::FreeBSD, llvm::Triple::aarch64, llvm::ELF::NT_ARM_PAC_MASK},
 {llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_PAC_MASK},
 };
 
Index: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
===
--- lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
+++ lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
@@ -36,8 +36,10 @@
 : public NativeRegisterContextFreeBSD,
   public NativeRegisterContextDBReg_arm64 {
 public:
-  NativeRegisterContextFreeBSD_arm64(const ArchSpec &target_arch,
- NativeThreadProtocol &native_thread);
+  NativeRegisterContextFreeBSD_arm64(
+  const ArchSpec &target_arch,
+  NativeThreadProtocol &native_thread,
+  std::unique_ptr register_info_up);
 
   uint32_t GetRegisterSetCount() const override;
 
@@ -65,6 +67,15 @@
   // a unittest to assert that).
   std::array m_reg_data;
   std::array m_fpreg_data;
+
+  struct arm64_addr_mask {
+uint64_t data_mask;
+uint64_t insn_mask;
+  };
+
+  bool m_addr_mask_is_valid;
+  struct arm64_addr_mask m_addr_mask;
+
 #ifdef LLDB_HAS_FREEBSD_WATCHPOINT
   dbreg m_dbreg;
   bool m_read_dbreg;
@@ -76,16 +87,22 @@
   void *GetFPRBuffer() { return &m_fpreg_data; }
   size_t GetFPRSize() { return sizeof(m_fpreg_data); }
 
+  void *GetAddrMaskBuffer() { return &m_addr_mask; }
+  size_t GetAddrMaskBufferSize() { return sizeof(m_addr_mask); }
+
   bool IsGPR(unsigned reg) const;
   bool IsFPR(unsigned reg) const;
+  bool IsAddrMask(unsigned reg) const;
 
   Status ReadGPR();
   Status WriteGPR();
+  Status ReadAddrMask();
 
   Status ReadFPR();
   Status WriteFPR();
 
   uint32_t CalculateFprOffset(const RegisterInfo *reg_info) const;
+  uint32_t CalculateAddrMaskOffset(const RegisterInfo *reg_info) const;
 
   llvm::Error ReadHardwareDebugInfo() override;
   llvm::Error WriteHardwareDebugRegs(DREGType hwbType) override;
Index: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
@@ -19,9 +19,11 @@
 #include "Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h"
 
 // clang-format off
+#include 
 #include 
 #include 
 #include 
+#include 
 // clang-format on
 
 #define REG_CONTEXT_SIZE (GetGPRSize() + GetFPRSize())
@@ -33,13 +35,19 @@
 NativeRegisterContextFreeBSD *
 NativeRegisterContextFreeBSD::CreateHostNativeRegisterContextFreeBSD(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread) {
-  return new NativeRegisterContextFreeBSD_arm64(target_arch, native_thread);
+  Flags opt_regsets;
+  opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskPAuth);
+  auto register_info_up =
+  std::make_unique(target_arch, opt_regsets);
+  return new NativeRegisterContextFreeBSD_arm64(target_arch, native_thread,
+  std::move(register_info_up));
 }
 
 NativeRegisterContextFreeBSD_arm64::NativeRegisterContextFreeBSD_arm64(
-const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
-: NativeRegisterContextRegisterInfo(
-  native_thread, new RegisterInfoPOSIX_arm64(target_arch, 0))
+const ArchSpec &target_arch, NativeThreadProtocol &native_thread,
+std::unique_ptr register_info_up)
+: NativeRegisterContextRegisterInfo(native_thread,
+register_info_up.release())
 #ifdef LLDB_HAS_FREEBSD_WATCHPOINT
   ,
   m_read_dbreg(false)
@@ -47,6 +55,9 @@
 {
   ::memset(&m_hwp_regs, 0, sizeof(m_hwp_regs));

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

[Andrew Turner via Phabricator via lldb-commits]

andrew added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:128
+
+#ifdef NT_ARM_ADDR_MASK
+  if (m_addr_mask_is_valid)

emaste wrote:
> This `#define` is coming from our headers? We need to provide this constant 
> within LLVM so this will be available on non-FreeBSD hosts.
My understanding is this file is used on FreeBSD as it's used when debugging a 
currently running process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120485

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

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

[Andrew Turner via Phabricator via lldb-commits]

andrew added a comment.

In D120485#3343538 , @emaste wrote:

> We need to make sure a test covers this as well, perhaps just enabling 
> lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py?

I have a local change to use the test, however it depends on forcing 
`isAArch64PAuth` in `lldb/packages/Python/lldbsuite/test/lldbtest.py` to return 
true. We'll need a way to detect which CPU features are supported. On Linux it 
reads `/proc/cpuinfo`, however there isn't a similar simple method on FreeBSD.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120485

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

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

[Andrew Turner via Phabricator via lldb-commits]

andrew added a comment.

It's easy to detect in the debugee as it can check which hardware capabilities 
are passed to it from the kernel.

Another option could be to have a build tool that prints these capabilities and 
have the test decide based on it as they are identical on FreeBSD and Linux.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120485

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

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

[Andrew Turner via Phabricator via lldb-commits]

andrew updated this revision to Diff 420205.
andrew added a comment.

Add support for the PAC test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120485

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
  lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c

Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
===
--- lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
@@ -4,8 +4,26 @@
 // To enable PAC return address signing compile with following clang arguments:
 // -march=armv8.3-a -mbranch-protection=pac-ret+leaf
 
+#include 
 #include 
 
+#if defined(__FreeBSD__)
+#include 
+
+static bool pac_supported(void) {
+  unsigned long hwcap;
+
+  if (elf_aux_info(AT_HWCAP, &hwcap, sizeof(hwcap)) != 0)
+return false;
+
+  return (hwcap & HWCAP_PACA) != 0;
+}
+#else
+static bool pac_supported(void) {
+  return true;
+}
+#endif
+
 static void __attribute__((noinline)) func_c(void) {
   exit(0); // Frame func_c
 }
@@ -19,6 +37,9 @@
 }
 
 int main(int argc, char *argv[]) {
+  if (!pac_supported())
+return 1;
+
   func_a(); // Frame main
   return 0;
 }
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -6,28 +6,26 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+import re
 
 
 class AArch64UnwindPAC(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
-@skipIf(archs=no_match(["aarch64"]))
-@skipIf(oslist=no_match(['linux']))
-def test(self):
-"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
-if not self.isAArch64PAuth():
-self.skipTest('Target must support Pointer Authentication.')
-
+def common(self, backtrace_tail):
 self.build()
 
 self.line = line_number('main.c', '// Frame func_c')
 
 exe = self.getBuildArtifact("a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
-
 lldbutil.run_break_set_by_file_and_line(
 self, "main.c", self.line, num_expected_locations=1)
 self.runCmd("run", RUN_SUCCEEDED)
+if re.match('Process .* exited with status = 1[^0-9]', self.res.GetOutput()):
+# No PAC support
+self.skipTest('Target must support Pointer Authentication.')
+
 self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
 substrs=["stop reason = breakpoint 1."])
 
@@ -35,13 +33,32 @@
 process = target.GetProcess()
 thread = process.GetThreadAtIndex(0)
 
-backtrace = ["func_c", "func_b", "func_a", "main", "__libc_start_main", "_start"]
-self.assertEqual(thread.GetNumFrames(), len(backtrace))
+backtrace = ["func_c", "func_b", "func_a", "main"] + backtrace_tail
+# This doesn't work as we get a ___lldb_unnamed_symbol on FreeBSD
+#self.assertEqual(thread.GetNumFrames(), len(backtrace))
 for frame_idx, frame in enumerate(thread.frames):
 frame = thread.GetFrameAtIndex(frame_idx)
+lldb_unnamed = "___lldb_unnamed_symbol"
+if frame.GetFunctionName()[:len(lldb_unnamed)] == lldb_unnamed:
+break
 self.assertTrue(frame)
 self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
 			# Check line number for functions in main.c
 if (frame_idx < 4):
 self.assertEqual(frame.GetLineEntry().GetLine(),
  line_number("main.c", "Frame " + backtrace[frame_idx]))
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['freebsd']))
+def test_freebsd(self):
+"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
+self.common(["__start"])
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['linux']))
+def test_linux(self):
+"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
+if not self.isAArch64PAuth():
+self.skipTest('Target must support Pointer Authentication.')
+
+self.common(["__libc_start_main", "_start"])
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
==

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

[Andrew Turner via Phabricator via lldb-commits]

andrew updated this revision to Diff 426993.
andrew marked 5 inline comments as done and an inline comment as not done.
andrew added a comment.

Cleanup based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120485

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
  lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
  lldb/test/API/python_api/sbvalue_const_addrof/main.cpp

Index: lldb/test/API/python_api/sbvalue_const_addrof/main.cpp
===
--- lldb/test/API/python_api/sbvalue_const_addrof/main.cpp
+++ lldb/test/API/python_api/sbvalue_const_addrof/main.cpp
@@ -1,5 +1,5 @@
-#include 
-#include 
+#include 
+#include 
 
 struct RegisterContext
 {
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
===
--- lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
@@ -4,8 +4,32 @@
 // To enable PAC return address signing compile with following clang arguments:
 // -march=armv8.3-a -mbranch-protection=pac-ret+leaf
 
+#include 
 #include 
 
+#if defined(__FreeBSD__)
+#include 
+
+// Added in FreeBSD 12.2, can be removed when all supported branches define it.
+#if !defined(HWCAP_PACA)
+#define	HWCAP_PACA	0x4000
+#endif
+
+static bool pac_supported(void) {
+  unsigned long hwcap;
+
+  if (elf_aux_info(AT_HWCAP, &hwcap, sizeof(hwcap)) != 0)
+return false;
+
+  return (hwcap & HWCAP_PACA) != 0;
+}
+#else
+static bool pac_supported(void) {
+  // We expect Linux to check for PAC up front using /proc/cpuinfo.
+  return true;
+}
+#endif
+
 static void __attribute__((noinline)) func_c(void) {
   exit(0); // Frame func_c
 }
@@ -19,6 +43,9 @@
 }
 
 int main(int argc, char *argv[]) {
+  if (!pac_supported())
+return 1;
+
   func_a(); // Frame main
   return 0;
 }
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -6,28 +6,26 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+import re
 
 
 class AArch64UnwindPAC(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
-@skipIf(archs=no_match(["aarch64"]))
-@skipIf(oslist=no_match(['linux']))
-def test(self):
-"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
-if not self.isAArch64PAuth():
-self.skipTest('Target must support Pointer Authentication.')
-
+def common(self, backtrace_tail):
 self.build()
 
 self.line = line_number('main.c', '// Frame func_c')
 
 exe = self.getBuildArtifact("a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
-
 lldbutil.run_break_set_by_file_and_line(
 self, "main.c", self.line, num_expected_locations=1)
 self.runCmd("run", RUN_SUCCEEDED)
+if re.match('Process .* exited with status = 1[^0-9]', self.res.GetOutput()):
+# No PAC support
+self.skipTest('Target must support Pointer Authentication.')
+
 self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
 substrs=["stop reason = breakpoint 1."])
 
@@ -35,13 +33,33 @@
 process = target.GetProcess()
 thread = process.GetThreadAtIndex(0)
 
-backtrace = ["func_c", "func_b", "func_a", "main", "__libc_start_main", "_start"]
-self.assertEqual(thread.GetNumFrames(), len(backtrace))
+backtrace = ["func_c", "func_b", "func_a", "main"] + backtrace_tail
 for frame_idx, frame in enumerate(thread.frames):
 frame = thread.GetFrameAtIndex(frame_idx)
+# On FreeBSD we get ___lldb_unnamed_symbol symbols at the bottom
+# of the stack. Ignore these and assume we have walked the entire
+# stack, however check this assumption is correct.
+if frame.GetFunctionName().startswith("___lldb_unnamed_symbol"):
+self.assertEqual(len(backtrace), frame_idx)
+break
 self.assertTrue(frame)
 self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
 			# Check line number for functions in main.c
 if (frame_idx < 4):
 self.assertEqual(frame.GetLineEntry().GetLine(),
   

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

[Andrew Turner via Phabricator via lldb-commits]

andrew updated this revision to Diff 427010.
andrew added a comment.

Remove a stray change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120485

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
  lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c

Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
===
--- lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
@@ -4,8 +4,32 @@
 // To enable PAC return address signing compile with following clang arguments:
 // -march=armv8.3-a -mbranch-protection=pac-ret+leaf
 
+#include 
 #include 
 
+#if defined(__FreeBSD__)
+#include 
+
+// Added in FreeBSD 12.2, can be removed when all supported branches define it.
+#if !defined(HWCAP_PACA)
+#define	HWCAP_PACA	0x4000
+#endif
+
+static bool pac_supported(void) {
+  unsigned long hwcap;
+
+  if (elf_aux_info(AT_HWCAP, &hwcap, sizeof(hwcap)) != 0)
+return false;
+
+  return (hwcap & HWCAP_PACA) != 0;
+}
+#else
+static bool pac_supported(void) {
+  // We expect Linux to check for PAC up front using /proc/cpuinfo.
+  return true;
+}
+#endif
+
 static void __attribute__((noinline)) func_c(void) {
   exit(0); // Frame func_c
 }
@@ -19,6 +43,9 @@
 }
 
 int main(int argc, char *argv[]) {
+  if (!pac_supported())
+return 1;
+
   func_a(); // Frame main
   return 0;
 }
Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -6,28 +6,26 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+import re
 
 
 class AArch64UnwindPAC(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
-@skipIf(archs=no_match(["aarch64"]))
-@skipIf(oslist=no_match(['linux']))
-def test(self):
-"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
-if not self.isAArch64PAuth():
-self.skipTest('Target must support Pointer Authentication.')
-
+def common(self, backtrace_tail):
 self.build()
 
 self.line = line_number('main.c', '// Frame func_c')
 
 exe = self.getBuildArtifact("a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
-
 lldbutil.run_break_set_by_file_and_line(
 self, "main.c", self.line, num_expected_locations=1)
 self.runCmd("run", RUN_SUCCEEDED)
+if re.match('Process .* exited with status = 1[^0-9]', self.res.GetOutput()):
+# No PAC support
+self.skipTest('Target must support Pointer Authentication.')
+
 self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
 substrs=["stop reason = breakpoint 1."])
 
@@ -35,13 +33,33 @@
 process = target.GetProcess()
 thread = process.GetThreadAtIndex(0)
 
-backtrace = ["func_c", "func_b", "func_a", "main", "__libc_start_main", "_start"]
-self.assertEqual(thread.GetNumFrames(), len(backtrace))
+backtrace = ["func_c", "func_b", "func_a", "main"] + backtrace_tail
 for frame_idx, frame in enumerate(thread.frames):
 frame = thread.GetFrameAtIndex(frame_idx)
+# On FreeBSD we get ___lldb_unnamed_symbol symbols at the bottom
+# of the stack. Ignore these and assume we have walked the entire
+# stack, however check this assumption is correct.
+if frame.GetFunctionName().startswith("___lldb_unnamed_symbol"):
+self.assertEqual(len(backtrace), frame_idx)
+break
 self.assertTrue(frame)
 self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
 			# Check line number for functions in main.c
 if (frame_idx < 4):
 self.assertEqual(frame.GetLineEntry().GetLine(),
  line_number("main.c", "Frame " + backtrace[frame_idx]))
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['freebsd']))
+def test_freebsd(self):
+"""Test that we can backtrace correctly when AArch64 PAC is enabled"""
+self.common(["__start"])
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['linux']))
+def test_linux(self):
+"""Test that we can backtrace correctly when AA

[Lldb-commits] [PATCH] D110545: [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

[Andrew Turner via Phabricator via lldb-commits]

andrew created this revision.
Herald added subscribers: omjavaid, kristof.beyls, emaste.
andrew requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110545

Files:
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp


Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);


Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110545: [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

[Andrew Turner via Phabricator via lldb-commits]

andrew updated this revision to Diff 375293.
andrew added a comment.

Set opt_regsets to eRegsetMaskDefault


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110545

Files:
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp


Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets = RegisterInfoPOSIX_arm64::eRegsetMaskDefault;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);


Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -325,8 +325,9 @@
   sizeof(fpreg::fbsd_reg)))
 
 TEST(RegisterContextFreeBSDTest, arm64) {
+  Flags opt_regsets = RegisterInfoPOSIX_arm64::eRegsetMaskDefault;
   ArchSpec arch{"aarch64-unknown-freebsd"};
-  RegisterInfoPOSIX_arm64 reg_ctx{arch};
+  RegisterInfoPOSIX_arm64 reg_ctx{arch, opt_regsets};
 
   EXPECT_GPR_ARM64(x0, x[0]);
   EXPECT_GPR_ARM64(x1, x[1]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110545: [lldb] [unittests] Fix building the FreeBSD arm64 Register Context test

[Andrew Turner via Phabricator via lldb-commits]

andrew added a comment.

Yes, I found it by trying to build the tests on FreeBSD/arm64


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110545

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

[Lldb-commits] [PATCH] D110569: [lldb] [Process/FreeBSD] Rework arm64 register access

[Andrew Turner via Phabricator via lldb-commits]

andrew created this revision.
andrew added a reviewer: mgorny.
Herald added subscribers: omjavaid, kristof.beyls, krytarowski, arichardson, 
emaste.
andrew requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

To simplify future register access rework the FreeBSD arm64 register
read/write functions to be similar to on Linux.

This will simplify adding more register sets, e.g. for Pointer
Authentication.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110569

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h

Index: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
===
--- lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
+++ lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
@@ -63,14 +63,29 @@
   // LLDB's GPR/FPU structs.  However, all fields have matching offsets
   // and sizes, so we do not have to worry about these (and we have
   // a unittest to assert that).
-  std::array m_reg_data;
+  std::array m_reg_data;
+  std::array m_fpreg_data;
 #ifdef LLDB_HAS_FREEBSD_WATCHPOINT
   dbreg m_dbreg;
   bool m_read_dbreg;
 #endif
 
-  Status ReadRegisterSet(uint32_t set);
-  Status WriteRegisterSet(uint32_t set);
+  void *GetGPRBuffer() { return &m_reg_data; }
+  size_t GetGPRBufferSize() { return sizeof(m_reg_data); }
+
+  void *GetFPRBuffer() { return &m_fpreg_data; }
+  size_t GetFPRSize() { return sizeof(m_fpreg_data); }
+
+  bool IsGPR(unsigned reg) const;
+  bool IsFPR(unsigned reg) const;
+
+  Status ReadGPR();
+  Status WriteGPR();
+
+  Status ReadFPR();
+  Status WriteFPR();
+
+  uint32_t CalculateFprOffset(const RegisterInfo *reg_info) const;
 
   llvm::Error ReadHardwareDebugInfo() override;
   llvm::Error WriteHardwareDebugRegs(DREGType hwbType) override;
Index: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
@@ -24,6 +24,8 @@
 #include 
 // clang-format on
 
+#define REG_CONTEXT_SIZE (GetGPRSize() + GetFPRSize())
+
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_freebsd;
@@ -68,30 +70,43 @@
   return count;
 }
 
-Status NativeRegisterContextFreeBSD_arm64::ReadRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
-   m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(
-PT_GETFPREGS, m_thread.GetID(),
-m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::ReadRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsGPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
+  RegisterInfoPOSIX_arm64::GPRegSet)
+return true;
+  return false;
 }
 
-Status NativeRegisterContextFreeBSD_arm64::WriteRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(),
-   m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(
-PT_SETFPREGS, m_thread.GetID(),
-m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::WriteRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsFPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
+  RegisterInfoPOSIX_arm64::FPRegSet)
+return true;
+  return false;
+}
+
+Status NativeRegisterContextFreeBSD_arm64::ReadGPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_GETREGS, m_thread.GetID(), m_reg_data.data());
+}
+
+Status NativeRegisterContextFreeBSD_arm64::WriteGPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_SETREGS, m_thread.GetID(), m_reg_data.data());
+}
+
+Status NativeRegisterContextFreeBSD_arm64::ReadFPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_GETFPREGS, m_thread.GetID(), m_fpreg_data.data());
+}
+
+Status NativeRegisterContextFreeBSD_arm64::WriteFPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+  PT_SETFPREGS, m_thread.GetID(), m_fpreg_data.data());
+}
+
+uint32_t NativeRegisterContextFreeBSD_arm64::CalculateFprOffset(
+const RegisterInfo *reg_info) const {
+  return reg_inf

[Lldb-commits] [PATCH] D110569: [lldb] [Process/FreeBSD] Rework arm64 register access

[Andrew Turner via Phabricator via lldb-commits]

andrew added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:73
 
-Status NativeRegisterContextFreeBSD_arm64::ReadRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
-   m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-return NativeProcessFreeBSD::PtraceWrapper(
-PT_GETFPREGS, m_thread.GetID(),
-m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::ReadRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsGPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==

mgorny wrote:
> These helpers don't seem very helpful. Isn't it better to use `switch` on the 
> value returned by `GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg)`?
Using a switch will be messy when adding pointer authentication or MTE 
registers as we need to call `RegisterInfoPOSIX_arm64::IsPAuthReg` [1] to check 
if a given register is a pointer authentication register, and similar for MTE. 
This is because they are dynamically added as needed.

[1] 
https://github.com/llvm/llvm-project/blob/76e47d4/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp#L392-L396



Comment at: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:133
+  if (IsGPR(reg)) {
+error = ReadGPR();
+if (error.Fail())

mgorny wrote:
> To be honest, it seems counterproductive to restore the duplication that I've 
> removed before. Is there any real advantage to having two methods here, and 
> calling them separately from inside the `if` instead of calling 
> `ReadRegisterSet(set)` before the `if`?
Because of the dynamic nature of the register sets we may not have a usable 
value to pass into `ReadRegisterSet` and any logic to decide which register set 
is being read in `ReadRegisterSet` would need to be repeated here and below 
when writing a register.

Calling the function `ReadRegisterSet` will also be confusing when I add 
`PT_GETREGSET` and `PT_SETREGSET` [1] to FreeBSD. This will be used to access 
the pointer authentication address masks on FreeBSD (and future registers, e.g. 
MTE & SVE).

See [2] for a WIP patch with how I plan to extend this for code & data address 
masks needed by pointer authentication.

[1] https://reviews.freebsd.org/D19831
[2] 
https://github.com/zxombie/llvm-project/commit/b801eca#diff-3d6a219398418eb59bf8104bda44a111f47d29081e076b4ce69336882e482cc2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110569

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

[Lldb-commits] [PATCH] D101361: [LLDB] Support AArch64/Linux watchpoint on tagged addresses

[Andrew Turner via Phabricator via lldb-commits]

andrew added a comment.

FreeBSD doesn't currently support TBI. I'm trying to decide if it will be 
enabled everywhere, or just when needed (e.g. for HWASAN) due to how it 
interacts with PAC.

I have a patch in review in the FreeBSD phabricator to add PAC support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101361

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