[Qemu-devel] [RISU PATCH v4 00/10] record/replay patches

[Qemu-devel] [RISU PATCH v4 00/10] record/replay patches

[Alex Bennée]

Hi,

After much messing about I finally got this re-base working. The
recent re-factoring work has made the code a lot simple. I also
ran into problems getting a decent cross-compiler that could link
against a zlib. This was manly down to multi-arch conflicts when I try
and install zlib1g-dev:arm64 on my Ubuntu 16.04 dev box. Happily we
already have a decent cross compile solution for QEMU so I tweaked the
build-all-archs script to offer a --use-docker option.

The paramterise patch is a little white space messy due to the
inconsistent formatting. Maybe it would be easier to nail down what
the indent should be and run indent over the source tree?

Regards,

Alex Bennée (10):
  .gitignore: ignore build directories
  build-all-archs: support cross building via docker
  build-all-archs: support --static flag
  risu: a bit more verbosity when running
  risu: paramterise send/receive functions
  risu: add header to trace stream
  risu: add simple trace and replay support
  risu: add support compressed tracefiles
  new: record_traces.sh helper script
  new: run_risu.sh script

 .gitignore       |   1 +
 Makefile         |   4 +-
 build-all-archs  |  76 ++++++++++++++++++++++--
 configure        |  55 +++++++++++++++++-
 record_traces.sh |  20 +++++++
 reginfo.c        | 173 ++++++++++++++++++++++++++++++++-----------------------
 risu.c           | 147 ++++++++++++++++++++++++++++++++++++++++++----
 risu.h           |  23 +++++++-
 risu_aarch64.c   |   8 +++
 risu_arm.c       |   8 +++
 risu_m68k.c      |   5 ++
 risu_ppc64.c     |   5 ++
 run_risu.sh      |  53 +++++++++++++++++
 13 files changed, 484 insertions(+), 94 deletions(-)
 create mode 100755 record_traces.sh
 create mode 100755 run_risu.sh

-- 
2.13.0

[Qemu-devel] [RISU PATCH v4 01/10] .gitignore: ignore build directories

[Alex Bennée]

These are generated by the build-all-arches script.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index fc84419..fca9128 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,3 +6,4 @@ risu
 core
 config.h
 Makefile.in
+build
-- 
2.13.0

[Qemu-devel] [RISU PATCH v4 02/10] build-all-archs: support cross building via docker

[Alex Bennée]

If we want to link to any other libraries we might find using simple
cross toolchains doesn't work so well. One way around this is to use a
dockerised cross-toolchain which then won't clash with your host
system. If the user specifies --use-docker the obvious will be done.

By default we use the QEMU projects qemu:debian-FOO-cross images as
RISU hackers are likely to be QEMU developers too. However any docker
tag can be passed on the command line.

If none of the docker images have usable compilers we fall back to
checking the host path.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 build-all-archs | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 66 insertions(+), 6 deletions(-)

diff --git a/build-all-archs b/build-all-archs
index 2768727..78f062e 100755
--- a/build-all-archs
+++ b/build-all-archs
@@ -11,15 +11,65 @@
 # Contributors:
 #     Peter Maydell (Linaro) - initial implementation
 
-# So we notice risugen failing even though it's in a pipeline
-set -o pipefail
+# Simple usage
+usage() {
+    cat <<-EOF
+	Usage: $0 [options]
+
+	Options include:
+	    --use-docker[=tags]    use docker cross compile
+
+        If specifying docker the default will be to use the any
+        qemu:debian-FOO-cross targets available on your system.
+EOF
+    exit 1
+}
+
+while [[ "$1" = -* ]]; do
+    opt="$1"; shift
+    arg=
+    if [[ "$opt" = *=* ]]; then
+	arg="${opt#*=}"
+	opt="${opt%%=*}"
+    fi
+    case "$opt" in
+	--use-docker)
+            if [ -z "$arg" ]; then
+                default_tags=$(docker images qemu --format "{{.Repository}}:{{.Tag}}" | grep "\(arm\|power\).*cross$")
+                docker_tags=$(echo $default_tags | sed 's/\n/\s/g' )
+            else
+	        docker_tags="$arg"
+            fi
+	    ;;
+	--help)
+	    usage
+	    ;;
+	*)
+	    usage
+	    ;;
+    esac
+done
 
 # Debian stretch and Ubuntu Xenial have cross compiler packages for
 # all of these:
-# gcc-arm-linux-gnueabihf gcc-aarch64-linux-gnu gcc-m68k-linux-gnu
-# gcc-powerpc64le-linux-gnu gcc-powerpc64-linux-gnu
+#   gcc-arm-linux-gnueabihf gcc-aarch64-linux-gnu gcc-m68k-linux-gnu
+#   gcc-powerpc64le-linux-gnu gcc-powerpc64-linux-gnu
+# If docker is enabled we just brute force the various images until we
+# can set the one that has a workable cross compiler.
+
+DOCKER_RUN="docker run --rm -t -u $(id -u) -v $(pwd):$(pwd) -w $(pwd)"
 
 program_exists() {
+    if [ ! -z "$docker_tags" ]; then
+        use_docker_tag=""
+        for tag in $docker_tags; do
+            if ${DOCKER_RUN} ${tag} /bin/bash -c "command -v $1 >/dev/null"; then
+                use_docker_tag=$tag
+                return
+            fi
+        done
+    fi
+
     command -v "$1" >/dev/null 2>&1
 }
 
@@ -30,19 +80,29 @@ for triplet in aarch64-linux-gnu arm-linux-gnueabihf m68k-linux-gnu \
     if ! program_exists "${triplet}-gcc"; then
         echo "Skipping ${triplet}: no compiler found"
         continue
+    else
+        echo "Building ${triplet} on ${use_docker_tag:-host}..."
     fi
 
     # Do a complete rebuild from scratch, because it's cheap enough.
     rm -rf build/${triplet}
     mkdir -p build/${triplet}
 
-    (cd build/${triplet} && CROSS_PREFIX="${triplet}-"  ../../configure)
-    make -C build/${triplet} EXTRA_CFLAGS=-Werror
+    CONFIGURE="cd build/${triplet} && CROSS_PREFIX="${triplet}-"  ../../configure"
+    MAKE="make -C build/${triplet} EXTRA_CFLAGS=-Werror"
 
+    if [ -z "$use_docker_tag" ]; then
+        /bin/bash -c "${CONFIGURE}"
+        ${MAKE}
+    else
+        ${DOCKER_RUN} $use_docker_tag /bin/bash -c "${CONFIGURE}"
+        ${DOCKER_RUN} $use_docker_tag /bin/bash -c "${MAKE}"
+    fi
 done
 
 # Now run risugen for all architectures
 mkdir -p build/risuout
+set -o pipefail # detect failures in pipeline
 
 for f in *.risu; do
     echo "Running risugen on $f..."
-- 
2.13.0

[Qemu-devel] [RISU PATCH v4 03/10] build-all-archs: support --static flag

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 build-all-archs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/build-all-archs b/build-all-archs
index 78f062e..788ba36 100755
--- a/build-all-archs
+++ b/build-all-archs
@@ -18,6 +18,7 @@ usage() {
 
 	Options include:
 	    --use-docker[=tags]    use docker cross compile
+            --static               build a static binary
 
         If specifying docker the default will be to use the any
         qemu:debian-FOO-cross targets available on your system.
@@ -41,6 +42,9 @@ while [[ "$1" = -* ]]; do
 	        docker_tags="$arg"
             fi
 	    ;;
+        --static)
+            CONF="--static"
+            ;;
 	--help)
 	    usage
 	    ;;
@@ -88,7 +92,7 @@ for triplet in aarch64-linux-gnu arm-linux-gnueabihf m68k-linux-gnu \
     rm -rf build/${triplet}
     mkdir -p build/${triplet}
 
-    CONFIGURE="cd build/${triplet} && CROSS_PREFIX="${triplet}-"  ../../configure"
+    CONFIGURE="cd build/${triplet} && CROSS_PREFIX=${triplet}-  ../../configure ${CONF}"
     MAKE="make -C build/${triplet} EXTRA_CFLAGS=-Werror"
 
     if [ -z "$use_docker_tag" ]; then
-- 
2.13.0

[Qemu-devel] [RISU PATCH v4 04/10] risu: a bit more verbosity when running

[Alex Bennée]

Before this is could seem a little quite when running as you had no
indication stuff was happening (or how fast). I only dump on the master
side as I want to minimise the amount of qemu logs to sift through.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

--
v3
  - use portable fmt string for image_start_address
  - include arm dumping position
---
 risu.c         | 15 +++++++++++++--
 risu.h         |  3 +++
 risu_aarch64.c |  3 +++
 risu_arm.c     |  3 +++
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/risu.c b/risu.c
index 7e42160..bcdc219 100644
--- a/risu.c
+++ b/risu.c
@@ -37,6 +37,16 @@ sigjmp_buf jmpbuf;
 /* Should we test for FP exception status bits? */
 int test_fp_exc = 0;
 
+long executed_tests = 0;
+void report_test_status(void *pc)
+{
+   executed_tests += 1;
+   if (executed_tests % 100 == 0) {
+      fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
+              executed_tests, pc);
+   }
+}
+
 void master_sigill(int sig, siginfo_t *si, void *uc)
 {
    switch (recv_and_compare_register_info(master_socket, uc))
@@ -61,6 +71,7 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc)
          return;
       case 1:
          /* end of test */
+         fprintf(stderr, "\nend of test\n");
          exit(0);
       default:
          /* mismatch */
@@ -129,7 +140,7 @@ int master(int sock)
    }
    master_socket = sock;
    set_sigill_handler(&master_sigill);
-   fprintf(stderr, "starting image\n");
+   fprintf(stderr, "starting master image at 0x%"PRIxPTR"\n", image_start_address);
    image_start();
    fprintf(stderr, "image returned unexpectedly\n");
    exit(1);
@@ -139,7 +150,7 @@ int apprentice(int sock)
 {
    apprentice_socket = sock;
    set_sigill_handler(&apprentice_sigill);
-   fprintf(stderr, "starting image\n");
+   fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n", image_start_address);
    image_start();
    fprintf(stderr, "image returned unexpectedly\n");
    exit(1);
diff --git a/risu.h b/risu.h
index 883bcf7..1eeb885 100644
--- a/risu.h
+++ b/risu.h
@@ -37,6 +37,7 @@ extern uintptr_t image_start_address;
 extern void *memblock;
 
 extern int test_fp_exc;
+extern int ismaster;
 
 /* Ops code under test can request from risu: */
 #define OP_COMPARE 0
@@ -72,6 +73,8 @@ int recv_and_compare_register_info(int sock, void *uc);
  */
 int report_match_status(void);
 
+void report_test_status(void *pc);
+
 /* Interface provided by CPU-specific code: */
 
 /* Move the PC past this faulting insn by adjusting ucontext
diff --git a/risu_aarch64.c b/risu_aarch64.c
index 9c6809d..5625979 100644
--- a/risu_aarch64.c
+++ b/risu_aarch64.c
@@ -16,6 +16,9 @@ void advance_pc(void *vuc)
 {
     ucontext_t *uc = vuc;
     uc->uc_mcontext.pc += 4;
+    if (ismaster) {
+      report_test_status((void *) uc->uc_mcontext.pc);
+    }
 }
 
 void set_ucontext_paramreg(void *vuc, uint64_t value)
diff --git a/risu_arm.c b/risu_arm.c
index f570828..eaf4f6c 100644
--- a/risu_arm.c
+++ b/risu_arm.c
@@ -44,6 +44,9 @@ void advance_pc(void *vuc)
 {
    ucontext_t *uc = vuc;
    uc->uc_mcontext.arm_pc += insnsize(uc);
+   if (ismaster) {
+      report_test_status((void *) uc->uc_mcontext.arm_pc);
+   }
 }
 
 
-- 
2.13.0

[Qemu-devel] [RISU PATCH v4 05/10] risu: paramterise send/receive functions

[Alex Bennée]

This is a precursor to record/playback support. Instead of passing the
socket fd we now pass helper functions for reading/writing and
responding. This will allow us to do the rest of the record/playback
cleanly outside of the main worker function.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v4
  - split header code
  - fix formatting foo-bar's
v3
  - new for v3
  - arm, aarch64, ppc64
---
 reginfo.c | 118 +++++++++++++++++++++++++++++++-------------------------------
 risu.c    |  23 ++++++++++--
 risu.h    |  11 ++++--
 3 files changed, 89 insertions(+), 63 deletions(-)

diff --git a/reginfo.c b/reginfo.c
index 96c6342..6498459 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -21,33 +21,35 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
 static int mem_used = 0;
 static int packet_mismatch = 0;
 
-int send_register_info(int sock, void *uc)
+int send_register_info(write_fn write_fn, void *uc)
 {
-    struct reginfo ri;
-    int op;
-    reginfo_init(&ri, uc);
-    op = get_risuop(&ri);
-
-    switch (op) {
-    case OP_COMPARE:
-    case OP_TESTEND:
-    default:
-        /* Do a simple register compare on (a) explicit request
-         * (b) end of test (c) a non-risuop UNDEF
-         */
-        return send_data_pkt(sock, &ri, sizeof(ri));
-    case OP_SETMEMBLOCK:
-        memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
-       break;
-    case OP_GETMEMBLOCK:
-        set_ucontext_paramreg(uc,
-                              get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
-        break;
-    case OP_COMPAREMEM:
-        return send_data_pkt(sock, memblock, MEMBLOCKLEN);
-        break;
-    }
-    return 0;
+   struct reginfo ri;
+   int op;
+   reginfo_init(&ri, uc);
+   op = get_risuop(&ri);
+
+   switch (op) {
+      case OP_TESTEND:
+         write_fn(&ri, sizeof(ri));
+         return 1;
+      case OP_SETMEMBLOCK:
+         memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
+         break;
+      case OP_GETMEMBLOCK:
+         set_ucontext_paramreg(uc,
+                               get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
+         break;
+      case OP_COMPAREMEM:
+         return write_fn(memblock, MEMBLOCKLEN);
+         break;
+      case OP_COMPARE:
+      default:
+         /* Do a simple register compare on (a) explicit request
+          * (b) end of test (c) a non-risuop UNDEF
+          */
+         return write_fn(&ri, sizeof(ri));
+   }
+   return 0;
 }
 
 /* Read register info from the socket and compare it with that from the
@@ -58,54 +60,52 @@ int send_register_info(int sock, void *uc)
  * that says whether it is register or memory data, so if the two
  * sides get out of sync then we will fail obscurely.
  */
-int recv_and_compare_register_info(int sock, void *uc)
+int recv_and_compare_register_info(read_fn read_fn, respond_fn resp_fn, void *uc)
 {
-    int resp = 0, op;
-
-    reginfo_init(&master_ri, uc);
-    op = get_risuop(&master_ri);
-
-    switch (op) {
-    case OP_COMPARE:
-    case OP_TESTEND:
-    default:
-        /* Do a simple register compare on (a) explicit request
-         * (b) end of test (c) a non-risuop UNDEF
-         */
-        if (recv_data_pkt(sock, &apprentice_ri, sizeof(apprentice_ri))) {
+   int resp = 0, op;
+
+   reginfo_init(&master_ri, uc);
+   op = get_risuop(&master_ri);
+
+   switch (op) {
+      case OP_COMPARE:
+      case OP_TESTEND:
+      default:
+         /* Do a simple register compare on (a) explicit request
+          * (b) end of test (c) a non-risuop UNDEF
+          */
+         if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {
             packet_mismatch = 1;
             resp = 2;
-
-        } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
+         } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
             /* register mismatch */
             resp = 2;
-
-        } else if (op == OP_TESTEND) {
+         } else if (op == OP_TESTEND) {
             resp = 1;
-        }
-        send_response_byte(sock, resp);
-        break;
+         }
+         resp_fn(resp);
+         break;
       case OP_SETMEMBLOCK:
-          memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
-          break;
+         memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
+         break;
       case OP_GETMEMBLOCK:
-          set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +
-                                (uintptr_t)memblock);
-          break;
+         set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +
+                               (uintptr_t)memblock);
+         break;
       case OP_COMPAREMEM:
          mem_used = 1;
-         if (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
-             packet_mismatch = 1;
-             resp = 2;
+         if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
+            packet_mismatch = 1;
+            resp = 2;
          } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
-             /* memory mismatch */
-             resp = 2;
+            /* memory mismatch */
+            resp = 2;
          }
-         send_response_byte(sock, resp);
+         resp_fn(resp);
          break;
    }
 
-    return resp;
+   return resp;
 }
 
 /* Print a useful report on the status of the last comparison
diff --git a/risu.c b/risu.c
index bcdc219..22571cd 100644
--- a/risu.c
+++ b/risu.c
@@ -47,9 +47,28 @@ void report_test_status(void *pc)
    }
 }
 
+/* Master functions */
+
+int read_sock(void *ptr, size_t bytes)
+{
+   return recv_data_pkt(master_socket, ptr, bytes);
+}
+
+void respond_sock(int r)
+{
+   send_response_byte(master_socket, r);
+}
+
+/* Apprentice function */
+
+int write_sock(void *ptr, size_t bytes)
+{
+   return send_data_pkt(apprentice_socket, ptr, bytes);
+}
+
 void master_sigill(int sig, siginfo_t *si, void *uc)
 {
-   switch (recv_and_compare_register_info(master_socket, uc))
+   switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
    {
       case 0:
          /* match OK */
@@ -63,7 +82,7 @@ void master_sigill(int sig, siginfo_t *si, void *uc)
 
 void apprentice_sigill(int sig, siginfo_t *si, void *uc)
 {
-   switch (send_register_info(apprentice_socket, uc))
+   switch (send_register_info(write_sock, uc))
    {
       case 0:
          /* match OK */
diff --git a/risu.h b/risu.h
index 1eeb885..71ea94f 100644
--- a/risu.h
+++ b/risu.h
@@ -53,17 +53,24 @@ struct reginfo;
 
 /* Functions operating on reginfo */
 
+/* To keep the read/write logic from multiplying across all arches
+ * we wrap up the function here to keep all the changes in one place
+ */
+typedef int (*write_fn) (void *ptr, size_t bytes);
+typedef int (*read_fn) (void *ptr, size_t bytes);
+typedef void (*respond_fn) (int response);
+
 /* Send the register information from the struct ucontext down the socket.
  * Return the response code from the master.
  * NB: called from a signal handler.
  */
-int send_register_info(int sock, void *uc);
+int send_register_info(write_fn write_fn, void *uc);
 
 /* Read register info from the socket and compare it with that from the
  * ucontext. Return 0 for match, 1 for end-of-test, 2 for mismatch.
  * NB: called from a signal handler.
  */
-int recv_and_compare_register_info(int sock, void *uc);
+int recv_and_compare_register_info(read_fn read_fn, respond_fn respond, void *uc);
 
 /* Print a useful report on the status of the last comparison
  * done in recv_and_compare_register_info(). This is called on
-- 
2.13.0

[Qemu-devel] [RISU PATCH v4 06/10] risu: add header to trace stream

[Alex Bennée]

I've also added a header packet with pc/risu op in it so we can keep
better track of how things are going.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v4
  - split from previous patch
---
 reginfo.c      | 103 +++++++++++++++++++++++++++++++++++++--------------------
 risu.h         |   9 +++++
 risu_aarch64.c |   5 +++
 risu_arm.c     |   5 +++
 risu_m68k.c    |   5 +++
 risu_ppc64.c   |   5 +++
 6 files changed, 96 insertions(+), 36 deletions(-)

diff --git a/reginfo.c b/reginfo.c
index 6498459..be7bcdb 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -24,10 +24,20 @@ static int packet_mismatch = 0;
 int send_register_info(write_fn write_fn, void *uc)
 {
    struct reginfo ri;
+   trace_header_t header;
    int op;
+
    reginfo_init(&ri, uc);
    op = get_risuop(&ri);
 
+   /* Write a header with PC/op to keep in sync */
+   header.pc = get_pc(&ri);
+   header.risu_op = op;
+   if (write_fn(&header, sizeof(header)) != 0) {
+      fprintf(stderr,"%s: failed header write\n", __func__);
+      return -1;
+   }
+
    switch (op) {
       case OP_TESTEND:
          write_fn(&ri, sizeof(ri));
@@ -63,51 +73,72 @@ int send_register_info(write_fn write_fn, void *uc)
 int recv_and_compare_register_info(read_fn read_fn, respond_fn resp_fn, void *uc)
 {
    int resp = 0, op;
+   trace_header_t header;
 
    reginfo_init(&master_ri, uc);
    op = get_risuop(&master_ri);
 
-   switch (op) {
-      case OP_COMPARE:
-      case OP_TESTEND:
-      default:
-         /* Do a simple register compare on (a) explicit request
-          * (b) end of test (c) a non-risuop UNDEF
-          */
-         if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {
-            packet_mismatch = 1;
-            resp = 2;
-         } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
-            /* register mismatch */
-            resp = 2;
-         } else if (op == OP_TESTEND) {
-            resp = 1;
-         }
-         resp_fn(resp);
-         break;
-      case OP_SETMEMBLOCK:
-         memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
-         break;
-      case OP_GETMEMBLOCK:
-         set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +
-                               (uintptr_t)memblock);
-         break;
-      case OP_COMPAREMEM:
-         mem_used = 1;
-         if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
-            packet_mismatch = 1;
-            resp = 2;
-         } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
-            /* memory mismatch */
-            resp = 2;
-         }
-         resp_fn(resp);
-         break;
+   if (read_fn(&header, sizeof(header)) != 0) {
+      fprintf(stderr,"%s: failed header read\n", __func__);
+      return -1;
+   }
+
+   if (header.risu_op == op ) {
+
+      /* send OK for the header */
+      resp_fn(0);
+
+      switch (op) {
+         case OP_COMPARE:
+         case OP_TESTEND:
+         default:
+            /* Do a simple register compare on (a) explicit request
+             * (b) end of test (c) a non-risuop UNDEF
+             */
+            if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {
+               packet_mismatch = 1;
+               resp = 2;
+
+            } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
+               /* register mismatch */
+               resp = 2;
+
+            } else if (op == OP_TESTEND) {
+               resp = 1;
+            }
+            resp_fn(resp);
+            break;
+         case OP_SETMEMBLOCK:
+            memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
+            break;
+         case OP_GETMEMBLOCK:
+            set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +
+                                  (uintptr_t)memblock);
+            break;
+         case OP_COMPAREMEM:
+            mem_used = 1;
+            if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
+               packet_mismatch = 1;
+               resp = 2;
+            } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
+               /* memory mismatch */
+               resp = 2;
+            }
+            resp_fn(resp);
+            break;
+      }
+   } else {
+      fprintf(stderr, "out of sync %" PRIxPTR "/%" PRIxPTR " %d/%d\n",
+              get_pc(&master_ri), header.pc,
+              op, header.risu_op);
+      resp = 2;
+      resp_fn(resp);
    }
 
    return resp;
 }
 
+
 /* Print a useful report on the status of the last comparison
  * done in recv_and_compare_register_info(). This is called on
  * exit, so need not restrict itself to signal-safe functions.
diff --git a/risu.h b/risu.h
index 71ea94f..c01a9c3 100644
--- a/risu.h
+++ b/risu.h
@@ -51,6 +51,12 @@ extern int ismaster;
 
 struct reginfo;
 
+typedef struct
+{
+   uintptr_t pc;
+   uint32_t risu_op;
+} trace_header_t;
+
 /* Functions operating on reginfo */
 
 /* To keep the read/write logic from multiplying across all arches
@@ -102,6 +108,9 @@ uint64_t get_reginfo_paramreg(struct reginfo *ri);
  */
 int get_risuop(struct reginfo *ri);
 
+/* Return the PC from a reginfo */
+uintptr_t get_pc(struct reginfo *ri);
+
 /* initialize structure from a ucontext */
 void reginfo_init(struct reginfo *ri, ucontext_t *uc);
 
diff --git a/risu_aarch64.c b/risu_aarch64.c
index 5625979..312bd6a 100644
--- a/risu_aarch64.c
+++ b/risu_aarch64.c
@@ -43,3 +43,8 @@ int get_risuop(struct reginfo *ri)
     uint32_t risukey = 0x00005af0;
     return (key != risukey) ? -1 : op;
 }
+
+uintptr_t get_pc(struct reginfo *ri)
+{
+   return ri->pc;
+}
diff --git a/risu_arm.c b/risu_arm.c
index eaf4f6c..a61256b 100644
--- a/risu_arm.c
+++ b/risu_arm.c
@@ -73,3 +73,8 @@ int get_risuop(struct reginfo *ri)
    uint32_t risukey = (isz == 2) ? 0xdee0 : 0xe7fe5af0;
    return (key != risukey) ? -1 : op;
 }
+
+uintptr_t get_pc(struct reginfo *ri)
+{
+   return ri->gpreg[15];
+}
diff --git a/risu_m68k.c b/risu_m68k.c
index f84ac7a..35b3c03 100644
--- a/risu_m68k.c
+++ b/risu_m68k.c
@@ -33,3 +33,8 @@ int get_risuop(struct reginfo *ri)
     uint32_t risukey = 0x4afc7000;
     return (key != risukey) ? -1 : op;
 }
+
+uintptr_t get_pc(struct reginfo *ri)
+{
+    return ri->gregs[R_PC];
+}
diff --git a/risu_ppc64.c b/risu_ppc64.c
index b575078..e702a00 100644
--- a/risu_ppc64.c
+++ b/risu_ppc64.c
@@ -38,3 +38,8 @@ int get_risuop(struct reginfo *ri)
     uint32_t risukey = 0x00005af0;
     return (key != risukey) ? -1 : op;
 }
+
+uintptr_t get_pc(struct reginfo *ri)
+{
+   return ri->nip;
+}
-- 
2.13.0

[Qemu-devel] [RISU PATCH v4 07/10] risu: add simple trace and replay support

[Alex Bennée]

This adds a very dumb and easily breakable trace and replay support. In
--master mode the various risu ops trigger a write of register/memory
state into a binary file which can be played back to an apprentice.
Currently there is no validation of the image source so feeding the
wrong image will fail straight away.

The trace files will get very big for any appreciable sized test file
and this will be addressed in later patches.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v4
  - fix formatting mess
  - abort() instead of reporting de-sync
  - don't fake return for compiler
v3
  - fix options parsing
  - re-factored so no need for copy & paste
v2
  - moved read/write functions into main risu.c
  - cleaned up formatting
  - report more in apprentice --trace mode
---
 risu.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 84 insertions(+), 13 deletions(-)

diff --git a/risu.c b/risu.c
index 22571cd..137cbbf 100644
--- a/risu.c
+++ b/risu.c
@@ -31,6 +31,7 @@
 void *memblock = 0;
 
 int apprentice_socket, master_socket;
+int trace_file = 0;
 
 sigjmp_buf jmpbuf;
 
@@ -40,10 +41,12 @@ int test_fp_exc = 0;
 long executed_tests = 0;
 void report_test_status(void *pc)
 {
-   executed_tests += 1;
-   if (executed_tests % 100 == 0) {
-      fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
-              executed_tests, pc);
+   if (ismaster || trace_file) {
+      executed_tests += 1;
+      if (executed_tests % 100 == 0) {
+         fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
+                 executed_tests, pc);
+      }
    }
 }
 
@@ -54,6 +57,12 @@ int read_sock(void *ptr, size_t bytes)
    return recv_data_pkt(master_socket, ptr, bytes);
 }
 
+int write_trace(void *ptr, size_t bytes)
+{
+   size_t res = write(trace_file, ptr, bytes);
+   return (res == bytes) ? 0 : 1;
+}
+
 void respond_sock(int r)
 {
    send_response_byte(master_socket, r);
@@ -66,9 +75,36 @@ int write_sock(void *ptr, size_t bytes)
    return send_data_pkt(apprentice_socket, ptr, bytes);
 }
 
+int read_trace(void *ptr, size_t bytes)
+{
+   size_t res = read(trace_file, ptr, bytes);
+   return (res == bytes) ? 0 : 1;
+}
+
+void respond_trace(int r)
+{
+   switch (r) {
+      case 0: /* test ok */
+      case 1: /* end of test */
+         break;
+      default:
+         /* should not get here */
+         abort();
+         break;
+   }
+}
+
 void master_sigill(int sig, siginfo_t *si, void *uc)
 {
-   switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
+   int r;
+
+   if (trace_file) {
+      r = send_register_info(write_trace, uc);
+   } else {
+      r = recv_and_compare_register_info(read_sock, respond_sock, uc);
+   }
+
+   switch (r)
    {
       case 0:
          /* match OK */
@@ -82,7 +118,15 @@ void master_sigill(int sig, siginfo_t *si, void *uc)
 
 void apprentice_sigill(int sig, siginfo_t *si, void *uc)
 {
-   switch (send_register_info(write_sock, uc))
+   int r;
+
+   if (trace_file) {
+      r = recv_and_compare_register_info(read_trace, respond_trace, uc);
+   } else {
+      r = send_register_info(write_sock, uc);
+   }
+
+   switch (r)
    {
       case 0:
          /* match OK */
@@ -94,6 +138,9 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc)
          exit(0);
       default:
          /* mismatch */
+         if (trace_file) {
+            report_match_status();
+         }
          exit(1);
    }
 }
@@ -155,7 +202,13 @@ int master(int sock)
 {
    if (sigsetjmp(jmpbuf, 1))
    {
-      return report_match_status();
+      if (trace_file) {
+         close(trace_file);
+         fprintf(stderr,"\nDone...\n");
+         return 0;
+      } else {
+         return report_match_status();
+      }
    }
    master_socket = sock;
    set_sigill_handler(&master_sigill);
@@ -184,6 +237,7 @@ void usage (void)
    fprintf(stderr, "between master and apprentice risu processes.\n\n");
    fprintf(stderr, "Options:\n");
    fprintf(stderr, "  --master          Be the master (server)\n");
+   fprintf(stderr, "  -t, --trace=FILE  Record/playback trace file\n");
    fprintf(stderr, "  -h, --host=HOST   Specify master host machine (apprentice only)\n");
    fprintf(stderr, "  -p, --port=PORT   Specify the port to connect to/listen on (default 9191)\n");
 }
@@ -194,6 +248,7 @@ int main(int argc, char **argv)
    uint16_t port = 9191;
    char *hostname = "localhost";
    char *imgfile;
+   char *trace_fn = NULL;
    int sock;
 
    // TODO clean this up later
@@ -204,13 +259,14 @@ int main(int argc, char **argv)
          {
             { "help", no_argument, 0, '?'},
             { "master", no_argument, &ismaster, 1 },
+            { "trace", required_argument, 0, 't' },
             { "host", required_argument, 0, 'h' },
             { "port", required_argument, 0, 'p' },
             { "test-fp-exc", no_argument, &test_fp_exc, 1 },
             { 0,0,0,0 }
          };
       int optidx = 0;
-      int c = getopt_long(argc, argv, "h:p:", longopts, &optidx);
+      int c = getopt_long(argc, argv, "h:p:t:", longopts, &optidx);
       if (c == -1)
       {
          break;
@@ -223,6 +279,11 @@ int main(int argc, char **argv)
             /* flag set by getopt_long, do nothing */
             break;
          }
+         case 't':
+         {
+           trace_fn = optarg;
+           break;
+         }
          case 'h':
          {
             hostname = optarg;
@@ -253,17 +314,27 @@ int main(int argc, char **argv)
    }
 
    load_image(imgfile);
-   
+
    if (ismaster)
    {
-      fprintf(stderr, "master port %d\n", port);
-      sock = master_connect(port);
+      if (trace_fn)
+      {
+         trace_file = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
+      } else {
+         fprintf(stderr, "master port %d\n", port);
+         sock = master_connect(port);
+      }
       return master(sock);
    }
    else
    {
-      fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
-      sock = apprentice_connect(hostname, port);
+      if (trace_fn)
+      {
+         trace_file = open(trace_fn, O_RDONLY);
+      } else {
+         fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
+         sock = apprentice_connect(hostname, port);
+      }
       return apprentice(sock);
    }
 }
-- 
2.13.0

[Qemu-devel] [RISU PATCH v4 08/10] risu: add support compressed tracefiles

[Alex Bennée]

This uses the magic of zlib's gzread/write interface to wrap the
tracefile in compression. The code changes are tiny. I spent more time
messing about with the configure/linker stuff to auto-detect bits.

As you need decent multi-arch support or a correctly setup cross
toolchain we fall back if we can't compile with zlib. This
unfortunately needs some #ifdef hackery around the zlib bits in
risu.c.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

--
v4
  - removed redundant config.h output, added HAVE_ZLIB
  - added BUILD_INC to deal with out-of-tree builds
---
 Makefile  |  4 ++--
 configure | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 risu.c    | 30 +++++++++++++++++++++++++++---
 3 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 9a29bb4..ca80eef 100644
--- a/Makefile
+++ b/Makefile
@@ -17,7 +17,7 @@ VPATH=$(SRCDIR)
 
 CFLAGS ?= -g
 
-ALL_CFLAGS = -Wall -D_GNU_SOURCE -DARCH=$(ARCH) $(CFLAGS) $(EXTRA_CFLAGS)
+ALL_CFLAGS = -Wall -D_GNU_SOURCE -DARCH=$(ARCH) $(BUILD_INC) $(CFLAGS) $(EXTRA_CFLAGS)
 
 PROG=risu
 SRCS=risu.c comms.c reginfo.c risu_$(ARCH).c risu_reginfo_$(ARCH).c
@@ -35,7 +35,7 @@ all: $(PROG) $(BINS)
 dump: $(RISU_ASMS)
 
 $(PROG): $(OBJS)
-	$(CC) $(STATIC) $(ALL_CFLAGS) -o $@ $^
+	$(CC) $(STATIC) $(ALL_CFLAGS) -o $@ $^ $(LDFLAGS)
 
 %.risu.asm: %.risu.bin
 	${OBJDUMP} -b binary -m $(ARCH) -D $^ > $@
diff --git a/configure b/configure
index c4b5adb..1dc527b 100755
--- a/configure
+++ b/configure
@@ -32,6 +32,10 @@ compile() {
     $CC $CFLAGS -c -o ${1}.o ${1}.c 2>/dev/null
 }
 
+link() {
+    $LD $LDFLAGS -l${2} -o ${1} ${1}.o 2>/dev/null
+}
+
 check_define() {
     c=${tmp_dir}/check_define_${1}
     cat > ${c}.c <<EOF
@@ -58,6 +62,48 @@ guess_arch() {
     fi
 }
 
+check_type() {
+    c=${tmp_dir}/check_type_${1}
+    cat > ${c}.c <<EOF
+#include <inttypes.h>
+#include <stdint.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+
+int main(void) { $1 thisone; return 0; }
+EOF
+    compile $c
+}
+
+check_lib() {
+    c=${tmp_dir}/check_lib${1}
+    cat > ${c}.c <<EOF
+#include <stdint.h>
+#include <$2.h>
+
+int main(void) { $3; return 0; }
+EOF
+    compile $c && link $c $1
+}
+
+generate_config() {
+    cfg=config.h
+    echo "generating config.h..."
+
+    echo "/* config.h - generated by the 'configure' script */" > $cfg
+    echo "#ifndef CONFIG_H" >> $cfg
+    echo "#define CONFIG_H 1" >> $cfg
+
+    if check_lib z zlib "zlibVersion()"; then
+        echo "#define HAVE_ZLIB 1" >> $cfg
+        LDFLAGS=-lz
+    fi
+
+    echo "#endif /* CONFIG_H */" >> $cfg
+
+    echo "...done"
+}
+
 generate_makefilein() {
     m=Makefile.in
     echo "generating Makefile.in..."
@@ -65,11 +111,13 @@ generate_makefilein() {
     echo "# Makefile.in - generated by the 'configure' script" > $m
     echo "ARCH:=${ARCH}" >> $m
     echo "CC:=${CC}" >> $m
+    echo "LDFLAGS:=${LDFLAGS}" >> $m
     echo "AS:=${AS}" >> $m
     echo "OBJCOPY:=${OBJCOPY}" >> $m
     echo "OBJDUMP:=${OBJDUMP}" >> $m
     echo "STATIC:=${STATIC}" >> $m
     echo "SRCDIR:=${SRCDIR}" >> $m
+    echo "BUILD_INC:=${BUILD_INC}" >> $m
 
     echo "...done"
 }
@@ -118,6 +166,7 @@ done
 
 CC="${CC-${CROSS_PREFIX}gcc}"
 AS="${AS-${CROSS_PREFIX}as}"
+LD="${LD-${CROSS_PREFIX}ld}"
 OBJCOPY="${OBJCOPY-${CROSS_PREFIX}objcopy}"
 OBJDUMP="${OBJDUMP-${CROSS_PREFIX}objdump}"
 
@@ -125,15 +174,17 @@ if test "x${ARCH}" = "x"; then
     guess_arch
 fi
 
-generate_makefilein
-
 # Are we in a separate build tree? If so, link the Makefile
 # so that 'make' works.
 if test ! -e Makefile; then
     echo "linking Makefile..."
+    BUILD_INC="-I $(pwd)"
     ln -s "${SRCDIR}/Makefile" .
 fi
 
+generate_config
+generate_makefilein
+
 rm -r "$tmp_dir"
 
 echo "type 'make' to start the build"
diff --git a/risu.c b/risu.c
index 137cbbf..275c5a9 100644
--- a/risu.c
+++ b/risu.c
@@ -26,6 +26,8 @@
 #include <fcntl.h>
 #include <string.h>
 
+#include "config.h"
+
 #include "risu.h"
 
 void *memblock = 0;
@@ -33,6 +35,11 @@ void *memblock = 0;
 int apprentice_socket, master_socket;
 int trace_file = 0;
 
+#ifdef HAVE_ZLIB
+#include <zlib.h>
+gzFile gz_trace_file;
+#endif
+
 sigjmp_buf jmpbuf;
 
 /* Should we test for FP exception status bits? */
@@ -59,7 +66,11 @@ int read_sock(void *ptr, size_t bytes)
 
 int write_trace(void *ptr, size_t bytes)
 {
+#ifdef HAVE_ZLIB
+   size_t res = gzwrite(gz_trace_file, ptr, bytes);
+#else
    size_t res = write(trace_file, ptr, bytes);
+#endif
    return (res == bytes) ? 0 : 1;
 }
 
@@ -77,7 +88,11 @@ int write_sock(void *ptr, size_t bytes)
 
 int read_trace(void *ptr, size_t bytes)
 {
+#ifdef HAVE_ZLIB
+   size_t res = gzread(gz_trace_file, ptr, bytes);
+#else
    size_t res = read(trace_file, ptr, bytes);
+#endif
    return (res == bytes) ? 0 : 1;
 }
 
@@ -202,9 +217,12 @@ int master(int sock)
 {
    if (sigsetjmp(jmpbuf, 1))
    {
-      if (trace_file) {
-         close(trace_file);
-         fprintf(stderr,"\nDone...\n");
+      if (trace_file)
+      {
+#ifdef HAVE_ZLIB
+         gzclose(gz_trace_file);
+#endif
+         fprintf(stderr,"Done...\n");
          return 0;
       } else {
          return report_match_status();
@@ -320,6 +338,9 @@ int main(int argc, char **argv)
       if (trace_fn)
       {
          trace_file = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
+#ifdef HAVE_ZLIB
+         gz_trace_file = gzdopen(trace_file, "wb9");
+#endif
       } else {
          fprintf(stderr, "master port %d\n", port);
          sock = master_connect(port);
@@ -331,6 +352,9 @@ int main(int argc, char **argv)
       if (trace_fn)
       {
          trace_file = open(trace_fn, O_RDONLY);
+#ifdef HAVE_ZLIB
+         gz_trace_file = gzdopen(trace_file, "rb");
+#endif
       } else {
          fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
          sock = apprentice_connect(hostname, port);
-- 
2.13.0

[Qemu-devel] [RISU PATCH v4 09/10] new: record_traces.sh helper script

[Alex Bennée]

A simple script to run through a bunch of binaries and generate their
trace files.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v3
  - allow overriding of RISU binary
---
 record_traces.sh | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100755 record_traces.sh

diff --git a/record_traces.sh b/record_traces.sh
new file mode 100755
index 0000000..78ce47f
--- /dev/null
+++ b/record_traces.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+#
+# Record traces
+#
+# A risu helper script to batch process a bunch of binaries and record their outputs
+#
+set -e
+
+if test -z "$RISU"; then
+    script_dir=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd -P)
+    RISU=${script_dir}/risu
+fi
+
+for f in $@; do
+    echo "Running risu against $f"
+    t="$f.trace"
+    ${RISU} --master $f -t $t
+    echo "Checking trace file OK"
+    ${RISU} $f -t $t
+done
-- 
2.13.0

[Qemu-devel] [RISU PATCH v4 10/10] new: run_risu.sh script

[Alex Bennée]

A simple script to work through running all of a bunch of files with
record/playback traces. Dumps a summary and the number of failed tests
at the end.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v3
  - tweak to allow specifying RISU binary
---
 run_risu.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100755 run_risu.sh

diff --git a/run_risu.sh b/run_risu.sh
new file mode 100755
index 0000000..7740d6f
--- /dev/null
+++ b/run_risu.sh
@@ -0,0 +1,53 @@
+#!/bin/bash
+#
+# Run risu against a set of binaries + trace files
+#
+#
+#set -e
+
+passed=()
+failed=()
+missing=()
+
+if test -z "$RISU"; then
+    script_dir=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd -P)
+    RISU=${script_dir}/risu
+fi
+
+for f in $@; do
+    t="$f.trace"
+    echo "Running $f against $t"
+    if [ -e $t ]; then
+        ${QEMU} ${RISU} $f -t $t
+        if [ $? == 0 ]; then
+            passed=( "${passed[@]}" $f )
+        else
+            failed=( "${failed[@]}" $f )
+        fi
+    else
+        missing=( "${missing[@]}" $f )
+    fi
+done
+
+if test ${#missing[@]} -gt 0; then
+    echo "Tests missing ${#missing[@]} trace files:"
+    for m in "${missing[@]}"; do
+        echo "$m"
+    done
+fi
+
+if test ${#passed[@]} -gt 0; then
+    echo "Passed ${#passed[@]} tests:"
+    for p in "${passed[@]}"; do
+        echo "$p"
+    done
+fi
+
+if test ${#failed[@]} -gt 0; then
+    echo "Failed ${#failed[@]} tests:"
+    for f in "${failed[@]}"; do
+        echo "$f"
+    done
+fi
+
+exit ${#failed[@]}
-- 
2.13.0

Re: [Qemu-devel] [RISU PATCH v4 01/10] .gitignore: ignore build directories

[Peter Maydell]

On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
> These are generated by the build-all-arches script.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  .gitignore | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/.gitignore b/.gitignore
> index fc84419..fca9128 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -6,3 +6,4 @@ risu
>  core
>  config.h
>  Makefile.in
> +build
> --
> 2.13.0
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [RISU PATCH v4 04/10] risu: a bit more verbosity when running

[Peter Maydell]

On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
> Before this is could seem a little quite when running as you had no
> indication stuff was happening (or how fast). I only dump on the master
> side as I want to minimise the amount of qemu logs to sift through.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> --
> v3
>   - use portable fmt string for image_start_address
>   - include arm dumping position
> ---
>  risu.c         | 15 +++++++++++++--
>  risu.h         |  3 +++
>  risu_aarch64.c |  3 +++
>  risu_arm.c     |  3 +++
>  4 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/risu.c b/risu.c
> index 7e42160..bcdc219 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -37,6 +37,16 @@ sigjmp_buf jmpbuf;
>  /* Should we test for FP exception status bits? */
>  int test_fp_exc = 0;
>
> +long executed_tests = 0;
> +void report_test_status(void *pc)
> +{
> +   executed_tests += 1;
> +   if (executed_tests % 100 == 0) {
> +      fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
> +              executed_tests, pc);

This gets called from signal handlers, so you can't use stdio.

> +   }
> +}
> +
>  void master_sigill(int sig, siginfo_t *si, void *uc)
>  {
>     switch (recv_and_compare_register_info(master_socket, uc))
> @@ -61,6 +71,7 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc)
>           return;
>        case 1:
>           /* end of test */
> +         fprintf(stderr, "\nend of test\n");

Can't use stdio in a signal handler.

>           exit(0);
>        default:
>           /* mismatch */
> @@ -129,7 +140,7 @@ int master(int sock)
>     }
>     master_socket = sock;
>     set_sigill_handler(&master_sigill);
> -   fprintf(stderr, "starting image\n");
> +   fprintf(stderr, "starting master image at 0x%"PRIxPTR"\n", image_start_address);
>     image_start();
>     fprintf(stderr, "image returned unexpectedly\n");
>     exit(1);
> @@ -139,7 +150,7 @@ int apprentice(int sock)
>  {
>     apprentice_socket = sock;
>     set_sigill_handler(&apprentice_sigill);
> -   fprintf(stderr, "starting image\n");
> +   fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n", image_start_address);
>     image_start();
>     fprintf(stderr, "image returned unexpectedly\n");
>     exit(1);
> diff --git a/risu.h b/risu.h
> index 883bcf7..1eeb885 100644
> --- a/risu.h
> +++ b/risu.h
> @@ -37,6 +37,7 @@ extern uintptr_t image_start_address;
>  extern void *memblock;
>
>  extern int test_fp_exc;
> +extern int ismaster;
>
>  /* Ops code under test can request from risu: */
>  #define OP_COMPARE 0
> @@ -72,6 +73,8 @@ int recv_and_compare_register_info(int sock, void *uc);
>   */
>  int report_match_status(void);
>
> +void report_test_status(void *pc);
> +
>  /* Interface provided by CPU-specific code: */
>
>  /* Move the PC past this faulting insn by adjusting ucontext
> diff --git a/risu_aarch64.c b/risu_aarch64.c
> index 9c6809d..5625979 100644
> --- a/risu_aarch64.c
> +++ b/risu_aarch64.c
> @@ -16,6 +16,9 @@ void advance_pc(void *vuc)
>  {
>      ucontext_t *uc = vuc;
>      uc->uc_mcontext.pc += 4;
> +    if (ismaster) {
> +      report_test_status((void *) uc->uc_mcontext.pc);
> +    }

If we're going to print something we should do it in the
arch-independent code that calls advance_pc(), rather than
duplicating the code in each arch backend.

>  }
>
>  void set_ucontext_paramreg(void *vuc, uint64_t value)
> diff --git a/risu_arm.c b/risu_arm.c
> index f570828..eaf4f6c 100644
> --- a/risu_arm.c
> +++ b/risu_arm.c
> @@ -44,6 +44,9 @@ void advance_pc(void *vuc)
>  {
>     ucontext_t *uc = vuc;
>     uc->uc_mcontext.arm_pc += insnsize(uc);
> +   if (ismaster) {
> +      report_test_status((void *) uc->uc_mcontext.arm_pc);
> +   }
>  }

thanks
-- PMM

Re: [Qemu-devel] [RISU PATCH v4 05/10] risu: paramterise send/receive functions

[Peter Maydell]

On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
> This is a precursor to record/playback support. Instead of passing the
> socket fd we now pass helper functions for reading/writing and
> responding. This will allow us to do the rest of the record/playback
> cleanly outside of the main worker function.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> -        } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
> +         } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
>              /* register mismatch */
>              resp = 2;
> -
> -        } else if (op == OP_TESTEND) {
> +         } else if (op == OP_TESTEND) {
>              resp = 1;
> -        }

[etc]

If you're going to fix the indent can you put that in
its own patch, please? Much easier to review that way.

thanks
-- PMM

Re: [Qemu-devel] [RISU PATCH v4 05/10] risu: paramterise send/receive functions

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
>> This is a precursor to record/playback support. Instead of passing the
>> socket fd we now pass helper functions for reading/writing and
>> responding. This will allow us to do the rest of the record/playback
>> cleanly outside of the main worker function.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
>> -        } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
>> +         } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
>>              /* register mismatch */
>>              resp = 2;
>> -
>> -        } else if (op == OP_TESTEND) {
>> +         } else if (op == OP_TESTEND) {
>>              resp = 1;
>> -        }
>
> [etc]
>
> If you're going to fix the indent can you put that in
> its own patch, please? Much easier to review that way.

Damn I forgotten that got munged. Can we agree on what the indent should
be for risu? 3 or 4, space or tabs?

--
Alex Bennée

Re: [Qemu-devel] [RISU PATCH v4 05/10] risu: paramterise send/receive functions

[Peter Maydell]

On 6 June 2017 at 11:13, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> If you're going to fix the indent can you put that in
>> its own patch, please? Much easier to review that way.
>
> Damn I forgotten that got munged. Can we agree on what the indent should
> be for risu? 3 or 4, space or tabs?

Same as QEMU coding style, please.

thanks
-- PMM

Re: [Qemu-devel] [RISU PATCH v4 07/10] risu: add simple trace and replay support

[Peter Maydell]

On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
> This adds a very dumb and easily breakable trace and replay support. In
> --master mode the various risu ops trigger a write of register/memory
> state into a binary file which can be played back to an apprentice.
> Currently there is no validation of the image source so feeding the
> wrong image will fail straight away.
>
> The trace files will get very big for any appreciable sized test file
> and this will be addressed in later patches.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v4
>   - fix formatting mess
>   - abort() instead of reporting de-sync
>   - don't fake return for compiler
> v3
>   - fix options parsing
>   - re-factored so no need for copy & paste
> v2
>   - moved read/write functions into main risu.c
>   - cleaned up formatting
>   - report more in apprentice --trace mode
> ---
>  risu.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 84 insertions(+), 13 deletions(-)
>
> diff --git a/risu.c b/risu.c
> index 22571cd..137cbbf 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -31,6 +31,7 @@
>  void *memblock = 0;
>
>  int apprentice_socket, master_socket;
> +int trace_file = 0;
>
>  sigjmp_buf jmpbuf;
>
> @@ -40,10 +41,12 @@ int test_fp_exc = 0;
>  long executed_tests = 0;
>  void report_test_status(void *pc)
>  {
> -   executed_tests += 1;
> -   if (executed_tests % 100 == 0) {
> -      fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
> -              executed_tests, pc);
> +   if (ismaster || trace_file) {
> +      executed_tests += 1;
> +      if (executed_tests % 100 == 0) {
> +         fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
> +                 executed_tests, pc);
> +      }
>     }
>  }
>
> @@ -54,6 +57,12 @@ int read_sock(void *ptr, size_t bytes)
>     return recv_data_pkt(master_socket, ptr, bytes);
>  }
>
> +int write_trace(void *ptr, size_t bytes)
> +{
> +   size_t res = write(trace_file, ptr, bytes);
> +   return (res == bytes) ? 0 : 1;
> +}
> +
>  void respond_sock(int r)
>  {
>     send_response_byte(master_socket, r);
> @@ -66,9 +75,36 @@ int write_sock(void *ptr, size_t bytes)
>     return send_data_pkt(apprentice_socket, ptr, bytes);
>  }
>
> +int read_trace(void *ptr, size_t bytes)
> +{
> +   size_t res = read(trace_file, ptr, bytes);
> +   return (res == bytes) ? 0 : 1;
> +}
> +
> +void respond_trace(int r)
> +{
> +   switch (r) {
> +      case 0: /* test ok */
> +      case 1: /* end of test */
> +         break;
> +      default:
> +         /* should not get here */
> +         abort();
> +         break;
> +   }
> +}
> +
>  void master_sigill(int sig, siginfo_t *si, void *uc)
>  {
> -   switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
> +   int r;
> +
> +   if (trace_file) {
> +      r = send_register_info(write_trace, uc);
> +   } else {
> +      r = recv_and_compare_register_info(read_sock, respond_sock, uc);
> +   }

It's a bit weird that in the trace file case we send the data
from the master, but in the normal case we send it from
the apprentice end.

> +
> +   switch (r)
>     {
>        case 0:
>           /* match OK */
> @@ -82,7 +118,15 @@ void master_sigill(int sig, siginfo_t *si, void *uc)
>
>  void apprentice_sigill(int sig, siginfo_t *si, void *uc)
>  {
> -   switch (send_register_info(write_sock, uc))
> +   int r;
> +
> +   if (trace_file) {
> +      r = recv_and_compare_register_info(read_trace, respond_trace, uc);
> +   } else {
> +      r = send_register_info(write_sock, uc);
> +   }
> +
> +   switch (r)
>     {
>        case 0:
>           /* match OK */
> @@ -94,6 +138,9 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc)
>           exit(0);
>        default:
>           /* mismatch */
> +         if (trace_file) {
> +            report_match_status();
> +         }

report_match_status() uses stdio, you can't call it from a signal handler.

>           exit(1);
>     }
>  }
> @@ -155,7 +202,13 @@ int master(int sock)
>  {
>     if (sigsetjmp(jmpbuf, 1))
>     {
> -      return report_match_status();
> +      if (trace_file) {
> +         close(trace_file);
> +         fprintf(stderr,"\nDone...\n");
> +         return 0;
> +      } else {
> +         return report_match_status();
> +      }
>     }
>     master_socket = sock;
>     set_sigill_handler(&master_sigill);
> @@ -184,6 +237,7 @@ void usage (void)
>     fprintf(stderr, "between master and apprentice risu processes.\n\n");
>     fprintf(stderr, "Options:\n");
>     fprintf(stderr, "  --master          Be the master (server)\n");
> +   fprintf(stderr, "  -t, --trace=FILE  Record/playback trace file\n");
>     fprintf(stderr, "  -h, --host=HOST   Specify master host machine (apprentice only)\n");
>     fprintf(stderr, "  -p, --port=PORT   Specify the port to connect to/listen on (default 9191)\n");
>  }
> @@ -194,6 +248,7 @@ int main(int argc, char **argv)
>     uint16_t port = 9191;
>     char *hostname = "localhost";
>     char *imgfile;
> +   char *trace_fn = NULL;
>     int sock;
>
>     // TODO clean this up later
> @@ -204,13 +259,14 @@ int main(int argc, char **argv)
>           {
>              { "help", no_argument, 0, '?'},
>              { "master", no_argument, &ismaster, 1 },
> +            { "trace", required_argument, 0, 't' },
>              { "host", required_argument, 0, 'h' },
>              { "port", required_argument, 0, 'p' },
>              { "test-fp-exc", no_argument, &test_fp_exc, 1 },
>              { 0,0,0,0 }
>           };
>        int optidx = 0;
> -      int c = getopt_long(argc, argv, "h:p:", longopts, &optidx);
> +      int c = getopt_long(argc, argv, "h:p:t:", longopts, &optidx);
>        if (c == -1)
>        {
>           break;
> @@ -223,6 +279,11 @@ int main(int argc, char **argv)
>              /* flag set by getopt_long, do nothing */
>              break;
>           }
> +         case 't':
> +         {
> +           trace_fn = optarg;
> +           break;
> +         }
>           case 'h':
>           {
>              hostname = optarg;
> @@ -253,17 +314,27 @@ int main(int argc, char **argv)
>     }
>
>     load_image(imgfile);
> -
> +
>     if (ismaster)
>     {
> -      fprintf(stderr, "master port %d\n", port);
> -      sock = master_connect(port);
> +      if (trace_fn)
> +      {
> +         trace_file = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
> +      } else {
> +         fprintf(stderr, "master port %d\n", port);
> +         sock = master_connect(port);
> +      }
>        return master(sock);

Doesn't this call master() with an uninitialized sock if
the trace file is in use ?

>     }
>     else
>     {
> -      fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
> -      sock = apprentice_connect(hostname, port);
> +      if (trace_fn)
> +      {
> +         trace_file = open(trace_fn, O_RDONLY);
> +      } else {
> +         fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
> +         sock = apprentice_connect(hostname, port);
> +      }
>        return apprentice(sock);
>     }
>  }
> --
> 2.13.0
>

thanks
-- PMM

Re: [Qemu-devel] [RISU PATCH v4 08/10] risu: add support compressed tracefiles

[Peter Maydell]

On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
> This uses the magic of zlib's gzread/write interface to wrap the
> tracefile in compression. The code changes are tiny. I spent more time
> messing about with the configure/linker stuff to auto-detect bits.
>
> As you need decent multi-arch support or a correctly setup cross
> toolchain we fall back if we can't compile with zlib. This
> unfortunately needs some #ifdef hackery around the zlib bits in
> risu.c.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> --
> v4
>   - removed redundant config.h output, added HAVE_ZLIB
>   - added BUILD_INC to deal with out-of-tree builds

I thought the trace files were so enormous that zlib was
basically mandatory for the record/replay to be useful?
I'm wondering if we should use a submodule for zlib and
just build it locally. That would let us just make it a
hard requirement (and avoid the need to do things with
docker).

thanks
-- PMM

Re: [Qemu-devel] [RISU PATCH v4 09/10] new: record_traces.sh helper script

[Peter Maydell]

On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
> A simple script to run through a bunch of binaries and generate their
> trace files.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v3
>   - allow overriding of RISU binary
> ---
>  record_traces.sh | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100755 record_traces.sh
>
> diff --git a/record_traces.sh b/record_traces.sh
> new file mode 100755
> index 0000000..78ce47f
> --- /dev/null
> +++ b/record_traces.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +#
> +# Record traces
> +#
> +# A risu helper script to batch process a bunch of binaries and record their outputs
> +#

Can we have a license statement, copyright line and a
usage summary, please? (Ditto patch 10). I'm not sure if
these scripts are intended for testing purposes or as
something that risu users might want to use -- if the
latter then also documenting them in README would be
a good idea.

> +set -e
> +
> +if test -z "$RISU"; then
> +    script_dir=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd -P)
> +    RISU=${script_dir}/risu
> +fi
> +
> +for f in $@; do
> +    echo "Running risu against $f"
> +    t="$f.trace"
> +    ${RISU} --master $f -t $t
> +    echo "Checking trace file OK"
> +    ${RISU} $f -t $t
> +done
> --
> 2.13.0

thanks
-- PMM

Re: [Qemu-devel] [RISU PATCH v4 07/10] risu: add simple trace and replay support

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
>> This adds a very dumb and easily breakable trace and replay support. In
>> --master mode the various risu ops trigger a write of register/memory
>> state into a binary file which can be played back to an apprentice.
>> Currently there is no validation of the image source so feeding the
>> wrong image will fail straight away.
>>
>> The trace files will get very big for any appreciable sized test file
>> and this will be addressed in later patches.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> ---
>> v4
>>   - fix formatting mess
>>   - abort() instead of reporting de-sync
>>   - don't fake return for compiler
>> v3
>>   - fix options parsing
>>   - re-factored so no need for copy & paste
>> v2
>>   - moved read/write functions into main risu.c
>>   - cleaned up formatting
>>   - report more in apprentice --trace mode
>> ---
>>  risu.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 84 insertions(+), 13 deletions(-)
>>
>> diff --git a/risu.c b/risu.c
>> index 22571cd..137cbbf 100644
>> --- a/risu.c
>> +++ b/risu.c
>> @@ -31,6 +31,7 @@
>>  void *memblock = 0;
>>
>>  int apprentice_socket, master_socket;
>> +int trace_file = 0;
>>
>>  sigjmp_buf jmpbuf;
>>
>> @@ -40,10 +41,12 @@ int test_fp_exc = 0;
>>  long executed_tests = 0;
>>  void report_test_status(void *pc)
>>  {
>> -   executed_tests += 1;
>> -   if (executed_tests % 100 == 0) {
>> -      fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
>> -              executed_tests, pc);
>> +   if (ismaster || trace_file) {
>> +      executed_tests += 1;
>> +      if (executed_tests % 100 == 0) {
>> +         fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
>> +                 executed_tests, pc);
>> +      }
>>     }
>>  }
>>
>> @@ -54,6 +57,12 @@ int read_sock(void *ptr, size_t bytes)
>>     return recv_data_pkt(master_socket, ptr, bytes);
>>  }
>>
>> +int write_trace(void *ptr, size_t bytes)
>> +{
>> +   size_t res = write(trace_file, ptr, bytes);
>> +   return (res == bytes) ? 0 : 1;
>> +}
>> +
>>  void respond_sock(int r)
>>  {
>>     send_response_byte(master_socket, r);
>> @@ -66,9 +75,36 @@ int write_sock(void *ptr, size_t bytes)
>>     return send_data_pkt(apprentice_socket, ptr, bytes);
>>  }
>>
>> +int read_trace(void *ptr, size_t bytes)
>> +{
>> +   size_t res = read(trace_file, ptr, bytes);
>> +   return (res == bytes) ? 0 : 1;
>> +}
>> +
>> +void respond_trace(int r)
>> +{
>> +   switch (r) {
>> +      case 0: /* test ok */
>> +      case 1: /* end of test */
>> +         break;
>> +      default:
>> +         /* should not get here */
>> +         abort();
>> +         break;
>> +   }
>> +}
>> +
>>  void master_sigill(int sig, siginfo_t *si, void *uc)
>>  {
>> -   switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
>> +   int r;
>> +
>> +   if (trace_file) {
>> +      r = send_register_info(write_trace, uc);
>> +   } else {
>> +      r = recv_and_compare_register_info(read_sock, respond_sock, uc);
>> +   }
>
> It's a bit weird that in the trace file case we send the data
> from the master, but in the normal case we send it from
> the apprentice end.

It seemed the natural way round (master generating the "gold" stream
that the apprentice checks). I could switch it or make the apprentice
responsible for checking it's own work?

>
>> +
>> +   switch (r)
>>     {
>>        case 0:
>>           /* match OK */
>> @@ -82,7 +118,15 @@ void master_sigill(int sig, siginfo_t *si, void *uc)
>>
>>  void apprentice_sigill(int sig, siginfo_t *si, void *uc)
>>  {
>> -   switch (send_register_info(write_sock, uc))
>> +   int r;
>> +
>> +   if (trace_file) {
>> +      r = recv_and_compare_register_info(read_trace, respond_trace, uc);
>> +   } else {
>> +      r = send_register_info(write_sock, uc);
>> +   }
>> +
>> +   switch (r)
>>     {
>>        case 0:
>>           /* match OK */
>> @@ -94,6 +138,9 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc)
>>           exit(0);
>>        default:
>>           /* mismatch */
>> +         if (trace_file) {
>> +            report_match_status();
>> +         }
>
> report_match_status() uses stdio, you can't call it from a signal
> handler.

Oops, I'll sigjmp.

>
>>           exit(1);
>>     }
>>  }
>> @@ -155,7 +202,13 @@ int master(int sock)
>>  {
>>     if (sigsetjmp(jmpbuf, 1))
>>     {
>> -      return report_match_status();
>> +      if (trace_file) {
>> +         close(trace_file);
>> +         fprintf(stderr,"\nDone...\n");
>> +         return 0;
>> +      } else {
>> +         return report_match_status();
>> +      }
>>     }
>>     master_socket = sock;
>>     set_sigill_handler(&master_sigill);
>> @@ -184,6 +237,7 @@ void usage (void)
>>     fprintf(stderr, "between master and apprentice risu processes.\n\n");
>>     fprintf(stderr, "Options:\n");
>>     fprintf(stderr, "  --master          Be the master (server)\n");
>> +   fprintf(stderr, "  -t, --trace=FILE  Record/playback trace file\n");
>>     fprintf(stderr, "  -h, --host=HOST   Specify master host machine (apprentice only)\n");
>>     fprintf(stderr, "  -p, --port=PORT   Specify the port to connect to/listen on (default 9191)\n");
>>  }
>> @@ -194,6 +248,7 @@ int main(int argc, char **argv)
>>     uint16_t port = 9191;
>>     char *hostname = "localhost";
>>     char *imgfile;
>> +   char *trace_fn = NULL;
>>     int sock;
>>
>>     // TODO clean this up later
>> @@ -204,13 +259,14 @@ int main(int argc, char **argv)
>>           {
>>              { "help", no_argument, 0, '?'},
>>              { "master", no_argument, &ismaster, 1 },
>> +            { "trace", required_argument, 0, 't' },
>>              { "host", required_argument, 0, 'h' },
>>              { "port", required_argument, 0, 'p' },
>>              { "test-fp-exc", no_argument, &test_fp_exc, 1 },
>>              { 0,0,0,0 }
>>           };
>>        int optidx = 0;
>> -      int c = getopt_long(argc, argv, "h:p:", longopts, &optidx);
>> +      int c = getopt_long(argc, argv, "h:p:t:", longopts, &optidx);
>>        if (c == -1)
>>        {
>>           break;
>> @@ -223,6 +279,11 @@ int main(int argc, char **argv)
>>              /* flag set by getopt_long, do nothing */
>>              break;
>>           }
>> +         case 't':
>> +         {
>> +           trace_fn = optarg;
>> +           break;
>> +         }
>>           case 'h':
>>           {
>>              hostname = optarg;
>> @@ -253,17 +314,27 @@ int main(int argc, char **argv)
>>     }
>>
>>     load_image(imgfile);
>> -
>> +
>>     if (ismaster)
>>     {
>> -      fprintf(stderr, "master port %d\n", port);
>> -      sock = master_connect(port);
>> +      if (trace_fn)
>> +      {
>> +         trace_file = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
>> +      } else {
>> +         fprintf(stderr, "master port %d\n", port);
>> +         sock = master_connect(port);
>> +      }
>>        return master(sock);
>
> Doesn't this call master() with an uninitialized sock if
> the trace file is in use ?

Hmm yes I can clean that up.

>
>>     }
>>     else
>>     {
>> -      fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
>> -      sock = apprentice_connect(hostname, port);
>> +      if (trace_fn)
>> +      {
>> +         trace_file = open(trace_fn, O_RDONLY);
>> +      } else {
>> +         fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
>> +         sock = apprentice_connect(hostname, port);
>> +      }
>>        return apprentice(sock);
>>     }
>>  }
>> --
>> 2.13.0
>>
>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [RISU PATCH v4 08/10] risu: add support compressed tracefiles

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
>> This uses the magic of zlib's gzread/write interface to wrap the
>> tracefile in compression. The code changes are tiny. I spent more time
>> messing about with the configure/linker stuff to auto-detect bits.
>>
>> As you need decent multi-arch support or a correctly setup cross
>> toolchain we fall back if we can't compile with zlib. This
>> unfortunately needs some #ifdef hackery around the zlib bits in
>> risu.c.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> --
>> v4
>>   - removed redundant config.h output, added HAVE_ZLIB
>>   - added BUILD_INC to deal with out-of-tree builds
>
> I thought the trace files were so enormous that zlib was
> basically mandatory for the record/replay to be useful?

It would still work. As mentioned on IRC we could look at streaming
through stdout if zlib is hard to do.

> I'm wondering if we should use a submodule for zlib and
> just build it locally. That would let us just make it a
> hard requirement (and avoid the need to do things with
> docker).

That would be another option. There doesn't seem to be a nice zlib
source repository and the license means having to mess about with
version markings if we import it into the tree.

>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [RISU PATCH v4 09/10] new: record_traces.sh helper script

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
>> A simple script to run through a bunch of binaries and generate their
>> trace files.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> ---
>> v3
>>   - allow overriding of RISU binary
>> ---
>>  record_traces.sh | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>  create mode 100755 record_traces.sh
>>
>> diff --git a/record_traces.sh b/record_traces.sh
>> new file mode 100755
>> index 0000000..78ce47f
>> --- /dev/null
>> +++ b/record_traces.sh
>> @@ -0,0 +1,20 @@
>> +#!/bin/sh
>> +#
>> +# Record traces
>> +#
>> +# A risu helper script to batch process a bunch of binaries and record their outputs
>> +#
>
> Can we have a license statement, copyright line and a
> usage summary, please? (Ditto patch 10). I'm not sure if
> these scripts are intended for testing purposes or as
> something that risu users might want to use -- if the
> latter then also documenting them in README would be
> a good idea.

They are more useful scripts I used for bulk generating stuff. Maybe I
should move them into contrib?

I'll update the usage and copyright lines.

>
>> +set -e
>> +
>> +if test -z "$RISU"; then
>> +    script_dir=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd -P)
>> +    RISU=${script_dir}/risu
>> +fi
>> +
>> +for f in $@; do
>> +    echo "Running risu against $f"
>> +    t="$f.trace"
>> +    ${RISU} --master $f -t $t
>> +    echo "Checking trace file OK"
>> +    ${RISU} $f -t $t
>> +done
>> --
>> 2.13.0
>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [RISU PATCH v4 07/10] risu: add simple trace and replay support

[Peter Maydell]

On 6 June 2017 at 15:19, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
>>> This adds a very dumb and easily breakable trace and replay support. In
>>> --master mode the various risu ops trigger a write of register/memory
>>> state into a binary file which can be played back to an apprentice.
>>> Currently there is no validation of the image source so feeding the
>>> wrong image will fail straight away.
>>>
>>> The trace files will get very big for any appreciable sized test file
>>> and this will be addressed in later patches.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>
>>> ---
>>> v4
>>>   - fix formatting mess
>>>   - abort() instead of reporting de-sync
>>>   - don't fake return for compiler
>>> v3
>>>   - fix options parsing
>>>   - re-factored so no need for copy & paste
>>> v2
>>>   - moved read/write functions into main risu.c
>>>   - cleaned up formatting
>>>   - report more in apprentice --trace mode
>>> ---
>>>  risu.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>>>  1 file changed, 84 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/risu.c b/risu.c
>>> index 22571cd..137cbbf 100644
>>> --- a/risu.c
>>> +++ b/risu.c
>>> @@ -31,6 +31,7 @@
>>>  void *memblock = 0;
>>>
>>>  int apprentice_socket, master_socket;
>>> +int trace_file = 0;
>>>
>>>  sigjmp_buf jmpbuf;
>>>
>>> @@ -40,10 +41,12 @@ int test_fp_exc = 0;
>>>  long executed_tests = 0;
>>>  void report_test_status(void *pc)
>>>  {
>>> -   executed_tests += 1;
>>> -   if (executed_tests % 100 == 0) {
>>> -      fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
>>> -              executed_tests, pc);
>>> +   if (ismaster || trace_file) {
>>> +      executed_tests += 1;
>>> +      if (executed_tests % 100 == 0) {
>>> +         fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
>>> +                 executed_tests, pc);
>>> +      }
>>>     }
>>>  }
>>>
>>> @@ -54,6 +57,12 @@ int read_sock(void *ptr, size_t bytes)
>>>     return recv_data_pkt(master_socket, ptr, bytes);
>>>  }
>>>
>>> +int write_trace(void *ptr, size_t bytes)
>>> +{
>>> +   size_t res = write(trace_file, ptr, bytes);
>>> +   return (res == bytes) ? 0 : 1;
>>> +}
>>> +
>>>  void respond_sock(int r)
>>>  {
>>>     send_response_byte(master_socket, r);
>>> @@ -66,9 +75,36 @@ int write_sock(void *ptr, size_t bytes)
>>>     return send_data_pkt(apprentice_socket, ptr, bytes);
>>>  }
>>>
>>> +int read_trace(void *ptr, size_t bytes)
>>> +{
>>> +   size_t res = read(trace_file, ptr, bytes);
>>> +   return (res == bytes) ? 0 : 1;
>>> +}
>>> +
>>> +void respond_trace(int r)
>>> +{
>>> +   switch (r) {
>>> +      case 0: /* test ok */
>>> +      case 1: /* end of test */
>>> +         break;
>>> +      default:
>>> +         /* should not get here */
>>> +         abort();
>>> +         break;
>>> +   }
>>> +}
>>> +
>>>  void master_sigill(int sig, siginfo_t *si, void *uc)
>>>  {
>>> -   switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
>>> +   int r;
>>> +
>>> +   if (trace_file) {
>>> +      r = send_register_info(write_trace, uc);
>>> +   } else {
>>> +      r = recv_and_compare_register_info(read_sock, respond_sock, uc);
>>> +   }
>>
>> It's a bit weird that in the trace file case we send the data
>> from the master, but in the normal case we send it from
>> the apprentice end.
>
> It seemed the natural way round (master generating the "gold" stream
> that the apprentice checks). I could switch it or make the apprentice
> responsible for checking it's own work?

I guess it's not worth switching unless it causes the code to
neatly simplify because you don't need the if when both branches
of it are calling the same function...

thanks
-- PMM