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

Early in the adoption of pointer authentication on Darwin systems, we had most 
environments running with the same number of addressing bits in use on AArch64 
systems, and didn't prioritize fetching that value dynamically. Our 
environments have grown so there is no correct fixed value, and the correct 
value is determined dynamically in most scenarios; the hardcoding of one value 
can cause problems when running on a larger address space environment that may 
set its value later.

Change ABIMacOSX_arm64 to mask off the top byte unconditionally when we have no 
dynamic information about the number of addressing bits, instead of hardcoding 
the old addressable bits value.

I had a test which created a corefile with no LC_NOTE markup for the number of 
addressable bits, and tested against that to confirm the hardcoded default 
value was in effect.  Remove that test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148603

Files:
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/test/API/macosx/corefile-default-ptrauth/Makefile
  lldb/test/API/macosx/corefile-default-ptrauth/TestCorefileDefaultPtrauth.py
  lldb/test/API/macosx/corefile-default-ptrauth/create-corefile.c
  lldb/test/API/macosx/corefile-default-ptrauth/main.c

Index: lldb/test/API/macosx/corefile-default-ptrauth/main.c
===================================================================
--- lldb/test/API/macosx/corefile-default-ptrauth/main.c
+++ /dev/null
@@ -1,6 +0,0 @@
-int main();
-int (*fmain)() = main;
-int main () {
-  return fmain();
-}
-
Index: lldb/test/API/macosx/corefile-default-ptrauth/create-corefile.c
===================================================================
--- lldb/test/API/macosx/corefile-default-ptrauth/create-corefile.c
+++ /dev/null
@@ -1,189 +0,0 @@
-#include <mach-o/loader.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <mach/machine.h>
-#include <string.h>
-#include <mach/machine/thread_state.h>
-#include <inttypes.h>
-#include <sys/syslimits.h>
-#include <uuid/uuid.h>
-
-// Given an executable binary with 
-//   "fmain" (a function pointer to main)
-//   "main"
-// symbols, create a fake arm64e corefile that
-// contains a memory segment for the fmain 
-// function pointer, with the value of the 
-// address of main() with ptrauth bits masked on.
-//
-// The corefile does not include the "addrable bits"
-// LC_NOTE, so lldb will need to fall back on its 
-// default value from the Darwin arm64 ABI.
-
-int main(int argc, char **argv)
-{
-  if (argc != 3) {
-    fprintf (stderr, "usage: %s executable-binary output-file\n", argv[0]);
-    exit(1);
-  }
-  FILE *exe = fopen(argv[1], "r");
-  if (!exe) {
-    fprintf (stderr, "Unable to open executable %s for reading\n", argv[1]);
-    exit(1);
-  }
-  FILE *out = fopen(argv[2], "w");
-  if (!out) {
-    fprintf (stderr, "Unable to open %s for writing\n", argv[2]);
-    exit(1);
-  }
-
-  char buf[PATH_MAX + 6];
-  sprintf (buf, "nm '%s'", argv[1]);
-  FILE *nm = popen(buf, "r");
-  if (!nm) {
-    fprintf (stderr, "Unable to run nm on '%s'", argv[1]);
-    exit (1);
-  }
-  uint64_t main_addr = 0;
-  uint64_t fmain_addr = 0;
-  while (fgets (buf, sizeof(buf), nm)) {
-    if (strstr (buf, "_fmain")) {
-      fmain_addr = strtoul (buf, NULL, 16);
-    }
-    if (strstr (buf, "_main")) {
-      main_addr = strtoul (buf, NULL, 16);
-    }
-  }
-  pclose (nm);
-
-  sprintf (buf, "dwarfdump -u '%s'", argv[1]);
-  FILE *dwarfdump = popen(buf, "r");
-  if (!dwarfdump) {
-    fprintf (stderr, "Unable to run dwarfdump -u on '%s'\n", argv[1]);
-    exit (1);
-  }
-  uuid_t uuid;
-  uuid_clear (uuid);
-  while (fgets (buf, sizeof(buf), dwarfdump)) {
-    if (strncmp (buf, "UUID: ", 6) == 0) {
-      buf[6 + 36] = '\0';
-      if (uuid_parse (buf + 6, uuid) != 0) {
-        fprintf (stderr, "Unable to parse UUID in '%s'\n", buf);
-        exit (1);
-      }
-    }
-  }
-  if (uuid_is_null(uuid)) {
-    fprintf (stderr, "Got a null uuid for the binary\n");
-    exit (1);
-  }
-
-  if (main_addr == 0 || fmain_addr == 0) {
-    fprintf(stderr, "Unable to find address of main or fmain in %s.\n",
-        argv[1]);
-    exit (1);
-  }
-
-  // Write out a corefile with contents in this order:
-  //    1. mach header
-  //    2. LC_THREAD load command
-  //    3. LC_SEGMENT_64 load command
-  //    4. LC_NOTE load command
-  //    5. memory segment contents
-  //    6. "load binary" note contents
-
-  // struct thread_command {
-  //       uint32_t        cmd;    
-  //       uint32_t        cmdsize;
-  //       uint32_t flavor      
-  //       uint32_t count       
-  //       struct XXX_thread_state state
-  int size_of_thread_cmd = 4 + 4 + 4 + 4 + sizeof (arm_thread_state64_t);
-
-  struct mach_header_64 mh;
-  mh.magic = 0xfeedfacf;
-  mh.cputype = CPU_TYPE_ARM64;
-  mh.cpusubtype = CPU_SUBTYPE_ARM64E;
-  mh.filetype = MH_CORE;
-  mh.ncmds = 3; // LC_THREAD, LC_SEGMENT_64, LC_NOTE
-  mh.sizeofcmds = size_of_thread_cmd + sizeof(struct segment_command_64) + sizeof(struct note_command);
-  mh.flags = 0;
-  mh.reserved = 0;
-
-  fwrite(&mh, sizeof (mh), 1, out);
-
-  struct note_command lcnote;
-  struct segment_command_64 seg;
-  seg.cmd = LC_SEGMENT_64;
-  seg.cmdsize = sizeof(seg);
-  memset (&seg.segname, 0, 16);
-  seg.vmaddr = fmain_addr;
-  seg.vmsize = 8;
-  // Offset to segment contents
-  seg.fileoff = sizeof (mh) + size_of_thread_cmd + sizeof(seg) + sizeof(lcnote);
-  seg.filesize = 8;
-  seg.maxprot = 3;
-  seg.initprot = 3;
-  seg.nsects = 0;
-  seg.flags = 0;
-
-  fwrite (&seg, sizeof (seg), 1, out);
-
-  uint32_t cmd = LC_THREAD;
-  fwrite (&cmd, sizeof (cmd), 1, out);
-  uint32_t cmdsize = size_of_thread_cmd;
-  fwrite (&cmdsize, sizeof (cmdsize), 1, out);
-  uint32_t flavor = ARM_THREAD_STATE64;
-  fwrite (&flavor, sizeof (flavor), 1, out);
-  // count is number of uint32_t's of the register context
-  uint32_t count = sizeof (arm_thread_state64_t) / 4;
-  fwrite (&count, sizeof (count), 1, out);
-  arm_thread_state64_t regstate;
-  memset (&regstate, 0, sizeof (regstate));
-  fwrite (&regstate, sizeof (regstate), 1, out);
-
-  lcnote.cmd = LC_NOTE;
-  lcnote.cmdsize = sizeof (lcnote);
-  strcpy (lcnote.data_owner, "load binary");
-
-  // 8 is the size of the LC_SEGMENT contents
-  lcnote.offset = sizeof (mh) + size_of_thread_cmd + sizeof(seg) + sizeof(lcnote) + 8;
-
-  // struct load_binary
-  // {
-  // uint32_t version;        // currently 1
-  // uuid_t   uuid;           // all zeroes if uuid not specified
-  // uint64_t load_address;   // virtual address where the macho is loaded, UINT64_MAX if unavail
-  // uint64_t slide;          // slide to be applied to file address to get load address, 0 if unavail
-  // char     name_cstring[]; // must be nul-byte terminated c-string, '\0' alone if name unavail
-  // } __attribute__((packed));
-  lcnote.size = 4 + 16 + 8 + 8 + sizeof("a.out");
-
-  fwrite (&lcnote, sizeof(lcnote), 1, out);
-
-  // Write the contents of the memory segment
-
-  // Or together a random PAC value from a system using 39 bits 
-  // of addressing with the address of main().  lldb will need
-  // to correctly strip off the high bits to find the address of
-  // main.
-  uint64_t segment_contents = 0xe46bff0000000000 | main_addr;
-  fwrite (&segment_contents, sizeof (segment_contents), 1, out);
-
-  // Now write the contents of the "load binary" LC_NOTE.
-  {
-    uint32_t version = 1;
-    fwrite (&version, sizeof (version), 1, out);
-    fwrite (&uuid, sizeof (uuid), 1, out);
-    uint64_t load_address = UINT64_MAX;
-    fwrite (&load_address, sizeof (load_address), 1, out);
-    uint64_t slide = 0;
-    fwrite (&slide, sizeof (slide), 1, out);
-    strcpy (buf, "a.out");
-    fwrite (buf, 6, 1, out);
-  }
-
-  fclose (out);
-
-  exit (0);
-}
Index: lldb/test/API/macosx/corefile-default-ptrauth/TestCorefileDefaultPtrauth.py
===================================================================
--- lldb/test/API/macosx/corefile-default-ptrauth/TestCorefileDefaultPtrauth.py
+++ /dev/null
@@ -1,51 +0,0 @@
-"""Test that lldb has a default mask for addressable bits on Darwin arm64 ABI"""
-
-
-import os
-import re
-import subprocess
-
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class TestCorefileDefaultPtrauth(TestBase):
-
-    @skipIf(debug_info=no_match(["dsym"]), bugnumber="This test is looking explicitly for a dSYM")
-    @skipIf(archs=no_match(['arm64','arm64e']))
-    @skipUnlessDarwin
-    @skipIfRemote
-    def test_lc_note(self):
-        self.build()
-        self.test_exe = self.getBuildArtifact("a.out")
-        self.create_corefile = self.getBuildArtifact("create-corefile")
-        self.corefile = self.getBuildArtifact("core")
-
-        ### Create our corefile
-        retcode = call(self.create_corefile + " " +  self.test_exe + " " + self.corefile, shell=True)
-
-        ## This corefile has no metadata telling us how many bits are
-        ## used for ptrauth signed function pointers.  We will need lldb
-        ## to fall back on its old default value for Darwin arm64 ABIs
-        ## to correctly strip the bits.
-
-        # Create a Target with our main executable binary to get it
-        # seeded in lldb's global module cache.  Then delete the Target.
-        # This way when the corefile searches for a binary with its UUID,
-        # it'll be found by that search.
-        initial_target = self.dbg.CreateTarget(self.test_exe)
-        self.dbg.DeleteTarget(initial_target)
-
-        self.target = self.dbg.CreateTarget('')
-        err = lldb.SBError()
-        self.process = self.target.LoadCore(self.corefile)
-        self.assertEqual(self.process.IsValid(), True)
-
-        # target variable should show us both the actual function
-        # pointer with ptrauth bits and the symbol it resolves to,
-        # with the ptrauth bits stripped, e.g.
-        #  (int (*)(...)) fmain = 0xe46bff0100003f90 (actual=0x0000000100003f90 a.out`main at main.c:3)
-
-        self.expect("target variable fmain", substrs=['fmain = 0x', '(actual=0x', 'main at main.c'])
Index: lldb/test/API/macosx/corefile-default-ptrauth/Makefile
===================================================================
--- lldb/test/API/macosx/corefile-default-ptrauth/Makefile
+++ /dev/null
@@ -1,12 +0,0 @@
-C_SOURCES := main.c
-
-# compile a.out and create-corefile
-# create-corefile will create a custom corefile using the symbols
-# addresses from the a.out binary.
-all: a.out create-corefile
-
-create-corefile:
-	$(MAKE) -f $(MAKEFILE_RULES) EXE=create-corefile \
-		C_SOURCES=create-corefile.c
-
-include Makefile.rules
Index: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
===================================================================
--- lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -816,15 +816,12 @@
 
 lldb::addr_t ABIMacOSX_arm64::FixAddress(addr_t pc, addr_t mask) {
   lldb::addr_t pac_sign_extension = 0x0080000000000000ULL;
-  // Darwin systems originally couldn't determine the proper value
-  // dynamically, so the most common value was hardcoded.  This has
-  // largely been cleaned up, but there are still a handful of
-  // environments that assume the default value is set to this value
-  // and there's no dynamic value to correct it.
-  // When no mask is specified, set it to 39 bits of addressing (0..38).
+  // When no mask is specified, clear/set the top byte; preserve
+  // the low 55 bits (00..54) for addressing and bit 55 to indicate
+  // sign.
   if (mask == 0) {
-    // ~((1ULL<<39)-1)
-    mask = 0xffffff8000000000;
+    // ~((1ULL<<55)-1)
+    mask = 0xff80000000000000;
   }
   return (pc & pac_sign_extension) ? pc | mask : pc & (~mask);
 }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] ... Jason Molenda via Phabricator via lldb-commits

Reply via email to