[Qemu-devel] [RFC PATCH v2 30/30] trace: Force compiler warnings on trace parameter type mismatches

[Eric Blake]

Our traces are processed through a function call (even though the
function is static inline).  While we could turn up compiler
warnings to flag cases of implicit narrowing through a function
parameter (such as a caller doing foo(my_uint64_t) to a function
prototyped as void foo(int)), it would make the build very noisy
as we have a lot of code that relies on that C-mandated behavior.
Using the attribute printf marking of qemu_log_mask (when the log
trace backend is enabled) is too late to catch scalar type changes
(it does catch that the format string matches the parameters of
the trace_foo() function, but does not detect format mismatches at
the callsite).  Completely emitting the traces as a macro is a
possibility since they are already marked static inline, but doing
so in a way that does not evaluate side effects more than once, and
uses proper \ line continuation for a (lengthy) macro, is a more
invasive change to the generator than I'm willing to make.

So this patch introduces a compromise solution: almost[1] every
trace_foo() is generated both as an inline function (unchanged
from before, with the bulk of the code) and a forwarding macro (new
in this patch) that uses a dead-code call to printf to trigger
-Wformat type mismatch warnings from the compiler.  The end result
does not change the size of emitted code, but does enable us to get
better checking; particularly useful since the format string in
trace-events is distant from the callsites to trace_foo in the
normal .c files.  Note that the generated macro has to insert a
trailing "\n" to shut up gcc's warning about an empty format
string, and we have to use the gcc/clang extension of ##__VA_ARGS__
for comma elision since there are a few 0-arg traces.

[1]The macro trick is NOT done for vcpu traces, because those
trace calls are special; the format string associated with those
events is in a different order than the resulting parameters to
the function call, due to the additional logic generated for
determining whether the vcpu needs filtering.

Earlier patches show scenarios that this patch was able to catch,
most of which just needed a tweak to the parameter types declared
in trace-events, but some which were actual bugs in the code being
traced.

Note that there is a missing dependency in the makefiles;
although trace/Makefile.objs defines $(tracetool-y) to include
all .py files, the change to h.py does NOT force a rebuild
of any of the generated trace.h files; and it probably has
something to do with how our trace files are themselves
considered a prerequisite of Makefile being up-to-date.  I was
unable to figure out the Makefile magic necessary to fix the
dependency, but was able to manually work around the problem
by running

make -B block/trace.h-timestamp

which forced a rerun of the tracetool, and therefore regenerated
all of the trace.h files.

Signed-off-by: Eric Blake <address@hidden>
---

RFC for two reasons:
1. the Makefile issue documented above means that incremental
builds won't benefit from this patch without manual intervention
(fresh builds, including docker, manage to test it, though)
2. there are still failures under 'make address@hidden'
due to more type mismatches that still need to be squashed. I'm
still working on fixing those, but wanted to at least post this
series for initial review, especially so the maintainer can weigh
in on how much (or little) belongs in 2.9

 scripts/tracetool/format/h.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index 3682f4e..491fd6e 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -73,6 +73,19 @@ def generate(events, backend, group):
         out('    }',
             '}')

+        if "vcpu" not in e.properties:
+            out('',
+                '#define %(api)s(...) \\',
+                '    do { \\',
+                '        if (0) { \\',
+                '            printf(%(fmt)s "\\n", ## __VA_ARGS__); \\',
+                '        } \\',
+                '        %(api)s(__VA_ARGS__); \\',
+                '    } while (0)',
+                api=e.api(),
+                fmt=e.fmt.rstrip("\n")
+            )
+
     backend.generate_end(events, group)

     out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
-- 
2.9.3