Pull the trace struct out of the low-level interface.

By pulling the struct from here, and moving it into yktrace, we can
offer the user a more abstracted interface, including a Drop and an
Iterator.

Also revise some comments while we are here.

Author: vext01

src/libcore/yk_swt.rs

@@ -7,51 +7,33 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-/// The result of `yk_swt_stop_tracing_impl()` is an array of this C struct.
+/// A MIR basic block location.
+/// FIXME: This shouldn't live here, as it will need to be shared across all tracing backends.
 #[repr(C)]
 #[derive(Debug)]
 pub struct MirLoc {
     /// Unique identifier for the crate.
-    pub crate_hash: u64,
+    crate_hash: u64,
     /// The definition index.
-    pub def_idx: u32,
+    def_idx: u32,
     /// The basic block index.
-    pub bb_idx: u32,
+    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,
-}
-
-impl SWTrace {
-    /// Returns the number of MIR locations recorded in the trace.
-    pub fn len(&self) -> usize {
-        self.len
+impl MirLoc {
+    /// Returns the crate hash of the location.
+    pub fn crate_hash(&self) -> u64 {
+        self.crate_hash
     }
 
-    /// Return a pointer to the raw trace buffer. The consumer *must* free this pointer.
-    pub fn buf(self) -> *mut MirLoc {
-        self.buf
+    /// Returns the definition index of the location.
+    pub fn def_idx(&self) -> u32 {
+        self.def_idx
     }
 
-    /// 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) }
-        }
+    /// Returns the basic block index of the location.
+    pub fn bb_idx(&self) -> u32 {
+        self.bb_idx
     }
 }
 
@@ -73,10 +55,11 @@ pub fn start_tracing() {
     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.
+/// Stop software tracing and on success return a tuple containing a pointer to the raw trace
+/// buffer, and the number of items inside. Returns `None` if the trace was invalidated, or if an
+/// error occurred. The current thread must already be tracing.
 #[cfg_attr(not(stage0), no_trace)]
-pub fn stop_tracing() -> Option<SWTrace> {
+pub fn stop_tracing() -> Option<(*mut MirLoc, usize)> {
     let len: usize = 0;
 
     extern "C" { fn yk_swt_stop_tracing_impl(ret_len: &usize) -> *mut MirLoc; }
@@ -85,7 +68,7 @@ pub fn stop_tracing() -> Option<SWTrace> {
     if buf.is_null() {
         None
     } else {
-        Some(SWTrace { buf, len })
+        Some((buf, len))
     }
 }
 

src/test/run-fail/yk_swt/oob-trace-idx.rs

@@ -28,10 +28,10 @@ pub fn main() {
     black_box(work2());
     let trace2 = stop_tracing().unwrap();
 
-    assert!(trace1.len() > trace2.len());
+    assert!(trace1.1 > trace2.1);
 
-    unsafe { libc::free(trace1.buf() as *mut libc::c_void) };
-    unsafe { libc::free(trace2.buf() as *mut libc::c_void) };
+    unsafe { libc::free(trace1.0 as *mut libc::c_void) };
+    unsafe { libc::free(trace2.0 as *mut libc::c_void) };
 }
 
 #[inline(never)]

src/test/run-pass/yk_swt/collect-trace-twice.rs

@@ -23,16 +23,11 @@ pub fn main() {
     black_box(work());
     let trace = stop_tracing().unwrap();
 
-    let len = trace.len();
     // The default capacity of the trace buffer is 1024. We want to be sure we've tested the case
     // where it had to be reallocated beyond its starting capacity.
-    assert!(len > 1024);
+    assert!(trace.1 > 1024);
 
-    for idx in 0..len {
-        trace.loc(idx); // All indices are in bounds, so should not panic.
-    }
-
-    unsafe { libc::free(trace.buf() as *mut libc::c_void) };
+    unsafe { libc::free(trace.0 as *mut libc::c_void) };
 }
 
 #[inline(never)]

src/test/run-pass/yk_swt/collect-trace.rs

@@ -15,7 +15,7 @@ extern crate core;
 extern crate libc;
 extern crate test;
 
-use core::yk_swt::{start_tracing, stop_tracing, SWTrace};
+use core::yk_swt::{start_tracing, stop_tracing, MirLoc};
 use std::thread;
 use test::black_box;
 
@@ -25,29 +25,30 @@ fn main() {
         start_tracing();
         let _ = work2();
         let raw_trace2 = stop_tracing().unwrap();
-        let trace2 = trace_to_vec(&raw_trace2);
-        unsafe { libc::free(raw_trace2.buf() as *mut libc::c_void) };
+        let trace2 = trace_to_vec(raw_trace2);
         trace2
     });
 
     start_tracing();
     black_box(work1());
     let raw_trace1 = stop_tracing().unwrap();
-    let trace1 = trace_to_vec(&raw_trace1);
-    unsafe { libc::free(raw_trace1.buf() as *mut libc::c_void) };
+    let trace1 = trace_to_vec(raw_trace1);
 
     let trace2 = thr2.join().unwrap();
 
     assert_ne!(trace1, trace2); // They should have been thread local.
 }
 
 // Copies a trace into a plain Rust Vec of tuples so we can compare them.
-fn trace_to_vec(t: &SWTrace) -> Vec<(u64, u32, u32)> {
+fn trace_to_vec(tup: (*mut MirLoc, usize)) -> Vec<(u64, u32, u32)> {
+    let (buf, len) = tup;
     let mut v = Vec::new();
-    for i in 0..t.len() {
-        let loc = t.loc(i);
-        v.push((loc.crate_hash, loc.def_idx, loc.bb_idx));
+    assert!(len < (isize::max_value() as usize)); // Or we can't do ptr arithmetic.
+    for i in 0..len {
+        let loc = unsafe { &*buf.offset(i as isize) };
+        v.push((loc.crate_hash(), loc.def_idx(), loc.bb_idx()));
     }
+    unsafe { libc::free(buf as *mut libc::c_void) };
     v
 }