* [PATCH 0/1] tricore: fixed faulty conditions for extr and imask @ 2021-02-10 8:26 David Brenken 2021-02-10 8:26 ` [PATCH 1/1] " David Brenken ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: David Brenken @ 2021-02-10 8:26 UTC (permalink / raw) To: qemu-devel; +Cc: kbastian, Andreas Konopik From: Andreas Konopik <andreas.konopik@efs-auto.de> Hello together, we have fixed a few conditions leading to incorrect intermediate code generation. RCPW_IMASK, RRPW_EXTR, RRPW_EXTR_U and RRPW_IMASK invoke undefined behavior for "pos + width > 32", which is also checked in tcg_gen_extract_tl(). RRRW_EXTR_U invokes undefined behavior for "width == 0", hence we removed that condition. Andreas Konopik (1): tricore: fixed faulty conditions for extr and imask target/tricore/translate.c | 37 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) -- 2.30.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] tricore: fixed faulty conditions for extr and imask 2021-02-10 8:26 [PATCH 0/1] tricore: fixed faulty conditions for extr and imask David Brenken @ 2021-02-10 8:26 ` David Brenken 2021-02-10 19:02 ` [PATCH 0/1] " Richard Henderson 2021-02-11 11:32 ` no-reply 2 siblings, 0 replies; 5+ messages in thread From: David Brenken @ 2021-02-10 8:26 UTC (permalink / raw) To: qemu-devel; +Cc: kbastian, David Brenken, Georg Hofstetter, Andreas Konopik From: Andreas Konopik <andreas.konopik@efs-auto.de> Signed-off-by: Andreas Konopik <andreas.konopik@efs-auto.de> Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de> Signed-off-by: David Brenken <david.brenken@efs-auto.de> --- target/tricore/translate.c | 37 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/target/tricore/translate.c b/target/tricore/translate.c index 7752630ac1..f451e085f8 100644 --- a/target/tricore/translate.c +++ b/target/tricore/translate.c @@ -5777,8 +5777,8 @@ static void decode_rcpw_insert(DisasContext *ctx) switch (op2) { case OPC2_32_RCPW_IMASK: CHECK_REG_PAIR(r2); - /* if pos + width > 31 undefined result */ - if (pos + width <= 31) { + /* if pos + width > 32 undefined result */ + if (pos + width <= 32) { tcg_gen_movi_tl(cpu_gpr_d[r2+1], ((1u << width) - 1) << pos); tcg_gen_movi_tl(cpu_gpr_d[r2], (const4 << pos)); } @@ -6999,29 +6999,16 @@ static void decode_rrpw_extract_insert(DisasContext *ctx) switch (op2) { case OPC2_32_RRPW_EXTR: - if (pos + width <= 31) { - /* optimize special cases */ - if ((pos == 0) && (width == 8)) { - tcg_gen_ext8s_tl(cpu_gpr_d[r3], cpu_gpr_d[r1]); - } else if ((pos == 0) && (width == 16)) { - tcg_gen_ext16s_tl(cpu_gpr_d[r3], cpu_gpr_d[r1]); - } else { - tcg_gen_shli_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], 32 - pos - width); - tcg_gen_sari_tl(cpu_gpr_d[r3], cpu_gpr_d[r3], 32 - width); - } - } + /* if pos + width > 32 undefined result */ + tcg_gen_sextract_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], pos, width); break; case OPC2_32_RRPW_EXTR_U: - if (width == 0) { - tcg_gen_movi_tl(cpu_gpr_d[r3], 0); - } else { - tcg_gen_shri_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], pos); - tcg_gen_andi_tl(cpu_gpr_d[r3], cpu_gpr_d[r3], ~0u >> (32-width)); - } + /* if pos + width > 32 undefined result */ + tcg_gen_extract_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], pos, width); break; case OPC2_32_RRPW_IMASK: CHECK_REG_PAIR(r3); - if (pos + width <= 31) { + if (pos + width <= 32) { tcg_gen_movi_tl(cpu_gpr_d[r3+1], ((1u << width) - 1) << pos); tcg_gen_shli_tl(cpu_gpr_d[r3], cpu_gpr_d[r2], pos); } @@ -8320,13 +8307,9 @@ static void decode_rrrw_extract_insert(DisasContext *ctx) tcg_gen_sari_tl(cpu_gpr_d[r4], cpu_gpr_d[r4], 32 - width); break; case OPC2_32_RRRW_EXTR_U: - if (width == 0) { - tcg_gen_movi_tl(cpu_gpr_d[r4], 0); - } else { - tcg_gen_andi_tl(temp, cpu_gpr_d[r3], 0x1f); - tcg_gen_shr_tl(cpu_gpr_d[r4], cpu_gpr_d[r1], temp); - tcg_gen_andi_tl(cpu_gpr_d[r4], cpu_gpr_d[r4], ~0u >> (32-width)); - } + tcg_gen_andi_tl(temp, cpu_gpr_d[r3], 0x1f); + tcg_gen_shr_tl(cpu_gpr_d[r4], cpu_gpr_d[r1], temp); + tcg_gen_andi_tl(cpu_gpr_d[r4], cpu_gpr_d[r4], ~0u >> (32-width)); break; case OPC2_32_RRRW_IMASK: temp2 = tcg_temp_new(); -- 2.30.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/1] tricore: fixed faulty conditions for extr and imask 2021-02-10 8:26 [PATCH 0/1] tricore: fixed faulty conditions for extr and imask David Brenken 2021-02-10 8:26 ` [PATCH 1/1] " David Brenken @ 2021-02-10 19:02 ` Richard Henderson 2021-02-11 10:02 ` Konopik, Andreas (EFS-GH2) 2021-02-11 11:32 ` no-reply 2 siblings, 1 reply; 5+ messages in thread From: Richard Henderson @ 2021-02-10 19:02 UTC (permalink / raw) To: David Brenken, qemu-devel; +Cc: kbastian, Andreas Konopik On 2/10/21 12:26 AM, David Brenken wrote: > From: Andreas Konopik <andreas.konopik@efs-auto.de> > > Hello together, > > we have fixed a few conditions leading to incorrect intermediate code > generation. RCPW_IMASK, RRPW_EXTR, RRPW_EXTR_U and RRPW_IMASK invoke > undefined behavior for "pos + width > 32", which is also checked in > tcg_gen_extract_tl(). RRRW_EXTR_U invokes undefined behavior for > "width == 0", hence we removed that condition. This is incorrect, because "undefined behavior" should not include a qemu abort. You could raise a guest exception, you could treat the faulty instruction as a nop, you could truncate the inputs to avoid the abort, you could write 0xdeadbeef to the destination. Or you could fix the couple of faulty conditions and leave the rest of the code as-is. r~ ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 0/1] tricore: fixed faulty conditions for extr and imask 2021-02-10 19:02 ` [PATCH 0/1] " Richard Henderson @ 2021-02-11 10:02 ` Konopik, Andreas (EFS-GH2) 0 siblings, 0 replies; 5+ messages in thread From: Konopik, Andreas (EFS-GH2) @ 2021-02-11 10:02 UTC (permalink / raw) To: Richard Henderson, qemu-devel; +Cc: kbastian, Brenken, David (EFS-GH5) Hi Richard, thank you for your feedback. > From: Richard Henderson <richard.henderson@linaro.org> > Sent: Wednesday, February 10, 2021 20:02 > To: David Brenken <david.brenken@efs-auto.org> > On 2/10/21 12:26 AM, David Brenken wrote: > > From: Andreas Konopik <andreas.konopik@efs-auto.de> > > > > Hello together, > > > > we have fixed a few conditions leading to incorrect intermediate code > > generation. RCPW_IMASK, RRPW_EXTR, RRPW_EXTR_U and RRPW_IMASK > invoke > > undefined behavior for "pos + width > 32", which is also checked in > > tcg_gen_extract_tl(). RRRW_EXTR_U invokes undefined behavior for > > "width == 0", hence we removed that condition. > > This is incorrect, because "undefined behavior" should not include a qemu > abort. I didn't notice that these checks terminated program execution. > You could raise a guest exception, you could treat the faulty instruction as a > nop, you could truncate the inputs to avoid the abort, you could write > 0xdeadbeef to the destination. > > Or you could fix the couple of faulty conditions and leave the rest of the code > as-is. We will submit the patch following your advice as soon as possible. Kind regards, Andreas INTERNAL ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/1] tricore: fixed faulty conditions for extr and imask 2021-02-10 8:26 [PATCH 0/1] tricore: fixed faulty conditions for extr and imask David Brenken 2021-02-10 8:26 ` [PATCH 1/1] " David Brenken 2021-02-10 19:02 ` [PATCH 0/1] " Richard Henderson @ 2021-02-11 11:32 ` no-reply 2 siblings, 0 replies; 5+ messages in thread From: no-reply @ 2021-02-11 11:32 UTC (permalink / raw) To: david.brenken; +Cc: kbastian, qemu-devel, andreas.konopik Patchew URL: https://patchew.org/QEMU/20210210082650.5516-1-david.brenken@efs-auto.org/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210210082650.5516-1-david.brenken@efs-auto.org Subject: [PATCH 0/1] tricore: fixed faulty conditions for extr and imask === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 05ead58 tricore: fixed faulty conditions for extr and imask === OUTPUT BEGIN === ERROR: spaces required around that '-' (ctx:VxV) #78: FILE: target/tricore/translate.c:8312: + tcg_gen_andi_tl(cpu_gpr_d[r4], cpu_gpr_d[r4], ~0u >> (32-width)); ^ total: 1 errors, 0 warnings, 60 lines checked Commit 05ead58660db (tricore: fixed faulty conditions for extr and imask) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20210210082650.5516-1-david.brenken@efs-auto.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-11 11:34 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-10 8:26 [PATCH 0/1] tricore: fixed faulty conditions for extr and imask David Brenken 2021-02-10 8:26 ` [PATCH 1/1] " David Brenken 2021-02-10 19:02 ` [PATCH 0/1] " Richard Henderson 2021-02-11 10:02 ` Konopik, Andreas (EFS-GH2) 2021-02-11 11:32 ` no-reply
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.