From c556aa40883e44f41b3040bb4fc302c47266928a Mon Sep 17 00:00:00 2001
From: Roland McGrath <mcgrathr@google.com>
Date: Sat, 11 May 2019 05:00:10 +0000
Subject: [PATCH] [zircon][gn][libc] Add ubsan-sancov-full variant

This new variant enables all the kinds of sancov instrumentation,
not just the basic one that the standard runtime handles.  This
exercises more of the instrumentation features that e.g. fuzzing
also uses.  The special magic in the dynamic linker to cope with
the instrumentation is extended to handle all the cases.

Bug: ZX-3986 #comment ubsan-sancov-full variant
Test: /boot/test/sys/noop-test.ubsan-sancov-full
Change-Id: I2ca2bbaca732526ee10e0d60822410f48d8eb1ef
---
 .../public/gn/config/instrumentation/BUILD.gn |  9 ++++
 zircon/public/gn/config/standard.gni          | 10 +++++
 .../system/ulib/c/sanitizers/sancov-stubs.cpp | 12 +++++
 .../system/ulib/c/sanitizers/sancov-stubs.h   | 37 ++++++++++++++++
 zircon/third_party/ulib/musl/ldso/BUILD.gn    | 17 ++++++-
 zircon/third_party/ulib/musl/ldso/dlstart.c   | 15 +------
 .../ulib/musl/ldso/dynlink-sancov.S           | 28 +++++++-----
 zircon/third_party/ulib/musl/ldso/dynlink.c   | 44 ++++++++++++-------
 8 files changed, 130 insertions(+), 42 deletions(-)
 create mode 100644 zircon/system/ulib/c/sanitizers/sancov-stubs.h

diff --git a/zircon/public/gn/config/instrumentation/BUILD.gn b/zircon/public/gn/config/instrumentation/BUILD.gn
index ada811bbae9..9ea97513bfe 100644
--- a/zircon/public/gn/config/instrumentation/BUILD.gn
+++ b/zircon/public/gn/config/instrumentation/BUILD.gn
@@ -113,6 +113,15 @@ if (toolchain.tags + [ "sancov" ] - [ "sancov" ] != toolchain.tags) {
     cflags = compiler_flags
     ldflags = compiler_flags
   }
+
+  # This enables all the different kinds of sancov callbacks, not just
+  # the basic ones actually implemented by the canonical runtime.
+  config("sancov-full") {
+    compiler_flags = [ "-fsanitize-coverage=indirect-calls,trace-cmp,trace-div,trace-gep,trace-pc-guard,inline-8bit-counters,pc-table" ]
+    asmflags = compiler_flags
+    cflags = compiler_flags
+    ldflags = compiler_flags
+  }
 }
 
 config("profile") {
diff --git a/zircon/public/gn/config/standard.gni b/zircon/public/gn/config/standard.gni
index 5a7501902ed..66c24f454c3 100644
--- a/zircon/public/gn/config/standard.gni
+++ b/zircon/public/gn/config/standard.gni
@@ -293,6 +293,16 @@ standard_variants += [
       tags = [ "sancov" ]
     }
   },
+  {
+    variant = {
+      name = "ubsan-sancov-full"
+      bases = [ "ubsan" ]
+
+      # The "sancov" tag is important: see $zx/system/ulib/c/sanitizers.
+      configs = [ "$zx/public/gn/config/instrumentation:sancov-full" ]
+      tags = [ "sancov" ]
+    }
+  },
 ]
 
 # This list is appended to the $variants list for the build.  It provides
diff --git a/zircon/system/ulib/c/sanitizers/sancov-stubs.cpp b/zircon/system/ulib/c/sanitizers/sancov-stubs.cpp
index 3caba985171..26bbda011a4 100644
--- a/zircon/system/ulib/c/sanitizers/sancov-stubs.cpp
+++ b/zircon/system/ulib/c/sanitizers/sancov-stubs.cpp
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "sancov-stubs.h"
+
 // This file defines all the entry points that -fsanitize-coverage=...
 // instrumentation calls.  Unfortunately, LLVM does not publish any header
 // file declaring those signatures, though they are all given in
@@ -33,3 +35,13 @@
         __builtin_trap();
     }
 }
+
+// There are many hooks that are called a lot but can always safely
+// just do nothing.
+
+#define SANCOV_STUB(name) \
+    [[gnu::weak]] extern "C" void __sanitizer_cov_##name() {}
+
+SANCOV_NOOP_STUBS
+
+#undef SANCOV_STUB
diff --git a/zircon/system/ulib/c/sanitizers/sancov-stubs.h b/zircon/system/ulib/c/sanitizers/sancov-stubs.h
new file mode 100644
index 00000000000..146911ce7c8
--- /dev/null
+++ b/zircon/system/ulib/c/sanitizers/sancov-stubs.h
@@ -0,0 +1,37 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#pragma once
+
+// These macros call SANCOV_STUB(NAME) for each __sanitizer_cov_NAME symbol
+// that represents a function called by instrumented code.
+//
+// SANCOV_STUBS covers all the entry points.
+// SANCOV_NOOP_STUBS covers only the subset of SANCOV_STUBS where each
+// entry point is ordinarily a no-op that might be called harmlessly
+// by code during early startup before the proper runtime is in place.
+
+#define SANCOV_STUBS                 \
+    SANCOV_STUB(trace_pc_guard)      \
+    SANCOV_STUB(trace_pc_guard_init) \
+    SANCOV_NOOP_STUBS
+
+#define SANCOV_NOOP_STUBS            \
+    SANCOV_STUB(trace_cmp)           \
+    SANCOV_STUB(trace_cmp1)          \
+    SANCOV_STUB(trace_cmp2)          \
+    SANCOV_STUB(trace_cmp4)          \
+    SANCOV_STUB(trace_cmp8)          \
+    SANCOV_STUB(trace_const_cmp1)    \
+    SANCOV_STUB(trace_const_cmp2)    \
+    SANCOV_STUB(trace_const_cmp4)    \
+    SANCOV_STUB(trace_const_cmp8)    \
+    SANCOV_STUB(trace_switch)        \
+    SANCOV_STUB(trace_div4)          \
+    SANCOV_STUB(trace_div8)          \
+    SANCOV_STUB(trace_gep)           \
+    SANCOV_STUB(trace_pc)            \
+    SANCOV_STUB(trace_pc_indir)      \
+    SANCOV_STUB(8bit_counters_init)  \
+    SANCOV_STUB(pcs_init)
diff --git a/zircon/third_party/ulib/musl/ldso/BUILD.gn b/zircon/third_party/ulib/musl/ldso/BUILD.gn
index 493979e2840..bfb0886079e 100644
--- a/zircon/third_party/ulib/musl/ldso/BUILD.gn
+++ b/zircon/third_party/ulib/musl/ldso/BUILD.gn
@@ -4,15 +4,19 @@
 
 source_set("ldso") {
   deps = [
+    ":dlstart",
     "$zx/system/ulib/ldmsg",
     "$zx/third_party/ulib/musl:musl_internal",
   ]
   sources = [
     "$zx/third_party/ulib/musl/arch/${toolchain.cpu}/dl-entry.S",
-    "dlstart.c",
     "dynlink-sancov.S",
     "dynlink.c",
   ]
+
+  # This is needed by dynlink-sancov.S and dynlink.c for "sancov-stubs.h".
+  include_dirs = [ "$zx/system/ulib/c/sanitizers" ]
+
   if (toolchain.libprefix != "") {
     # The libprefix always ends with a / but that's not part of the
     # "config" string in the loader-service protocol.
@@ -21,3 +25,14 @@ source_set("ldso") {
     defines = [ "DYNLINK_LDSVC_CONFIG=\"$ldsvc_config\"" ]
   }
 }
+
+source_set("dlstart") {
+  visibility = [ ":*" ]
+  deps = [
+    "$zx/third_party/ulib/musl:musl_internal",
+  ]
+  sources = [
+    "dlstart.c",
+  ]
+  configs += [ "$zx/public/gn/config:no_sanitizers" ]
+}
diff --git a/zircon/third_party/ulib/musl/ldso/dlstart.c b/zircon/third_party/ulib/musl/ldso/dlstart.c
index 0cc62080c6d..23e6f818fff 100644
--- a/zircon/third_party/ulib/musl/ldso/dlstart.c
+++ b/zircon/third_party/ulib/musl/ldso/dlstart.c
@@ -5,20 +5,7 @@
 #include <stdatomic.h>
 #include <stddef.h>
 
-#ifdef __clang__
-// TODO(mcgrathr): Really we want to compile just this file without
-// -fsanitize-coverage, but this works around the issue for now.
-__asm__(".weakref __sanitizer_cov_trace_pc_guard, _dlstart_sancov_dummy");
-__asm__(".pushsection .text._dlstart_sancov_dummy,\"ax\",%progbits\n"
-        ".local _dlstart_sancov_dummy\n"
-        ".type _dlstart_sancov_dummy,%function\n"
-        "_dlstart_sancov_dummy: ret\n"
-        ".size _dlstart_sancov_dummy, . - _dlstart_sancov_dummy\n"
-        ".popsection");
-#endif
-
-__LOCAL __NO_SAFESTACK NO_ASAN dl_start_return_t _dl_start(void* start_arg,
-                                                           void* vdso) {
+__LOCAL dl_start_return_t _dl_start(void* start_arg, void* vdso) {
     ElfW(Addr) base = (uintptr_t)__ehdr_start;
     const ElfW(Rel)* rel = NULL;
     const ElfW(Rela)* rela = NULL;
diff --git a/zircon/third_party/ulib/musl/ldso/dynlink-sancov.S b/zircon/third_party/ulib/musl/ldso/dynlink-sancov.S
index 606029d3bc2..a0d0d976766 100644
--- a/zircon/third_party/ulib/musl/ldso/dynlink-sancov.S
+++ b/zircon/third_party/ulib/musl/ldso/dynlink-sancov.S
@@ -4,21 +4,27 @@
 
 #include <asm.h>
 
-// See the end of dynlink.c for what this is about.
-
-.section .text._dynlink_sancov_trace_pc_guard,"ax",%progbits
+#include "sancov-stubs.h"
 
-.weak __sanitizer_cov_trace_pc_guard
+// See the end of dynlink.c for what this is about.
 
-ENTRY(_dynlink_sancov_trace_pc_guard)
+.macro sancov_stub name
+    .pushsection .text._dynlink_sancov_\name,"ax",%progbits
+    .weak __sanitizer_cov_\name
+    ENTRY(_dynlink_sancov_\name)
 #ifdef __x86_64__
-    jmp *__sanitizer_cov_trace_pc_guard@GOTPCREL(%rip)
+        jmp *__sanitizer_cov_\name@GOTPCREL(%rip)
 #elif defined(__aarch64__)
-    adrp x16, :got:__sanitizer_cov_trace_pc_guard
-    ldr x16, [x16, #:got_lo12:__sanitizer_cov_trace_pc_guard]
-    br x16
+        adrp x16, :got:__sanitizer_cov_\name
+        ldr x16, [x16, #:got_lo12:__sanitizer_cov_\name]
+        br x16
 #else
 # error unsupported architecture
 #endif
-END(_dynlink_sancov_trace_pc_guard)
-.hidden _dynlink_sancov_trace_pc_guard
+    END(_dynlink_sancov_\name)
+    .hidden _dynlink_sancov_\name
+    .popsection
+.endm
+
+#define SANCOV_STUB(name) sancov_stub name;
+SANCOV_STUBS
diff --git a/zircon/third_party/ulib/musl/ldso/dynlink.c b/zircon/third_party/ulib/musl/ldso/dynlink.c
index bf724807676..1774fe56b7c 100644
--- a/zircon/third_party/ulib/musl/ldso/dynlink.c
+++ b/zircon/third_party/ulib/musl/ldso/dynlink.c
@@ -1696,8 +1696,7 @@ __NO_SAFESTACK static void update_tls_size(void) {
 static dl_start_return_t __dls3(void* start_arg);
 
 __NO_SAFESTACK NO_ASAN __attribute__((__visibility__("hidden")))
-dl_start_return_t __dls2(
-    void* start_arg, void* vdso_map) {
+dl_start_return_t __dls2(void* start_arg, void* vdso_map) {
     ldso.l_map.l_addr = (uintptr_t)__ehdr_start;
 
     Ehdr* ehdr = (void*)ldso.l_map.l_addr;
@@ -2702,26 +2701,39 @@ zx_status_t __sanitizer_change_code_protection(uintptr_t addr, size_t len,
 // directly to our definition.  The trampoline checks the 'runtime' flag to
 // distinguish calls before final relocation is complete, and only calls
 // into the sanitizer runtime once it's actually up.  Because of the
-// .weakref chicanery, _dynlink_sancov_trace_pc_guard must be in a separate
+// .weakref chicanery, the _dynlink_sancov_* symbols must be in a separate
 // assembly file.
-__asm__(".weakref __sanitizer_cov_trace_pc_guard, _dynlink_sancov_trampoline");
-__asm__(".hidden _dynlink_sancov_trace_pc_guard");
-__asm__(".pushsection .text._dynlink_sancov_trampoline,\"ax\",%progbits\n"
-        ".local _dynlink_sancov_trampoline\n"
-        ".type _dynlink_sancov_trampoline,%function\n"
-        "_dynlink_sancov_trampoline:\n"
+
+# include "sancov-stubs.h"
+
+# define SANCOV_STUB(name) SANCOV_STUB_ASM(#name)
+# define SANCOV_STUB_ASM(name) \
+    __asm__( \
+    ".weakref __sanitizer_cov_" name ", _dynlink_sancov_trampoline_" name "\n" \
+    ".hidden _dynlink_sancov_" name "\n" \
+    ".pushsection .text._dynlink_sancov_trampoline_" name ",\"ax\",%progbits\n"\
+    ".local _dynlink_sancov_trampoline_" name "\n" \
+    ".type _dynlink_sancov_trampoline_" name ",%function\n" \
+    "_dynlink_sancov_trampoline_" name ":\n" \
+     SANCOV_STUB_ASM_BODY(name) \
+    ".size _dynlink_sancov_trampoline_" name ", . - _dynlink_sancov_trampoline_" name "\n" \
+    ".popsection");
+
 # ifdef __x86_64__
-        "cmpl $0, _dynlink_runtime(%rip)\n"
-        "jne _dynlink_sancov_trace_pc_guard\n"
+#  define SANCOV_STUB_ASM_BODY(name) \
+        "cmpl $0, _dynlink_runtime(%rip)\n" \
+        "jne _dynlink_sancov_" name "\n" \
         "ret\n"
 # elif defined(__aarch64__)
-        "adrp x16, _dynlink_runtime\n"
-        "ldr w16, [x16, #:lo12:_dynlink_runtime]\n"
-        "cbnz w16, _dynlink_sancov_trace_pc_guard\n"
+#  define SANCOV_STUB_ASM_BODY(name) \
+        "adrp x16, _dynlink_runtime\n" \
+        "ldr w16, [x16, #:lo12:_dynlink_runtime]\n" \
+        "cbnz w16, _dynlink_sancov_" name "\n" \
         "ret\n"
 # else
 #  error unsupported architecture
 # endif
-        ".size _dynlink_sancov_trampoline, . - _dynlink_sancov_trampoline\n"
-        ".popsection");
+
+SANCOV_STUBS
+
 #endif
-- 
GitLab