Review comments

Author: zhengyu123

ddprof-lib/src/main/cpp/safeAccess.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2025 Datadog, Inc
+ * Copyright 2025, 2026 Datadog, Inc
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,8 @@
 
 
 #include "safeAccess.h"
+#include <cstdio>
+#include <cstdlib>
 #include <signal.h>
 #include <ucontext.h>
 
@@ -30,6 +32,28 @@ extern "C" int64_t safefetch64_cont(int64_t* adr, int64_t errValue);
 extern "C" int safecopy_impl(void* dst, const void* src, size_t len);
 extern "C" int safecopy_cont(void* dst, const void* src, size_t len);
 
+#ifdef DEBUG
+// handle_safefetch protects safeCopy by treating any fault whose pc lands in
+// [safecopy_impl, safecopy_cont) as a recoverable read fault. That range is
+// only valid if the assembler/linker keeps safecopy_cont strictly above
+// safecopy_impl (they are emitted adjacently in one asm block). A future
+// toolchain change could in principle reorder them, collapsing the range to
+// empty and silently disabling fault protection. Assert the invariant once at
+// load time so such a regression aborts loudly at startup instead of turning
+// safeCopy into an unprotected crash. Debug builds only: this is a
+// toolchain-regression tripwire, not a runtime safety check.
+__attribute__((constructor))
+static void verify_safecopy_range() {
+  if ((uintptr_t)safecopy_cont <= (uintptr_t)safecopy_impl) {
+    fprintf(stderr,
+            "FATAL: safecopy_cont (%p) is not above safecopy_impl (%p); "
+            "safeCopy fault protection would be lost\n",
+            (void*)safecopy_cont, (void*)safecopy_impl);
+    abort();
+  }
+}
+#endif // DEBUG
+
 #ifdef __APPLE__
     #if defined(__x86_64__)
       #define current_pc context_rip

ddprof-lib/src/main/cpp/safeAccess.h

@@ -1,5 +1,6 @@
 /*
  * Copyright 2021 Andrei Pangin
+* Copyright 2026 Datadog, Inc
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.

ddprof-lib/src/test/cpp/safefetch_ut.cpp

@@ -1,3 +1,8 @@
+/*
+ * Copyright 2026, Datadog, Inc.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include <climits>
@@ -281,6 +286,40 @@ TEST_F(SafeFetchTest, safeCopy_requestedRangeCrossesUnmappedPage_returnsFalse) {
   munmap(region, page_size);
 }
 
+TEST_F(SafeFetchTest, safeCopy_partialPrefixCopiedBeforeFault) {
+  // When the requested range straddles the boundary into an unmapped page,
+  // safeCopy returns false but the byte-granular copy still writes every
+  // readable byte before the fault. Verify the readable prefix landed in dst.
+  long page_size = sysconf(_SC_PAGESIZE);
+  ASSERT_GT(page_size, 0);
+
+  void* region = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
+                      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  ASSERT_NE(region, MAP_FAILED);
+  ASSERT_EQ(0, munmap((char*)region + page_size, page_size));
+
+  char* mapped_end = (char*)region + page_size;
+  // Place src so exactly `prefix` bytes are readable and the rest fall in
+  // the unmapped page.
+  const size_t prefix = 5;
+  char* src = mapped_end - prefix;
+  static const char kPrefix[] = "HELLO";  // 5 known readable bytes
+  memcpy(src, kPrefix, prefix);
+
+  // Request more than the readable prefix so the copy faults partway.
+  const size_t requested = prefix + 4;
+  char dst[16];
+  memset(dst, 0x5A, sizeof(dst));
+  EXPECT_FALSE(SafeAccess::safeCopy(dst, src, requested));
+
+  // The readable prefix must have been copied faithfully before the fault.
+  EXPECT_EQ(0, memcmp(dst, kPrefix, prefix));
+  // Bytes past the prefix were never written (fault stopped the copy).
+  EXPECT_EQ((char)0x5A, dst[prefix]);
+
+  munmap(region, page_size);
+}
+
 TEST_F(SafeFetchTest, safeCopy_unalignedSource_allMisalignments) {
   // The front fixup must correctly extract leading bytes from the
   // previous-aligned-word fetch for every misalignment k ∈ {1, 2, 3}.