From 6745c8a01f759b64d3c4cd1e0a69bb793cead268 Mon Sep 17 00:00:00 2001 From: Doug Evans Date: Wed, 25 Nov 2020 13:52:45 -0800 Subject: trace: Send "-d trace:help" output to stdout ... for consistency with "-d help". Signed-off-by: Doug Evans Message-id: 20201125215245.3514695-1-dje@google.com Signed-off-by: Stefan Hajnoczi --- trace/control.c | 12 ++++++------ trace/control.h | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/trace/control.c b/trace/control.c index b82fb87..cd04dd4 100644 --- a/trace/control.c +++ b/trace/control.c @@ -125,18 +125,18 @@ TraceEvent *trace_event_iter_next(TraceEventIter *iter) return NULL; } -void trace_list_events(void) +void trace_list_events(FILE *f) { TraceEventIter iter; TraceEvent *ev; trace_event_iter_init(&iter, NULL); while ((ev = trace_event_iter_next(&iter)) != NULL) { - fprintf(stderr, "%s\n", trace_event_get_name(ev)); + fprintf(f, "%s\n", trace_event_get_name(ev)); } #ifdef CONFIG_TRACE_DTRACE - fprintf(stderr, "This list of names of trace points may be incomplete " - "when using the DTrace/SystemTap backends.\n" - "Run 'qemu-trace-stap list %s' to print the full list.\n", + fprintf(f, "This list of names of trace points may be incomplete " + "when using the DTrace/SystemTap backends.\n" + "Run 'qemu-trace-stap list %s' to print the full list.\n", error_get_progname()); #endif } @@ -176,7 +176,7 @@ static void do_trace_enable_events(const char *line_buf) void trace_enable_events(const char *line_buf) { if (is_help_option(line_buf)) { - trace_list_events(); + trace_list_events(stdout); if (monitor_cur() == NULL) { exit(0); } diff --git a/trace/control.h b/trace/control.h index 05b95ea..9522a7b 100644 --- a/trace/control.h +++ b/trace/control.h @@ -201,10 +201,11 @@ void trace_fini_vcpu(CPUState *vcpu); /** * trace_list_events: + * @f: Where to send output. * * List all available events. */ -void trace_list_events(void); +void trace_list_events(FILE *f); /** * trace_enable_events: -- cgit v1.1 From c05012a365c2d7d42d205b1efa895bf2144bab88 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 27 Aug 2020 15:29:12 +0100 Subject: tracetool: add output filename command-line argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tracetool.py script writes to stdout. This means the output filename is not available to the script. Add the output filename to the command-line so that the script has access to the filename. This also simplifies the tracetool.py invocation. It's no longer necessary to use meson's custom_build(capture : true) to save output. Signed-off-by: Stefan Hajnoczi Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Message-Id: <20200827142915.108730-2-stefanha@redhat.com> --- docs/devel/tracing.txt | 3 ++- meson.build | 3 +-- scripts/tracetool.py | 12 +++++++----- scripts/tracetool/__init__.py | 18 ++++++++++++++++-- trace/meson.build | 21 +++++++-------------- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt index d216065..dba43fc 100644 --- a/docs/devel/tracing.txt +++ b/docs/devel/tracing.txt @@ -318,7 +318,8 @@ probes: --target-type system \ --target-name x86_64 \ --group=all \ - trace-events-all >qemu.stp + trace-events-all \ + qemu.stp To facilitate simple usage of systemtap where there merely needs to be printf logging of certain probes, a helper script "qemu-trace-stap" is provided. diff --git a/meson.build b/meson.build index 372576f..4a92213 100644 --- a/meson.build +++ b/meson.build @@ -1901,7 +1901,6 @@ foreach target : target_dirs custom_target(exe['name'] + stp['ext'], input: trace_events_all, output: exe['name'] + stp['ext'], - capture: true, install: stp['install'], install_dir: get_option('datadir') / 'systemtap/tapset', command: [ @@ -1910,7 +1909,7 @@ foreach target : target_dirs '--target-name=' + target_name, '--target-type=' + target_type, '--probe-prefix=qemu.' + target_type + '.' + target_name, - '@INPUT@', + '@INPUT@', '@OUTPUT@' ]) endforeach endif diff --git a/scripts/tracetool.py b/scripts/tracetool.py index 3114624..ab7653a 100755 --- a/scripts/tracetool.py +++ b/scripts/tracetool.py @@ -16,7 +16,7 @@ __email__ = "stefanha@redhat.com" import sys import getopt -from tracetool import error_write, out +from tracetool import error_write, out, out_open import tracetool.backend import tracetool.format @@ -32,7 +32,7 @@ def error_opt(msg = None): format_descr = "\n".join([ " %-15s %s" % (n, d) for n,d in tracetool.format.get_list() ]) error_write("""\ -Usage: %(script)s --format= --backends= [] +Usage: %(script)s --format= --backends= [] ... Backends: %(backends)s @@ -135,13 +135,15 @@ def main(args): if probe_prefix is None: probe_prefix = ".".join(["qemu", target_type, target_name]) - if len(args) < 1: - error_opt("missing trace-events filepath") + if len(args) < 2: + error_opt("missing trace-events and output filepaths") events = [] - for arg in args: + for arg in args[:-1]: with open(arg, "r") as fh: events.extend(tracetool.read_events(fh, arg)) + out_open(args[-1]) + try: tracetool.generate(events, arg_group, arg_format, arg_backends, binary=binary, probe_prefix=probe_prefix) diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py index 3ee54be..a601338 100644 --- a/scripts/tracetool/__init__.py +++ b/scripts/tracetool/__init__.py @@ -31,14 +31,28 @@ def error(*lines): sys.exit(1) +out_filename = '' +out_fobj = sys.stdout + +def out_open(filename): + global out_filename, out_fobj + out_filename = filename + out_fobj = open(filename, 'wt') + def out(*lines, **kwargs): """Write a set of output lines. You can use kwargs as a shorthand for mapping variables when formatting all the strings in lines. + + The 'out_filename' kwarg is automatically added with the output filename. """ - lines = [ l % kwargs for l in lines ] - sys.stdout.writelines("\n".join(lines) + "\n") + output = [] + for l in lines: + kwargs['out_filename'] = out_filename + output.append(l % kwargs) + + out_fobj.writelines("\n".join(output) + "\n") # We only want to allow standard C types or fixed sized # integer types. We don't want QEMU specific types diff --git a/trace/meson.build b/trace/meson.build index 843ea14..8569e8a 100644 --- a/trace/meson.build +++ b/trace/meson.build @@ -11,20 +11,17 @@ foreach dir : [ '.' ] + trace_events_subdirs trace_h = custom_target(fmt.format('trace', 'h'), output: fmt.format('trace', 'h'), input: trace_events_file, - command: [ tracetool, group, '--format=h', '@INPUT@' ], - capture: true) + command: [ tracetool, group, '--format=h', '@INPUT@', '@OUTPUT@' ]) genh += trace_h trace_c = custom_target(fmt.format('trace', 'c'), output: fmt.format('trace', 'c'), input: trace_events_file, - command: [ tracetool, group, '--format=c', '@INPUT@' ], - capture: true) + command: [ tracetool, group, '--format=c', '@INPUT@', '@OUTPUT@' ]) if 'CONFIG_TRACE_UST' in config_host trace_ust_h = custom_target(fmt.format('trace-ust', 'h'), output: fmt.format('trace-ust', 'h'), input: trace_events_file, - command: [ tracetool, group, '--format=ust-events-h', '@INPUT@' ], - capture: true) + command: [ tracetool, group, '--format=ust-events-h', '@INPUT@', '@OUTPUT@' ]) trace_ss.add(trace_ust_h, lttng, urcubp) genh += trace_ust_h endif @@ -33,8 +30,7 @@ foreach dir : [ '.' ] + trace_events_subdirs trace_dtrace = custom_target(fmt.format('trace-dtrace', 'dtrace'), output: fmt.format('trace-dtrace', 'dtrace'), input: trace_events_file, - command: [ tracetool, group, '--format=d', '@INPUT@' ], - capture: true) + command: [ tracetool, group, '--format=d', '@INPUT@', '@OUTPUT@' ]) trace_dtrace_h = custom_target(fmt.format('trace-dtrace', 'h'), output: fmt.format('trace-dtrace', 'h'), input: trace_dtrace, @@ -69,8 +65,7 @@ foreach d : [ gen = custom_target(d[0], output: d[0], input: meson.source_root() / 'trace-events', - command: [ tracetool, '--group=root', '--format=@0@'.format(d[1]), '@INPUT@' ], - capture: true) + command: [ tracetool, '--group=root', '--format=@0@'.format(d[1]), '@INPUT@', '@OUTPUT@' ]) specific_ss.add(gen) endforeach @@ -78,13 +73,11 @@ if 'CONFIG_TRACE_UST' in config_host trace_ust_all_h = custom_target('trace-ust-all.h', output: 'trace-ust-all.h', input: trace_events_files, - command: [ tracetool, '--group=all', '--format=ust-events-h', '@INPUT@' ], - capture: true) + command: [ tracetool, '--group=all', '--format=ust-events-h', '@INPUT@', '@OUTPUT@' ]) trace_ust_all_c = custom_target('trace-ust-all.c', output: 'trace-ust-all.c', input: trace_events_files, - command: [ tracetool, '--group=all', '--format=ust-events-c', '@INPUT@' ], - capture: true) + command: [ tracetool, '--group=all', '--format=ust-events-c', '@INPUT@', '@OUTPUT@' ]) trace_ss.add(trace_ust_all_h, trace_ust_all_c) genh += trace_ust_all_h endif -- cgit v1.1 From 294170c1ddda454f2d8de65a4a26346fb2a7f715 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 27 Aug 2020 15:29:13 +0100 Subject: tracetool: add out_lineno and out_next_lineno to out() Make the output file line number and next line number available to out(). A later patch will use this to improve error messages. Signed-off-by: Stefan Hajnoczi Reviewed-by: Peter Maydell Message-Id: <20200827142915.108730-3-stefanha@redhat.com> --- scripts/tracetool/__init__.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py index a601338..da5004e 100644 --- a/scripts/tracetool/__init__.py +++ b/scripts/tracetool/__init__.py @@ -31,6 +31,7 @@ def error(*lines): sys.exit(1) +out_lineno = 1 out_filename = '' out_fobj = sys.stdout @@ -45,12 +46,19 @@ def out(*lines, **kwargs): You can use kwargs as a shorthand for mapping variables when formatting all the strings in lines. - The 'out_filename' kwarg is automatically added with the output filename. + The 'out_lineno' kwarg is automatically added to reflect the current output + file line number. The 'out_next_lineno' kwarg is also automatically added + with the next output line number. The 'out_filename' kwarg is automatically + added with the output filename. """ + global out_lineno output = [] for l in lines: + kwargs['out_lineno'] = out_lineno + kwargs['out_next_lineno'] = out_lineno + 1 kwargs['out_filename'] = out_filename output.append(l % kwargs) + out_lineno += 1 out_fobj.writelines("\n".join(output) + "\n") -- cgit v1.1 From 4e66c9ef643b5f6bc01de33eac6fa0335e4c0753 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 27 Aug 2020 15:29:14 +0100 Subject: tracetool: add input filename and line number to Event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Store the input filename and line number in Event. A later patch will use this to improve error messages. Signed-off-by: Stefan Hajnoczi Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Message-Id: <20200827142915.108730-4-stefanha@redhat.com> --- scripts/tracetool/__init__.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py index da5004e..96b1cd6 100644 --- a/scripts/tracetool/__init__.py +++ b/scripts/tracetool/__init__.py @@ -218,6 +218,10 @@ class Event(object): Properties of the event. args : Arguments The event arguments. + lineno : int + The line number in the input file. + filename : str + The path to the input file. """ @@ -230,7 +234,7 @@ class Event(object): _VALID_PROPS = set(["disable", "tcg", "tcg-trans", "tcg-exec", "vcpu"]) - def __init__(self, name, props, fmt, args, orig=None, + def __init__(self, name, props, fmt, args, lineno, filename, orig=None, event_trans=None, event_exec=None): """ Parameters @@ -243,6 +247,10 @@ class Event(object): Event printing format string(s). args : Arguments Event arguments. + lineno : int + The line number in the input file. + filename : str + The path to the input file. orig : Event or None Original Event before transformation/generation. event_trans : Event or None @@ -255,6 +263,8 @@ class Event(object): self.properties = props self.fmt = fmt self.args = args + self.lineno = int(lineno) + self.filename = str(filename) self.event_trans = event_trans self.event_exec = event_exec @@ -276,16 +286,21 @@ class Event(object): def copy(self): """Create a new copy.""" return Event(self.name, list(self.properties), self.fmt, - self.args.copy(), self, self.event_trans, self.event_exec) + self.args.copy(), self.lineno, self.filename, + self, self.event_trans, self.event_exec) @staticmethod - def build(line_str): + def build(line_str, lineno, filename): """Build an Event instance from a string. Parameters ---------- line_str : str Line describing the event. + lineno : int + Line number in input file. + filename : str + Path to input file. """ m = Event._CRE.match(line_str) assert m is not None @@ -315,7 +330,7 @@ class Event(object): if "tcg" in props and isinstance(fmt, str): raise ValueError("Events with 'tcg' property must have two format strings") - event = Event(name, props, fmt, args) + event = Event(name, props, fmt, args, lineno, filename) # add implicit arguments when using the 'vcpu' property import tracetool.vcpu @@ -360,6 +375,8 @@ class Event(object): list(self.properties), self.fmt, self.args.transform(*trans), + self.lineno, + self.filename, self) @@ -386,7 +403,7 @@ def read_events(fobj, fname): continue try: - event = Event.build(line) + event = Event.build(line, lineno, fname) except ValueError as e: arg0 = 'Error at %s:%d: %s' % (fname, lineno, e.args[0]) e.args = (arg0,) + e.args[1:] -- cgit v1.1 From 7fb48c0ee1bbf5cc4c905e900b054096250e9f39 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 27 Aug 2020 15:29:15 +0100 Subject: tracetool: show trace-events filename/lineno in fmt string errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compiler encounters trace event format strings in generated code. Format strings are error-prone and therefore clear compiler errors are important. Use the #line directive to show the trace-events filename and line number in format string errors: https://gcc.gnu.org/onlinedocs/gcc-10.2.0/cpp/Line-Control.html For example, if the cpu_in trace event's %u is changed to %p the following error is reported: trace-events:29:18: error: format ‘%p’ expects argument of type ‘void *’, but argument 7 has type ‘unsigned int’ [-Werror=format=] Line 29 in trace-events is where cpu_in is defined. This works for any trace-events file in the QEMU source tree and the correct path is displayed. Unfortunately there does not seem to be a way to set the column, so "18" is not the right character on that line. Suggested-by: Peter Maydell Signed-off-by: Stefan Hajnoczi Reviewed-by: Peter Maydell Message-Id: <20200827142915.108730-5-stefanha@redhat.com> --- scripts/tracetool/backend/ftrace.py | 4 ++++ scripts/tracetool/backend/log.py | 4 ++++ scripts/tracetool/backend/syslog.py | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py index e9844dd..5fa30cc 100644 --- a/scripts/tracetool/backend/ftrace.py +++ b/scripts/tracetool/backend/ftrace.py @@ -33,8 +33,10 @@ def generate_h(event, group): ' int unused __attribute__ ((unused));', ' int trlen;', ' if (trace_event_get_state(%(event_id)s)) {', + '#line %(event_lineno)d "%(event_filename)s"', ' trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,', ' "%(name)s " %(fmt)s "\\n" %(argnames)s);', + '#line %(out_next_lineno)d "%(out_filename)s"', ' trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);', ' unused = write(trace_marker_fd, ftrace_buf, trlen);', ' }', @@ -42,6 +44,8 @@ def generate_h(event, group): name=event.name, args=event.args, event_id="TRACE_" + event.name.upper(), + event_lineno=event.lineno, + event_filename=event.filename, fmt=event.fmt.rstrip("\n"), argnames=argnames) diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py index 877222b..bc43dbb 100644 --- a/scripts/tracetool/backend/log.py +++ b/scripts/tracetool/backend/log.py @@ -37,12 +37,16 @@ def generate_h(event, group): out(' if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {', ' struct timeval _now;', ' gettimeofday(&_now, NULL);', + '#line %(event_lineno)d "%(event_filename)s"', ' qemu_log("%%d@%%zu.%%06zu:%(name)s " %(fmt)s "\\n",', ' qemu_get_thread_id(),', ' (size_t)_now.tv_sec, (size_t)_now.tv_usec', ' %(argnames)s);', + '#line %(out_next_lineno)d "%(out_filename)s"', ' }', cond=cond, + event_lineno=event.lineno, + event_filename=event.filename, name=event.name, fmt=event.fmt.rstrip("\n"), argnames=argnames) diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py index 1373a90..5a3a00f 100644 --- a/scripts/tracetool/backend/syslog.py +++ b/scripts/tracetool/backend/syslog.py @@ -35,9 +35,13 @@ def generate_h(event, group): cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper()) out(' if (%(cond)s) {', + '#line %(event_lineno)d "%(event_filename)s"', ' syslog(LOG_INFO, "%(name)s " %(fmt)s %(argnames)s);', + '#line %(out_next_lineno)d "%(out_filename)s"', ' }', cond=cond, + event_lineno=event.lineno, + event_filename=event.filename, name=event.name, fmt=event.fmt.rstrip("\n"), argnames=argnames) -- cgit v1.1