Compilation failure

Hi Stefan,
I think this needs to be resolved.

CC trace.o
CC simpletrace.o
cc1: warnings being treated as errors /home/prerna/qemu-testing/git/qemu/simpletrace.c: In function ‘write_header’:
/home/prerna/qemu-testing/git/qemu/simpletrace.c:46: error: integer constant is too large for ‘long’ type /home/prerna/qemu-testing/git/qemu/simpletrace.c:46: error: large integer implicitly truncated to unsigned type
make: *** [simpletrace.o] Error 1

The error arises due to :

TraceRecord header = {
        .event = -1UL, /* max avoids conflicting with TraceEventIDs */
        .timestamp_ns = 0xf2b177cb0aa429b4, /* magic number */
			^^^^^^^^ error.

Also, it would be better to #define the magic number to some macro, and use that instead of using the constant directly.

Explicitly use 64-bit fields in trace records so that timestamps and magic numbers work for 32-bit host builds.

Signed-off-by Stefan Hajnoczi<[email protected]>

---
  simpletrace.c  |   31 +++++++++++++++++++++----------
  simpletrace.h  |   11 ++++++-----
  simpletrace.py |    2 +-
  tracetool      |    6 +++---
  4 files changed, 31 insertions(+), 19 deletions(-)

simpletrace.c +21 -10

index 954cc4e..01acfc5 100644
@@ -9,18 +9,29 @@
   */
 
  #include<stdlib.h>
+#include<stdint.h>
  #include<stdio.h>
  #include<time.h>
  #include "trace.h"
 
+/** Trace file header event ID */
+#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with TraceEventIDs */
+
+/** Trace file magic number */
+#define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
+
+/** Trace file version number, bump if format changes */
+#define HEADER_VERSION 0
+
+/** Trace buffer entry */
  typedef struct {
-    unsigned long event;
-    unsigned long timestamp_ns;
-    unsigned long x1;
-    unsigned long x2;
-    unsigned long x3;
-    unsigned long x4;
-    unsigned long x5;
+    uint64_t event;
+    uint64_t timestamp_ns;
+    uint64_t x1;
+    uint64_t x2;
+    uint64_t x3;
+    uint64_t x4;
+    uint64_t x5;
  } TraceRecord;
 
  enum {

@@ -42,9 +53,9 @@ void st_print_trace_file_status(FILE *stream, int (*stream_printf)(FILE *stream,
  static bool write_header(FILE *fp)
  {
      TraceRecord header = {
-        .event = -1UL, /* max avoids conflicting with TraceEventIDs */
-        .timestamp_ns = 0xf2b177cb0aa429b4, /* magic number */
-        .x1 = 0, /* bump this version number if file format changes */
+        .event = HEADER_EVENT_ID,
+        .timestamp_ns = HEADER_MAGIC,
+        .x1 = HEADER_VERSION,
      };
 
      return fwrite(&header, sizeof header, 1, fp) == 1;

simpletrace.h +0 -0

index 6a2b8d9..f81aa8e 100644

simpletrace.h +5 -5

@@ -20,11 +21,11 @@ typedef struct {
  } TraceEvent;
 
  void trace0(TraceEventID event);
-void trace1(TraceEventID event, unsigned long x1);
-void trace2(TraceEventID event, unsigned long x1, unsigned long x2);
-void trace3(TraceEventID event, unsigned long x1, unsigned long x2, unsigned long x3);
-void trace4(TraceEventID event, unsigned long x1, unsigned long x2, unsigned long x3, unsigned long x4);
-void trace5(TraceEventID event, unsigned long x1, unsigned long x2, unsigned long x3, unsigned long x4, unsigned long x5);
+void trace1(TraceEventID event, uint64_t x1);
+void trace2(TraceEventID event, uint64_t x1, uint64_t x2);
+void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3);
+void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4);
+void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5);
  void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
  void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...));
  void st_change_trace_event_state(const char *tname, bool tstate);

simpletrace.py +1 -1

index 979d911..fdf0eb5 100755
@@ -17,7 +17,7 @@ header_event_id = 0xffffffffffffffff
  header_magic    = 0xf2b177cb0aa429b4
  header_version  = 0
 
-trace_fmt = 'LLLLLLL'
+trace_fmt = '=QQQQQQQ'
  trace_len = struct.calcsize(trace_fmt)
  event_re  = re.compile(r'(disable\s+)?([a-zA-Z0-9_]+)\(([^)]*)\)\s+"([^"]*)"')
 

tracetool +0 -0

index c5a5bdc..b78cd97 100755

tracetool +1 -1

      done
  }
 

@@ -173,7 +173,7 @@ linetoh_simple()
      trace_args="$simple_event_num"
      if [ "$argc" -gt 0 ]
      then
-        trace_args="$trace_args, $(cast_args_to_ulong "$1")"
+        trace_args="$trace_args, $(cast_args_to_uint64_t "$1")"
      fi
 
      cat<<EOF

Quoted 72 lines · Stefan Hajnoczi

Explicitly use 64-bit fields in trace records so that timestamps and magic numbers work for 32-bit host builds.

Signed-off-by Stefan Hajnoczi<[email protected]>

---
  simpletrace.c  |   31 +++++++++++++++++++++----------
  simpletrace.h  |   11 ++++++-----
  simpletrace.py |    2 +-
  tracetool      |    6 +++---
  4 files changed, 31 insertions(+), 19 deletions(-)

simpletrace.c +3 -0

index 954cc4e..01acfc5 100644
@@ -9,18 +9,29 @@
   */
 
  #include<stdlib.h>
+#include<stdint.h>
  #include<stdio.h>
  #include<time.h>
  #include "trace.h"
 
+/** Trace file header event ID */
+#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with

TraceEventIDs */

simpletrace.c +18 -10

+
+/** Trace file magic number */
+#define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
+
+/** Trace file version number, bump if format changes */
+#define HEADER_VERSION 0
+
+/** Trace buffer entry */
  typedef struct {
-    unsigned long event;
-    unsigned long timestamp_ns;
-    unsigned long x1;
-    unsigned long x2;
-    unsigned long x3;
-    unsigned long x4;
-    unsigned long x5;
+    uint64_t event;
+    uint64_t timestamp_ns;
+    uint64_t x1;
+    uint64_t x2;
+    uint64_t x3;
+    uint64_t x4;
+    uint64_t x5;
  } TraceRecord;
 
  enum {

@@ -42,9 +53,9 @@ void st_print_trace_file_status(FILE *stream, int
 (*stream_printf)(FILE *stream,
  static bool write_header(FILE *fp)
  {
      TraceRecord header = {
-        .event = -1UL, /* max avoids conflicting with TraceEventIDs */
-        .timestamp_ns = 0xf2b177cb0aa429b4, /* magic number */
-        .x1 = 0, /* bump this version number if file format changes */
+        .event = HEADER_EVENT_ID,
+        .timestamp_ns = HEADER_MAGIC,
+        .x1 = HEADER_VERSION,
      };
 

      return fwrite(&header, sizeof header, 1, fp) == 1;

simpletrace.h +0 -0

index 6a2b8d9..f81aa8e 100644

simpletrace.h line 16

Quoted 7 lines

simpletrace.h +1 -0

@@ -10,6 +10,7 @@
  #define SIMPLETRACE_H
 
  #include<stdbool.h>
+#include<stdint.h>
  #include<stdio.h>
 
  typedef unsigned int TraceEventID;

It would be useful to have :

typedef uint64_t TraceEventID;

This ensures that the maximum number of trace events available on both 32 and 64 bit builds is same.

Quoted 46 lines

simpletrace.h +4 -5

@@ -20,11 +21,11 @@ typedef struct {
  } TraceEvent;
 
  void trace0(TraceEventID event);
-void trace1(TraceEventID event, unsigned long x1);
-void trace2(TraceEventID event, unsigned long x1, unsigned long x2);
-void trace3(TraceEventID event, unsigned long x1, unsigned long x2,
 unsigned long x3);
-void trace4(TraceEventID event, unsigned long x1, unsigned long x2,
 unsigned long x3, unsigned long x4);
-void trace5(TraceEventID event, unsigned long x1, unsigned long x2,
 unsigned long x3, unsigned long x4, unsigned long x5);
+void trace1(TraceEventID event, uint64_t x1);
+void trace2(TraceEventID event, uint64_t x1, uint64_t x2);
+void trace3(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3);
+void trace4(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3,

uint64_t x4);
+void trace5(TraceEventID event, uint64_t x1, uint64_t x2, uint64_t x3, uint64_t x4, uint64_t x5);
void st_print_trace(FILE *stream, int (*stream_printf)(FILE *stream, const
char *fmt, ...));
void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE
*stream, const char *fmt, ...));
void st_change_trace_event_state(const char *tname, bool tstate);

simpletrace.py +1 -1

index 979d911..fdf0eb5 100755
@@ -17,7 +17,7 @@ header_event_id = 0xffffffffffffffff
  header_magic    = 0xf2b177cb0aa429b4
  header_version  = 0
 
-trace_fmt = 'LLLLLLL'
+trace_fmt = '=QQQQQQQ'
  trace_len = struct.calcsize(trace_fmt)
  event_re  =
 re.compile(r'(disable\s+)?([a-zA-Z0-9_]+)\(([^)]*)\)\s+"([^"]*)"')

tracetool +0 -0

index c5a5bdc..b78cd97 100755
@@ -151,11 +151,11 @@ EOF
      simple_event_num=0
  }
 

tracetool EOF · line 158

Quoted 7 lines

tracetool +2 -2

-cast_args_to_ulong()
+cast_args_to_uint64_t()
  {
      local arg
      for arg in $(get_argnames "$1"); do
-        echo -n "(unsigned long)$arg"
+        echo -n "(uint64_t)$arg"

Tested this on a 32 bit host. It throws up some warnings, and we need :
echo -n "(uint64_t)(uintptr_t)$arg"

Quoted 13 lines

tracetool +1 -1

      done
  }
 

@@ -173,7 +173,7 @@ linetoh_simple()
      trace_args="$simple_event_num"
      if [ "$argc" -gt 0 ]
      then
-        trace_args="$trace_args, $(cast_args_to_ulong "$1")"
+        trace_args="$trace_args, $(cast_args_to_uint64_t "$1")"
      fi
 
      cat<<EOF