From be0aa7ac8990c7d62f2d41a66e8c50784e7ee9ac Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 13 Feb 2018 14:00:29 +0000 Subject: log-for-trace.h: Split out parts of log.h used by trace.h A persistent build problem we see is where a source file accidentally omits the #include of log.h. This slips through local developer testing because if you configure with the default (log) trace backend trace.h will pull in log.h for you. Compilation fails only if some other backend is selected. To make this error cause a compile failure regardless of the configured trace backend, split out the parts of log.h that trace.h requires into a new log-for-trace.h header. Since almost all manual uses of the log.h functions will use constants or functions which aren't in log-for-trace.h, this will let us catch missing #include "qemu/log.h" more consistently. Signed-off-by: Peter Maydell Reviewed-by: Eric Blake Reviewed-by: Richard Henderson Message-id: 20180213140029.8308-1-peter.maydell@linaro.org Signed-off-by: Stefan Hajnoczi --- scripts/tracetool/backend/log.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'scripts') diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py index da86f6b882..78933d03ad 100644 --- a/scripts/tracetool/backend/log.py +++ b/scripts/tracetool/backend/log.py @@ -20,7 +20,7 @@ PUBLIC = True def generate_h_begin(events, group): - out('#include "qemu/log.h"', + out('#include "qemu/log-for-trace.h"', '') @@ -35,14 +35,13 @@ def generate_h(event, group): else: cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) - out(' if (%(cond)s) {', + out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', ' struct timeval _now;', ' gettimeofday(&_now, NULL);', - ' qemu_log_mask(LOG_TRACE,', - ' "%%d@%%zd.%%06zd:%(name)s " %(fmt)s "\\n",', - ' getpid(),', - ' (size_t)_now.tv_sec, (size_t)_now.tv_usec', - ' %(argnames)s);', + ' qemu_log("%%d@%%zd.%%06zd:%(name)s " %(fmt)s "\\n",', + ' getpid(),', + ' (size_t)_now.tv_sec, (size_t)_now.tv_usec', + ' %(argnames)s);', ' }', cond=cond, name=event.name, -- cgit v1.2.3 From e42860ae836dc6f768eff2d51223c47103d2dc3b Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 22 Feb 2018 16:39:01 +0000 Subject: simpletrace: fix timestamp argument type The timestamp argument to a trace event method is documented as follows: The method can also take a timestamp argument before the trace event arguments: def runstate_set(self, timestamp, new_state): ... Timestamps have the uint64_t type and are in nanoseconds. In reality methods with a timestamp argument actually receive a tuple like (123456789,) as the timestamp argument. This is due to a bug in simpletrace.py. This patch unpacks the tuple so that methods receive the correct timestamp argument type. Signed-off-by: Stefan Hajnoczi Message-id: 20180222163901.14095-1-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi --- scripts/simpletrace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts') diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py index a3a6315055..be3d1affaf 100755 --- a/scripts/simpletrace.py +++ b/scripts/simpletrace.py @@ -199,7 +199,7 @@ def process(events, log, analyzer, read_header=True): fn_argcount = len(inspect.getargspec(fn)[0]) - 1 if fn_argcount == event_argcount + 1: # Include timestamp as first argument - return lambda _, rec: fn(*((rec[1:2],) + rec[3:3 + event_argcount])) + return lambda _, rec: fn(*(rec[1:2] + rec[3:3 + event_argcount])) elif fn_argcount == event_argcount + 2: # Include timestamp and pid return lambda _, rec: fn(*rec[1:3 + event_argcount]) -- cgit v1.2.3 From 86b5aacfb972ffe0fa5fac6028e9f0bc61050dda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Tue, 6 Mar 2018 15:46:50 +0000 Subject: trace: include filename when printing parser error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improves error messages from: ValueError: Error on line 72: need more than 1 value to unpack To ValueError: Error at /home/berrange/src/virt/qemu/trace-events:72: need more than 1 value to unpack Signed-off-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Message-id: 20180306154650.24075-1-berrange@redhat.com Signed-off-by: Stefan Hajnoczi --- scripts/simpletrace.py | 4 ++-- scripts/tracetool.py | 2 +- scripts/tracetool/__init__.py | 6 ++++-- 3 files changed, 7 insertions(+), 5 deletions(-) (limited to 'scripts') diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py index be3d1affaf..9d45c6ba4e 100755 --- a/scripts/simpletrace.py +++ b/scripts/simpletrace.py @@ -168,7 +168,7 @@ class Analyzer(object): def process(events, log, analyzer, read_header=True): """Invoke an analyzer on each event in a log.""" if isinstance(events, str): - events = read_events(open(events, 'r')) + events = read_events(open(events, 'r'), events) if isinstance(log, str): log = open(log, 'rb') @@ -233,7 +233,7 @@ def run(analyzer): '\n' % sys.argv[0]) sys.exit(1) - events = read_events(open(sys.argv[1], 'r')) + events = read_events(open(sys.argv[1], 'r'), sys.argv[1]) process(events, sys.argv[2], analyzer, read_header=read_header) if __name__ == '__main__': diff --git a/scripts/tracetool.py b/scripts/tracetool.py index c55a21518b..fe2b0771f2 100755 --- a/scripts/tracetool.py +++ b/scripts/tracetool.py @@ -142,7 +142,7 @@ def main(args): events = [] for arg in args: with open(arg, "r") as fh: - events.extend(tracetool.read_events(fh)) + events.extend(tracetool.read_events(fh, arg)) try: tracetool.generate(events, arg_group, arg_format, arg_backends, diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py index 3646c2b9fc..4236062650 100644 --- a/scripts/tracetool/__init__.py +++ b/scripts/tracetool/__init__.py @@ -291,13 +291,15 @@ class Event(object): self) -def read_events(fobj): +def read_events(fobj, fname): """Generate the output for the given (format, backends) pair. Parameters ---------- fobj : file Event description file. + fname : str + Name of event file Returns a list of Event objects """ @@ -312,7 +314,7 @@ def read_events(fobj): try: event = Event.build(line) except ValueError as e: - arg0 = 'Error on line %d: %s' % (lineno, e.args[0]) + arg0 = 'Error at %s:%d: %s' % (fname, lineno, e.args[0]) e.args = (arg0,) + e.args[1:] raise -- cgit v1.2.3 From 73ff061032efa0b260bcd1a177ac89b57a1642d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Thu, 8 Mar 2018 15:55:24 +0000 Subject: trace: only permit standard C types and fixed size integer types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some trace backends will compile code based on the declared trace events. It should not be assumed that the backends can resolve any QEMU specific typedefs. So trace events should restrict their argument types to the standard C types and fixed size integer types. Any complex pointer types can be declared as "void *" for purposes of trace events, since nothing will be dereferencing these pointer arguments. Signed-off-by: Daniel P. Berrangé Message-id: 20180308155524.5082-3-berrange@redhat.com Signed-off-by: Stefan Hajnoczi --- scripts/tracetool/__init__.py | 46 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) (limited to 'scripts') diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py index 4236062650..b20fac34a3 100644 --- a/scripts/tracetool/__init__.py +++ b/scripts/tracetool/__init__.py @@ -41,6 +41,51 @@ def out(*lines, **kwargs): lines = [ l % kwargs for l in lines ] sys.stdout.writelines("\n".join(lines) + "\n") +# We only want to allow standard C types or fixed sized +# integer types. We don't want QEMU specific types +# as we can't assume trace backends can resolve all the +# typedefs +ALLOWED_TYPES = [ + "int", + "long", + "short", + "char", + "bool", + "unsigned", + "signed", + "float", + "double", + "int8_t", + "uint8_t", + "int16_t", + "uint16_t", + "int32_t", + "uint32_t", + "int64_t", + "uint64_t", + "void", + "size_t", + "ssize_t", + "uintptr_t", + "ptrdiff_t", + # Magic substitution is done by tracetool + "TCGv", +] + +def validate_type(name): + bits = name.split(" ") + for bit in bits: + bit = re.sub("\*", "", bit) + if bit == "": + continue + if bit == "const": + continue + if bit not in ALLOWED_TYPES: + raise ValueError("Argument type '%s' is not in whitelist. " + "Only standard C types and fixed size integer " + "types should be used. struct, union, and " + "other complex pointer types should be " + "declared as 'void *'" % name) class Arguments: """Event arguments description.""" @@ -87,6 +132,7 @@ class Arguments: else: arg_type, identifier = arg.rsplit(None, 1) + validate_type(arg_type) res.append((arg_type, identifier)) return Arguments(res) -- cgit v1.2.3