Merge #4

4: Software Trace Function in C. r=ltratt a=vext01

This is a DNMY (do no merge yet) PR. It needs some discussion.

As discussed earlier this week, this is an attempt to implement the tracing function in C so that our MIR pass doesn't see the innards of the tracing mechanisms and it's dependencies.

This means we can trace a lot more than we could have before, where e.g. thread locals and vectors used in the tracing mechanism were blocked from being traced.

I printed the trace from one of my tests, and it looks highly plausable (tm):
```
MirLoc { crate_hash: 7687613001418581878, def_idx: 14, bb_idx: 1 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 0 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 1 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 2 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 3 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 6 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 7 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 8 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 9 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 2 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 3 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 6 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 7 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 8 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 9 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 2 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 3 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 6 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 7 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 8 }
MirLoc { crate_hash: 7687613001418581878, def_idx: 16, bb_idx: 9 }
...
```

I can see a loop!

# Discussion Points

## Low-level of abstraction.

Because the tracing function is now in C, and because we no longer need `Vec` or `std::thread` from `libstd` to trace, I opted to move the whole mechanism into `libcore`. This is better in the sense that we can trace stuff even if libstd isn't present (a dependency of the crate being traced).

On the other hand, working in libcore is limiting and it's evident from the low level of abstraction in the API you see. I'd have liked to, for example, have given the user an iterator and a `Drop` implementation for the trace struct, but these are `libstd` concepts.

I propose that in the next PR we make the libcore interface even more low-level -- make `stop_tracing` return a tuple, not a struct -- and then move the trace struct up into `libstd` where it can have better abstraction.

This should mean that VM authors get a nice API to start and stop tracing via libstd, but they can still trace `no_std` crates that their VMs might depend upon.

## Traits

Looking to the future, we will eventually have two backends (sw/hw) and a compile time switch to choose one. I suspect that we will want to offer the same API regardless of the backend, so we probably want a place to keep the common parts (e.g. `struct MirLoc` can probably be shared) and a trait for the API which includes stuff like getting an iterator over a trace.

This should be a PR in the near future.

## C Errors

Currently the C tracing code will crash hard (with `err()` or `errx()`) if there is an error. We could implement something like we did in hwtracer where each FFI call passes down an error struct pointer, which can be read by Rust after the call.

Since this change can be hidden from the user in abstractions, I think this is low-priority for now.




Do you agree with the above plan?

Cheers

Co-authored-by: Edd Barrett <vext01@gmail.com>

Author: bors[bot]

src/Cargo.lock

@@ -444,6 +444,7 @@ dependencies = [
 name = "core"
 version = "0.0.0"
 dependencies = [
+ "cc 1.0.25 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.5.5 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 

src/libcore/Cargo.toml

@@ -21,3 +21,6 @@ path = "../libcore/benches/lib.rs"
 
 [dev-dependencies]
 rand = "0.5"
+
+[build-dependencies]
+cc = "1.0"

src/libcore/build.rs

@@ -0,0 +1,22 @@
+// Copyright 2019 King's College London.
+// Created by the Software Development Team <http://soft-dev.org/>.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+extern crate cc;
+
+fn main() {
+    let mut c_build = cc::Build::new();
+
+    c_build.file("yk_swt_impl.c");
+    c_build.compile("yk_swt_impl");
+    c_build.flag("-std=c11");
+    c_build.warnings(true);
+    c_build.extra_warnings(true);
+
+    println!("cargo:rerun-if-changed=yk_swt_impl.c");
+}

src/libcore/lib.rs

@@ -229,7 +229,9 @@ mod nonzero;
 mod tuple;
 mod unit;
 
-mod yk_swt;
+/// Yorick software tracing.
+#[unstable(feature = "yk_swt", issue = "0")]
+pub mod yk_swt;
 
 // Pull in the `coresimd` crate directly into libcore. This is where all the
 // architecture-specific (and vendor-specific) intrinsics are defined. AKA

src/libcore/yk_swt.rs

@@ -7,20 +7,91 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-/// The software trace recorder function.
-/// This is a weak language item, it actually resides in libstd. It has to be weak to allow libcore
-/// to call up to libstd (libstd is not a dependency of libcore).
-extern "Rust" {
-    #[cfg_attr(not(stage0), lang="yk_swt_rec_loc")]
-    fn yk_swt_rec_loc(crate_hash: u64, def_idx: u32, bb: u32);
+/// The result of `yk_swt_stop_tracing_impl()` is an array of this C struct.
+#[repr(C)]
+#[derive(Debug)]
+pub struct MirLoc {
+    /// Unique identifier for the crate.
+    pub crate_hash: u64,
+    /// The definition index.
+    pub def_idx: u32,
+    /// The basic block index.
+    pub bb_idx: u32,
+}
+
+/// Wraps the raw C trace buffer and exposes a more "Rusty" interface to it.
+#[derive(Debug)]
+#[allow(dead_code)]
+pub struct SWTrace {
+    /// A heap allocated array of `MirLoc` structs, which the consumer must free. Ideally we'd
+    /// have this struct implement `std::ops::Drop` or at least provide a method to do the freeing,
+    /// but we can do neither due to libcore restrictions.
+    buf: *mut MirLoc,
+    /// The number of items in the above array.
+    len: usize,
 }
 
-/// Wrapper lang item to call the above wrapper function.
-/// This has to be a lang item too, as a MIR terminator cannot call a weak language item directly.
+impl SWTrace {
+    /// Returns the number of MIR locations recorded in the trace.
+    pub fn len(&self) -> usize {
+        self.len
+    }
+
+    /// Return a pointer to the raw trace buffer. The consumer *must* free this pointer.
+    pub fn buf(self) -> *mut MirLoc {
+        self.buf
+    }
+
+    /// Returns the location at index `idx` or `None` if the index is out of bounds.
+    pub fn loc<'a>(&'a self, idx: usize) -> &'a MirLoc {
+        if idx >= self.len {
+            panic!("software trace index out of bounds: len={}, idx={}", self.len, idx);
+        } else {
+            if idx > isize::max_value() as usize {
+                panic!("index too large for ptr arithmetic");
+            }
+            unsafe { &*self.buf.offset(idx as isize) }
+        }
+    }
+}
+
+/// The software trace recorder function.
+/// This is implemented in C so that: the `yk_swt_calls` MIR pass doesn't see inside.
 #[allow(dead_code)] // Used only indirectly in a MIR pass.
-#[cfg_attr(not(stage0), lang="yk_swt_rec_loc_wrap")]
+#[cfg_attr(not(stage0), lang="yk_swt_rec_loc")]
+#[cfg_attr(not(stage0), no_trace)]
+#[cfg(not(test))]
+fn yk_swt_rec_loc(crate_hash: u64, def_idx: u32, bb_idx: u32) {
+    extern "C" { fn yk_swt_rec_loc_impl(crate_hash: u64, def_idx: u32, bb_idx: u32); }
+    unsafe { yk_swt_rec_loc_impl(crate_hash, def_idx, bb_idx); }
+}
+
+/// Start software tracing on the current thread. The current thread must not already be tracing.
 #[cfg_attr(not(stage0), no_trace)]
-fn yk_swt_rec_loc_wrap(crate_hash: u64, def_idx: u32, bb: u32) {
-    unsafe { yk_swt_rec_loc(crate_hash, def_idx, bb) };
+pub fn start_tracing() {
+    extern "C" { fn yk_swt_start_tracing_impl(); }
+    unsafe { yk_swt_start_tracing_impl(); }
 }
 
+/// Stop software tracing and return the trace, or `None` if the trace was invalidated.
+/// The current thread must already be tracing.
+#[cfg_attr(not(stage0), no_trace)]
+pub fn stop_tracing() -> Option<SWTrace> {
+    let len: usize = 0;
+
+    extern "C" { fn yk_swt_stop_tracing_impl(ret_len: &usize) -> *mut MirLoc; }
+    let buf = unsafe { yk_swt_stop_tracing_impl(&len) };
+
+    if buf.is_null() {
+        None
+    } else {
+        Some(SWTrace { buf, len })
+    }
+}
+
+/// Invalidate the software trace, if one is being collected.
+#[cfg_attr(not(stage0), no_trace)]
+pub fn invalidate_trace() {
+    extern "C" { fn yk_swt_invalidate_trace_impl(); }
+    unsafe { yk_swt_invalidate_trace_impl(); }
+}

src/libcore/yk_swt_impl.c

@@ -0,0 +1,130 @@
+// Copyright 2019 King's College London.
+// Created by the Software Development Team <http://soft-dev.org/>.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#include <stdint.h>
+#include <stdlib.h>
+#include <err.h>
+#include <stdbool.h>
+#include <stdatomic.h>
+
+struct mir_loc {
+    uint64_t crate_hash;
+    uint32_t def_idx;
+    uint32_t bb_idx;
+};
+
+#define TL_TRACE_INIT_CAP       1024
+#define TL_TRACE_REALLOC_CAP    1024
+
+void yk_swt_start_tracing_impl(void);
+void yk_swt_rec_loc_impl(uint64_t crate_hash, uint32_t def_idx, uint32_t bb_idx);
+struct mir_loc *yk_swt_stop_tracing_impl(size_t *ret_trace_len);
+void yk_swt_invalidate_trace_impl(void);
+
+// The trace buffer.
+static __thread struct mir_loc *trace_buf = NULL;
+// The number of elements in the trace buffer.
+static __thread size_t trace_buf_len = 0;
+// The allocation capacity of the trace buffer (in elements).
+static __thread size_t trace_buf_cap = 0;
+// Is the current thread tracing?
+// true = we are tracing, false = we are not tracing or an error occurred.
+static __thread volatile atomic_bool tracing = false;
+
+// Start tracing on the current thread.
+// A new trace buffer is allocated and MIR locations will be written into it on
+// subsequent calls to `yk_swt_rec_loc_impl`. If the current thread is already
+// tracing, calling this will lead to undefined behaviour.
+void
+yk_swt_start_tracing_impl(void) {
+    trace_buf = calloc(TL_TRACE_INIT_CAP, sizeof(struct mir_loc));
+    if (trace_buf == NULL) {
+        err(EXIT_FAILURE, "%s: calloc: ", __func__);
+    }
+
+    trace_buf_cap = TL_TRACE_INIT_CAP;
+    atomic_store_explicit(&tracing, true, memory_order_relaxed);
+}
+
+// Record a location into the trace buffer if tracing is enabled on the current thread.
+void
+yk_swt_rec_loc_impl(uint64_t crate_hash, uint32_t def_idx, uint32_t bb_idx)
+{
+    if (!atomic_load_explicit(&tracing, memory_order_relaxed)) {
+        return;
+    }
+
+    // Check if we need more space and reallocate if necessary.
+    if (trace_buf_len == trace_buf_cap) {
+        if (trace_buf_cap >= SIZE_MAX - TL_TRACE_REALLOC_CAP) {
+            // Trace capacity would overflow.
+            atomic_store_explicit(&tracing, false, memory_order_relaxed);
+            return;
+        }
+        size_t new_cap = trace_buf_cap + TL_TRACE_REALLOC_CAP;
+
+        if (new_cap > SIZE_MAX / sizeof(struct mir_loc)) {
+            // New buffer size would overflow.
+            atomic_store_explicit(&tracing, false, memory_order_relaxed);
+            return;
+        }
+        size_t new_size = new_cap * sizeof(struct mir_loc);
+
+        trace_buf = realloc(trace_buf, new_size);
+        if (trace_buf == NULL) {
+            atomic_store_explicit(&tracing, false, memory_order_relaxed);
+            return;
+        }
+
+        trace_buf_cap = new_cap;
+    }
+
+    struct mir_loc loc = { crate_hash, def_idx, bb_idx };
+    trace_buf[trace_buf_len] = loc;
+    trace_buf_len ++;
+}
+
+
+// Stop tracing on the current thread.
+// On success the trace buffer is returned and the number of locations it
+// holds is written to `*ret_trace_len`. It is the responsibility of the caller
+// to free the returned trace buffer. A NULL pointer is returned on error.
+// Calling this function when tracing was not started with
+// `yk_swt_start_tracing_impl()` results in undefined behaviour.
+struct mir_loc *
+yk_swt_stop_tracing_impl(size_t *ret_trace_len) {
+    if (!atomic_load_explicit(&tracing, memory_order_relaxed)) {
+        free(trace_buf);
+        trace_buf = NULL;
+        trace_buf_len = 0;
+    }
+
+    // We hand ownership of the trace to Rust now. Rust is responsible for
+    // freeing the trace.
+    struct mir_loc *ret_trace = trace_buf;
+    *ret_trace_len = trace_buf_len;
+
+    // Now reset all off the recorder's state.
+    // We reset `trace_invalid` when tracing is restarted, because signals
+    // handlers which set this flag may arrive in the meantime.
+    trace_buf = NULL;
+    tracing = false;
+    trace_buf_len = 0;
+    trace_buf_cap = 0;
+
+    return ret_trace;
+}
+
+// Call this to safely mark the trace invalid.
+void
+yk_swt_invalidate_trace_impl(void) {
+    // We don't free the trace buffer here, as this may be called in a signal
+    // handler and thus needs to be reentrant.
+    atomic_store_explicit(&tracing, false, memory_order_relaxed);
+}

src/librustc/middle/lang_items.rs

@@ -377,7 +377,6 @@ language_item_table! {
     U128ShroFnLangItem,          "u128_shro",          u128_shro_fn,            Target::Fn;
 
     YkSwtRecLocLangItem,         "yk_swt_rec_loc",     yk_swt_rec_loc,          Target::Fn;
-    YkSwtRecLocWrapLangItem,     "yk_swt_rec_loc_wrap",yk_swt_rec_loc_wrap,     Target::Fn;
 
     // Align offset for stride != 1, must not panic.
     AlignOffsetLangItem,         "align_offset",       align_offset_fn,         Target::Fn;

src/librustc/middle/weak_lang_items.rs

@@ -168,5 +168,4 @@ weak_lang_items! {
     eh_personality,     EhPersonalityLangItem,      rust_eh_personality;
     eh_unwind_resume,   EhUnwindResumeLangItem,     rust_eh_unwind_resume;
     oom,                OomLangItem,                rust_oom;
-    yk_swt_rec_loc,     YkSwtRecLocLangItem,        rust_yk_swt_rec_loc;
 }

src/librustc_mir/transform/add_yk_swt_calls.rs

@@ -51,7 +51,7 @@ impl MirPass for AddYkSWTCalls {
             return;
         }
 
-        let rec_fn_defid = tcx.get_lang_items(LOCAL_CRATE).yk_swt_rec_loc_wrap()
+        let rec_fn_defid = tcx.get_lang_items(LOCAL_CRATE).yk_swt_rec_loc()
             .expect("couldn't find software trace recorder function");
 
         let unit_ty = tcx.mk_unit();

src/libstd/lib.rs

@@ -310,6 +310,7 @@
 #![feature(panic_info_message)]
 #![feature(non_exhaustive)]
 #![feature(alloc_layout_extra)]
+#![feature(yk_swt)]
 
 #![default_lib_allocator]
 
@@ -518,10 +519,6 @@ mod coresimd {
 #[cfg(all(not(stage0), not(test)))]
 pub use stdsimd::arch;
 
-/// Yorick software tracing.
-#[unstable(feature = "yk_swt", issue = "0")]
-pub mod yk_swt;
-
 // Include a number of private modules that exist solely to provide
 // the rustdoc documentation for primitive types. Using `include!`
 // because rustdoc only looks for these modules at the crate level.