[LLDB] AArch64 Linux and elf-core PAC stack unwinder support

[omjavaid]

Summary

This patch builds on D100521 and other related patches to add support
for unwinding stack on AArch64 systems with pointer authentication
feature enabled.

We override FixCodeAddress and FixDataAddress function in ABISysV_arm64
class. We now try to calculate and set code and data masks after reading
data_mask and code_mask registers exposed by AArch64 targets running Linux.

This patch utilizes core file linux-aarch64-pac.core for testing that
LLDB can successfully unwind stack frames in the presence of signed
return address after masking off ignored bits.

This patch also includes a AArch64 Linux native test case to demonstrate
successful back trace calculation in presence of pointer authentication
feature.

[labath]

Setting the address size from the register context is a bit awkward. We have one register context per thread, and they all will be competing to set the value (it should always be the same value under normal circumstances, but it still makes for a strange relationship). It would be better if all of this could be contained in the ABI class. The ABI classes have intimate knowledge of the registers anyway, so it would not be unusual for it to access a specific register to get the mask (here I am assuming that the mask is actually stored in a register).

Then we'd have no need for Process::GetAddressBitsInUse and worry whether it has been initialized, as the abi class could fetch the value when it gets constructed, or something...

Do you envision using this in some other way than through the ABI class?

lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp

41–42

I think this should be an assert (here, or even somewhere earlier).

43–45

return llvm::SignExtend64(pc, address_shift_width)

[jasonmolenda]

Hi Omair, sorry for the delay in looking at this. It seems like we have two overlapping patches here, @justincohen patch in https://reviews.llvm.org/D98886 and this one.

I'm not convinced that we'll see a target where we have noncontiguous bits used for addressing (the best reason for using a mask), but Justin points out that Linux ARM's NT_ARM_PAC_MASK (gettable by ptrace PTRACE_GETREGSET) is expressed as a mask (and googling around, it looks like you can have a different mask for code pointers and data pointers, so Process would really need to store both, but it may be that for crashpad only the code mask needs to be tracked in Process).

lldb needs to allow for users to set the number of bits used for addressing in terms of a number -- 52 in your example -- but given that Linux wants to give us a mask via the ptrace call, I think we should preserve and use that mask. Darwin's API also provide the value in terms of the number of bits used for addressing, and as a user setting I think that's the most natural way of handling it and we should allow it. But internal to Process, we should convert this to a CodeAddress mask and down in the ABI's where we support AArch v8.3 ptrauth, we use bit 55 to clear/set the non-addressing high bits.

Omair, Justin, what do you think here? I don't think it's especially hard to accept this in terms of # of bits OR a mask, and we should use the more general internal rep in lldb. Another alternative would be "the mask should be converted to the # of bits in addressing and stored in Process in those terms".

(Honestly the idea of an address mask makes me nervous all over - if the rule is "take bit 55 and set all non-addressing bits to 0 or 1 depending on that", then we're implicitly saying "the mask only masks off high bits". If it ever masked off bit 0, for instance, on a code address, then we're going to set that to 0 or 1 when we set/clear non-addressing bits? Obviously not. So it's not REALLY a mask, is it, it's just the # of bits used for addressing in another more flexible format that you can't actually flex or it all breaks. :)

[justincohen]

In D99944#2684280, @jasonmolenda wrote:

Omair, Justin, what do you think here? I don't think it's especially hard to accept this in terms of # of bits OR a mask, and we should use the more general internal rep in lldb. Another alternative would be "the mask should be converted to the # of bits in addressing and stored in Process in those terms".

From a minidump/crashpad perspective, we are fine with either approach. We plan to a use a mask within the minidump format, but we can convert it to # of bits as necessary.

[jasonmolenda]

I think this will come down to, who wants to implement a patch that can set/get either form through Process API. The mask might be a more general format, so if Justin is willing to update his patch to that we can add Omair's test case to his patch?

[omjavaid]

In D99944#2684280, @jasonmolenda wrote:

Hi Omair, sorry for the delay in looking at this. It seems like we have two overlapping patches here, @justincohen patch in https://reviews.llvm.org/D98886 and this one.

I'm not convinced that we'll see a target where we have noncontiguous bits used for addressing (the best reason for using a mask), but Justin points out that Linux ARM's NT_ARM_PAC_MASK (gettable by ptrace PTRACE_GETREGSET) is expressed as a mask (and googling around, it looks like you can have a different mask for code pointers and data pointers, so Process would really need to store both, but it may be that for crashpad only the code mask needs to be tracked in Process).

lldb needs to allow for users to set the number of bits used for addressing in terms of a number -- 52 in your example -- but given that Linux wants to give us a mask via the ptrace call, I think we should preserve and use that mask. Darwin's API also provide the value in terms of the number of bits used for addressing, and as a user setting I think that's the most natural way of handling it and we should allow it. But internal to Process, we should convert this to a CodeAddress mask and down in the ABI's where we support AArch v8.3 ptrauth, we use bit 55 to clear/set the non-addressing high bits.

Omair, Justin, what do you think here? I don't think it's especially hard to accept this in terms of # of bits OR a mask, and we should use the more general internal rep in lldb. Another alternative would be "the mask should be converted to the # of bits in addressing and stored in Process in those terms".

(Honestly the idea of an address mask makes me nervous all over - if the rule is "take bit 55 and set all non-addressing bits to 0 or 1 depending on that", then we're implicitly saying "the mask only masks off high bits". If it ever masked off bit 0, for instance, on a code address, then we're going to set that to 0 or 1 when we set/clear non-addressing bits? Obviously not. So it's not REALLY a mask, is it, it's just the # of bits used for addressing in another more flexible format that you can't actually flex or it all breaks. :)

Frankly I am also not too sure about keeping single mask for both code and data masks. For now Linux kernel ensures both are the same but they can differ in future. For user-space Linux apps top-byte is used for tags and bit 55 only gives us a hint on whether we mask the top byte only or also need to mask off bits (54:VA_Bits). For unwinder code mask is relevant but we would also like to use the mask for clearing top-bits off watchpoint addresses in-case of a tagged data pointer and if we have a separate data_mask we ll essentially need to store that information separately.

On linux masks are readable through code_mask and data_mask registers. We dont really need to keep this information in process as @labath pointed out but that is only fine for user-space apps. In case we intend to debug linux kernel or any bare-metal app utilizing the top byte(s) which i am not aware of any user of LLDB right now using it to debug the Linux kernel but I dont wanna introduce anything here which makes LLDB user-app specific so thats why one of the option is to send the information over gdb-remote protocol qHostInfo but that also restricts LLDB to lldb-server only.

In any case we ll need atleast one mask for sure and I dont mind @justincohen writing that patch and I ll rebase this patch on top of it once its merged.

[JDevlieghere]

Coincidentally, I was just in need of something similar. Jason pointed me to this discussion so I took the liberty of implementing his suggestion in D100515

[omjavaid]

Moved mask calculation to ABISysV_arm64 class.

[omjavaid]

This merges separate patch D99947 for Linux into this one. Linux specific changes are no longer required as we have moved mask calculation into ABISysV_arm64 class.

[DavidSpickett]

The code LGTM but others should decide if this fits with their efforts.

lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile

4

Nit: armv8.3-a would also be fine here

[DavidSpickett]

Realised that you've got an empty program file and no core file. I compiled main.c with:

$ aarch64-unknown-linux-gnu-gcc -g -march=armv8.3-a -mbranch-protection=pac-ret+leaf main.c -o linux-aarch64-pac.out -nostdlib -static

Then generated a core dump in qemu and after fixing up the PID it works as expected.

[omjavaid]

This add skipped linux-aarch64-pac.out file.

[omjavaid]

Uploading linux-aarch64-pac.out using arc

differential was not uploading binary file properly using arc to do the same.

[DavidSpickett]

I realised my mistake, I thought this was adding a new core file but in fact it's using the one you added for the register tests. So now the outfile is there the test passes.

lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py

42

Strange indent here

lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c

6

I'd rather use armv8.3-a here just for correctness.

[DavidSpickett]

Resigning to remove my requested changes, if that works. Looks good from my point of view.

@labath You had earlier questions about using the register context, and a few comments that have gotten disconnected from the original diff somehow. Those concerns still relevant?

lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h

public:
   static lldb_private::ConstString GetPluginNameStatic();
 
   // PluginInterface protocol
 
   lldb_private::ConstString GetPluginName() override;
 
   uint32_t GetPluginVersion() override;
 
+  lldb::addr_t FixCodeAddress(lldb::addr_t pc) override;
+  lldb::addr_t FixDataAddress(lldb::addr_t pc) override;
+
 protected:
   lldb::ValueObjectSP
   GetReturnValueObjectImpl(lldb_private::Thread &thread,
                            lldb_private::CompilerType &ast_type) const override;
 
 private:
   using ABIAArch64::ABIAArch64; // Call CreateInstance instead.
 };
 
 #endif // LLDB_SOURCE_PLUGINS_ABI_AARCH64_ABISYSV_ARM64_H

lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp

 using namespace lldb;
 using namespace lldb_private;
 
 bool ABISysV_arm64::GetPointerReturnRegister(const char *&name) {
   name = "x0";
   return true;
 }
 
 size_t ABISysV_arm64::GetRedZoneSize() const { return 128; }
 
 // Static Functions
 
 ABISP
 ABISysV_arm64::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
   const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
   const llvm::Triple::VendorType vendor_type = arch.GetTriple().getVendor();
 
   if (vendor_type != llvm::Triple::Apple) {
     if (arch_type == llvm::Triple::aarch64 ||
         arch_type == llvm::Triple::aarch64_32) {
       return ABISP(
ValueObjectSP ABISysV_arm64::GetReturnValueObjectImpl(
   return return_valobj_sp;
 }
 
 lldb::addr_t ABISysV_arm64::FixAddress(addr_t pc, addr_t mask) {
   lldb::addr_t pac_sign_extension = 0x0080000000000000ULL;
   return (pc & pac_sign_extension) ? pc | mask : pc & (~mask);
 }
 
+// Reads code or data address mask for the current Linux process.
+static lldb::addr_t ReadLinuxProcessAddressMask(lldb::ProcessSP process_sp,
+                                                llvm::StringRef reg_name) {
+  // Linux configures user-space virtual addresses with top byte ignored.
+  // We set default value of mask such that top byte is masked out.
+  uint64_t address_mask = ~((1ULL << 56) - 1);
+  // If Pointer Authentication feature is enabled then Linux exposes
+  // PAC data and code mask register. Try reading relevant register
+  // below and merge it with default address mask calculated above.
+  lldb::ThreadSP thread_sp = process_sp->GetThreadList().GetSelectedThread();
+  if (thread_sp) {
+    lldb::RegisterContextSP reg_ctx_sp = thread_sp->GetRegisterContext();
+    if (reg_ctx_sp) {
+      const RegisterInfo *reg_info =
+          reg_ctx_sp->GetRegisterInfoByName(reg_name, 0);
+      if (reg_info) {
+        lldb::addr_t mask_reg_val = reg_ctx_sp->ReadRegisterAsUnsigned(
+            reg_info->kinds[eRegisterKindLLDB], LLDB_INVALID_ADDRESS);
+        if (mask_reg_val != LLDB_INVALID_ADDRESS)
+          address_mask |= mask_reg_val;
+      }
+    }
+  }
+  return address_mask;
+}
+
+lldb::addr_t ABISysV_arm64::FixCodeAddress(lldb::addr_t pc) {
+  if (lldb::ProcessSP process_sp = GetProcessSP()) {
+    if (process_sp->GetTarget().GetArchitecture().GetTriple().isOSLinux() &&
+        !process_sp->GetCodeAddressMask())
+      process_sp->SetCodeAddressMask(
+          ReadLinuxProcessAddressMask(process_sp, "code_mask"));
+
+    return FixAddress(pc, process_sp->GetCodeAddressMask());
+  }
+  return pc;
+}
+
+lldb::addr_t ABISysV_arm64::FixDataAddress(lldb::addr_t pc) {
+  if (lldb::ProcessSP process_sp = GetProcessSP()) {
+    if (process_sp->GetTarget().GetArchitecture().GetTriple().isOSLinux() &&
+        !process_sp->GetDataAddressMask())
+      process_sp->SetDataAddressMask(
+          ReadLinuxProcessAddressMask(process_sp, "data_mask"));
+
+    return FixAddress(pc, process_sp->GetDataAddressMask());
+  }
+  return pc;
+}
+
 void ABISysV_arm64::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
                                 "SysV ABI for AArch64 targets", CreateInstance);
 }
 
 void ABISysV_arm64::Terminate() {
   PluginManager::UnregisterPlugin(CreateInstance);
 }

lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py

 
 
 class LinuxCoreTestCase(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
     mydir = TestBase.compute_mydir(__file__)
 
     _aarch64_pid = 37688
+    _aarch64_pac_pid = 387
     _i386_pid = 32306
     _x86_64_pid = 32259
     _s390x_pid = 1045
     _ppc64le_pid = 28147
 
     _aarch64_regions = 4
     _i386_regions = 4
     _x86_64_regions = 5
def test_x86_64_sysroot(self):
         mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
         self.assertEqual(mod_path, exe_inside)
         self.check_all(process, self._x86_64_pid,
                        self._x86_64_regions, "a.out")
 
         self.dbg.DeleteTarget(target)
 
     @skipIfLLVMTargetMissing("AArch64")
+    def test_aarch64_pac(self):
+        """Test that lldb can unwind stack for AArch64 elf core file with PAC enabled."""
+
+        target = self.dbg.CreateTarget("linux-aarch64-pac.out")
+        self.assertTrue(target, VALID_TARGET)
+        process = target.LoadCore("linux-aarch64-pac.core")
+
+        self.check_all(process, self._aarch64_pac_pid, self._aarch64_regions, "a.out")
+
+        self.dbg.DeleteTarget(target)
+
+    @skipIfLLVMTargetMissing("AArch64")
     @expectedFailureAll(archs=["aarch64"], oslist=["freebsd"],
                         bugnumber="llvm.org/pr49415")
     def test_aarch64_regs(self):
         # check 64 bit ARM core files
         target = self.dbg.CreateTarget(None)
         self.assertTrue(target, VALID_TARGET)
         process = target.LoadCore("linux-aarch64-neon.core")
 

lldb/test/API/functionalities/unwind/aarch64_unwind_pac/Makefile

+C_SOURCES := main.c
+
+CFLAGS := -g -Os -march=armv8.3-a -mbranch-protection=pac-ret+leaf
+
+include Makefile.rules

lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py

+"""
+Test that we can backtrace correctly when AArch64 PAC is enabled
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+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"""
+        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)
+        self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+                    substrs=["stop reason = breakpoint 1."])
+
+        target = self.dbg.GetSelectedTarget()
+        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))
+        for frame_idx, frame in enumerate(thread.frames):
+            frame = thread.GetFrameAtIndex(frame_idx)
+            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]))

lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c

+// This program makes a multi tier nested function call to test AArch64
+// Pointer Authentication feature.
+
+// To enable PAC return address signing compile with following clang arguments:
+// -march=armv8.3-a -mbranch-protection=pac-ret+leaf
+
+#include <stdlib.h>
+
+static void __attribute__((noinline)) func_c(void) {
+  exit(0); // Frame func_c
+}
+
+static void __attribute__((noinline)) func_b(void) {
+  func_c(); // Frame func_b
+}
+
+static void __attribute__((noinline)) func_a(void) {
+  func_b(); // Frame func_a
+}
+
+int main(int argc, char *argv[]) {
+  func_a(); // Frame main
+  return 0;
+}