All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: "Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Greg Kurz" <groug@kaod.org>,
	"Luis Fernando Fujita Pires" <luis.pires@eldorado.org.br>,
	"Hervé Poussineau" <hpoussin@reactos.org>,
	"Cédric Le Goater" <clg@kaod.org>, qemu-ppc <qemu-ppc@nongnu.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PULL 33/44] target/ppc: Fix 64-bit decrementer
Date: Sat, 2 Oct 2021 11:39:57 +0100	[thread overview]
Message-ID: <CAFEAcA8AxuMRRjxpya3_H+57SmrduD1dVqg7vjSOadV1pCQaSQ@mail.gmail.com> (raw)
In-Reply-To: <20210930054426.357344-34-david@gibson.dropbear.id.au>

On Thu, 30 Sept 2021 at 06:44, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> From: Cédric Le Goater <clg@kaod.org>
>
> The current way the mask is built can overflow with a 64-bit decrementer.
> Use sextract64() to extract the signed values and remove the logic to
> handle negative values which has become useless.
>
> Cc: Luis Fernando Fujita Pires <luis.pires@eldorado.org.br>
> Fixes: a8dafa525181 ("target/ppc: Implement large decrementer support for TCG")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Message-Id: <20210920061203.989563-5-clg@kaod.org>
> Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Hi; Coverity complains about dead code here (CID 1464061):



>       * On MSB edge based DEC implementations the MSB going from 0 -> 1 triggers
>       * an edge interrupt, so raise it here too.
>       */
> -    if ((value < 3) ||
> -        ((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && negative) ||
> -        ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && negative
> -          && !(decr & (1ULL << (nr_bits - 1))))) {
> +    if ((signed_value < 3) ||
> +        ((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) ||
> +        ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && signed_value < 0
> +          && signed_decr >= 0)) {
>          (*raise_excp)(cpu);
>          return;
>      }

If signed_value < 3 then the first clause of the || evaluates as true,
and so we will definitely take the if() clause. So if we're evaluating
the second operand to the || then we know that signed_value > 3,
which means that 'signed_value < 0' is always false and in turn that
neither of the other two '||' options can be true. The whole expression
is equivalent to just "if (signed_value < 3)".

What was intended here? If this was supposed to be a no-behaviour-change
commit (apart from fixing the 64-bit case) then the condition should
have stayed as "(value < 3)", I think.

thanks
-- PMM


  reply	other threads:[~2021-10-02 10:41 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-30  5:43 [PULL 00/44] ppc-for-6.2 queue 20210930 David Gibson
2021-09-30  5:43 ` [PULL 01/44] host-utils: Fix overflow detection in divu128() David Gibson
2021-09-30  5:43 ` [PULL 02/44] host-utils: fix missing zero-extension in divs128 David Gibson
2021-09-30  5:43 ` [PULL 03/44] host-utils: introduce uabs64() David Gibson
2021-09-30  5:43 ` [PULL 04/44] i386/kvm: Replace abs64() with uabs64() from host-utils David Gibson
2021-09-30  5:43 ` [PULL 05/44] ppc/spapr: Add a POWER10 DD2 CPU David Gibson
2021-09-30  5:43 ` [PULL 06/44] ppc/pnv: Add a comment on the "primary-topology-index" property David Gibson
2021-09-30  5:43 ` [PULL 07/44] ppc/pnv: Remove useless variable David Gibson
2021-09-30  5:43 ` [PULL 08/44] ppc/xive: Export priority_to_ipb() helper David Gibson
2021-09-30  5:43 ` [PULL 09/44] ppc/xive: Export xive_tctx_word2() helper David Gibson
2021-09-30  5:43 ` [PULL 10/44] ppc/pnv: Rename "id" to "quad-id" in PnvQuad David Gibson
2021-09-30  5:43 ` [PULL 11/44] docs/system: ppc: Update the URL for OpenPOWER firmware images David Gibson
2021-09-30  5:43 ` [PULL 12/44] ppc/pnv: Add an assert when calculating the RAM distribution on chips David Gibson
2021-09-30  5:43 ` [PULL 13/44] target/ppc: fix setting of CR flags in bcdcfsq David Gibson
2021-09-30  5:43 ` [PULL 14/44] memory_hotplug.c: handle dev->id = NULL in acpi_memory_hotplug_write() David Gibson
2021-09-30  5:43 ` [PULL 15/44] spapr.c: handle dev->id in spapr_memory_unplug_rollback() David Gibson
2021-09-30  5:43 ` [PULL 16/44] spapr_drc.c: do not error_report() when drc->dev->id == NULL David Gibson
2021-09-30  5:43 ` [PULL 17/44] qapi/qdev.json: fix DEVICE_DELETED parameters doc David Gibson
2021-09-30  5:44 ` [PULL 18/44] qapi/qdev.json: add DEVICE_UNPLUG_GUEST_ERROR QAPI event David Gibson
2021-09-30  5:44 ` [PULL 19/44] spapr: use DEVICE_UNPLUG_GUEST_ERROR to report unplug errors David Gibson
2021-09-30  5:44 ` [PULL 20/44] memory_hotplug.c: send DEVICE_UNPLUG_GUEST_ERROR in acpi_memory_hotplug_write() David Gibson
2021-09-30  5:44 ` [PULL 21/44] target/ppc: Convert debug to trace events (exceptions) David Gibson
2021-09-30  5:44 ` [PULL 22/44] target/ppc: Replace debug messages by asserts for unknown IRQ pins David Gibson
2021-09-30  5:44 ` [PULL 23/44] target/ppc: add LPCR[HR] to DisasContext and hflags David Gibson
2021-09-30  5:44 ` [PULL 24/44] target/ppc: Check privilege level based on PSR and LPCR[HR] in tlbie[l] David Gibson
2021-09-30  5:44 ` [PULL 25/44] spapr_numa.c: split FORM1 code into helpers David Gibson
2021-09-30  5:44 ` [PULL 26/44] spapr_numa.c: scrap 'legacy_numa' concept David Gibson
2021-09-30  5:44 ` [PULL 27/44] spapr_numa.c: parametrize FORM1 macros David Gibson
2021-09-30  5:44 ` [PULL 28/44] spapr_numa.c: rename numa_assoc_array to FORM1_assoc_array David Gibson
2021-09-30  5:44 ` [PULL 29/44] spapr: move FORM1 verifications to post CAS David Gibson
2021-09-30  5:44 ` [PULL 30/44] spapr_numa.c: FORM2 NUMA affinity support David Gibson
2021-09-30  5:44 ` [PULL 31/44] spapr_numa.c: handle auto NUMA node with no distance info David Gibson
2021-09-30  5:44 ` [PULL 32/44] target/ppc: Convert debug to trace events (decrementer and IRQ) David Gibson
2021-09-30  5:44 ` [PULL 33/44] target/ppc: Fix 64-bit decrementer David Gibson
2021-10-02 10:39   ` Peter Maydell [this message]
2021-10-04  6:54     ` Cédric Le Goater
2021-09-30  5:44 ` [PULL 34/44] hw/intc: openpic: Correct the reset value of IPIDR for FSL chipset David Gibson
2021-09-30  5:44 ` [PULL 35/44] hw/intc: openpic: Drop Raven related codes David Gibson
2021-09-30  5:44 ` [PULL 36/44] hw/intc: openpic: Clean up the styles David Gibson
2021-09-30  5:44 ` [PULL 37/44] spapr_numa.c: fixes in spapr_numa_FORM2_write_rtas_tables() David Gibson
2021-09-30  5:44 ` [PULL 38/44] spapr/xive: Fix kvm_xive_source_reset trace event David Gibson
2021-09-30  5:44 ` [PULL 39/44] MAINTAINERS: Remove machine specific files from ppc TCG CPUs entry David Gibson
2021-09-30  5:44 ` [PULL 40/44] MAINTAINERS: Remove David & Greg as reviewers for a number of boards David Gibson
2021-09-30  5:44 ` [PULL 41/44] MAINTAINERS: Orphan obscure ppc platforms David Gibson
2021-09-30  5:44 ` [PULL 42/44] MAINTAINERS: Remove David & Greg as reviewers/co-maintainers of powernv David Gibson
2021-09-30  5:44 ` [PULL 43/44] MAINTAINERS: Add information for OpenPIC David Gibson
2021-09-30  5:44 ` [PULL 44/44] MAINTAINERS: Demote sPAPR from "Supported" to "Maintained" David Gibson
2021-09-30 16:37 ` [PULL 00/44] ppc-for-6.2 queue 20210930 Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFEAcA8AxuMRRjxpya3_H+57SmrduD1dVqg7vjSOadV1pCQaSQ@mail.gmail.com \
    --to=peter.maydell@linaro.org \
    --cc=clg@kaod.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=hpoussin@reactos.org \
    --cc=luis.pires@eldorado.org.br \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.