All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches
@ 2017-06-19 10:46 Alex Bennée
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 01/13] .gitignore: ignore build directories Alex Bennée
                   ` (12 more replies)
  0 siblings, 13 replies; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, Alex Bennée

Hi,

Here is v5 of the RISU record/replay patches. The big changes are:

  - new patch to sync coding standards
  - handle stdin/out as a trace input/destination
  - remove fprintf's from signal handlers
  - swapped the static build flag with docker

I've left the "build with docker" patches in but they can be easily
dropped if you want as the --static build patch goes in first. I still
think it is a handy solution if the users host cross-arch setup is
broken.

The alternative is to use the stdin/out pipeline and pass trace data
through a (potentially native) compression binary.

Details in the commits.

Alex Bennée (13):
  .gitignore: ignore build directories
  README: document the coding style used for risu
  all: fix up code consitency
  build-all-archs: support --static flag
  build-all-archs: support cross building via docker
  risu: a bit more verbosity when starting
  risu: paramterise send/receive functions
  risu: add header to trace stream
  risu: add simple trace and replay support
  risu: handle trace through stdin/stdout
  risu: add support compressed tracefiles
  new: record_traces.sh helper script
  new: run_risu.sh script

 .dir-locals.el         |   2 +
 .gitignore             |   1 +
 Makefile               |   4 +-
 README                 |   9 +
 build-all-archs        |  76 +++++++-
 comms.c                | 325 ++++++++++++++++------------------
 configure              |  55 +++++-
 record_traces.sh       |  32 ++++
 reginfo.c              | 189 +++++++++++---------
 risu.c                 | 465 ++++++++++++++++++++++++++++++++-----------------
 risu.h                 |  27 ++-
 risu_aarch64.c         |   5 +
 risu_arm.c             |  63 +++----
 risu_i386.c            | 150 ++++++++--------
 risu_m68k.c            |   7 +-
 risu_ppc64.c           |   7 +-
 risu_reginfo_aarch64.c |  60 ++++---
 risu_reginfo_aarch64.h |   3 +-
 risu_reginfo_arm.c     | 290 +++++++++++++++---------------
 risu_reginfo_arm.h     |   3 +-
 risu_reginfo_m68k.c    |  20 +--
 risu_reginfo_m68k.h    |   3 +-
 risu_reginfo_ppc64.c   |  29 ++-
 risu_reginfo_ppc64.h   |   3 +-
 run_risu.sh            |  66 +++++++
 25 files changed, 1150 insertions(+), 744 deletions(-)
 create mode 100644 .dir-locals.el
 create mode 100755 record_traces.sh
 create mode 100755 run_risu.sh

-- 
2.13.0

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 01/13] .gitignore: ignore build directories
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 02/13] README: document the coding style used for risu Alex Bennée
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, Alex Bennée

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

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 02/13] README: document the coding style used for risu
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 01/13] .gitignore: ignore build directories Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 12:54   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 03/13] all: fix up code consitency Alex Bennée
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, Alex Bennée

We also include the an Emacs .dir-locals (as per QEMU) that enforces
this layout.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .dir-locals.el | 2 ++
 README         | 9 +++++++++
 2 files changed, 11 insertions(+)
 create mode 100644 .dir-locals.el

diff --git a/.dir-locals.el b/.dir-locals.el
new file mode 100644
index 0000000..3ac0cfc
--- /dev/null
+++ b/.dir-locals.el
@@ -0,0 +1,2 @@
+((c-mode . ((c-file-style . "stroustrup")
+	    (indent-tabs-mode . nil))))
diff --git a/README b/README
index 4b37b4e..858a349 100644
--- a/README
+++ b/README
@@ -42,6 +42,15 @@ architecture that we support and that you have a cross compiler
 installed for. This is useful for confirming that your changes
 to risu haven't broken anything.
 
+Coding Style
+------------
+
+risu follows the same coding style as the QEMU project, namely 4
+spaces (no tabs) for indentation and the One True Brace Style variant
+of K&R. The source tree includes a .dir-locals.el for Emacs users that
+will set this automatically. Other editors are available.
+
+
 Usage
 -----
 
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 03/13] all: fix up code consitency
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 01/13] .gitignore: ignore build directories Alex Bennée
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 02/13] README: document the coding style used for risu Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 13:13   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 04/13] build-all-archs: support --static flag Alex Bennée
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, Alex Bennée

This is pretty much a mechanical change where I ran:

  indent -kr

Across all the files and then fixed up all but a few violations of:

  ../../qemu.git/scripts/checkpatch.pl -f *.c *.h > checkpatch.out

Along with heavy use of M-x untabify to make everything consistent.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 comms.c                | 325 +++++++++++++++++++++++--------------------------
 reginfo.c              | 114 ++++++++---------
 risu.c                 | 310 +++++++++++++++++++++++-----------------------
 risu.h                 |   4 +-
 risu_arm.c             |  58 +++++----
 risu_i386.c            | 150 +++++++++++------------
 risu_m68k.c            |   2 +-
 risu_ppc64.c           |   2 +-
 risu_reginfo_aarch64.c |  60 +++++----
 risu_reginfo_aarch64.h |   3 +-
 risu_reginfo_arm.c     | 290 +++++++++++++++++++++----------------------
 risu_reginfo_arm.h     |   3 +-
 risu_reginfo_m68k.c    |  20 +--
 risu_reginfo_m68k.h    |   3 +-
 risu_reginfo_ppc64.c   |  29 ++---
 risu_reginfo_ppc64.h   |   3 +-
 16 files changed, 672 insertions(+), 704 deletions(-)

diff --git a/comms.c b/comms.c
index 31be846..2900c33 100644
--- a/comms.c
+++ b/comms.c
@@ -7,7 +7,7 @@
  *
  * Contributors:
  *     Peter Maydell (Linaro) - initial implementation
- *******************************************************************************/
+ ******************************************************************************/
 
 /* Routines for the socket communication between master and apprentice. */
 
@@ -24,76 +24,70 @@
 
 int apprentice_connect(const char *hostname, int port)
 {
-   /* We are the client end of the TCP connection */
-   int sock;
-   struct sockaddr_in sa;
-   sock = socket(PF_INET, SOCK_STREAM, 0);
-   if (sock < 0)
-   {
-      perror("socket");
-      exit(1);
-   }
-   struct hostent *hostinfo;
-   sa.sin_family = AF_INET;
-   sa.sin_port = htons(port);
-   hostinfo = gethostbyname(hostname);
-   if (!hostinfo)
-   {
-      fprintf(stderr, "Unknown host %s\n", hostname);
-      exit(1);
-   }
-   sa.sin_addr = *(struct in_addr*)hostinfo->h_addr;
-   if (connect(sock, (struct sockaddr*)&sa, sizeof(sa)) < 0)
-   {
-      perror("connect");
-      exit(1);
-   }
-   return sock;
+    /* We are the client end of the TCP connection */
+    int sock;
+    struct sockaddr_in sa;
+    sock = socket(PF_INET, SOCK_STREAM, 0);
+    if (sock < 0) {
+        perror("socket");
+        exit(1);
+    }
+    struct hostent *hostinfo;
+    sa.sin_family = AF_INET;
+    sa.sin_port = htons(port);
+    hostinfo = gethostbyname(hostname);
+    if (!hostinfo) {
+        fprintf(stderr, "Unknown host %s\n", hostname);
+        exit(1);
+    }
+    sa.sin_addr = *(struct in_addr *) hostinfo->h_addr;
+    if (connect(sock, (struct sockaddr *) &sa, sizeof(sa)) < 0) {
+        perror("connect");
+        exit(1);
+    }
+    return sock;
 }
 
 int master_connect(int port)
 {
-   int sock;
-   struct sockaddr_in sa;
-   sock = socket(PF_INET, SOCK_STREAM, 0);
-   if (sock < 0)
-   {
-      perror("socket");
-      exit(1);
-   }
-   int sora = 1;
-   if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &sora, sizeof(sora)) != 0)
-   {
-      perror("setsockopt(SO_REUSEADDR)");
-      exit(1);
-   }
+    int sock;
+    struct sockaddr_in sa;
+    sock = socket(PF_INET, SOCK_STREAM, 0);
+    if (sock < 0) {
+        perror("socket");
+        exit(1);
+    }
+    int sora = 1;
+    if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &sora, sizeof(sora)) !=
+        0) {
+        perror("setsockopt(SO_REUSEADDR)");
+        exit(1);
+    }
 
-   sa.sin_family = AF_INET;
-   sa.sin_port = htons(port);
-   sa.sin_addr.s_addr = htonl(INADDR_ANY);
-   if (bind(sock, (struct sockaddr*)&sa, sizeof(sa)) < 0)
-   {
-      perror("bind");
-      exit(1);
-   }
-   if (listen(sock, 1) < 0)
-   {
-      perror("listen");
-      exit(1);
-   }
-   /* Just block until we get a connection */
-   fprintf(stderr, "master: waiting for connection on port %d...\n", port);
-   struct sockaddr_in csa;
-   socklen_t csasz = sizeof(csa);
-   int nsock = accept(sock, (struct sockaddr*)&csa, &csasz);
-   if (nsock < 0)
-   {
-      perror("accept");
-      exit(1);
-   }
-   /* We're done with the server socket now */
-   close(sock);
-   return nsock;
+    sa.sin_family = AF_INET;
+    sa.sin_port = htons(port);
+    sa.sin_addr.s_addr = htonl(INADDR_ANY);
+    if (bind(sock, (struct sockaddr *) &sa, sizeof(sa)) < 0) {
+        perror("bind");
+        exit(1);
+    }
+    if (listen(sock, 1) < 0) {
+        perror("listen");
+        exit(1);
+    }
+    /* Just block until we get a connection */
+    fprintf(stderr, "master: waiting for connection on port %d...\n",
+            port);
+    struct sockaddr_in csa;
+    socklen_t csasz = sizeof(csa);
+    int nsock = accept(sock, (struct sockaddr *) &csa, &csasz);
+    if (nsock < 0) {
+        perror("accept");
+        exit(1);
+    }
+    /* We're done with the server socket now */
+    close(sock);
+    return nsock;
 }
 
 /* Utility functions which are just wrappers around read and writev
@@ -101,77 +95,68 @@ int master_connect(int port)
  */
 static void recv_bytes(int sock, void *pkt, int pktlen)
 {
-   char *p = pkt;
-   while (pktlen)
-   {
-      int i = read(sock, p, pktlen);
-      if (i <= 0)
-      {
-         if (errno == EINTR)
-         {
-            continue;
-         }
-         perror("read failed");
-         exit(1);
-      }
-      pktlen -= i;
-      p += i;
-   }
+    char *p = pkt;
+    while (pktlen) {
+        int i = read(sock, p, pktlen);
+        if (i <= 0) {
+            if (errno == EINTR) {
+                continue;
+            }
+            perror("read failed");
+            exit(1);
+        }
+        pktlen -= i;
+        p += i;
+    }
 }
 
 static void recv_and_discard_bytes(int sock, int pktlen)
 {
-   /* Read and discard bytes */
-   char dumpbuf[64];
-   while (pktlen)
-   {
-      int i;
-      int len = sizeof(dumpbuf);
-      if (len > pktlen)
-      {
-         len = pktlen;
-      }
-      i = read(sock, dumpbuf, len);
-      if (i <= 0)
-      {
-         if (errno == EINTR)
-         {
-            continue;
-         }
-         perror("read failed");
-         exit(1);
-      }
-      pktlen -= i;
-   }
+    /* Read and discard bytes */
+    char dumpbuf[64];
+    while (pktlen) {
+        int i;
+        int len = sizeof(dumpbuf);
+        if (len > pktlen) {
+            len = pktlen;
+        }
+        i = read(sock, dumpbuf, len);
+        if (i <= 0) {
+            if (errno == EINTR) {
+                continue;
+            }
+            perror("read failed");
+            exit(1);
+        }
+        pktlen -= i;
+    }
 }
 
 ssize_t safe_writev(int fd, struct iovec *iov_in, int iovcnt)
 {
-   /* writev, retrying for EINTR and short writes */
-   int r = 0;
-   struct iovec *iov = iov_in;
-   for (;;)
-   {
-      ssize_t i = writev(fd, iov, iovcnt);
-      if (i == -1)
-      {
-         if (errno == EINTR)
-            continue;
-         return -1;
-      }
-      r += i;
-      /* Move forward through iov to account for data transferred */
-      while (i >= iov->iov_len)
-      {
-         i -= iov->iov_len;
-         iov++;
-         iovcnt--;
-         if (iovcnt == 0) {
-            return r;
-         }
-      }
-      iov->iov_len -= i;
-   }
+    /* writev, retrying for EINTR and short writes */
+    int r = 0;
+    struct iovec *iov = iov_in;
+    for (;;) {
+        ssize_t i = writev(fd, iov, iovcnt);
+        if (i == -1) {
+            if (errno == EINTR) {
+                continue;
+            }
+            return -1;
+        }
+        r += i;
+        /* Move forward through iov to account for data transferred */
+        while (i >= iov->iov_len) {
+            i -= iov->iov_len;
+            iov++;
+            iovcnt--;
+            if (iovcnt == 0) {
+                return r;
+            }
+        }
+        iov->iov_len -= i;
+    }
 }
 
 /* Low level comms routines:
@@ -184,57 +169,53 @@ ssize_t safe_writev(int fd, struct iovec *iov_in, int iovcnt)
  */
 int send_data_pkt(int sock, void *pkt, int pktlen)
 {
-   unsigned char resp;
-   /* First we send the packet length as a network-order 32 bit value.
-    * This avoids silent deadlocks if the two sides disagree over
-    * what size data packet they are transferring. We use writev()
-    * so that both length and packet are sent in one packet; otherwise
-    * we get 300x slowdown because we hit Nagle's algorithm.
-    */
-   uint32_t net_pktlen = htonl(pktlen);
-   struct iovec iov[2];
-   iov[0].iov_base = &net_pktlen;
-   iov[0].iov_len = sizeof(net_pktlen);
-   iov[1].iov_base = pkt;
-   iov[1].iov_len = pktlen;
+    unsigned char resp;
+    /* First we send the packet length as a network-order 32 bit value.
+     * This avoids silent deadlocks if the two sides disagree over
+     * what size data packet they are transferring. We use writev()
+     * so that both length and packet are sent in one packet; otherwise
+     * we get 300x slowdown because we hit Nagle's algorithm.
+     */
+    uint32_t net_pktlen = htonl(pktlen);
+    struct iovec iov[2];
+    iov[0].iov_base = &net_pktlen;
+    iov[0].iov_len = sizeof(net_pktlen);
+    iov[1].iov_base = pkt;
+    iov[1].iov_len = pktlen;
 
-   if (safe_writev(sock, iov, 2) == -1)
-   {
-      perror("writev failed");
-      exit(1);
-   }
+    if (safe_writev(sock, iov, 2) == -1) {
+        perror("writev failed");
+        exit(1);
+    }
 
-   if (read(sock, &resp, 1) != 1)
-   {
-      perror("read failed");
-      exit(1);
-   }
-   return resp;
+    if (read(sock, &resp, 1) != 1) {
+        perror("read failed");
+        exit(1);
+    }
+    return resp;
 }
 
 int recv_data_pkt(int sock, void *pkt, int pktlen)
 {
-   uint32_t net_pktlen;
-   recv_bytes(sock, &net_pktlen, sizeof(net_pktlen));
-   net_pktlen = ntohl(net_pktlen);
-   if (pktlen != net_pktlen)
-   {
-      /* Mismatch. Read the data anyway so we can send
-       * a response back.
-       */
-      recv_and_discard_bytes(sock, net_pktlen);
-      return 1;
-   }
-   recv_bytes(sock, pkt, pktlen);
-   return 0;
+    uint32_t net_pktlen;
+    recv_bytes(sock, &net_pktlen, sizeof(net_pktlen));
+    net_pktlen = ntohl(net_pktlen);
+    if (pktlen != net_pktlen) {
+        /* Mismatch. Read the data anyway so we can send
+         * a response back.
+         */
+        recv_and_discard_bytes(sock, net_pktlen);
+        return 1;
+    }
+    recv_bytes(sock, pkt, pktlen);
+    return 0;
 }
 
 void send_response_byte(int sock, int resp)
 {
-   unsigned char r = resp;
-   if (write(sock, &r, 1) != 1)
-   {
-      perror("write failed");
-      exit(1);
-   }
+    unsigned char r = resp;
+    if (write(sock, &r, 1) != 1) {
+        perror("write failed");
+        exit(1);
+    }
 }
diff --git a/reginfo.c b/reginfo.c
index 96c6342..31bb99f 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -18,8 +18,8 @@ struct reginfo master_ri, apprentice_ri;
 
 uint8_t apprentice_memblock[MEMBLOCKLEN];
 
-static int mem_used = 0;
-static int packet_mismatch = 0;
+static int mem_used;
+static int packet_mismatch;
 
 int send_register_info(int sock, void *uc)
 {
@@ -37,11 +37,12 @@ int send_register_info(int sock, void *uc)
          */
         return send_data_pkt(sock, &ri, sizeof(ri));
     case OP_SETMEMBLOCK:
-        memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
-       break;
+        memblock = (void *) (uintptr_t) get_reginfo_paramreg(&ri);
+        break;
     case OP_GETMEMBLOCK:
         set_ucontext_paramreg(uc,
-                              get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
+                              get_reginfo_paramreg(&ri) +
+                              (uintptr_t) memblock);
         break;
     case OP_COMPAREMEM:
         return send_data_pkt(sock, memblock, MEMBLOCKLEN);
@@ -85,25 +86,25 @@ int recv_and_compare_register_info(int sock, void *uc)
         }
         send_response_byte(sock, 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 (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
-             packet_mismatch = 1;
-             resp = 2;
-         } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
-             /* memory mismatch */
-             resp = 2;
-         }
-         send_response_byte(sock, 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 (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
+            packet_mismatch = 1;
+            resp = 2;
+        } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
+            /* memory mismatch */
+            resp = 2;
+        }
+        send_response_byte(sock, resp);
+        break;
+    }
 
     return resp;
 }
@@ -116,36 +117,37 @@ int recv_and_compare_register_info(int sock, void *uc)
  */
 int report_match_status(void)
 {
-   int resp = 0;
-   fprintf(stderr, "match status...\n");
-   if (packet_mismatch) {
-       fprintf(stderr, "packet mismatch (probably disagreement "
-               "about UNDEF on load/store)\n");
-       /* We don't have valid reginfo from the apprentice side
-        * so stop now rather than printing anything about it.
-        */
-       fprintf(stderr, "master reginfo:\n");
-       reginfo_dump(&master_ri, stderr);
-       return 1;
-   }
-   if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
-       fprintf(stderr, "mismatch on regs!\n");
-       resp = 1;
-   }
-   if (mem_used && memcmp(memblock, &apprentice_memblock, MEMBLOCKLEN) != 0) {
-       fprintf(stderr, "mismatch on memory!\n");
-       resp = 1;
-   }
-   if (!resp) {
-       fprintf(stderr, "match!\n");
-       return 0;
-   }
-
-   fprintf(stderr, "master reginfo:\n");
-   reginfo_dump(&master_ri, stderr);
-   fprintf(stderr, "apprentice reginfo:\n");
-   reginfo_dump(&apprentice_ri, stderr);
-
-   reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
-   return resp;
+    int resp = 0;
+    fprintf(stderr, "match status...\n");
+    if (packet_mismatch) {
+        fprintf(stderr, "packet mismatch (probably disagreement "
+                "about UNDEF on load/store)\n");
+        /* We don't have valid reginfo from the apprentice side
+         * so stop now rather than printing anything about it.
+         */
+        fprintf(stderr, "master reginfo:\n");
+        reginfo_dump(&master_ri, stderr);
+        return 1;
+    }
+    if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
+        fprintf(stderr, "mismatch on regs!\n");
+        resp = 1;
+    }
+    if (mem_used
+        && memcmp(memblock, &apprentice_memblock, MEMBLOCKLEN) != 0) {
+        fprintf(stderr, "mismatch on memory!\n");
+        resp = 1;
+    }
+    if (!resp) {
+        fprintf(stderr, "match!\n");
+        return 0;
+    }
+
+    fprintf(stderr, "master reginfo:\n");
+    reginfo_dump(&master_ri, stderr);
+    fprintf(stderr, "apprentice reginfo:\n");
+    reginfo_dump(&apprentice_ri, stderr);
+
+    reginfo_dump_mismatch(&master_ri, &apprentice_ri, stderr);
+    return resp;
 }
diff --git a/risu.c b/risu.c
index 7e42160..2cd6d22 100644
--- a/risu.c
+++ b/risu.c
@@ -28,59 +28,56 @@
 
 #include "risu.h"
 
-void *memblock = 0;
+void *memblock;
 
 int apprentice_socket, master_socket;
 
 sigjmp_buf jmpbuf;
 
 /* Should we test for FP exception status bits? */
-int test_fp_exc = 0;
+int test_fp_exc;
 
 void master_sigill(int sig, siginfo_t *si, void *uc)
 {
-   switch (recv_and_compare_register_info(master_socket, uc))
-   {
-      case 0:
-         /* match OK */
-         advance_pc(uc);
-         return;
-      default:
-         /* mismatch, or end of test */
-         siglongjmp(jmpbuf, 1);
-   }
+    switch (recv_and_compare_register_info(master_socket, uc)) {
+    case 0:
+        /* match OK */
+        advance_pc(uc);
+        return;
+    default:
+        /* mismatch, or end of test */
+        siglongjmp(jmpbuf, 1);
+    }
 }
 
 void apprentice_sigill(int sig, siginfo_t *si, void *uc)
 {
-   switch (send_register_info(apprentice_socket, uc))
-   {
-      case 0:
-         /* match OK */
-         advance_pc(uc);
-         return;
-      case 1:
-         /* end of test */
-         exit(0);
-      default:
-         /* mismatch */
-         exit(1);
-   }
+    switch (send_register_info(apprentice_socket, uc)) {
+    case 0:
+        /* match OK */
+        advance_pc(uc);
+        return;
+    case 1:
+        /* end of test */
+        exit(0);
+    default:
+        /* mismatch */
+        exit(1);
+    }
 }
 
-static void set_sigill_handler(void (*fn)(int, siginfo_t *, void *))
+static void set_sigill_handler(void (*fn) (int, siginfo_t *, void *))
 {
-   struct sigaction sa;
-   memset(&sa, 0, sizeof(struct sigaction));
-
-   sa.sa_sigaction = fn;
-   sa.sa_flags = SA_SIGINFO;
-   sigemptyset(&sa.sa_mask);
-   if (sigaction(SIGILL, &sa, 0) != 0)
-   {
-      perror("sigaction");
-      exit(1);
-   }
+    struct sigaction sa;
+    memset(&sa, 0, sizeof(struct sigaction));
+
+    sa.sa_sigaction = fn;
+    sa.sa_flags = SA_SIGINFO;
+    sigemptyset(&sa.sa_mask);
+    if (sigaction(SIGILL, &sa, 0) != 0) {
+        perror("sigaction");
+        exit(1);
+    }
 }
 
 typedef void entrypoint_fn(void);
@@ -90,152 +87,147 @@ entrypoint_fn *image_start;
 
 void load_image(const char *imgfile)
 {
-   /* Load image file into memory as executable */
-   struct stat st;
-   fprintf(stderr, "loading test image %s...\n", imgfile);
-   int fd = open(imgfile, O_RDONLY);
-   if (fd < 0)
-   {
-      fprintf(stderr, "failed to open image file %s\n", imgfile);
-      exit(1);
-   }
-   if (fstat(fd, &st) != 0)
-   {
-      perror("fstat");
-      exit(1);
-   }
-   size_t len = st.st_size;
-   void *addr;
-
-   /* Map writable because we include the memory area for store
-    * testing in the image.
-    */
-   addr = mmap(0, len, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE, fd, 0);
-   if (!addr)
-   {
-      perror("mmap");
-      exit(1);
-   }
-   close(fd);
-   image_start = addr;
-   image_start_address = (uintptr_t)addr;
+    /* Load image file into memory as executable */
+    struct stat st;
+    fprintf(stderr, "loading test image %s...\n", imgfile);
+    int fd = open(imgfile, O_RDONLY);
+    if (fd < 0) {
+        fprintf(stderr, "failed to open image file %s\n", imgfile);
+        exit(1);
+    }
+    if (fstat(fd, &st) != 0) {
+        perror("fstat");
+        exit(1);
+    }
+    size_t len = st.st_size;
+    void *addr;
+
+    /* Map writable because we include the memory area for store
+     * testing in the image.
+     */
+    addr =
+        mmap(0, len, PROT_READ | PROT_WRITE | PROT_EXEC, MAP_PRIVATE, fd,
+             0);
+    if (!addr) {
+        perror("mmap");
+        exit(1);
+    }
+    close(fd);
+    image_start = addr;
+    image_start_address = (uintptr_t) addr;
 }
 
 int master(int sock)
 {
-   if (sigsetjmp(jmpbuf, 1))
-   {
-      return report_match_status();
-   }
-   master_socket = sock;
-   set_sigill_handler(&master_sigill);
-   fprintf(stderr, "starting image\n");
-   image_start();
-   fprintf(stderr, "image returned unexpectedly\n");
-   exit(1);
+    if (sigsetjmp(jmpbuf, 1)) {
+        return report_match_status();
+    }
+    master_socket = sock;
+    set_sigill_handler(&master_sigill);
+    fprintf(stderr, "starting image\n");
+    image_start();
+    fprintf(stderr, "image returned unexpectedly\n");
+    exit(1);
 }
 
 int apprentice(int sock)
 {
-   apprentice_socket = sock;
-   set_sigill_handler(&apprentice_sigill);
-   fprintf(stderr, "starting image\n");
-   image_start();
-   fprintf(stderr, "image returned unexpectedly\n");
-   exit(1);
+    apprentice_socket = sock;
+    set_sigill_handler(&apprentice_sigill);
+    fprintf(stderr, "starting image\n");
+    image_start();
+    fprintf(stderr, "image returned unexpectedly\n");
+    exit(1);
 }
 
 int ismaster;
 
-void usage (void)
+void usage(void)
 {
-   fprintf(stderr, "Usage: risu [--master] [--host <ip>] [--port <port>] <image file>\n\n");
-   fprintf(stderr, "Run through the pattern file verifying each instruction\n");
-   fprintf(stderr, "between master and apprentice risu processes.\n\n");
-   fprintf(stderr, "Options:\n");
-   fprintf(stderr, "  --master          Be the master (server)\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");
+    fprintf(stderr,
+            "Usage: risu [--master] [--host <ip>] [--port <port>] <image file>"
+            "\n\n");
+    fprintf(stderr,
+            "Run through the pattern file verifying each instruction\n");
+    fprintf(stderr, "between master and apprentice risu processes.\n\n");
+    fprintf(stderr, "Options:\n");
+    fprintf(stderr, "  --master          Be the master (server)\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");
 }
 
 int main(int argc, char **argv)
 {
-   // some handy defaults to make testing easier
-   uint16_t port = 9191;
-   char *hostname = "localhost";
-   char *imgfile;
-   int sock;
-
-   // TODO clean this up later
-   
-   for (;;)
-   {
-      static struct option longopts[] = 
-         {
-            { "help", no_argument, 0, '?'},
-            { "master", no_argument, &ismaster, 1 },
-            { "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);
-      if (c == -1)
-      {
-         break;
-      }
-      
-      switch (c)
-      {
-         case 0:
-         {
+    /* some handy defaults to make testing easier */
+    uint16_t port = 9191;
+    char *hostname = "localhost";
+    char *imgfile;
+    int sock;
+
+    /* TODO clean this up later */
+
+    for (;;) {
+        static struct option longopts[] = {
+            {"help", no_argument, 0, '?'},
+            {"master", no_argument, &ismaster, 1},
+            {"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);
+        if (c == -1) {
+            break;
+        }
+
+        switch (c) {
+        case 0:
+        {
             /* flag set by getopt_long, do nothing */
             break;
-         }
-         case 'h':
-         {
+        }
+        case 'h':
+        {
             hostname = optarg;
             break;
-         }
-         case 'p':
-         {
-            // FIXME err handling
+        }
+        case 'p':
+        {
+            /* FIXME err handling */
             port = strtol(optarg, 0, 10);
             break;
-         }
-         case '?':
-         {
+        }
+        case '?':
+        {
             usage();
             exit(1);
-         }
-         default:
+        }
+        default:
             abort();
-      }
-   }
-
-   imgfile = argv[optind];
-   if (!imgfile)
-   {
-      fprintf(stderr, "Error: must specify image file name\n\n");
-      usage();
-      exit(1);
-   }
-
-   load_image(imgfile);
-   
-   if (ismaster)
-   {
-      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);
-      return apprentice(sock);
-   }
+        }
+    }
+
+    imgfile = argv[optind];
+    if (!imgfile) {
+        fprintf(stderr, "Error: must specify image file name\n\n");
+        usage();
+        exit(1);
+    }
+
+    load_image(imgfile);
+
+    if (ismaster) {
+        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);
+        return apprentice(sock);
+    }
 }
-
-   
diff --git a/risu.h b/risu.h
index 883bcf7..3fbeda8 100644
--- a/risu.h
+++ b/risu.h
@@ -7,7 +7,7 @@
  *
  * Contributors:
  *     Peter Maydell (Linaro) - initial implementation
- *******************************************************************************/
+ ******************************************************************************/
 
 #ifndef RISU_H
 #define RISU_H
@@ -99,7 +99,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc);
 int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2);
 
 /* print reginfo state to a stream, returns 1 on success, 0 on failure */
-int reginfo_dump(struct reginfo *ri, FILE *f);
+int reginfo_dump(struct reginfo *ri, FILE * f);
 
 /* reginfo_dump_mismatch: print mismatch details to a stream, ret nonzero=ok */
 int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f);
diff --git a/risu_arm.c b/risu_arm.c
index f570828..a55c6c2 100644
--- a/risu_arm.c
+++ b/risu_arm.c
@@ -7,7 +7,7 @@
  *
  * Contributors:
  *     Peter Maydell (Linaro) - initial implementation
- *******************************************************************************/
+ ******************************************************************************/
 
 #include <stdio.h>
 #include <ucontext.h>
@@ -18,39 +18,37 @@
 
 int insnsize(ucontext_t *uc)
 {
-   /* Return instruction size in bytes of the
-    * instruction at PC
-    */
-   if (uc->uc_mcontext.arm_cpsr & 0x20) 
-   {
-      uint16_t faulting_insn = *((uint16_t*)uc->uc_mcontext.arm_pc);
-      switch (faulting_insn & 0xF800)
-      {
-         case 0xE800:
-         case 0xF000:
-         case 0xF800:
+    /* Return instruction size in bytes of the
+     * instruction at PC
+     */
+    if (uc->uc_mcontext.arm_cpsr & 0x20) {
+        uint16_t faulting_insn = *((uint16_t *) uc->uc_mcontext.arm_pc);
+        switch (faulting_insn & 0xF800) {
+        case 0xE800:
+        case 0xF000:
+        case 0xF800:
             /* 32 bit Thumb2 instruction */
             return 4;
-         default:
+        default:
             /* 16 bit Thumb instruction */
             return 2;
-      }
-   }
-   /* ARM instruction */
-   return 4;
+        }
+    }
+    /* ARM instruction */
+    return 4;
 }
 
 void advance_pc(void *vuc)
 {
-   ucontext_t *uc = vuc;
-   uc->uc_mcontext.arm_pc += insnsize(uc);
+    ucontext_t *uc = vuc;
+    uc->uc_mcontext.arm_pc += insnsize(uc);
 }
 
 
 void set_ucontext_paramreg(void *vuc, uint64_t value)
 {
-   ucontext_t *uc = vuc;
-   uc->uc_mcontext.arm_r0 = value;
+    ucontext_t *uc = vuc;
+    uc->uc_mcontext.arm_r0 = value;
 }
 
 uint64_t get_reginfo_paramreg(struct reginfo *ri)
@@ -60,13 +58,13 @@ uint64_t get_reginfo_paramreg(struct reginfo *ri)
 
 int get_risuop(struct reginfo *ri)
 {
-   /* Return the risuop we have been asked to do
-    * (or -1 if this was a SIGILL for a non-risuop insn)
-    */
-   uint32_t insn = ri->faulting_insn;
-   int isz = ri->faulting_insn_size;
-   uint32_t op = insn & 0xf;
-   uint32_t key = insn & ~0xf;
-   uint32_t risukey = (isz == 2) ? 0xdee0 : 0xe7fe5af0;
-   return (key != risukey) ? -1 : op;
+    /* Return the risuop we have been asked to do
+     * (or -1 if this was a SIGILL for a non-risuop insn)
+     */
+    uint32_t insn = ri->faulting_insn;
+    int isz = ri->faulting_insn_size;
+    uint32_t op = insn & 0xf;
+    uint32_t key = insn & ~0xf;
+    uint32_t risukey = (isz == 2) ? 0xdee0 : 0xe7fe5af0;
+    return (key != risukey) ? -1 : op;
 }
diff --git a/risu_i386.c b/risu_i386.c
index bfcb4e1..5e7e01d 100644
--- a/risu_i386.c
+++ b/risu_i386.c
@@ -7,7 +7,7 @@
  *
  * Contributors:
  *     Peter Maydell (Linaro) - initial implementation
- *******************************************************************************/
+ ******************************************************************************/
 
 #include <stdio.h>
 #include <ucontext.h>
@@ -19,10 +19,9 @@
  * It is a simplified and reduced subset of what can
  * be obtained with a ucontext_t*
  */
-struct reginfo
-{
-      uint32_t faulting_insn;
-      gregset_t gregs;
+struct reginfo {
+    uint32_t faulting_insn;
+    gregset_t gregs;
 };
 
 #ifndef REG_GS
@@ -42,34 +41,32 @@ struct reginfo master_ri, apprentice_ri;
 
 static int insn_is_ud2(uint32_t insn)
 {
-   return ((insn & 0xffff) == 0x0b0f);
+    return ((insn & 0xffff) == 0x0b0f);
 }
 
 void advance_pc(void *vuc)
 {
-   /* We assume that this is either UD1 or UD2.
-    * This would need tweaking if we want to test
-    * expected undefs on x86.
-    */
-   ucontext_t *uc = vuc;
-   uc->uc_mcontext.gregs[REG_EIP] += 2;
+    /* We assume that this is either UD1 or UD2.
+     * This would need tweaking if we want to test
+     * expected undefs on x86.
+     */
+    ucontext_t *uc = vuc;
+    uc->uc_mcontext.gregs[REG_EIP] += 2;
 }
 
-static void fill_reginfo(struct reginfo *ri, ucontext_t *uc)
+static void fill_reginfo(struct reginfo *ri, ucontext_t * uc)
 {
-   int i;
-   for (i = 0; i < NGREG; i++)
-   {
-      switch(i)
-      {
-         case REG_ESP:
-         case REG_UESP:
-         case REG_GS:
-         case REG_FS:
-         case REG_ES:
-         case REG_DS:
-         case REG_TRAPNO:
-         case REG_EFL:
+    int i;
+    for (i = 0; i < NGREG; i++) {
+        switch (i) {
+        case REG_ESP:
+        case REG_UESP:
+        case REG_GS:
+        case REG_FS:
+        case REG_ES:
+        case REG_DS:
+        case REG_TRAPNO:
+        case REG_EFL:
             /* Don't store these registers as it results in mismatches.
              * In particular valgrind has different values for some
              * segment registers, and they're boring anyway.
@@ -78,27 +75,27 @@ static void fill_reginfo(struct reginfo *ri, ucontext_t *uc)
              */
             ri->gregs[i] = 0xDEADBEEF;
             break;
-         case REG_EIP:
+        case REG_EIP:
             /* Store the offset from the start of the test image */
             ri->gregs[i] = uc->uc_mcontext.gregs[i] - image_start_address;
             break;
-         default:
+        default:
             ri->gregs[i] = uc->uc_mcontext.gregs[i];
             break;
-      }
-   }
-   /* x86 insns aren't 32 bit but we're not really testing x86 so
-    * this is just to distinguish 'do compare' from 'stop'
-    */
-   ri->faulting_insn = *((uint32_t*)uc->uc_mcontext.gregs[REG_EIP]);
+        }
+    }
+    /* x86 insns aren't 32 bit but we're not really testing x86 so
+     * this is just to distinguish 'do compare' from 'stop'
+     */
+    ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.gregs[REG_EIP]);
 }
 
 
 int send_register_info(int sock, void *uc)
 {
-   struct reginfo ri;
-   fill_reginfo(&ri, uc);
-   return send_data_pkt(sock, &ri, sizeof(ri));
+    struct reginfo ri;
+    fill_reginfo(&ri, uc);
+    return send_data_pkt(sock, &ri, sizeof(ri));
 }
 
 /* Read register info from the socket and compare it with that from the
@@ -107,43 +104,37 @@ int send_register_info(int sock, void *uc)
  */
 int recv_and_compare_register_info(int sock, void *uc)
 {
-   int resp;
-   fill_reginfo(&master_ri, uc);
-   recv_data_pkt(sock, &apprentice_ri, sizeof(apprentice_ri));
-   if (memcmp(&master_ri, &apprentice_ri, sizeof(master_ri)) != 0)
-   {
-      /* mismatch */
-      resp = 2;
-   }
-   else if (insn_is_ud2(master_ri.faulting_insn))
-   {
-      /* end of test */
-      resp = 1;
-   }
-   else
-   {
-      /* either successful match or expected undef */
-      resp = 0;
-   }
-   send_response_byte(sock, resp);
-   return resp;
+    int resp;
+    fill_reginfo(&master_ri, uc);
+    recv_data_pkt(sock, &apprentice_ri, sizeof(apprentice_ri));
+    if (memcmp(&master_ri, &apprentice_ri, sizeof(master_ri)) != 0) {
+        /* mismatch */
+        resp = 2;
+    } else if (insn_is_ud2(master_ri.faulting_insn)) {
+        /* end of test */
+        resp = 1;
+    } else {
+        /* either successful match or expected undef */
+        resp = 0;
+    }
+    send_response_byte(sock, resp);
+    return resp;
 }
 
-static char *regname[] = 
-{
-   "GS", "FS", "ES" ,"DS", "EDI", "ESI", "EBP", "ESP",
-   "EBX", "EDX", "ECX", "EAX", "TRAPNO", "ERR", "EIP",
-   "CS", "EFL", "UESP", "SS", 0
+static char *regname[] = {
+    "GS", "FS", "ES", "DS", "EDI", "ESI", "EBP", "ESP",
+    "EBX", "EDX", "ECX", "EAX", "TRAPNO", "ERR", "EIP",
+    "CS", "EFL", "UESP", "SS", 0
 };
 
 static void dump_reginfo(struct reginfo *ri)
 {
-   int i;
-   fprintf(stderr, "  faulting insn %x\n", ri->faulting_insn);
-   for (i = 0; i < NGREG; i++)
-   {
-      fprintf(stderr, "  %s: %x\n", regname[i] ? regname[i] : "???", ri->gregs[i]);
-   }
+    int i;
+    fprintf(stderr, "  faulting insn %x\n", ri->faulting_insn);
+    for (i = 0; i < NGREG; i++) {
+        fprintf(stderr, "  %s: %x\n", regname[i] ? regname[i] : "???",
+                ri->gregs[i]);
+    }
 }
 
 
@@ -155,16 +146,15 @@ static void dump_reginfo(struct reginfo *ri)
  */
 int report_match_status(void)
 {
-   fprintf(stderr, "match status...\n");
-   fprintf(stderr, "master reginfo:\n");
-   dump_reginfo(&master_ri);
-   fprintf(stderr, "apprentice reginfo:\n");
-   dump_reginfo(&apprentice_ri);
-   if (memcmp(&master_ri, &apprentice_ri, sizeof(master_ri)) == 0)
-   {
-      fprintf(stderr, "match!\n");
-      return 0;
-   }
-   fprintf(stderr, "mismatch!\n");
-   return 1;
+    fprintf(stderr, "match status...\n");
+    fprintf(stderr, "master reginfo:\n");
+    dump_reginfo(&master_ri);
+    fprintf(stderr, "apprentice reginfo:\n");
+    dump_reginfo(&apprentice_ri);
+    if (memcmp(&master_ri, &apprentice_ri, sizeof(master_ri)) == 0) {
+        fprintf(stderr, "match!\n");
+        return 0;
+    }
+    fprintf(stderr, "mismatch!\n");
+    return 1;
 }
diff --git a/risu_m68k.c b/risu_m68k.c
index f84ac7a..0bf5c14 100644
--- a/risu_m68k.c
+++ b/risu_m68k.c
@@ -10,7 +10,7 @@
 
 void advance_pc(void *vuc)
 {
-    ucontext_t *uc = (ucontext_t*)vuc;
+    ucontext_t *uc = (ucontext_t *) vuc;
     uc->uc_mcontext.gregs[R_PC] += 4;
 }
 
diff --git a/risu_ppc64.c b/risu_ppc64.c
index b575078..eb60573 100644
--- a/risu_ppc64.c
+++ b/risu_ppc64.c
@@ -15,7 +15,7 @@
 
 void advance_pc(void *vuc)
 {
-    ucontext_t *uc = (ucontext_t*)vuc;
+    ucontext_t *uc = (ucontext_t *) vuc;
     uc->uc_mcontext.regs->nip += 4;
 }
 
diff --git a/risu_reginfo_aarch64.c b/risu_reginfo_aarch64.c
index fe567de..e3fadde 100644
--- a/risu_reginfo_aarch64.c
+++ b/risu_reginfo_aarch64.c
@@ -26,33 +26,36 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
     /* necessary to be able to compare with memcmp later */
     memset(ri, 0, sizeof(*ri));
 
-    for (i = 0; i < 31; i++)
+    for (i = 0; i < 31; i++) {
         ri->regs[i] = uc->uc_mcontext.regs[i];
+    }
 
     ri->sp = 0xdeadbeefdeadbeef;
     ri->pc = uc->uc_mcontext.pc - image_start_address;
-    ri->flags = uc->uc_mcontext.pstate & 0xf0000000; /* get only flags */
+    ri->flags = uc->uc_mcontext.pstate & 0xf0000000;    /* get only flags */
 
     ri->fault_address = uc->uc_mcontext.fault_address;
-    ri->faulting_insn = *((uint32_t *)uc->uc_mcontext.pc);
+    ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.pc);
 
-    ctx = (struct _aarch64_ctx *)&uc->uc_mcontext.__reserved[0];
+    ctx = (struct _aarch64_ctx *) &uc->uc_mcontext.__reserved[0];
 
     while (ctx->magic != FPSIMD_MAGIC && ctx->size != 0) {
         ctx += (ctx->size + sizeof(*ctx) - 1) / sizeof(*ctx);
     }
 
     if (ctx->magic != FPSIMD_MAGIC || ctx->size != sizeof(*fp)) {
-        fprintf(stderr, "risu_reginfo_aarch64: failed to get FP/SIMD state\n");
+        fprintf(stderr,
+                "risu_reginfo_aarch64: failed to get FP/SIMD state\n");
         return;
     }
 
-    fp = (struct fpsimd_context *)ctx;
+    fp = (struct fpsimd_context *) ctx;
     ri->fpsr = fp->fpsr;
     ri->fpcr = fp->fpcr;
 
-    for (i = 0; i < 32; i++)
+    for (i = 0; i < 32; i++) {
         ri->vregs[i] = fp->vregs[i];
+    }
 };
 
 /* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
@@ -62,13 +65,14 @@ int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
 }
 
 /* reginfo_dump: print state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE *f)
+int reginfo_dump(struct reginfo *ri, FILE * f)
 {
     int i;
     fprintf(f, "  faulting insn %08x\n", ri->faulting_insn);
 
-    for (i = 0; i < 31; i++)
+    for (i = 0; i < 31; i++) {
         fprintf(f, "  X%2d   : %016" PRIx64 "\n", i, ri->regs[i]);
+    }
 
     fprintf(f, "  sp    : %016" PRIx64 "\n", ri->sp);
     fprintf(f, "  pc    : %016" PRIx64 "\n", ri->pc);
@@ -76,16 +80,17 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
     fprintf(f, "  fpsr  : %08x\n", ri->fpsr);
     fprintf(f, "  fpcr  : %08x\n", ri->fpcr);
 
-    for (i = 0; i < 32; i++)
+    for (i = 0; i < 32; i++) {
         fprintf(f, "  V%2d   : %016" PRIx64 "%016" PRIx64 "\n", i,
-                (uint64_t)(ri->vregs[i] >> 64),
-                (uint64_t)(ri->vregs[i] & 0xffffffffffffffff));
+                (uint64_t) (ri->vregs[i] >> 64),
+                (uint64_t) (ri->vregs[i] & 0xffffffffffffffff));
+    }
 
     return !ferror(f);
 }
 
 /* reginfo_dump_mismatch: print mismatch details to a stream, ret nonzero=ok */
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
+int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
 {
     int i;
     fprintf(f, "mismatch detail (master : apprentice):\n");
@@ -94,37 +99,44 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
                 m->faulting_insn, a->faulting_insn);
     }
     for (i = 0; i < 31; i++) {
-        if (m->regs[i] != a->regs[i])
+        if (m->regs[i] != a->regs[i]) {
             fprintf(f, "  X%2d   : %016" PRIx64 " vs %016" PRIx64 "\n",
                     i, m->regs[i], a->regs[i]);
+        }
     }
 
-    if (m->sp != a->sp)
+    if (m->sp != a->sp) {
         fprintf(f, "  sp    : %016" PRIx64 " vs %016" PRIx64 "\n",
                 m->sp, a->sp);
+    }
 
-    if (m->pc != a->pc)
+    if (m->pc != a->pc) {
         fprintf(f, "  pc    : %016" PRIx64 " vs %016" PRIx64 "\n",
                 m->pc, a->pc);
+    }
 
-    if (m->flags != a->flags)
+    if (m->flags != a->flags) {
         fprintf(f, "  flags : %08x vs %08x\n", m->flags, a->flags);
+    }
 
-    if (m->fpsr != a->fpsr)
+    if (m->fpsr != a->fpsr) {
         fprintf(f, "  fpsr  : %08x vs %08x\n", m->fpsr, a->fpsr);
+    }
 
-    if (m->fpcr != a->fpcr)
+    if (m->fpcr != a->fpcr) {
         fprintf(f, "  fpcr  : %08x vs %08x\n", m->fpcr, a->fpcr);
+    }
 
     for (i = 0; i < 32; i++) {
-        if (m->vregs[i] != a->vregs[i])
+        if (m->vregs[i] != a->vregs[i]) {
             fprintf(f, "  V%2d   : "
                     "%016" PRIx64 "%016" PRIx64 " vs "
                     "%016" PRIx64 "%016" PRIx64 "\n", i,
-                    (uint64_t)(m->vregs[i] >> 64),
-                    (uint64_t)(m->vregs[i] & 0xffffffffffffffff),
-                    (uint64_t)(a->vregs[i] >> 64),
-                    (uint64_t)(a->vregs[i] & 0xffffffffffffffff));
+                    (uint64_t) (m->vregs[i] >> 64),
+                    (uint64_t) (m->vregs[i] & 0xffffffffffffffff),
+                    (uint64_t) (a->vregs[i] >> 64),
+                    (uint64_t) (a->vregs[i] & 0xffffffffffffffff));
+        }
     }
 
     return !ferror(f);
diff --git a/risu_reginfo_aarch64.h b/risu_reginfo_aarch64.h
index 3d1b2fd..a05fb4e 100644
--- a/risu_reginfo_aarch64.h
+++ b/risu_reginfo_aarch64.h
@@ -13,8 +13,7 @@
 #ifndef RISU_REGINFO_AARCH64_H
 #define RISU_REGINFO_AARCH64_H
 
-struct reginfo
-{
+struct reginfo {
     uint64_t fault_address;
     uint64_t regs[31];
     uint64_t sp;
diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
index 5309f3a..8e6736b 100644
--- a/risu_reginfo_arm.c
+++ b/risu_reginfo_arm.c
@@ -26,173 +26,173 @@ extern int insnsize(ucontext_t *uc);
 
 static void reginfo_init_vfp(struct reginfo *ri, ucontext_t *uc)
 {
-   // Read VFP registers. These live in uc->uc_regspace, which is
-   // a sequence of
-   //   u32 magic
-   //   u32 size
-   //   data....
-   // blocks. We have to skip through to find the one for VFP.
-   unsigned long *rs = uc->uc_regspace;
-
-   for (;;)
-   {
-      switch (*rs++)
-      {
-         case 0:
-         {
-            /* We didn't find any VFP at all (probably a no-VFP
-             * kernel). Zero out all the state to avoid mismatches.
-             */
-            int j;
-            for (j = 0; j < 32; j++)
-               ri->fpregs[j] = 0;
-            ri->fpscr = 0;
-            return;
-         }
-         case 0x56465001: /* VFP_MAGIC */
-         {
-            /* This is the one we care about. The format (after the size word)
-             * is 32 * 64 bit registers, then the 32 bit fpscr, then some stuff
-             * we don't care about.
-             */
-            int i;
-            /* Skip if it's smaller than we expected (should never happen!) */
-            if (*rs < ((32*2)+1))
+    /* Read VFP registers. These live in uc->uc_regspace, which is
+     * a sequence of
+     *   u32 magic
+     *   u32 size
+     *   data....
+     * blocks. We have to skip through to find the one for VFP.
+     */
+    unsigned long *rs = uc->uc_regspace;
+
+    for (;;) {
+        switch (*rs++) {
+        case 0:
             {
-               rs += (*rs / 4);
-               break;
+                /* We didn't find any VFP at all (probably a no-VFP
+                 * kernel). Zero out all the state to avoid mismatches.
+                 */
+                int j;
+                for (j = 0; j < 32; j++) {
+                    ri->fpregs[j] = 0;
+                }
+                ri->fpscr = 0;
+                return;
             }
-            rs++;
-            for (i = 0; i < 32; i++)
+        case 0x56465001:        /* VFP_MAGIC */
             {
-               ri->fpregs[i] = *rs++;
-               ri->fpregs[i] |= (uint64_t)(*rs++) << 32;
+                /* This is the one we care about. The format (after
+                 *  the size word is 32 * 64 bit registers, then the
+                 *  32 bit fpscr, then some stuff we don't care about.
+                 */
+                int i;
+                /* Skip if it's smaller than we expected (should never happen!) */
+                if (*rs < ((32 * 2) + 1)) {
+                    rs += (*rs / 4);
+                    break;
+                }
+                rs++;
+                for (i = 0; i < 32; i++) {
+                    ri->fpregs[i] = *rs++;
+                    ri->fpregs[i] |= (uint64_t) (*rs++) << 32;
+                }
+                /* Ignore the UNK/SBZP bits. We also ignore the cumulative
+                 * exception bits unless we were specifically asked to test
+                 * them on the risu command line -- too much of qemu gets
+                 * them wrong and they aren't actually very important.
+                 */
+                ri->fpscr = (*rs) & 0xffff9f9f;
+                if (!test_fp_exc) {
+                    ri->fpscr &= ~0x9f;
+                }
+                /* Clear the cumulative exception flags. This is a bit
+                 * unclean, but makes sense because otherwise we'd have to
+                 * insert explicit bit-clearing code in the generated code
+                 * to avoid the test becoming useless once all the bits
+                 * get set.
+                 */
+                (*rs) &= ~0x9f;
+                return;
             }
-            /* Ignore the UNK/SBZP bits. We also ignore the cumulative
-             * exception bits unless we were specifically asked to test
-             * them on the risu command line -- too much of qemu gets
-             * them wrong and they aren't actually very important.
-             */
-            ri->fpscr = (*rs) & 0xffff9f9f;
-            if (!test_fp_exc) {
-               ri->fpscr &= ~0x9f;
-            }
-            /* Clear the cumulative exception flags. This is a bit
-             * unclean, but makes sense because otherwise we'd have to
-             * insert explicit bit-clearing code in the generated code
-             * to avoid the test becoming useless once all the bits
-             * get set.
-             */
-            (*rs) &= ~0x9f;
-            return;
-         }
-         default:
+        default:
             /* Some other kind of block, ignore it */
             rs += (*rs / 4);
             break;
-      }
-   }
+        }
+    }
 }
 
 void reginfo_init(struct reginfo *ri, ucontext_t *uc)
 {
-   memset(ri, 0, sizeof(*ri)); /* necessary for memcmp later */
-
-   ri->gpreg[0] = uc->uc_mcontext.arm_r0;
-   ri->gpreg[1] = uc->uc_mcontext.arm_r1;
-   ri->gpreg[2] = uc->uc_mcontext.arm_r2;
-   ri->gpreg[3] = uc->uc_mcontext.arm_r3;
-   ri->gpreg[4] = uc->uc_mcontext.arm_r4;
-   ri->gpreg[5] = uc->uc_mcontext.arm_r5;
-   ri->gpreg[6] = uc->uc_mcontext.arm_r6;
-   ri->gpreg[7] = uc->uc_mcontext.arm_r7;
-   ri->gpreg[8] = uc->uc_mcontext.arm_r8;
-   ri->gpreg[9] = uc->uc_mcontext.arm_r9;
-   ri->gpreg[10] = uc->uc_mcontext.arm_r10;
-   ri->gpreg[11] = uc->uc_mcontext.arm_fp;
-   ri->gpreg[12] = uc->uc_mcontext.arm_ip;
-   ri->gpreg[14] = uc->uc_mcontext.arm_lr;
-   ri->gpreg[13] = 0xdeadbeef;
-   ri->gpreg[15] = uc->uc_mcontext.arm_pc - image_start_address;
-   // Mask out everything except NZCVQ GE
-   // In theory we should be OK to compare everything
-   // except the reserved bits, but valgrind for one
-   // doesn't fill in enough fields yet.
-   ri->cpsr = uc->uc_mcontext.arm_cpsr & 0xF80F0000;
-
-   ri->faulting_insn = *((uint16_t*)uc->uc_mcontext.arm_pc);
-   ri->faulting_insn_size = insnsize(uc);
-   if (ri->faulting_insn_size != 2)
-   {
-      ri->faulting_insn |= (*((uint16_t*)uc->uc_mcontext.arm_pc+1)) << 16;
-   }
-
-   reginfo_init_vfp(ri, uc);
+    memset(ri, 0, sizeof(*ri));         /* necessary for memcmp later */
+
+    ri->gpreg[0] = uc->uc_mcontext.arm_r0;
+    ri->gpreg[1] = uc->uc_mcontext.arm_r1;
+    ri->gpreg[2] = uc->uc_mcontext.arm_r2;
+    ri->gpreg[3] = uc->uc_mcontext.arm_r3;
+    ri->gpreg[4] = uc->uc_mcontext.arm_r4;
+    ri->gpreg[5] = uc->uc_mcontext.arm_r5;
+    ri->gpreg[6] = uc->uc_mcontext.arm_r6;
+    ri->gpreg[7] = uc->uc_mcontext.arm_r7;
+    ri->gpreg[8] = uc->uc_mcontext.arm_r8;
+    ri->gpreg[9] = uc->uc_mcontext.arm_r9;
+    ri->gpreg[10] = uc->uc_mcontext.arm_r10;
+    ri->gpreg[11] = uc->uc_mcontext.arm_fp;
+    ri->gpreg[12] = uc->uc_mcontext.arm_ip;
+    ri->gpreg[14] = uc->uc_mcontext.arm_lr;
+    ri->gpreg[13] = 0xdeadbeef;
+    ri->gpreg[15] = uc->uc_mcontext.arm_pc - image_start_address;
+    /* Mask out everything except NZCVQ GE
+     * In theory we should be OK to compare everything
+     * except the reserved bits, but valgrind for one
+     * doesn't fill in enough fields yet.
+     */
+    ri->cpsr = uc->uc_mcontext.arm_cpsr & 0xF80F0000;
+
+    ri->faulting_insn = *((uint16_t *) uc->uc_mcontext.arm_pc);
+    ri->faulting_insn_size = insnsize(uc);
+    if (ri->faulting_insn_size != 2) {
+        ri->faulting_insn |=
+            (*((uint16_t *) uc->uc_mcontext.arm_pc + 1)) << 16;
+    }
+
+    reginfo_init_vfp(ri, uc);
 }
 
 /* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
 int reginfo_is_eq(struct reginfo *r1, struct reginfo *r2)
 {
-    return memcmp(r1, r2, sizeof(*r1)) == 0; /* ok since we memset 0 */
+    return memcmp(r1, r2, sizeof(*r1)) == 0;    /* ok since we memset 0 */
 }
 
 /* reginfo_dump: print the state to a stream, returns nonzero on success */
 int reginfo_dump(struct reginfo *ri, FILE *f)
 {
-   int i;
-   if (ri->faulting_insn_size == 2)
-      fprintf(f, "  faulting insn %04x\n", ri->faulting_insn);
-   else
-      fprintf(f, "  faulting insn %08x\n", ri->faulting_insn);
-   for (i = 0; i < 16; i++)
-   {
-      fprintf(f, "  r%d: %08x\n", i, ri->gpreg[i]);
-   }
-   fprintf(f, "  cpsr: %08x\n", ri->cpsr);
-   for (i = 0; i < 32; i++)
-   {
-      fprintf(f, "  d%d: %016llx\n",
-              i, (unsigned long long)ri->fpregs[i]);
-   }
-   fprintf(f, "  fpscr: %08x\n", ri->fpscr);
-
-   return !ferror(f);
+    int i;
+    if (ri->faulting_insn_size == 2) {
+        fprintf(f, "  faulting insn %04x\n", ri->faulting_insn);
+    } else {
+        fprintf(f, "  faulting insn %08x\n", ri->faulting_insn);
+    }
+    for (i = 0; i < 16; i++) {
+        fprintf(f, "  r%d: %08x\n", i, ri->gpreg[i]);
+    }
+    fprintf(f, "  cpsr: %08x\n", ri->cpsr);
+    for (i = 0; i < 32; i++) {
+        fprintf(f, "  d%d: %016llx\n",
+                i, (unsigned long long) ri->fpregs[i]);
+    }
+    fprintf(f, "  fpscr: %08x\n", ri->fpscr);
+
+    return !ferror(f);
 }
 
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a,
-                          FILE *f)
+int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
 {
-   int i;
-   fprintf(f, "mismatch detail (master : apprentice):\n");
-
-   if (m->faulting_insn_size != a->faulting_insn_size)
-      fprintf(f, "  faulting insn size mismatch %d vs %d\n",
-              m->faulting_insn_size, a->faulting_insn_size);
-   else if (m->faulting_insn != a->faulting_insn)
-   {
-      if (m->faulting_insn_size == 2)
-         fprintf(f, "  faulting insn mismatch %04x vs %04x\n",
-                 m->faulting_insn, a->faulting_insn);
-      else
-         fprintf(f, "  faulting insn mismatch %08x vs %08x\n",
-                 m->faulting_insn, a->faulting_insn);
-   }
-   for (i = 0; i < 16; i++)
-   {
-      if (m->gpreg[i] != a->gpreg[i])
-         fprintf(f, "  r%d: %08x vs %08x\n", i, m->gpreg[i], a->gpreg[i]);
-   }
-   if (m->cpsr != a->cpsr)
-      fprintf(f, "  cpsr: %08x vs %08x\n", m->cpsr, a->cpsr);
-   for (i = 0; i < 32; i++)
-   {
-      if (m->fpregs[i] != a->fpregs[i])
-         fprintf(f, "  d%d: %016llx vs %016llx\n", i,
-                 (unsigned long long)m->fpregs[i],
-                 (unsigned long long)a->fpregs[i]);
-   }
-   if (m->fpscr != a->fpscr)
-      fprintf(f, "  fpscr: %08x vs %08x\n", m->fpscr, a->fpscr);
-
-   return !ferror(f);
+    int i;
+    fprintf(f, "mismatch detail (master : apprentice):\n");
+
+    if (m->faulting_insn_size != a->faulting_insn_size) {
+        fprintf(f, "  faulting insn size mismatch %d vs %d\n",
+                m->faulting_insn_size, a->faulting_insn_size);
+    } else if (m->faulting_insn != a->faulting_insn) {
+        if (m->faulting_insn_size == 2) {
+            fprintf(f, "  faulting insn mismatch %04x vs %04x\n",
+                    m->faulting_insn, a->faulting_insn);
+        } else {
+            fprintf(f, "  faulting insn mismatch %08x vs %08x\n",
+                    m->faulting_insn, a->faulting_insn);
+        }
+    }
+    for (i = 0; i < 16; i++) {
+        if (m->gpreg[i] != a->gpreg[i]) {
+            fprintf(f, "  r%d: %08x vs %08x\n", i, m->gpreg[i],
+                    a->gpreg[i]);
+        }
+    }
+    if (m->cpsr != a->cpsr) {
+        fprintf(f, "  cpsr: %08x vs %08x\n", m->cpsr, a->cpsr);
+    }
+    for (i = 0; i < 32; i++) {
+        if (m->fpregs[i] != a->fpregs[i]) {
+            fprintf(f, "  d%d: %016llx vs %016llx\n", i,
+                    (unsigned long long) m->fpregs[i],
+                    (unsigned long long) a->fpregs[i]);
+        }
+    }
+    if (m->fpscr != a->fpscr) {
+        fprintf(f, "  fpscr: %08x vs %08x\n", m->fpscr, a->fpscr);
+    }
+
+    return !ferror(f);
 }
diff --git a/risu_reginfo_arm.h b/risu_reginfo_arm.h
index 96e5791..60754a9 100644
--- a/risu_reginfo_arm.h
+++ b/risu_reginfo_arm.h
@@ -13,8 +13,7 @@
 #ifndef RISU_REGINFO_ARM_H
 #define RISU_REGINFO_ARM_H
 
-struct reginfo
-{
+struct reginfo {
     uint64_t fpregs[32];
     uint32_t faulting_insn;
     uint32_t faulting_insn_size;
diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index 45950b1..4ff0aa8 100644
--- a/risu_reginfo_m68k.c
+++ b/risu_reginfo_m68k.c
@@ -20,7 +20,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
     int i;
     memset(ri, 0, sizeof(*ri));
 
-    ri->faulting_insn = *((uint32_t *)uc->uc_mcontext.gregs[R_PC]);
+    ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.gregs[R_PC]);
     ri->pc = uc->uc_mcontext.gregs[R_PC] - image_start_address;
 
     for (i = 0; i < NGREG; i++) {
@@ -78,8 +78,7 @@ int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
 int reginfo_dump(struct reginfo *ri, FILE *f)
 {
     int i;
-    fprintf(f, "  pc            \e[1;101;37m0x%08x\e[0m\n",
-            ri->pc);
+    fprintf(f, "  pc            \e[1;101;37m0x%08x\e[0m\n", ri->pc);
 
     fprintf(f, "\tPC: %08x\n", ri->gregs[R_PC]);
     fprintf(f, "\tPS: %04x\n", ri->gregs[R_PS]);
@@ -101,14 +100,14 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
     return !ferror(f);
 }
 
-int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
+int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE * f)
 {
     int i;
 
     if (m->gregs[R_PS] != a->gregs[R_PS]) {
-            fprintf(f, "Mismatch: Register PS\n");
-            fprintf(f, "master: [%x] - apprentice: [%x]\n",
-                    m->gregs[R_PS], a->gregs[R_PS]);
+        fprintf(f, "Mismatch: Register PS\n");
+        fprintf(f, "master: [%x] - apprentice: [%x]\n",
+                m->gregs[R_PS], a->gregs[R_PS]);
     }
 
     for (i = 0; i < 16; i++) {
@@ -116,9 +115,10 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
             continue;
         }
         if (m->gregs[i] != a->gregs[i]) {
-            fprintf(f, "Mismatch: Register %c%d\n", i < 8 ? 'D' : 'A', i % 8);
-            fprintf(f, "master: [%x] - apprentice: [%x]\n",
-                    m->gregs[i], a->gregs[i]);
+            fprintf(f, "Mismatch: Register %c%d\n", i < 8 ? 'D' : 'A',
+                    i % 8);
+            fprintf(f, "master: [%x] - apprentice: [%x]\n", m->gregs[i],
+                    a->gregs[i]);
         }
     }
 
diff --git a/risu_reginfo_m68k.h b/risu_reginfo_m68k.h
index 06ea61d..c1c9fe6 100644
--- a/risu_reginfo_m68k.h
+++ b/risu_reginfo_m68k.h
@@ -9,8 +9,7 @@
 #ifndef RISU_REGINFO_M68K_H
 #define RISU_REGINFO_M68K_H
 
-struct reginfo
-{
+struct reginfo {
     uint32_t faulting_insn;
     uint32_t pc;
     gregset_t gregs;
diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index ae86263..eb9c12b 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -28,7 +28,7 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
     int i;
     memset(ri, 0, sizeof(*ri));
 
-    ri->faulting_insn = *((uint32_t *)uc->uc_mcontext.regs->nip);
+    ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.regs->nip);
     ri->nip = uc->uc_mcontext.regs->nip - image_start_address;
 
     for (i = 0; i < NGREG; i++) {
@@ -76,16 +76,16 @@ int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
             continue;
         }
 
-        if  (m->fpregs[i] != a->fpregs[i]) {
+        if (m->fpregs[i] != a->fpregs[i]) {
             return 0;
         }
     }
 
     for (i = 0; i < 32; i++) {
         if (m->vrregs.vrregs[i][0] != a->vrregs.vrregs[i][0] ||
-                m->vrregs.vrregs[i][1] != a->vrregs.vrregs[i][1] ||
-                m->vrregs.vrregs[i][2] != a->vrregs.vrregs[i][2] ||
-                m->vrregs.vrregs[i][3] != a->vrregs.vrregs[i][3]) {
+            m->vrregs.vrregs[i][1] != a->vrregs.vrregs[i][1] ||
+            m->vrregs.vrregs[i][2] != a->vrregs.vrregs[i][2] ||
+            m->vrregs.vrregs[i][3] != a->vrregs.vrregs[i][3]) {
             return 0;
         }
     }
@@ -93,7 +93,7 @@ int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
 }
 
 /* reginfo_dump: print state to a stream, returns nonzero on success */
-int reginfo_dump(struct reginfo *ri, FILE *f)
+int reginfo_dump(struct reginfo *ri, FILE * f)
 {
     int i;
 
@@ -153,14 +153,12 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
 
     if (m->gregs[XER] != a->gregs[XER]) {
         fprintf(f, "Mismatch: XER\n");
-        fprintf(f, "m: [%lx] != a: [%lx]\n",
-                m->gregs[XER], a->gregs[XER]);
+        fprintf(f, "m: [%lx] != a: [%lx]\n", m->gregs[XER], a->gregs[XER]);
     }
 
     if (m->gregs[CCR] != a->gregs[CCR]) {
         fprintf(f, "Mismatch: Cond. Register\n");
-        fprintf(f, "m: [%lx] != a: [%lx]\n",
-                m->gregs[CCR], a->gregs[CCR]);
+        fprintf(f, "m: [%lx] != a: [%lx]\n", m->gregs[CCR], a->gregs[CCR]);
     }
 
     for (i = 0; i < 32; i++) {
@@ -168,18 +166,17 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
             continue;
         }
 
-        if  (m->fpregs[i] != a->fpregs[i]) {
+        if (m->fpregs[i] != a->fpregs[i]) {
             fprintf(f, "Mismatch: Register r%d\n", i);
-            fprintf(f, "m: [%f] != a: [%f]\n",
-                    m->fpregs[i], a->fpregs[i]);
+            fprintf(f, "m: [%f] != a: [%f]\n", m->fpregs[i], a->fpregs[i]);
         }
     }
 
     for (i = 0; i < 32; i++) {
         if (m->vrregs.vrregs[i][0] != a->vrregs.vrregs[i][0] ||
-                m->vrregs.vrregs[i][1] != a->vrregs.vrregs[i][1] ||
-                m->vrregs.vrregs[i][2] != a->vrregs.vrregs[i][2] ||
-                m->vrregs.vrregs[i][3] != a->vrregs.vrregs[i][3]) {
+            m->vrregs.vrregs[i][1] != a->vrregs.vrregs[i][1] ||
+            m->vrregs.vrregs[i][2] != a->vrregs.vrregs[i][2] ||
+            m->vrregs.vrregs[i][3] != a->vrregs.vrregs[i][3]) {
 
             fprintf(f, "Mismatch: Register vr%d\n", i);
             fprintf(f, "m: [%x, %x, %x, %x] != a: [%x, %x, %x, %x]\n",
diff --git a/risu_reginfo_ppc64.h b/risu_reginfo_ppc64.h
index 826143e..7f2c962 100644
--- a/risu_reginfo_ppc64.h
+++ b/risu_reginfo_ppc64.h
@@ -14,8 +14,7 @@
 #ifndef RISU_REGINFO_PPC64LE_H
 #define RISU_REGINFO_PPC64LE_H
 
-struct reginfo
-{
+struct reginfo {
     uint32_t faulting_insn;
     uint32_t prev_insn;
     uint64_t nip;
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 04/13] build-all-archs: support --static flag
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
                   ` (2 preceding siblings ...)
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 03/13] all: fix up code consitency Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 14:07   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 05/13] build-all-archs: support cross building via docker Alex Bennée
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, Alex Bennée

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

---
v5
  - swap with docker patch so later can be dropped if not wanted
---
 build-all-archs | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/build-all-archs b/build-all-archs
index 2768727..581a1b4 100755
--- a/build-all-archs
+++ b/build-all-archs
@@ -14,6 +14,38 @@
 # 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:
+            --static               build a static binary
+
+EOF
+    exit 1
+}
+
+while [[ "$1" = -* ]]; do
+    opt="$1"; shift
+    arg=
+    if [[ "$opt" = *=* ]]; then
+        arg="${opt#*=}"
+        opt="${opt%%=*}"
+    fi
+    case "$opt" in
+        --static)
+            CONF="--static"
+            ;;
+        --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
@@ -36,7 +68,7 @@ for triplet in aarch64-linux-gnu arm-linux-gnueabihf m68k-linux-gnu \
     rm -rf build/${triplet}
     mkdir -p build/${triplet}
 
-    (cd build/${triplet} && CROSS_PREFIX="${triplet}-"  ../../configure)
+    (cd build/${triplet} && CROSS_PREFIX="${triplet}-"  ../../configure ${CONF})
     make -C build/${triplet} EXTRA_CFLAGS=-Werror
 
 done
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 05/13] build-all-archs: support cross building via docker
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
                   ` (3 preceding siblings ...)
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 04/13] build-all-archs: support --static flag Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 14:09   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 06/13] risu: a bit more verbosity when starting Alex Bennée
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, 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>

---
v5
  - swapped with --static patch so this can be dropped if desired
---
 build-all-archs | 46 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/build-all-archs b/build-all-archs
index 581a1b4..63918e5 100755
--- a/build-all-archs
+++ b/build-all-archs
@@ -11,9 +11,6 @@
 # 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
@@ -21,7 +18,10 @@ usage() {
 
         Options include:
             --static               build a static binary
+            --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
 }
@@ -37,6 +37,14 @@ while [[ "$1" = -* ]]; do
         --static)
             CONF="--static"
             ;;
+        --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
             ;;
@@ -48,10 +56,24 @@ 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
 }
 
@@ -62,19 +84,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 ${CONF})
-    make -C build/${triplet} EXTRA_CFLAGS=-Werror
+    CONFIGURE="cd build/${triplet} && CROSS_PREFIX="${triplet}-"  ../../configure ${CONF}"
+    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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 06/13] risu: a bit more verbosity when starting
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
                   ` (4 preceding siblings ...)
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 05/13] build-all-archs: support cross building via docker Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 13:33   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive functions Alex Bennée
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, Alex Bennée

When debugging faults it is useful to know where the generated
instructions are living.

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

--
v5
  - dropped all the status update due to signal handler contraints
v3
  - use portable fmt string for image_start_address
  - include arm dumping position
---
 risu.c | 4 ++++
 risu.h | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/risu.c b/risu.c
index 2cd6d22..a10422a 100644
--- a/risu.c
+++ b/risu.c
@@ -124,6 +124,8 @@ int master(int sock)
     }
     master_socket = sock;
     set_sigill_handler(&master_sigill);
+    fprintf(stderr, "starting master image at 0x%"PRIxPTR"\n",
+            image_start_address);
     fprintf(stderr, "starting image\n");
     image_start();
     fprintf(stderr, "image returned unexpectedly\n");
@@ -134,6 +136,8 @@ int apprentice(int sock)
 {
     apprentice_socket = sock;
     set_sigill_handler(&apprentice_sigill);
+    fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n",
+            image_start_address);
     fprintf(stderr, "starting image\n");
     image_start();
     fprintf(stderr, "image returned unexpectedly\n");
diff --git a/risu.h b/risu.h
index 3fbeda8..78a7313 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
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive functions
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
                   ` (5 preceding siblings ...)
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 06/13] risu: a bit more verbosity when starting Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 13:40   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 08/13] risu: add header to trace stream Alex Bennée
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, 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>

---
v5
  - re-base without tab/format cleanps
v4
  - split header code
  - fix formatting foo-bar's
v3
  - new for v3
  - arm, aarch64, ppc64
---
 reginfo.c | 39 +++++++++++++++++++--------------------
 risu.c    | 23 +++++++++++++++++++++--
 risu.h    | 11 +++++++++--
 3 files changed, 49 insertions(+), 24 deletions(-)

diff --git a/reginfo.c b/reginfo.c
index 31bb99f..90cea8f 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -21,7 +21,7 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
 static int mem_used;
 static int packet_mismatch;
 
-int send_register_info(int sock, void *uc)
+int send_register_info(write_fn write_fn, void *uc)
 {
     struct reginfo ri;
     int op;
@@ -29,24 +29,25 @@ int send_register_info(int sock, void *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));
+        write_fn(&ri, sizeof(ri));
+        return 1;
     case OP_SETMEMBLOCK:
-        memblock = (void *) (uintptr_t) get_reginfo_paramreg(&ri);
+        memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
         break;
     case OP_GETMEMBLOCK:
         set_ucontext_paramreg(uc,
-                              get_reginfo_paramreg(&ri) +
-                              (uintptr_t) memblock);
+                              get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
         break;
     case OP_COMPAREMEM:
-        return send_data_pkt(sock, memblock, MEMBLOCKLEN);
+        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;
 }
@@ -59,7 +60,7 @@ 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;
 
@@ -73,36 +74,34 @@ int recv_and_compare_register_info(int sock, void *uc)
         /* 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))) {
+        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;
         }
-        send_response_byte(sock, resp);
+        resp_fn(resp);
         break;
     case OP_SETMEMBLOCK:
-        memblock = (void *) (uintptr_t) get_reginfo_paramreg(&master_ri);
+        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);
+                              (uintptr_t)memblock);
         break;
     case OP_COMPAREMEM:
         mem_used = 1;
-        if (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
+        if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
             packet_mismatch = 1;
             resp = 2;
         } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
             /* memory mismatch */
             resp = 2;
         }
-        send_response_byte(sock, resp);
+        resp_fn(resp);
         break;
     }
 
diff --git a/risu.c b/risu.c
index a10422a..88e586c 100644
--- a/risu.c
+++ b/risu.c
@@ -37,9 +37,28 @@ sigjmp_buf jmpbuf;
 /* Should we test for FP exception status bits? */
 int test_fp_exc;
 
+/* 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 */
         advance_pc(uc);
@@ -52,7 +71,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 */
         advance_pc(uc);
diff --git a/risu.h b/risu.h
index 78a7313..32241bc 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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 08/13] risu: add header to trace stream
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
                   ` (6 preceding siblings ...)
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive functions Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 13:55   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 09/13] risu: add simple trace and replay support Alex Bennée
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, 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>

---
v5
  - re-base without formatting fixes
  - dropped fprintfs in signal context
v4
  - split from previous patch
---
 reginfo.c      | 94 +++++++++++++++++++++++++++++++++++++---------------------
 risu.h         |  9 ++++++
 risu_aarch64.c |  5 ++++
 risu_arm.c     |  5 ++++
 risu_m68k.c    |  5 ++++
 risu_ppc64.c   |  5 ++++
 6 files changed, 89 insertions(+), 34 deletions(-)

diff --git a/reginfo.c b/reginfo.c
index 90cea8f..4ff937f 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -24,10 +24,19 @@ static int packet_mismatch;
 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) {
+        return -1;
+    }
+
     switch (op) {
     case OP_TESTEND:
         write_fn(&ri, sizeof(ri));
@@ -63,46 +72,63 @@ 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;
+    if (read_fn(&header, sizeof(header)) != 0) {
+        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 {
+        /* We are out of sync */
+        resp = 2;
         resp_fn(resp);
-        break;
     }
 
     return resp;
diff --git a/risu.h b/risu.h
index 32241bc..e9e70ab 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 9c6809d..492d141 100644
--- a/risu_aarch64.c
+++ b/risu_aarch64.c
@@ -40,3 +40,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 a55c6c2..5fcb2a5 100644
--- a/risu_arm.c
+++ b/risu_arm.c
@@ -68,3 +68,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 0bf5c14..1056eef 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 eb60573..83f8d1f 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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 09/13] risu: add simple trace and replay support
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
                   ` (7 preceding siblings ...)
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 08/13] risu: add header to trace stream Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 14:06   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 10/13] risu: handle trace through stdin/stdout Alex Bennée
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, 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>

---
v5
  - re-base without formatting fixes
  - unify socket/file descriptors
  - count number of signals/checkpoint
  - all output from sigjmp (not in signhandlers)
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 | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 93 insertions(+), 19 deletions(-)

diff --git a/risu.c b/risu.c
index 88e586c..93c274b 100644
--- a/risu.c
+++ b/risu.c
@@ -30,7 +30,9 @@
 
 void *memblock;
 
-int apprentice_socket, master_socket;
+int apprentice_fd, master_fd;
+int trace;
+size_t signal_count;
 
 sigjmp_buf jmpbuf;
 
@@ -41,24 +43,60 @@ int test_fp_exc;
 
 int read_sock(void *ptr, size_t bytes)
 {
-    return recv_data_pkt(master_socket, ptr, bytes);
+    return recv_data_pkt(master_fd, ptr, bytes);
+}
+
+int write_trace(void *ptr, size_t bytes)
+{
+    size_t res = write(master_fd, ptr, bytes);
+    return (res == bytes) ? 0 : 1;
 }
 
 void respond_sock(int r)
 {
-    send_response_byte(master_socket, r);
+    send_response_byte(master_fd, r);
 }
 
 /* Apprentice function */
 
 int write_sock(void *ptr, size_t bytes)
 {
-    return send_data_pkt(apprentice_socket, ptr, bytes);
+    return send_data_pkt(apprentice_fd, ptr, bytes);
+}
+
+int read_trace(void *ptr, size_t bytes)
+{
+    size_t res = read(apprentice_fd, 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:
+        /* mismatch - if tracing we need to report, otherwise barf */
+        if (!trace) {
+           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;
+    signal_count++;
+
+    if (trace) {
+        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 */
         advance_pc(uc);
@@ -71,7 +109,16 @@ 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;
+    signal_count++;
+
+    if (trace) {
+        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 */
         advance_pc(uc);
@@ -81,6 +128,9 @@ void apprentice_sigill(int sig, siginfo_t *si, void *uc)
         exit(0);
     default:
         /* mismatch */
+        if (trace) {
+            siglongjmp(jmpbuf, 1);
+        }
         exit(1);
     }
 }
@@ -136,12 +186,17 @@ void load_image(const char *imgfile)
     image_start_address = (uintptr_t) addr;
 }
 
-int master(int sock)
+int master(void)
 {
     if (sigsetjmp(jmpbuf, 1)) {
-        return report_match_status();
+        close(master_fd);
+        if (trace) {
+            fprintf(stderr, "trace complete after %zd checkpoints\n", signal_count);
+            return 0;
+        } else {
+            return report_match_status();
+        }
     }
-    master_socket = sock;
     set_sigill_handler(&master_sigill);
     fprintf(stderr, "starting master image at 0x%"PRIxPTR"\n",
             image_start_address);
@@ -151,9 +206,13 @@ int master(int sock)
     exit(1);
 }
 
-int apprentice(int sock)
+int apprentice(void)
 {
-    apprentice_socket = sock;
+    if (sigsetjmp(jmpbuf, 1)) {
+        close(apprentice_fd);
+        fprintf(stderr, "finished early after %zd checkpoints\n", signal_count);
+        return report_match_status();
+    }
     set_sigill_handler(&apprentice_sigill);
     fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n",
             image_start_address);
@@ -175,6 +234,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");
@@ -189,7 +249,7 @@ int main(int argc, char **argv)
     uint16_t port = 9191;
     char *hostname = "localhost";
     char *imgfile;
-    int sock;
+    char *trace_fn = NULL;
 
     /* TODO clean this up later */
 
@@ -203,7 +263,7 @@ int main(int argc, char **argv)
             {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;
         }
@@ -214,6 +274,12 @@ int main(int argc, char **argv)
             /* flag set by getopt_long, do nothing */
             break;
         }
+        case 't':
+        {
+            trace_fn = optarg;
+            trace = 1;
+            break;
+        }
         case 'h':
         {
             hostname = optarg;
@@ -245,12 +311,20 @@ int main(int argc, char **argv)
     load_image(imgfile);
 
     if (ismaster) {
-        fprintf(stderr, "master port %d\n", port);
-        sock = master_connect(port);
-        return master(sock);
+        if (trace) {
+            master_fd = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
+        } else {
+            fprintf(stderr, "master port %d\n", port);
+            master_fd = master_connect(port);
+        }
+        return master();
     } else {
-        fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
-        sock = apprentice_connect(hostname, port);
-        return apprentice(sock);
+        if (trace) {
+            apprentice_fd = open(trace_fn, O_RDONLY);
+        } else {
+            fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
+            apprentice_fd = apprentice_connect(hostname, port);
+        }
+        return apprentice();
     }
 }
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 10/13] risu: handle trace through stdin/stdout
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
                   ` (8 preceding siblings ...)
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 09/13] risu: add simple trace and replay support Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 14:14   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 11/13] risu: add support compressed tracefiles Alex Bennée
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, Alex Bennée

Trace files can get quite large so it would be useful to be able to
just capture the trace stream with stdin/stdout for processing in a
pipe line. The sort of case where this is useful is for building
static binaries where zlib support is missing for whatever reason.

It can also be used in testing pipelines.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 risu.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/risu.c b/risu.c
index 93c274b..e94b54b 100644
--- a/risu.c
+++ b/risu.c
@@ -312,7 +312,11 @@ int main(int argc, char **argv)
 
     if (ismaster) {
         if (trace) {
-            master_fd = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
+            if (strncmp(trace_fn, "-", strlen(trace_fn))==0) {
+                master_fd = fileno(stdout);
+            } else {
+                master_fd = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
+            }
         } else {
             fprintf(stderr, "master port %d\n", port);
             master_fd = master_connect(port);
@@ -320,7 +324,11 @@ int main(int argc, char **argv)
         return master();
     } else {
         if (trace) {
-            apprentice_fd = open(trace_fn, O_RDONLY);
+            if (strncmp(trace_fn, "-", strlen(trace_fn))==0) {
+                apprentice_fd = fileno(stdin);
+            } else {
+                apprentice_fd = open(trace_fn, O_RDONLY);
+            }
         } else {
             fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
             apprentice_fd = apprentice_connect(hostname, port);
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 11/13] risu: add support compressed tracefiles
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
                   ` (9 preceding siblings ...)
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 10/13] risu: handle trace through stdin/stdout Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 14:15   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 12/13] new: record_traces.sh helper script Alex Bennée
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 13/13] new: run_risu.sh script Alex Bennée
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, 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>

--
v5
  - re-base
  - also don't use zlib if using stdio fds
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    | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 101 insertions(+), 6 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 e94b54b..6986e4c 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;
@@ -34,6 +36,11 @@ int apprentice_fd, master_fd;
 int trace;
 size_t signal_count;
 
+#ifdef HAVE_ZLIB
+#include <zlib.h>
+gzFile gz_trace_file;
+#endif
+
 sigjmp_buf jmpbuf;
 
 /* Should we test for FP exception status bits? */
@@ -48,7 +55,17 @@ int read_sock(void *ptr, size_t bytes)
 
 int write_trace(void *ptr, size_t bytes)
 {
-    size_t res = write(master_fd, ptr, bytes);
+    size_t res;
+
+#ifdef HAVE_ZLIB
+    if (master_fd == STDOUT_FILENO) {
+#endif
+        res = write(master_fd, ptr, bytes);
+#ifdef HAVE_ZLIB
+    } else {
+        res = gzwrite(gz_trace_file, ptr, bytes);
+    }
+#endif
     return (res == bytes) ? 0 : 1;
 }
 
@@ -66,7 +83,18 @@ int write_sock(void *ptr, size_t bytes)
 
 int read_trace(void *ptr, size_t bytes)
 {
-    size_t res = read(apprentice_fd, ptr, bytes);
+    size_t res;
+
+#ifdef HAVE_ZLIB
+    if (apprentice_fd == STDIN_FILENO) {
+#endif
+        res = read(apprentice_fd, ptr, bytes);
+#ifdef HAVE_ZLIB
+    } else {
+        res = gzread(gz_trace_file, ptr, bytes);
+    }
+#endif
+
     return (res == bytes) ? 0 : 1;
 }
 
@@ -189,6 +217,11 @@ void load_image(const char *imgfile)
 int master(void)
 {
     if (sigsetjmp(jmpbuf, 1)) {
+#ifdef HAVE_ZLIB
+        if (trace && master_fd != STDOUT_FILENO) {
+            gzclose(gz_trace_file);
+        }
+#endif
         close(master_fd);
         if (trace) {
             fprintf(stderr, "trace complete after %zd checkpoints\n", signal_count);
@@ -209,6 +242,11 @@ int master(void)
 int apprentice(void)
 {
     if (sigsetjmp(jmpbuf, 1)) {
+#ifdef HAVE_ZLIB
+        if (trace && apprentice_fd != STDIN_FILENO) {
+            gzclose(gz_trace_file);
+        }
+#endif
         close(apprentice_fd);
         fprintf(stderr, "finished early after %zd checkpoints\n", signal_count);
         return report_match_status();
@@ -316,6 +354,9 @@ int main(int argc, char **argv)
                 master_fd = fileno(stdout);
             } else {
                 master_fd = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
+#ifdef HAVE_ZLIB
+                gz_trace_file = gzdopen(master_fd, "wb9");
+#endif
             }
         } else {
             fprintf(stderr, "master port %d\n", port);
@@ -328,6 +369,9 @@ int main(int argc, char **argv)
                 apprentice_fd = fileno(stdin);
             } else {
                 apprentice_fd = open(trace_fn, O_RDONLY);
+#ifdef HAVE_ZLIB
+                gz_trace_file = gzdopen(apprentice_fd, "rb");
+#endif
             }
         } else {
             fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
-- 
2.13.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 12/13] new: record_traces.sh helper script
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
                   ` (10 preceding siblings ...)
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 11/13] risu: add support compressed tracefiles Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 14:12   ` Peter Maydell
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 13/13] new: run_risu.sh script Alex Bennée
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, 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>

---
v5
  - author, license and usage header
v3
  - allow overriding of RISU binary
---
 record_traces.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100755 record_traces.sh

diff --git a/record_traces.sh b/record_traces.sh
new file mode 100755
index 0000000..1234a61
--- /dev/null
+++ b/record_traces.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+#
+# A risu helper script to batch process a bunch of binaries and record their outputs
+#
+# Copyright (c) 2017 Linaro Limited
+# All rights reserved. This program and the accompanying materials
+# are made available under the terms of the Eclipse Public License v1.0
+# which accompanies this distribution, and is available at
+# http://www.eclipse.org/legal/epl-v10.html
+#
+# Contributors:
+#     Alex Bennée <alex.bennee@linaro.org> - initial implementation
+#
+# Usage:
+#   export RISU=/path/to/risu
+#   ./record_traces.sh  ./testcases.aarch64/*.bin
+#
+
+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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RISU PATCH v5 13/13] new: run_risu.sh script
  2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
                   ` (11 preceding siblings ...)
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 12/13] new: record_traces.sh helper script Alex Bennée
@ 2017-06-19 10:46 ` Alex Bennée
  2017-06-20 14:11   ` Peter Maydell
  12 siblings, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2017-06-19 10:46 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-devel, 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>

---
v5
  - author, license, usage header
v3
  - tweak to allow specifying RISU binary
---
 run_risu.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100755 run_risu.sh

diff --git a/run_risu.sh b/run_risu.sh
new file mode 100755
index 0000000..439cd36
--- /dev/null
+++ b/run_risu.sh
@@ -0,0 +1,66 @@
+#!/bin/bash
+#
+# Run risu against a set of binaries + trace files
+#
+# Copyright (c) 2017 Linaro Limited
+# All rights reserved. This program and the accompanying materials
+# are made available under the terms of the Eclipse Public License v1.0
+# which accompanies this distribution, and is available at
+# http://www.eclipse.org/legal/epl-v10.html
+#
+# Contributors:
+#     Alex Bennée <alex.bennee@linaro.org> - initial implementation
+#
+# Usage:
+#   (optional) export QEMU=/path/to/qemu
+#   (optional) export RISU=/path/to/risu
+#   ./run_risu.sh  ./testcases.aarch64/*.bin
+
+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

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 02/13] README: document the coding style used for risu
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 02/13] README: document the coding style used for risu Alex Bennée
@ 2017-06-20 12:54   ` Peter Maydell
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 12:54 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
> We also include the an Emacs .dir-locals (as per QEMU) that enforces
> this layout.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  .dir-locals.el | 2 ++
>  README         | 9 +++++++++
>  2 files changed, 11 insertions(+)
>  create mode 100644 .dir-locals.el
>
> diff --git a/.dir-locals.el b/.dir-locals.el
> new file mode 100644
> index 0000000..3ac0cfc
> --- /dev/null
> +++ b/.dir-locals.el
> @@ -0,0 +1,2 @@
> +((c-mode . ((c-file-style . "stroustrup")
> +           (indent-tabs-mode . nil))))

This prompted me to look at this, and this is wrong: it
overrides my local emacs config with something that gets
some bits of style wrong :-( Indeed there's one in QEMU
now I see, and that is why my emacs behaviour has regressed.
For instance given

static const VMStateDescription cmsdk_apb_uart_vmstate = {
    .name = "cmsdk-apb-uart",
    .version_id = 1,
    .minimum_version_id = 1,
    .fields = (VMStateField[]) {
        VMSTATE_END_OF_LIST()


with cursor on the blank line at the end, if I type '}' then I get
     }\n
(ie the cursor moves to a new line) but the cursor should
stay after the '}' so I can type the ','.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 03/13] all: fix up code consitency
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 03/13] all: fix up code consitency Alex Bennée
@ 2017-06-20 13:13   ` Peter Maydell
  2017-06-20 14:16     ` Peter Maydell
  0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 13:13 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
> This is pretty much a mechanical change where I ran:
>
>   indent -kr
>
> Across all the files and then fixed up all but a few violations of:
>
>   ../../qemu.git/scripts/checkpatch.pl -f *.c *.h > checkpatch.out
>
> Along with heavy use of M-x untabify to make everything consistent.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>


> diff --git a/risu_reginfo_arm.c b/risu_reginfo_arm.c
> index 5309f3a..8e6736b 100644
> --- a/risu_reginfo_arm.c
> +++ b/risu_reginfo_arm.c
> @@ -26,173 +26,173 @@ extern int insnsize(ucontext_t *uc);
>
>  static void reginfo_init_vfp(struct reginfo *ri, ucontext_t *uc)
>  {
> -   // Read VFP registers. These live in uc->uc_regspace, which is
> -   // a sequence of
> -   //   u32 magic
> -   //   u32 size
> -   //   data....
> -   // blocks. We have to skip through to find the one for VFP.
> -   unsigned long *rs = uc->uc_regspace;
> -
> -   for (;;)
> -   {
> -      switch (*rs++)
> -      {
> -         case 0:
> -         {
> -            /* We didn't find any VFP at all (probably a no-VFP
> -             * kernel). Zero out all the state to avoid mismatches.
> -             */
> -            int j;
> -            for (j = 0; j < 32; j++)
> -               ri->fpregs[j] = 0;
> -            ri->fpscr = 0;
> -            return;
> -         }
> -         case 0x56465001: /* VFP_MAGIC */
> -         {
> -            /* This is the one we care about. The format (after the size word)
> -             * is 32 * 64 bit registers, then the 32 bit fpscr, then some stuff
> -             * we don't care about.
> -             */
> -            int i;
> -            /* Skip if it's smaller than we expected (should never happen!) */
> -            if (*rs < ((32*2)+1))
> +    /* Read VFP registers. These live in uc->uc_regspace, which is
> +     * a sequence of
> +     *   u32 magic
> +     *   u32 size
> +     *   data....
> +     * blocks. We have to skip through to find the one for VFP.
> +     */
> +    unsigned long *rs = uc->uc_regspace;
> +
> +    for (;;) {
> +        switch (*rs++) {
> +        case 0:
>              {
> -               rs += (*rs / 4);
> -               break;
> +                /* We didn't find any VFP at all (probably a no-VFP
> +                 * kernel). Zero out all the state to avoid mismatches.
> +                 */
> +                int j;
> +                for (j = 0; j < 32; j++) {
> +                    ri->fpregs[j] = 0;
> +                }
> +                ri->fpscr = 0;
> +                return;
>              }
> -            rs++;
> -            for (i = 0; i < 32; i++)
> +        case 0x56465001:        /* VFP_MAGIC */
>              {
> -               ri->fpregs[i] = *rs++;
> -               ri->fpregs[i] |= (uint64_t)(*rs++) << 32;
> +                /* This is the one we care about. The format (after


Something seems to have gone wrong in this file -- note the extra indent
of {} blocks after "case 0:". The others are all fine though so I'll
just fix this locally.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 06/13] risu: a bit more verbosity when starting
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 06/13] risu: a bit more verbosity when starting Alex Bennée
@ 2017-06-20 13:33   ` Peter Maydell
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 13:33 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
> When debugging faults it is useful to know where the generated
> instructions are living.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> --

Separator should be '---'.

> v5
>   - dropped all the status update due to signal handler contraints
> v3
>   - use portable fmt string for image_start_address
>   - include arm dumping position
> ---
>  risu.c | 4 ++++
>  risu.h | 3 +++
>  2 files changed, 7 insertions(+)
>
> diff --git a/risu.c b/risu.c
> index 2cd6d22..a10422a 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -124,6 +124,8 @@ int master(int sock)
>      }
>      master_socket = sock;
>      set_sigill_handler(&master_sigill);
> +    fprintf(stderr, "starting master image at 0x%"PRIxPTR"\n",
> +            image_start_address);
>      fprintf(stderr, "starting image\n");
>      image_start();
>      fprintf(stderr, "image returned unexpectedly\n");
> @@ -134,6 +136,8 @@ int apprentice(int sock)
>  {
>      apprentice_socket = sock;
>      set_sigill_handler(&apprentice_sigill);
> +    fprintf(stderr, "starting apprentice image at 0x%"PRIxPTR"\n",
> +            image_start_address);
>      fprintf(stderr, "starting image\n");
>      image_start();
>      fprintf(stderr, "image returned unexpectedly\n");
> diff --git a/risu.h b/risu.h
> index 3fbeda8..78a7313 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
> --

Stray extra changes here ?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive functions
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive functions Alex Bennée
@ 2017-06-20 13:40   ` Peter Maydell
  2017-06-20 13:54     ` Alex Bennée
  2017-06-29 15:45     ` Alex Bennée
  0 siblings, 2 replies; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 13:40 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, 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>
>
> ---
> v5
>   - re-base without tab/format cleanps
> v4
>   - split header code
>   - fix formatting foo-bar's
> v3
>   - new for v3
>   - arm, aarch64, ppc64
> ---
>  reginfo.c | 39 +++++++++++++++++++--------------------
>  risu.c    | 23 +++++++++++++++++++++--
>  risu.h    | 11 +++++++++--
>  3 files changed, 49 insertions(+), 24 deletions(-)
>
> diff --git a/reginfo.c b/reginfo.c
> index 31bb99f..90cea8f 100644
> --- a/reginfo.c
> +++ b/reginfo.c
> @@ -21,7 +21,7 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
>  static int mem_used;
>  static int packet_mismatch;
>
> -int send_register_info(int sock, void *uc)
> +int send_register_info(write_fn write_fn, void *uc)
>  {
>      struct reginfo ri;
>      int op;
> @@ -29,24 +29,25 @@ int send_register_info(int sock, void *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));
> +        write_fn(&ri, sizeof(ri));
> +        return 1;

Why the change from "return write function's return value"
to "ignore return value and always return 1" for OP_TESTEND ?

>      case OP_COMPAREMEM:
> -        return send_data_pkt(sock, memblock, MEMBLOCKLEN);
> +        return write_fn(memblock, MEMBLOCKLEN);
>          break;

...here we still just pass the return value through, for instance.

> +    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;
>  }
> @@ -59,7 +60,7 @@ 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;
>
> @@ -73,36 +74,34 @@ int recv_and_compare_register_info(int sock, void *uc)
>          /* 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))) {
> +        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;
>          }
> -        send_response_byte(sock, resp);
> +        resp_fn(resp);
>          break;
>      case OP_SETMEMBLOCK:
> -        memblock = (void *) (uintptr_t) get_reginfo_paramreg(&master_ri);
> +        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);
> +                              (uintptr_t)memblock);
>          break;
>      case OP_COMPAREMEM:
>          mem_used = 1;
> -        if (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
> +        if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
>              packet_mismatch = 1;
>              resp = 2;
>          } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
>              /* memory mismatch */
>              resp = 2;
>          }
> -        send_response_byte(sock, resp);
> +        resp_fn(resp);
>          break;
>      }
>
> diff --git a/risu.c b/risu.c
> index a10422a..88e586c 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -37,9 +37,28 @@ sigjmp_buf jmpbuf;
>  /* Should we test for FP exception status bits? */
>  int test_fp_exc;
>
> +/* 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 */
>          advance_pc(uc);
> @@ -52,7 +71,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 */
>          advance_pc(uc);
> diff --git a/risu.h b/risu.h
> index 78a7313..32241bc 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
> + */

Stale comment? The calling code is not per-arch any more.

> +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
>

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive functions
  2017-06-20 13:40   ` Peter Maydell
@ 2017-06-20 13:54     ` Alex Bennée
  2017-06-29 15:45     ` Alex Bennée
  1 sibling, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2017-06-20 13:54 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers


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

> On 19 June 2017 at 11:46, 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>
>>
>> ---
>> v5
>>   - re-base without tab/format cleanps
>> v4
>>   - split header code
>>   - fix formatting foo-bar's
>> v3
>>   - new for v3
>>   - arm, aarch64, ppc64
>> ---
>>  reginfo.c | 39 +++++++++++++++++++--------------------
>>  risu.c    | 23 +++++++++++++++++++++--
>>  risu.h    | 11 +++++++++--
>>  3 files changed, 49 insertions(+), 24 deletions(-)
>>
>> diff --git a/reginfo.c b/reginfo.c
>> index 31bb99f..90cea8f 100644
>> --- a/reginfo.c
>> +++ b/reginfo.c
>> @@ -21,7 +21,7 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
>>  static int mem_used;
>>  static int packet_mismatch;
>>
>> -int send_register_info(int sock, void *uc)
>> +int send_register_info(write_fn write_fn, void *uc)
>>  {
>>      struct reginfo ri;
>>      int op;
>> @@ -29,24 +29,25 @@ int send_register_info(int sock, void *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));
>> +        write_fn(&ri, sizeof(ri));
>> +        return 1;
>
> Why the change from "return write function's return value"
> to "ignore return value and always return 1" for OP_TESTEND ?

Hmm possibly a re-base snaggle. I'll fix it up.

>
>>      case OP_COMPAREMEM:
>> -        return send_data_pkt(sock, memblock, MEMBLOCKLEN);
>> +        return write_fn(memblock, MEMBLOCKLEN);
>>          break;
>
> ...here we still just pass the return value through, for instance.
>
>> +    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;
>>  }
>> @@ -59,7 +60,7 @@ 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;
>>
>> @@ -73,36 +74,34 @@ int recv_and_compare_register_info(int sock, void *uc)
>>          /* 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))) {
>> +        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;
>>          }
>> -        send_response_byte(sock, resp);
>> +        resp_fn(resp);
>>          break;
>>      case OP_SETMEMBLOCK:
>> -        memblock = (void *) (uintptr_t) get_reginfo_paramreg(&master_ri);
>> +        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);
>> +                              (uintptr_t)memblock);
>>          break;
>>      case OP_COMPAREMEM:
>>          mem_used = 1;
>> -        if (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
>> +        if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
>>              packet_mismatch = 1;
>>              resp = 2;
>>          } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
>>              /* memory mismatch */
>>              resp = 2;
>>          }
>> -        send_response_byte(sock, resp);
>> +        resp_fn(resp);
>>          break;
>>      }
>>
>> diff --git a/risu.c b/risu.c
>> index a10422a..88e586c 100644
>> --- a/risu.c
>> +++ b/risu.c
>> @@ -37,9 +37,28 @@ sigjmp_buf jmpbuf;
>>  /* Should we test for FP exception status bits? */
>>  int test_fp_exc;
>>
>> +/* 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 */
>>          advance_pc(uc);
>> @@ -52,7 +71,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 */
>>          advance_pc(uc);
>> diff --git a/risu.h b/risu.h
>> index 78a7313..32241bc 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
>> + */
>
> Stale comment? The calling code is not per-arch any more.

Yes, I'll remove

>
>> +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
>>
>
> thanks
> -- PMM


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 08/13] risu: add header to trace stream
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 08/13] risu: add header to trace stream Alex Bennée
@ 2017-06-20 13:55   ` Peter Maydell
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 13:55 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
> 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>
>
> ---
> v5
>   - re-base without formatting fixes
>   - dropped fprintfs in signal context
> v4
>   - split from previous patch
> ---
>  reginfo.c      | 94 +++++++++++++++++++++++++++++++++++++---------------------
>  risu.h         |  9 ++++++
>  risu_aarch64.c |  5 ++++
>  risu_arm.c     |  5 ++++
>  risu_m68k.c    |  5 ++++
>  risu_ppc64.c   |  5 ++++
>  6 files changed, 89 insertions(+), 34 deletions(-)
>
> diff --git a/reginfo.c b/reginfo.c
> index 90cea8f..4ff937f 100644
> --- a/reginfo.c
> +++ b/reginfo.c
> @@ -24,10 +24,19 @@ static int packet_mismatch;
>  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) {
> +        return -1;
> +    }
> +
>      switch (op) {
>      case OP_TESTEND:
>          write_fn(&ri, sizeof(ri));
> @@ -63,46 +72,63 @@ 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;
> +    if (read_fn(&header, sizeof(header)) != 0) {
> +        return -1;
> +    }
> +
> +    if (header.risu_op == op ) {

Stray extra space.

> +
> +        /* 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 {
> +        /* We are out of sync */
> +        resp = 2;
>          resp_fn(resp);
> -        break;

This is probably easier to read if structured the other way
up, so:

     if (header.risu_op != op) {
        /* out of sync */
        ...
        return resp;
     }

and then you can leave the rest of the function alone.


>      }
>
>      return resp;
> diff --git a/risu.h b/risu.h
> index 32241bc..e9e70ab 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 9c6809d..492d141 100644
> --- a/risu_aarch64.c
> +++ b/risu_aarch64.c
> @@ -40,3 +40,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;

Indent here (and in some of the other versions of this
function) isn't matching the standard set up in earlier patches.

> +}
> diff --git a/risu_arm.c b/risu_arm.c
> index a55c6c2..5fcb2a5 100644
> --- a/risu_arm.c
> +++ b/risu_arm.c
> @@ -68,3 +68,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 0bf5c14..1056eef 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 eb60573..83f8d1f 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;
> +}
> --

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

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 09/13] risu: add simple trace and replay support
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 09/13] risu: add simple trace and replay support Alex Bennée
@ 2017-06-20 14:06   ` Peter Maydell
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 14:06 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, 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>

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

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 04/13] build-all-archs: support --static flag
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 04/13] build-all-archs: support --static flag Alex Bennée
@ 2017-06-20 14:07   ` Peter Maydell
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 14:07 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v5
>   - swap with docker patch so later can be dropped if not wanted
> ---

Applied to master, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 05/13] build-all-archs: support cross building via docker
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 05/13] build-all-archs: support cross building via docker Alex Bennée
@ 2017-06-20 14:09   ` Peter Maydell
  2017-06-20 14:57     ` Alex Bennée
  0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 14:09 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
> 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 --use-docker
Got permission denied while trying to connect to the Docker daemon
socket at unix:///var/run/docker.sock: Get
http://%2Fvar%2Frun%2Fdocker.sock/v1.27/images/json?filters=%7B%22reference%22%3A%7B%22qemu%22%3Atrue%7D%7D:
dial unix /var/run/docker.sock: connect: permission denied

but maybe my local docker setup is just broken?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 13/13] new: run_risu.sh script
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 13/13] new: run_risu.sh script Alex Bennée
@ 2017-06-20 14:11   ` Peter Maydell
  2017-06-20 14:57     ` Alex Bennée
  0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 14:11 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
> 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>
>
> ---
> v5
>   - author, license, usage header
> v3
>   - tweak to allow specifying RISU binary
> ---
>  run_risu.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100755 run_risu.sh
>
> diff --git a/run_risu.sh b/run_risu.sh

I think this should go into contrib/.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 12/13] new: record_traces.sh helper script
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 12/13] new: record_traces.sh helper script Alex Bennée
@ 2017-06-20 14:12   ` Peter Maydell
  2017-06-20 14:57     ` Alex Bennée
  0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 14:12 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, 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>
>
> ---
> v5
>   - author, license and usage header
> v3
>   - allow overriding of RISU binary
> ---
>  record_traces.sh | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100755 record_traces.sh

Another one for contrib/, I think.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 10/13] risu: handle trace through stdin/stdout
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 10/13] risu: handle trace through stdin/stdout Alex Bennée
@ 2017-06-20 14:14   ` Peter Maydell
  2017-06-20 14:58     ` Alex Bennée
  0 siblings, 1 reply; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 14:14 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
> Trace files can get quite large so it would be useful to be able to
> just capture the trace stream with stdin/stdout for processing in a
> pipe line. The sort of case where this is useful is for building
> static binaries where zlib support is missing for whatever reason.
>
> It can also be used in testing pipelines.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  risu.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/risu.c b/risu.c
> index 93c274b..e94b54b 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -312,7 +312,11 @@ int main(int argc, char **argv)
>
>      if (ismaster) {
>          if (trace) {
> -            master_fd = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
> +            if (strncmp(trace_fn, "-", strlen(trace_fn))==0) {

Why not just strcmp() ?

> +                master_fd = fileno(stdout);

   = STDOUT_FILENO;

> +            } else {
> +                master_fd = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
> +            }
>          } else {
>              fprintf(stderr, "master port %d\n", port);
>              master_fd = master_connect(port);
> @@ -320,7 +324,11 @@ int main(int argc, char **argv)
>          return master();
>      } else {
>          if (trace) {
> -            apprentice_fd = open(trace_fn, O_RDONLY);
> +            if (strncmp(trace_fn, "-", strlen(trace_fn))==0) {
> +                apprentice_fd = fileno(stdin);

 = STDIN_FILENO;

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

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 11/13] risu: add support compressed tracefiles
  2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 11/13] risu: add support compressed tracefiles Alex Bennée
@ 2017-06-20 14:15   ` Peter Maydell
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 14:15 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 19 June 2017 at 11:46, 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>

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

(it would be nice if we could just mandate gzip, but
we could spend a lot of time messing about here that
doesn't seem worthwhile.)

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 03/13] all: fix up code consitency
  2017-06-20 13:13   ` Peter Maydell
@ 2017-06-20 14:16     ` Peter Maydell
  0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2017-06-20 14:16 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers

On 20 June 2017 at 14:13, Peter Maydell <peter.maydell@linaro.org> wrote:
> Something seems to have gone wrong in this file -- note the extra indent
> of {} blocks after "case 0:". The others are all fine though so I'll
> just fix this locally.

...fixed up version applied to risu master.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 05/13] build-all-archs: support cross building via docker
  2017-06-20 14:09   ` Peter Maydell
@ 2017-06-20 14:57     ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2017-06-20 14:57 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers


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

> On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
>> 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 --use-docker
> Got permission denied while trying to connect to the Docker daemon
> socket at unix:///var/run/docker.sock: Get
> http://%2Fvar%2Frun%2Fdocker.sock/v1.27/images/json?filters=%7B%22reference%22%3A%7B%22qemu%22%3Atrue%7D%7D:
> dial unix /var/run/docker.sock: connect: permission denied
>
> but maybe my local docker setup is just broken?

Generally you need to add yourself to the "docker" group to have access.
This isn't recommended in production but the alternative is to preface
all docker calls with sudo and set up permissions that way. The official
docs discuss it here:

  https://docs.docker.com/engine/installation/linux/linux-postinstall/

The advice that "only trusted users should be allowed to control your
Docker daemon." is valid but irrelevant to developers who generally have
complete control of their own box.

>
> thanks
> -- PMM


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 13/13] new: run_risu.sh script
  2017-06-20 14:11   ` Peter Maydell
@ 2017-06-20 14:57     ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2017-06-20 14:57 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers


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

> On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
>> 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>
>>
>> ---
>> v5
>>   - author, license, usage header
>> v3
>>   - tweak to allow specifying RISU binary
>> ---
>>  run_risu.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 66 insertions(+)
>>  create mode 100755 run_risu.sh
>>
>> diff --git a/run_risu.sh b/run_risu.sh
>
> I think this should go into contrib/.

OK will do.

>
> thanks
> -- PMM


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 12/13] new: record_traces.sh helper script
  2017-06-20 14:12   ` Peter Maydell
@ 2017-06-20 14:57     ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2017-06-20 14:57 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers


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

> On 19 June 2017 at 11:46, 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>
>>
>> ---
>> v5
>>   - author, license and usage header
>> v3
>>   - allow overriding of RISU binary
>> ---
>>  record_traces.sh | 32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>>  create mode 100755 record_traces.sh
>
> Another one for contrib/, I think.

OK will do.

>
> thanks
> -- PMM


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 10/13] risu: handle trace through stdin/stdout
  2017-06-20 14:14   ` Peter Maydell
@ 2017-06-20 14:58     ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2017-06-20 14:58 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers


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

> On 19 June 2017 at 11:46, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Trace files can get quite large so it would be useful to be able to
>> just capture the trace stream with stdin/stdout for processing in a
>> pipe line. The sort of case where this is useful is for building
>> static binaries where zlib support is missing for whatever reason.
>>
>> It can also be used in testing pipelines.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  risu.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/risu.c b/risu.c
>> index 93c274b..e94b54b 100644
>> --- a/risu.c
>> +++ b/risu.c
>> @@ -312,7 +312,11 @@ int main(int argc, char **argv)
>>
>>      if (ismaster) {
>>          if (trace) {
>> -            master_fd = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
>> +            if (strncmp(trace_fn, "-", strlen(trace_fn))==0) {
>
> Why not just strcmp() ?

Years of habit...

>
>> +                master_fd = fileno(stdout);
>
>    = STDOUT_FILENO;

heh , the original version used that. I'll put it back.

>
>> +            } else {
>> +                master_fd = open(trace_fn, O_WRONLY|O_CREAT, S_IRWXU);
>> +            }
>>          } else {
>>              fprintf(stderr, "master port %d\n", port);
>>              master_fd = master_connect(port);
>> @@ -320,7 +324,11 @@ int main(int argc, char **argv)
>>          return master();
>>      } else {
>>          if (trace) {
>> -            apprentice_fd = open(trace_fn, O_RDONLY);
>> +            if (strncmp(trace_fn, "-", strlen(trace_fn))==0) {
>> +                apprentice_fd = fileno(stdin);
>
>  = STDIN_FILENO;
>
>> +            } else {
>> +                apprentice_fd = open(trace_fn, O_RDONLY);
>> +            }
>>          } else {
>>              fprintf(stderr, "apprentice host %s port %d\n", hostname, port);
>>              apprentice_fd = apprentice_connect(hostname, port);
>> --
>> 2.13.0
>>
>
> thanks
> -- PMM


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive functions
  2017-06-20 13:40   ` Peter Maydell
  2017-06-20 13:54     ` Alex Bennée
@ 2017-06-29 15:45     ` Alex Bennée
  1 sibling, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2017-06-29 15:45 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers


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

> On 19 June 2017 at 11:46, 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>
>>
>> ---
>> v5
>>   - re-base without tab/format cleanps
>> v4
>>   - split header code
>>   - fix formatting foo-bar's
>> v3
>>   - new for v3
>>   - arm, aarch64, ppc64
>> ---
>>  reginfo.c | 39 +++++++++++++++++++--------------------
>>  risu.c    | 23 +++++++++++++++++++++--
>>  risu.h    | 11 +++++++++--
>>  3 files changed, 49 insertions(+), 24 deletions(-)
>>
>> diff --git a/reginfo.c b/reginfo.c
>> index 31bb99f..90cea8f 100644
>> --- a/reginfo.c
>> +++ b/reginfo.c
>> @@ -21,7 +21,7 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
>>  static int mem_used;
>>  static int packet_mismatch;
>>
>> -int send_register_info(int sock, void *uc)
>> +int send_register_info(write_fn write_fn, void *uc)
>>  {
>>      struct reginfo ri;
>>      int op;
>> @@ -29,24 +29,25 @@ int send_register_info(int sock, void *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));
>> +        write_fn(&ri, sizeof(ri));
>> +        return 1;
>
> Why the change from "return write function's return value"
> to "ignore return value and always return 1" for OP_TESTEND ?

And now I remember why ;-)

As write_fn may no longer return a ack code from the apprentice we need
to ensure we always respond here. There is a bit of discrepancy here
between send_data_pkt which returns a result from afar and write_trace
which returns 0 when bytes_written == what we wanted to write.

>
>>      case OP_COMPAREMEM:
>> -        return send_data_pkt(sock, memblock, MEMBLOCKLEN);
>> +        return write_fn(memblock, MEMBLOCKLEN);
>>          break;
>
> ...here we still just pass the return value through, for instance.
>
>> +    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;
>>  }
>> @@ -59,7 +60,7 @@ 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;
>>
>> @@ -73,36 +74,34 @@ int recv_and_compare_register_info(int sock, void *uc)
>>          /* 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))) {
>> +        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;
>>          }
>> -        send_response_byte(sock, resp);
>> +        resp_fn(resp);
>>          break;
>>      case OP_SETMEMBLOCK:
>> -        memblock = (void *) (uintptr_t) get_reginfo_paramreg(&master_ri);
>> +        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);
>> +                              (uintptr_t)memblock);
>>          break;
>>      case OP_COMPAREMEM:
>>          mem_used = 1;
>> -        if (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
>> +        if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
>>              packet_mismatch = 1;
>>              resp = 2;
>>          } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
>>              /* memory mismatch */
>>              resp = 2;
>>          }
>> -        send_response_byte(sock, resp);
>> +        resp_fn(resp);
>>          break;
>>      }
>>
>> diff --git a/risu.c b/risu.c
>> index a10422a..88e586c 100644
>> --- a/risu.c
>> +++ b/risu.c
>> @@ -37,9 +37,28 @@ sigjmp_buf jmpbuf;
>>  /* Should we test for FP exception status bits? */
>>  int test_fp_exc;
>>
>> +/* 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 */
>>          advance_pc(uc);
>> @@ -52,7 +71,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 */
>>          advance_pc(uc);
>> diff --git a/risu.h b/risu.h
>> index 78a7313..32241bc 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
>> + */
>
> Stale comment? The calling code is not per-arch any more.
>
>> +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
>>
>
> thanks
> -- PMM


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2017-06-29 15:45 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-19 10:46 [Qemu-devel] [RISU PATCH v5 00/13] RISU record/replay patches Alex Bennée
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 01/13] .gitignore: ignore build directories Alex Bennée
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 02/13] README: document the coding style used for risu Alex Bennée
2017-06-20 12:54   ` Peter Maydell
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 03/13] all: fix up code consitency Alex Bennée
2017-06-20 13:13   ` Peter Maydell
2017-06-20 14:16     ` Peter Maydell
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 04/13] build-all-archs: support --static flag Alex Bennée
2017-06-20 14:07   ` Peter Maydell
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 05/13] build-all-archs: support cross building via docker Alex Bennée
2017-06-20 14:09   ` Peter Maydell
2017-06-20 14:57     ` Alex Bennée
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 06/13] risu: a bit more verbosity when starting Alex Bennée
2017-06-20 13:33   ` Peter Maydell
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive functions Alex Bennée
2017-06-20 13:40   ` Peter Maydell
2017-06-20 13:54     ` Alex Bennée
2017-06-29 15:45     ` Alex Bennée
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 08/13] risu: add header to trace stream Alex Bennée
2017-06-20 13:55   ` Peter Maydell
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 09/13] risu: add simple trace and replay support Alex Bennée
2017-06-20 14:06   ` Peter Maydell
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 10/13] risu: handle trace through stdin/stdout Alex Bennée
2017-06-20 14:14   ` Peter Maydell
2017-06-20 14:58     ` Alex Bennée
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 11/13] risu: add support compressed tracefiles Alex Bennée
2017-06-20 14:15   ` Peter Maydell
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 12/13] new: record_traces.sh helper script Alex Bennée
2017-06-20 14:12   ` Peter Maydell
2017-06-20 14:57     ` Alex Bennée
2017-06-19 10:46 ` [Qemu-devel] [RISU PATCH v5 13/13] new: run_risu.sh script Alex Bennée
2017-06-20 14:11   ` Peter Maydell
2017-06-20 14:57     ` Alex Bennée

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.