* [PATCH 0/3] target/arm: Coverity tweaks
@ 2020-03-20 16:06 Richard Henderson
2020-03-20 16:06 ` [PATCH 1/3] target/arm: Rearrange disabled check for watchpoints Richard Henderson
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Richard Henderson @ 2020-03-20 16:06 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Only the first of these appears to be a real bug.
The other two are adjustments to help satisfy Coverity.
r~
Richard Henderson (3):
target/arm: Rearrange disabled check for watchpoints
target/arm: Assert immh != 0 in disas_simd_shift_imm
target/arm: Move computation of index in handle_simd_dupe
target/arm/helper.c | 11 ++++++-----
target/arm/translate-a64.c | 6 +++++-
2 files changed, 11 insertions(+), 6 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] target/arm: Rearrange disabled check for watchpoints
2020-03-20 16:06 [PATCH 0/3] target/arm: Coverity tweaks Richard Henderson
@ 2020-03-20 16:06 ` Richard Henderson
2020-03-20 16:06 ` [PATCH 2/3] target/arm: Assert immh != 0 in disas_simd_shift_imm Richard Henderson
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2020-03-20 16:06 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Coverity rightly notes that ctz32(bas) on 0 will return 32,
which makes the len calculation a BAD_SHIFT.
A value of 0 in DBGWCR<n>_EL1.BAS is reserved. Simply move
the existing check we have for this case.
Reported-by: Coverity (CID 1421964)
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/helper.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index d2ec2c5351..b7b6887241 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6340,17 +6340,18 @@ void hw_watchpoint_update(ARMCPU *cpu, int n)
int bas = extract64(wcr, 5, 8);
int basstart;
- if (bas == 0) {
- /* This must act as if the watchpoint is disabled */
- return;
- }
-
if (extract64(wvr, 2, 1)) {
/* Deprecated case of an only 4-aligned address. BAS[7:4] are
* ignored, and BAS[3:0] define which bytes to watch.
*/
bas &= 0xf;
}
+
+ if (bas == 0) {
+ /* This must act as if the watchpoint is disabled */
+ return;
+ }
+
/* The BAS bits are supposed to be programmed to indicate a contiguous
* range of bytes. Otherwise it is CONSTRAINED UNPREDICTABLE whether
* we fire for each byte in the word/doubleword addressed by the WVR.
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] target/arm: Assert immh != 0 in disas_simd_shift_imm
2020-03-20 16:06 [PATCH 0/3] target/arm: Coverity tweaks Richard Henderson
2020-03-20 16:06 ` [PATCH 1/3] target/arm: Rearrange disabled check for watchpoints Richard Henderson
@ 2020-03-20 16:06 ` Richard Henderson
2020-03-20 16:06 ` [PATCH 3/3] target/arm: Move computation of index in handle_simd_dupe Richard Henderson
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2020-03-20 16:06 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Coverity raised a shed-load of errors cascading from inferring
that clz32(immh) might yield 32, from immh might be 0.
While immh cannot be 0 from encoding, it is not obvious even to
a human how we've checked that: via the filtering provided by
data_proc_simd[].
Reported-by: Coverity (CID 1421923, and more)
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate-a64.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 8fffb52203..032478614c 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -10405,6 +10405,9 @@ static void disas_simd_shift_imm(DisasContext *s, uint32_t insn)
bool is_u = extract32(insn, 29, 1);
bool is_q = extract32(insn, 30, 1);
+ /* data_proc_simd[] has sent immh == 0 to disas_simd_mod_imm. */
+ assert(immh != 0);
+
switch (opcode) {
case 0x08: /* SRI */
if (!is_u) {
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] target/arm: Move computation of index in handle_simd_dupe
2020-03-20 16:06 [PATCH 0/3] target/arm: Coverity tweaks Richard Henderson
2020-03-20 16:06 ` [PATCH 1/3] target/arm: Rearrange disabled check for watchpoints Richard Henderson
2020-03-20 16:06 ` [PATCH 2/3] target/arm: Assert immh != 0 in disas_simd_shift_imm Richard Henderson
@ 2020-03-20 16:06 ` Richard Henderson
2020-03-22 10:40 ` [PATCH 0/3] target/arm: Coverity tweaks Philippe Mathieu-Daudé
2020-03-23 9:35 ` Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2020-03-20 16:06 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu-arm
Coverity reports a BAD_SHIFT with ctz32(imm5), with imm5 == 0.
This is an invalid encoding, but we diagnose that just below
by rejecting size > 3. Avoid the warning by sinking the
computation of index below the check.
Reported-by: Coverity (CID 1421965)
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/arm/translate-a64.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 032478614c..7580e46367 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -7422,7 +7422,7 @@ static void handle_simd_dupe(DisasContext *s, int is_q, int rd, int rn,
int imm5)
{
int size = ctz32(imm5);
- int index = imm5 >> (size + 1);
+ int index;
if (size > 3 || (size == 3 && !is_q)) {
unallocated_encoding(s);
@@ -7433,6 +7433,7 @@ static void handle_simd_dupe(DisasContext *s, int is_q, int rd, int rn,
return;
}
+ index = imm5 >> (size + 1);
tcg_gen_gvec_dup_mem(size, vec_full_reg_offset(s, rd),
vec_reg_offset(s, rn, index, size),
is_q ? 16 : 8, vec_full_reg_size(s));
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] target/arm: Coverity tweaks
2020-03-20 16:06 [PATCH 0/3] target/arm: Coverity tweaks Richard Henderson
` (2 preceding siblings ...)
2020-03-20 16:06 ` [PATCH 3/3] target/arm: Move computation of index in handle_simd_dupe Richard Henderson
@ 2020-03-22 10:40 ` Philippe Mathieu-Daudé
2020-03-23 9:35 ` Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-22 10:40 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: peter.maydell, qemu-arm
On 3/20/20 5:06 PM, Richard Henderson wrote:
> Only the first of these appears to be a real bug.
> The other two are adjustments to help satisfy Coverity.
>
>
> r~
>
>
> Richard Henderson (3):
> target/arm: Rearrange disabled check for watchpoints
> target/arm: Assert immh != 0 in disas_simd_shift_imm
> target/arm: Move computation of index in handle_simd_dupe
>
> target/arm/helper.c | 11 ++++++-----
> target/arm/translate-a64.c | 6 +++++-
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
Series:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] target/arm: Coverity tweaks
2020-03-20 16:06 [PATCH 0/3] target/arm: Coverity tweaks Richard Henderson
` (3 preceding siblings ...)
2020-03-22 10:40 ` [PATCH 0/3] target/arm: Coverity tweaks Philippe Mathieu-Daudé
@ 2020-03-23 9:35 ` Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2020-03-23 9:35 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-arm, QEMU Developers
On Fri, 20 Mar 2020 at 16:06, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Only the first of these appears to be a real bug.
> The other two are adjustments to help satisfy Coverity.
>
>
> r~
>
>
> Richard Henderson (3):
> target/arm: Rearrange disabled check for watchpoints
> target/arm: Assert immh != 0 in disas_simd_shift_imm
> target/arm: Move computation of index in handle_simd_dupe
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-23 9:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20 16:06 [PATCH 0/3] target/arm: Coverity tweaks Richard Henderson
2020-03-20 16:06 ` [PATCH 1/3] target/arm: Rearrange disabled check for watchpoints Richard Henderson
2020-03-20 16:06 ` [PATCH 2/3] target/arm: Assert immh != 0 in disas_simd_shift_imm Richard Henderson
2020-03-20 16:06 ` [PATCH 3/3] target/arm: Move computation of index in handle_simd_dupe Richard Henderson
2020-03-22 10:40 ` [PATCH 0/3] target/arm: Coverity tweaks Philippe Mathieu-Daudé
2020-03-23 9:35 ` Peter Maydell
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.