| Message ID | 20210406165307.5993-1-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] scripts/tracetool: don't barf validating TCG types | expand |
On Tue, Apr 06, 2021 at 05:53:07PM +0100, Alex Bennée wrote: > TCG types will be transformed into the appropriate host types later on > in the tool. Try and work around this by detecting those cases and > pressing on. > > [AJB: this seems a bit too hacky - but the problem is validate_type is > buried a few layers deep. Maybe we should just drop TCGv from > ALLOWED_TYPES and manually do if bit.startswith("TCGv_") in validate_type?] Please include a line from a trace-events file that triggers the issue. It's unclear to me what the problem is although I guess it's related to TCGv_* types being rejected by validate_type. The bit.startswith("TCGv_") change you mentioned sounds fine or a slightly more general ALLOWED_TYPE_PREFIXES = ['TCGv_']. > Fixes: 73ff061032 ("trace: only permit standard C types and fixed size integer types") > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Matheus Ferst <matheus.ferst@eldorado.org.br> > --- > scripts/tracetool/__init__.py | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py > index 5bc94d95cf..ea078e34b4 100644 > --- a/scripts/tracetool/__init__.py > +++ b/scripts/tracetool/__init__.py > @@ -87,10 +87,11 @@ def out(*lines, **kwargs): > "ssize_t", > "uintptr_t", > "ptrdiff_t", > - # Magic substitution is done by tracetool > + # Magic substitution is done by tracetool TCG_2_HOST This makes it clearer that "TCG_2_HOST" is a code reference: # Magic substitution is done using tracetool.transform.TCG_2_HOST
Stefan Hajnoczi <stefanha@redhat.com> writes: > [[PGP Signed Part:Undecided]] > On Tue, Apr 06, 2021 at 05:53:07PM +0100, Alex Bennée wrote: >> TCG types will be transformed into the appropriate host types later on >> in the tool. Try and work around this by detecting those cases and >> pressing on. >> >> [AJB: this seems a bit too hacky - but the problem is validate_type is >> buried a few layers deep. Maybe we should just drop TCGv from >> ALLOWED_TYPES and manually do if bit.startswith("TCGv_") in validate_type?] > > Please include a line from a trace-events file that triggers the issue. > It's unclear to me what the problem is although I guess it's related to > TCGv_* types being rejected by validate_type. The > bit.startswith("TCGv_") change you mentioned sounds fine or a slightly > more general ALLOWED_TYPE_PREFIXES = ['TCGv_']. I tried: modified tcg/tcg-op.c @@ -2715,6 +2715,9 @@ void tcg_gen_lookup_and_goto_ptr(void) plugin_gen_disable_mem_helpers(); ptr = tcg_temp_new_ptr(); gen_helper_lookup_tb_ptr(ptr, cpu_env); + + trace_goto_tb_tcg(0, ptr); + tcg_gen_op1i(INDEX_op_goto_ptr, tcgv_ptr_arg(ptr)); tcg_temp_free_ptr(ptr); } else { modified trace-events @@ -136,6 +136,10 @@ vcpu guest_cpu_reset(void) # Targets: TCG(all) vcpu tcg guest_mem_before(TCGv vaddr, uint16_t info) "info=%d", "vaddr=0x%016"PRIx64" info=%d" +# Mode: user, softmmu +# Targets: TCG(all) +tcg goto_ptr(int ignore, TCGv_ptr ptr) "goto_ptr_trans=%d", "goto_ptr=%p (%d)" + Which fails with: FAILED: trace/trace-root.c /usr/bin/python3 ../../scripts/tracetool.py --backend=log --group=root --format=c /home/alex/lsrc/qemu.git/./trace-events trace/trace-root.c Traceback (most recent call last): File "../../scripts/tracetool.py", line 154, in <module> main(sys.argv) File "../../scripts/tracetool.py", line 143, in main events.extend(tracetool.read_events(fh, arg)) File "/home/alex/lsrc/qemu.git/scripts/tracetool/__init__.py", line 406, in read_events event = Event.build(line, lineno, fname) File "/home/alex/lsrc/qemu.git/scripts/tracetool/__init__.py", line 322, in build args = Arguments.build(groups["args"]) File "/home/alex/lsrc/qemu.git/scripts/tracetool/__init__.py", line 154, in build validate_type(arg_type) File "/home/alex/lsrc/qemu.git/scripts/tracetool/__init__.py", line 107, in validate_type "declared as 'void *'" % name) ValueError: Error at /home/alex/lsrc/qemu.git/./trace-events:141: Argument type 'TCGv_ptr' is not allowed. Only standard C types and fixed size integer types should be used. struct, union, and other complex pointer types should be declared as 'void *' > >> Fixes: 73ff061032 ("trace: only permit standard C types and fixed size integer types") >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Cc: Matheus Ferst <matheus.ferst@eldorado.org.br> >> --- >> scripts/tracetool/__init__.py | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py >> index 5bc94d95cf..ea078e34b4 100644 >> --- a/scripts/tracetool/__init__.py >> +++ b/scripts/tracetool/__init__.py >> @@ -87,10 +87,11 @@ def out(*lines, **kwargs): >> "ssize_t", >> "uintptr_t", >> "ptrdiff_t", >> - # Magic substitution is done by tracetool >> + # Magic substitution is done by tracetool TCG_2_HOST > > This makes it clearer that "TCG_2_HOST" is a code reference: > > # Magic substitution is done using tracetool.transform.TCG_2_HOST > > [[End of PGP Signed Part]] -- Alex Bennée
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py index 5bc94d95cf..ea078e34b4 100644 --- a/scripts/tracetool/__init__.py +++ b/scripts/tracetool/__init__.py @@ -87,10 +87,11 @@ def out(*lines, **kwargs): "ssize_t", "uintptr_t", "ptrdiff_t", - # Magic substitution is done by tracetool + # Magic substitution is done by tracetool TCG_2_HOST "TCGv", ] + def validate_type(name): bits = name.split(" ") for bit in bits: @@ -405,9 +406,13 @@ def read_events(fobj, fname): try: 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:] - raise + # these get translated by TCG_2_HOST later + if "TCGv_" not in e.args[0]: + arg0 = 'Error at %s:%d: %s' % (fname, lineno, e.args[0]) + e.args = (arg0,) + e.args[1:] + raise + else: + pass # transform TCG-enabled events if "tcg" not in event.properties:
TCG types will be transformed into the appropriate host types later on in the tool. Try and work around this by detecting those cases and pressing on. [AJB: this seems a bit too hacky - but the problem is validate_type is buried a few layers deep. Maybe we should just drop TCGv from ALLOWED_TYPES and manually do if bit.startswith("TCGv_") in validate_type?] Fixes: 73ff061032 ("trace: only permit standard C types and fixed size integer types") Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Matheus Ferst <matheus.ferst@eldorado.org.br> --- scripts/tracetool/__init__.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) -- 2.20.1