commit 3bd8fb59c9f3791003aaadd271492ba43c2a50d5
parent cf17408509b01121408381cac3ab8724dc0d43f8
Author: kyle <kyle@0x30.net>
Date: Tue, 8 Nov 2016 23:52:15 -0700
src: copy a struct instead of hand packing data
Running on sparc64 showed that hand packing data in pages is a bad idea.
Copy a struct that should be properly aligned instead.
Diffstat:
9 files changed, 79 insertions(+), 162 deletions(-)
diff --git a/src/rt.c b/src/rt.c
@@ -21,6 +21,7 @@
#include <errno.h> /* EEXIST */
#include <fcntl.h> /* O_CREAT */
#include <limits.h> /* PATH_MAX */
+#include <stddef.h> /* offsetof */
#include <stdlib.h> /* get{env,progname} */
#include <string.h> /* strnlen */
#include <unistd.h> /* get{cwd,pid,ppid,pgrp} */
@@ -34,34 +35,6 @@ static int init = 0;
static int shm_fd = 0;
static size_t shm_len = 0;
-static size_t
-add_1(uint8_t *shm, size_t shm_pos, uint8_t data)
-{
- shm[shm_pos] = data;
- return shm_pos + sizeof(data);
-}
-
-static size_t
-add_2(uint8_t *shm, size_t shm_pos, uint16_t data)
-{
- memcpy(shm + shm_pos, &data, sizeof(data));
- return shm_pos + sizeof(data);
-}
-
-static size_t
-add_4(uint8_t *shm, size_t shm_pos, uint32_t data)
-{
- memcpy(shm + shm_pos, &data, sizeof(data));
- return shm_pos + sizeof(data);
-}
-
-static size_t
-add_str(uint8_t *shm, size_t shm_pos, const char *str, uint16_t len)
-{
- memcpy(shm + shm_pos, str, len);
- return shm_pos + len;
-}
-
/*
* Extends the memory mapping of shm_fd some number of bytes rounded up to the
* next page size.
@@ -94,105 +67,41 @@ shm_extend(int bytes)
return shm;
}
+struct citrun_header {
+ char magic[6];
+ uint8_t major;
+ uint8_t minor;
+ uint32_t pids[3];
+ char progname[PATH_MAX];
+ char cwd[PATH_MAX];
+};
+
/*
- * Add a header region to a newly created shared memory file. It contains:
- * - 6 byte magic value
- * - 2 bytes for major and minor versions
- * - 12 bytes for process id, parent process id, group process id
- * - 2 + N bytes for program name prefixed with its length
- * - 2 + N bytes for current working directory prefixed with its length
- * Header size is rounded up to next page size multiple. Exits on error.
+ * Add a header region to a newly created shared memory file. Header size is
+ * rounded up to next page size multiple. Exits on error.
*/
static void
shm_add_header()
{
- char magic[6] = "citrun";
- char *cwd_buf;
- const char *progname;
uint8_t *shm;
- size_t sz;
- size_t shm_pos;
- uint16_t prog_sz, cwd_sz;
-
- progname = getprogname();
- if ((cwd_buf = getcwd(NULL, 0)) == NULL)
- err(1, "getcwd");
-
- prog_sz = strnlen(progname, PATH_MAX);
- cwd_sz = strnlen(cwd_buf, PATH_MAX);
- sz = 0;
- sz += sizeof(magic);
- sz += sizeof(uint8_t) * 2;
- sz += sizeof(uint32_t) * 3;
- sz += sizeof(prog_sz);
- sz += prog_sz;
- sz += sizeof(cwd_sz);
- sz += cwd_sz;
+ struct citrun_header header = {
+ "citrun",
+ citrun_major,
+ citrun_minor
+ };
- shm = shm_extend(sz);
+ header.pids[0] = getpid();
+ header.pids[1] = getppid();
+ header.pids[2] = getpgrp();
- shm_pos = 0;
- shm_pos = add_str(shm, shm_pos, magic, sizeof(magic));
+ strlcpy(header.progname, getprogname(), PATH_MAX);
- shm_pos = add_1(shm, shm_pos, citrun_major);
- shm_pos = add_1(shm, shm_pos, citrun_minor);
-
- shm_pos = add_4(shm, shm_pos, getpid());
- shm_pos = add_4(shm, shm_pos, getppid());
- shm_pos = add_4(shm, shm_pos, getpgrp());
-
- shm_pos = add_2(shm, shm_pos, prog_sz);
- shm_pos = add_str(shm, shm_pos, progname, prog_sz);
-
- shm_pos = add_2(shm, shm_pos, cwd_sz);
- shm_pos = add_str(shm, shm_pos, cwd_buf, cwd_sz);
-
- assert(shm_pos == sz);
-}
-
-/*
- * Adds a new citrun_node to the shared memory file. Contains:
- * - 4 bytes for the number of source code lines
- * - 2 + N bytes for the file name used when compiling the source code
- * - 2 + N bytes for the source code's absolute file path
- * - 8 * L bytes (L = total number of source code lines) for the execution
- * count buffers that store how many times each source code line executed.
- * Node size is rounded up to the next page size multiple.
- * Function exits on failure.
- */
-static void
-shm_add_node(struct citrun_node *n)
-{
- uint8_t *shm;
- size_t sz;
- size_t shm_pos;
- uint16_t comp_sz, abs_sz;
-
- comp_sz = strnlen(n->comp_file_path, PATH_MAX);
- abs_sz = strnlen(n->abs_file_path, PATH_MAX);
-
- sz = 0;
- sz += sizeof(uint32_t);
- sz += sizeof(comp_sz);
- sz += comp_sz;
- sz += sizeof(abs_sz);
- sz += abs_sz;
- sz += n->size * sizeof(uint64_t);
-
- shm = shm_extend(sz);
-
- shm_pos = 0;
- shm_pos = add_4(shm, shm_pos, n->size);
- shm_pos = add_2(shm, shm_pos, comp_sz);
- shm_pos = add_str(shm, shm_pos, n->comp_file_path, comp_sz);
- shm_pos = add_2(shm, shm_pos, abs_sz);
- shm_pos = add_str(shm, shm_pos, n->abs_file_path, abs_sz);
-
- n->data = (uint64_t *)&shm[shm_pos];
- shm_pos += n->size * sizeof(uint64_t);
+ if (getcwd(header.cwd, PATH_MAX) == NULL)
+ err(1, "getcwd");
- assert(shm_pos == sz);
+ shm = shm_extend(sizeof(struct citrun_header));
+ memcpy(shm, &header, sizeof(struct citrun_header));
}
/*
@@ -230,22 +139,36 @@ shm_create()
}
/*
- * Public interface: Add a node to shared memory.
+ * Public interface, called by instrumented translation units.
+ *
+ * Copies the passed in citrun_node into the shared memory file.
+ * Care is taken to allocate enough memory for the execution buffers which are
+ * 8 * L bytes (L = total number of source code lines).
+ * Node size is rounded up to the next page size multiple.
* Exits on failure.
*/
void
citrun_node_add(uint8_t node_major, uint8_t node_minor, struct citrun_node *n)
{
- /*
- * Binary compatibility between versions is not guaranteed.
- * A user is forced to rebuild their project in this case.
- */
+ uint8_t *shm;
+ size_t sz = 0;
+
+ /* Binary compatibility between versions not guaranteed. */
if (node_major != citrun_major || node_minor != citrun_minor)
- errx(1, "libcitrun %i.%i: incompatible node version %i.%i",
+ errx(1, "libcitrun-%i.%i: incompatible version %i.%i, "
+ "try cleaning and rebuilding your project",
citrun_major, citrun_minor, node_major, node_minor);
if (!init)
shm_create();
- shm_add_node(n);
+ sz += sizeof(struct citrun_node);
+ sz += n->size * sizeof(uint64_t);
+
+ shm = shm_extend(sz);
+
+ /* n->data = (uint64_t *)(shm + offsetof(struct citrun_node,data)); */
+ n->data = 0xF0F0F0F0F0F0F0F0;
+ memcpy(shm, n, sizeof(struct citrun_node));
+ n->data = (uint64_t *)(shm + sizeof(struct citrun_node));
}
diff --git a/src/rt.h b/src/rt.h
@@ -1,8 +1,9 @@
-#include <stdint.h>
+#include <limits.h> /* PATH_MAX */
+#include <stdint.h> /* uint{64,32,8}_t */
struct citrun_node {
- uint32_t size;
- const char *comp_file_path;
- const char *abs_file_path;
- uint64_t *data;
+ uint32_t size;
+ const char comp_file_path[PATH_MAX];
+ const char abs_file_path[PATH_MAX];
+ uint64_t *data;
};
void citrun_node_add(uint8_t, uint8_t, struct citrun_node *);
diff --git a/t/inst_preamble.sh b/t/inst_preamble.sh
@@ -12,12 +12,13 @@ cat <<EOF > preamble.c.good
#ifdef __cplusplus
extern "" {
#endif
-#include <stdint.h>
+#include <limits.h> /* PATH_MAX */
+#include <stdint.h> /* uint{64,32,8}_t */
struct citrun_node {
- uint32_t size;
- const char *comp_file_path;
- const char *abs_file_path;
- uint64_t *data;
+ uint32_t size;
+ const char comp_file_path[PATH_MAX];
+ const char abs_file_path[PATH_MAX];
+ uint64_t *data;
};
void citrun_node_add(uint8_t, uint8_t, struct citrun_node *);
static struct citrun_node _citrun = {
diff --git a/t/rt_badver.sh b/t/rt_badver.sh
@@ -19,5 +19,5 @@ EOF
ok "compile fake node" cc -include $CITRUN_TOOLS/rt.h -c main.c
ok "link fake node to libcitrun.a" cc -o main main.o $CITRUN_TOOLS/libcitrun.a
-output_good="main: libcitrun 0.0: incompatible node version 0.255"
+output_good="main: libcitrun-0.0: incompatible version 0.255, try cleaning and rebuilding your project"
ok_program "running fake node" 1 "$output_good" ./main
diff --git a/t/rt_exectotals.t b/t/rt_exectotals.t
@@ -27,7 +27,9 @@ for (0..99) {
my $execs = $shm->execs_for($_);
$total += $_ for (@$execs);
}
+
cmp_ok $total, '>', $last_total, "new total > old total";
+ $last_total = $total;
}
kill 'TERM', $child_pid;
diff --git a/t/rt_header.t b/t/rt_header.t
@@ -3,7 +3,7 @@
#
use strict;
use warnings;
-use Test::More tests => 15;
+use Test::More tests => 11;
use tlib::program;
use tlib::shm;
@@ -23,10 +23,5 @@ cmp_ok $ppid, '>', 0, "ppid is greater than min pid";
cmp_ok $pgrp, '<', 100 * 1000, "pgrp is less than max pid";
cmp_ok $pgrp, '>', 0, "pgrp is greater than min pid";
-cmp_ok $shm->{prg_sz}, '<', 1024, 'is size of program name less than 1024';
-cmp_ok $shm->{prg_sz}, '>', 0, 'is size of program name greater than 0';
is $shm->{progname}, "program", 'is test program name correct';
-
-cmp_ok $shm->{cwd_sz}, '<', 1024, 'is size of working dir less than 1024';
-cmp_ok $shm->{cwd_sz}, '>', 0, 'is size of working dir greater than 0';
# is $cwd, "/home/...", 'is working directory believable';
diff --git a/t/rt_translunit.t b/t/rt_translunit.t
@@ -13,10 +13,10 @@ is $ret >> 8, 0, "is program exit code 0";
my $shm = tlib::shm->new();
my ($tu1, $tu2, $tu3) = @{ $shm->{translation_units} };
-is $tu1->{size}, 9, "is transl unit 1 9 lines";
-cmp_ok $tu1->{cmp_sz}, '<', 1024, 'is size of compiler file name less than 1024';
-cmp_ok $tu1->{cmp_sz}, '>', 0, 'is size of compiler file name greater than 0';
-#is $tu1->{comp_file_name}, 'three.c', 'is compiler file name right';
-cmp_ok $tu1->{abs_sz}, '<', 1024, 'is size of absolute file path less than 1024';
-cmp_ok $tu1->{abs_sz}, '>', 0, 'is size of absolute file path greater than 0';
+is $tu1->{size}, 9, "is translation unit 1 9 lines";
+is $tu1->{comp_file_name}, 'three.c', 'is compiler file name right';
like $tu1->{abs_file_path}, qr/.*three.c/, 'is absolute file path believable';
+
+is $tu2->{size}, 11, "is translation unit 2 9 lines";
+is $tu2->{comp_file_name}, 'two.c', 'is compiler file name right';
+like $tu2->{abs_file_path}, qr/.*two.c/, 'is absolute file path believable';
diff --git a/tlib/shm.pm b/tlib/shm.pm
@@ -6,6 +6,8 @@ use POSIX;
# Triggers runtime to use alternate shm path.
$ENV{CITRUN_TOOLS} = 1;
+my $pagesize = POSIX::sysconf(POSIX::_SC_PAGESIZE);
+
sub new {
my ($class) = @_;
@@ -13,28 +15,21 @@ sub new {
bless($self, $class);
open(my $fh, "<:mmap", "procfile.shm") or die $!;
+
$self->{fh} = $fh;
$self->{size} = (stat "procfile.shm")[7];
- ($self->{magic}) = xread($fh, 6);
- ($self->{major}, $self->{minor}) = unpack("C2", xread($fh, 2));
- @{ $self->{pids} } = unpack("L3", xread($fh, 12));
-
- ($self->{prg_sz}) = unpack("S", xread($fh, 2));
- ($self->{progname}) = xread($fh, $self->{prg_sz});
- ($self->{cwd_sz}) = unpack("S", xread($fh, 2));
- ($self->{cwd}) = xread($fh, $self->{cwd_sz});
- $self->next_page();
+ ( $self->{magic}, $self->{major}, $self->{minor},
+ $self->{pids}[0], $self->{pids}[1], $self->{pids}[2],
+ $self->{progname}, $self->{cwd}
+ ) = unpack("Z6CCLLLZ" . PATH_MAX . "Z" . PATH_MAX, xread($fh, $pagesize));
my @translation_units;
while (tell $fh < $self->{size}) {
my %tu;
- ($tu{size}) = unpack("L", xread($fh, 4));
- ($tu{cmp_sz}) = unpack("S", xread($fh, 2));
- $tu{comp_file_name} = xread($fh, $tu{cmp_sz});
- ($tu{abs_sz}) = unpack("S", xread($fh, 2));
- $tu{abs_file_path} = xread($fh, $tu{abs_sz});
+ ($tu{size}, $tu{comp_file_name}, $tu{abs_file_path}) =
+ unpack("LZ" . PATH_MAX . "Z" . PATH_MAX, xread($fh, 4 + 2 * 1024 + 4 + 8));
$tu{exec_buf_pos} = tell $fh;
xread($fh, $tu{size} * 8);
@@ -50,7 +45,6 @@ sub new {
sub next_page {
my ($self) = @_;
- my $pagesize = POSIX::sysconf(POSIX::_SC_PAGESIZE);
my $cur_pos = tell $self->{fh};
xread($self->{fh}, $pagesize - ($cur_pos % $pagesize));
}
@@ -61,6 +55,7 @@ sub execs_for {
my $tu = $self->{translation_units}->[$tu_num];
seek $self->{fh}, $tu->{exec_buf_pos}, 0;
my @execs = unpack("Q$tu->{size}", xread($self->{fh}, $tu->{size} * 8));
+
return \@execs;
}
diff --git a/tlib/utils.sh b/tlib/utils.sh
@@ -6,7 +6,7 @@ export CITRUN_TOOLS="`pwd`/src"
function strip_preamble
{
file="${1}"
- tail -n +24 $file.citrun > $file.citrun_nohdr
+ tail -n +25 $file.citrun > $file.citrun_nohdr
}
function strip_log