From: Paul Floyd <pjfloyd@wanadoo.fr>
Date: Sun, 25 Feb 2024 18:10:37 +0000 (+0100)
Subject: FreeBSD: experimental fix for --sanity-level=3 and above
X-Git-Tag: VALGRIND_3_23_0~135
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a306848d8bce107f2e72e81229feedece6954358;p=thirdparty%2Fvalgrind.git

FreeBSD: experimental fix for --sanity-level=3 and above

Previously this failed due to split mmap mappings for MAP_STACK.
This change tries to piece together such stack mappings. This
mainly affects multithreaded apps when they create their thread
stacks.
---

diff --git a/coregrind/m_aspacemgr/aspacemgr-linux.c b/coregrind/m_aspacemgr/aspacemgr-linux.c
index aab9eb8d18..4ed8e40ff9 100644
--- a/coregrind/m_aspacemgr/aspacemgr-linux.c
+++ b/coregrind/m_aspacemgr/aspacemgr-linux.c
@@ -3906,6 +3906,68 @@ Bool VG_(get_changed_segments)(
 /*------BEGIN-procmaps-parser-for-Freebsd------------------------*/
 #elif defined(VGO_freebsd)
 
+/*
+ * Some more nasty hacks.
+ *
+ * On FreeBSD mmap with MAP_STACK will result in TWO adjacent areas being mapped.
+ * Assuming a grow down stack, the one in the lower address is a growth guard
+ * area. The one in the higher area is the stack. The kernel will automatically
+ * extend the stack into the growth guard. Valgrind doesn't see any of that.
+ * When we see mapped memory like that, we need to try to merge them so that
+ * they match the mmap that Valgrind saw and recorded.
+ *
+ * There is also the initial stack. Valgrind will have already recorded that
+ * with parse_procselfmaps. So we don't want to merge that.
+ */
+static char* maybe_merge_procmap_stack(char* p,  struct vki_kinfo_vmentry *kve, Addr* pEndPlusOne, UInt* pProt)
+{
+   static Bool sgrowsiz_read = False;
+   static SizeT kern_sgrowsiz;
+   if (!sgrowsiz_read) {
+      SizeT sysctl_size = sizeof(SizeT);
+      VG_(sysctlbyname)("kern.sgrowsiz", &kern_sgrowsiz, &sysctl_size, NULL, 0);
+      sgrowsiz_read = True;
+   }
+   char* p_next = p + kve->kve_structsize;
+   struct vki_kinfo_vmentry *kve_next = (struct vki_kinfo_vmentry *)(p_next);
+
+#if defined(VGP_amd64_freebsd)
+   // I think that this is the stacksize rlimit
+   // I could use sysctl kern.maxssiz for this
+   if ( *pEndPlusOne + kern_sgrowsiz - kve->kve_start == 512ULL*1024ULL*1024ULL) {
+      return p;
+   }
+#elif defined(VGP_x86_freebsd)
+   // sysctl kern.maxssiz OK for x86 on x86 but not x86 on amd64
+   if ( *pEndPlusOne + kern_sgrowsiz - kve->kve_start == 64ULL*1024ULL*1024ULL) {
+      return p;
+   }
+#endif
+
+   while (kve_next->kve_protection & VKI_KVME_PROT_READ &&
+          kve_next->kve_protection & VKI_KVME_PROT_WRITE &&
+          kve_next->kve_flags & VKI_KVME_FLAG_GROWS_DOWN &&
+          kve_next->kve_end - kve_next->kve_start == kern_sgrowsiz) {
+
+
+      *pEndPlusOne += kern_sgrowsiz;
+      if (kve_next->kve_protection & VKI_KVME_PROT_READ) {
+         *pProt |= VKI_PROT_READ;
+      }
+      if (kve_next->kve_protection & VKI_KVME_PROT_WRITE) {
+         *pProt |= VKI_PROT_WRITE;
+      }
+      if (kve_next->kve_protection & VKI_KVME_PROT_EXEC) {
+         *pProt |= VKI_PROT_EXEC;
+      }
+      p_next += kve->kve_structsize;
+      kve_next = (struct vki_kinfo_vmentry *)(p_next);
+   }
+   p_next -= kve->kve_structsize;
+   return p_next;
+}
+
+
 /*
  * PJF 2023-09-23
  *
@@ -3928,7 +3990,7 @@ Bool VG_(get_changed_segments)(
  * the RW PT_LOAD.
  *
  * For instance, objdump -p for memcheck-amd64-freebsd contains
- *     LOAD off    0x0000000000000000 vaddr 0x0000000038000000 paddr 0x0000000038000000 align 2**12
+ *    LOAD off    0x0000000000000000 vaddr 0x0000000038000000 paddr 0x0000000038000000 align 2**12
  *         filesz 0x00000000000c5124 memsz 0x00000000000c5124 flags r--
  *    LOAD off    0x00000000000c5130 vaddr 0x00000000380c6130 paddr 0x00000000380c6130 align 2**12
  *         filesz 0x00000000001b10df memsz 0x00000000001b10df flags r-x
@@ -4048,6 +4110,12 @@ static void parse_procselfmaps (
       if (record_gap && gapStart < start)
          (*record_gap) ( gapStart, start-gapStart );
 
+      if (kve->kve_type == VKI_KVME_TYPE_GUARD &&
+          record_mapping == sync_check_mapping_callback &&
+          VG_(clo_sanity_level) >= 3)  {
+         p = maybe_merge_procmap_stack(p, kve, &endPlusOne, &prot);
+      }
+
       if (record_mapping && start < endPlusOne) {
          (*record_mapping) ( start, endPlusOne-start,
                              prot, dev, ino,
@@ -4064,6 +4132,11 @@ static void parse_procselfmaps (
       }
 
       gapStart = endPlusOne;
+      // PJF I think that we need to walk this based on each entry's kve_structsize
+      // because sysctl kern.coredump_pack_fileinfo (on by default) can cause this
+      // array to be packed (for core dumps)
+      // the packing consists of only storing the used part of kve_path rather than
+      // the full 1024 bytes
       p += kve->kve_structsize;
    }
  
diff --git a/coregrind/m_aspacemgr/priv_aspacemgr.h b/coregrind/m_aspacemgr/priv_aspacemgr.h
index 161c5c2954..0c8a807f22 100644
--- a/coregrind/m_aspacemgr/priv_aspacemgr.h
+++ b/coregrind/m_aspacemgr/priv_aspacemgr.h
@@ -53,6 +53,10 @@
 
 #include "pub_core_options.h"    // VG_(clo_sanity_level)
 
+#if defined(VGO_freebsd)
+#include "pub_core_libcproc.h"   // VG_(sysctlbyname)
+#endif
+
 #include "pub_core_aspacemgr.h"  // self
 
 
diff --git a/coregrind/m_main.c b/coregrind/m_main.c
index 042b2787c7..10b2d9be97 100644
--- a/coregrind/m_main.c
+++ b/coregrind/m_main.c
@@ -1110,6 +1110,13 @@ void main_process_cmd_line_options( void )
          here. */
    }
 
+#if defined(VGO_freebsd)
+   if (VG_(clo_sanity_level) >= 3) {
+      VG_(debugLog)(0, "main", "Warning: due to transparent memory mappings with MAP_STACK\n");
+      VG_(debugLog)(0, "main", "--sanity-level=3 and above may give spurious errors.\n");
+   }
+#endif
+
    /* All non-logging-related options have been checked.  If the logging
       option specified is ok, we can switch to it, as we know we won't
       have to generate any other command-line-related error messages.
diff --git a/include/vki/vki-freebsd.h b/include/vki/vki-freebsd.h
index bd4f099765..1267eb3b3c 100644
--- a/include/vki/vki-freebsd.h
+++ b/include/vki/vki-freebsd.h
@@ -2164,6 +2164,8 @@ typedef struct vki_cap_rights       vki_cap_rights_t;
 #define VKI_KVME_TYPE_DEVICE        4
 #define VKI_KVME_TYPE_PHYS          5
 #define VKI_KVME_TYPE_DEAD          6
+#define VKI_KVME_TYPE_MGTDEVICE     8
+#define VKI_KVME_TYPE_GUARD         9
 #define VKI_KVME_TYPE_UNKNOWN       255
 
 #define VKI_KVME_PROT_READ          0x00000001
@@ -2172,6 +2174,11 @@ typedef struct vki_cap_rights       vki_cap_rights_t;
 
 #define VKI_KVME_FLAG_COW           0x00000001
 #define VKI_KVME_FLAG_NEEDS_COPY    0x00000002
+#define VKI_KVME_FLAG_NOCOREDUMP    0x00000004
+#define VKI_KVME_FLAG_SUPER         0x00000008
+#define VKI_KVME_FLAG_GROWS_UP      0x00000010
+#define VKI_KVME_FLAG_GROWS_DOWN    0x00000020
+#define VKI_KVME_FLAG_USER_WIRED    0x00000040
 
 struct vki_kinfo_vmentry {
    int   kve_structsize;
@@ -2559,8 +2566,9 @@ struct vki_ps_strings {
 
 
 #define VKI_NT_FREEBSD_ABI_TAG 1
-#define VKI_NT_FREEBSD_FEATURE_CTL	4
-#define VKI_NT_FREEBSD_FCTL_WXNEEDED	0x00000008
+#define VKI_NT_FREEBSD_FEATURE_CTL 4
+#define VKI_NT_FREEBSD_FCTL_STKGAP_DISABLE 0x00000004
+#define VKI_NT_FREEBSD_FCTL_WXNEEDED 0x00000008
 
 // See syswrap-freebsd.c PRE/POST(sys_ioctl)
 #if 0