From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1D2C3C4338F for ; Tue, 27 Jul 2021 15:04:36 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2896E61B20 for ; Tue, 27 Jul 2021 15:04:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 2896E61B20 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 4501182E94; Tue, 27 Jul 2021 17:04:32 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="XSrfzs+e"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 753EA82E94; Tue, 27 Jul 2021 17:04:29 +0200 (CEST) Received: from mail-qt1-x835.google.com (mail-qt1-x835.google.com [IPv6:2607:f8b0:4864:20::835]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id ADDDC8262F for ; Tue, 27 Jul 2021 17:04:24 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qt1-x835.google.com with SMTP id t18so9696085qta.8 for ; Tue, 27 Jul 2021 08:04:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=RIhu3+bLQfvo6IX3A+KzObiv7XxLKx5/3mT5uys5uAU=; b=XSrfzs+eK5Vg5i9i1thWzpe3P3jKb6UkbfrDb5NKRpxYdUV1awGnOenoBfr0tVsEQj Xyoyil7ZlT0q14iTUxPs6JmwfOgtoQ5mkKpazp9Rxi4wva8Uzey7KWWYKNAnJNGVq8ad mvo5lBeiaS/jJY3I7C4W17s+LVWPSsLwGgMl8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=RIhu3+bLQfvo6IX3A+KzObiv7XxLKx5/3mT5uys5uAU=; b=fNCKj+k04KZcMclU/H+G5mp8/PJMfrDDHvJDajWZFpCODJEbtGSzYH6VaMSAS0J2j9 e61uUG+V0zPT2FIMV6MGDb99Q1rK78xZZOLliFZ5jaPsiU24aR9HAO7GntQ4lwvkCVfk +VA8Ba8i6RNm8MDiIbE5YSMBnteXyln6T7KX2GTjg5wzTHSHmdQat0mHYehA7kE2luso oCSmGqsko3SxEnPRiRRDP8gJZc6hlYrtekRYMc30kHiFOleIY0XoYrCsvfeNJiIumdDt J6AZNa5T9zex8cW1akFWqKvkYn6xkJEDSFUI+nk/OFM2dxIG9JoyihQIp843uV27MWIR LP8w== X-Gm-Message-State: AOAM533DQ64WOPImVsWoZv4q4Bz4l4B0OJYzpI4LxMUB4sDvg93GDG8V 8RYeqe2Vl0CBRYCHbhj17hh4iQ== X-Google-Smtp-Source: ABdhPJzM9I/AxSuw3dkMScEeFdgsYvuJuYXnl1lYDgPB6ZGf36gIbXYL6c9tpIIhux8bVTxx34Mj7g== X-Received: by 2002:ac8:5d07:: with SMTP id f7mr20105259qtx.384.1627398263495; Tue, 27 Jul 2021 08:04:23 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b01-cbda-f53f-8332-072c-4f35.res6.spectrum.com. [2603:6081:7b01:cbda:f53f:8332:72c:4f35]) by smtp.gmail.com with ESMTPSA id d25sm1483716qtq.55.2021.07.27.08.04.22 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Tue, 27 Jul 2021 08:04:22 -0700 (PDT) Date: Tue, 27 Jul 2021 11:04:20 -0400 From: Tom Rini To: Sean Anderson Cc: u-boot@lists.denx.de, Simon Glass Subject: Re: [scan-admin@coverity.com: New Defects reported by Coverity Scan for Das U-Boot] Message-ID: <20210727150420.GG9379@bill-the-cat> References: <20210727025210.GE9379@bill-the-cat> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="GLdS9qjAGFrs7Ts6" Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean --GLdS9qjAGFrs7Ts6 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jul 26, 2021 at 11:26:39PM -0400, Sean Anderson wrote: > On 7/26/21 10:52 PM, Tom Rini wrote: > > ----- Forwarded message from scan-admin@coverity.com ----- > >=20 > > Date: Tue, 27 Jul 2021 01:10:27 +0000 (UTC) > > From: scan-admin@coverity.com > > To: tom.rini@gmail.com > > Subject: New Defects reported by Coverity Scan for Das U-Boot > >=20 > > Hi, > >=20 > > Please find the latest report on new defect(s) introduced to Das U-Boot= found with Coverity Scan. > >=20 > > 6 new defect(s) introduced to Das U-Boot found with Coverity Scan. > > 9 defect(s), reported by Coverity Scan earlier, were marked fixed in th= e recent build analyzed by Coverity Scan. > >=20 > > New defect(s) Reported-by: Coverity Scan > > Showing 6 of 6 defect(s) > >=20 > >=20 > > ** CID 332931: Control flow issues (NO_EFFECT) > > /drivers/clk/clk_kendryte.c: 852 in k210_pll_set_rate() > >=20 > >=20 > > _______________________________________________________________________= _________________________________ > > *** CID 332931: Control flow issues (NO_EFFECT) > > /drivers/clk/clk_kendryte.c: 852 in k210_pll_set_rate() > > 846 int err; > > 847 const struct k210_pll_params *pll =3D &k210_plls[id]; > > 848 struct k210_pll_config config =3D {}; > > 849 u32 reg; > > 850 ulong calc_rate; > > 851 > > > > > CID 332931: Control flow issues (NO_EFFECT) > > > > > This less-than-zero comparison of an unsigned value is never= true. "rate_in < 0UL". > > 852 if (rate_in < 0) > > 853 return rate_in; > > 854 > > 855 err =3D k210_pll_calc_config(rate, rate_in, &config); > > 856 if (err) > > 857 return err; > >=20 >=20 > > ** CID 332929: Integer handling issues (NO_EFFECT) > > /drivers/clk/clk_kendryte.c: 898 in k210_pll_get_rate() > >=20 > >=20 > > _______________________________________________________________________= _________________________________ > > *** CID 332929: Integer handling issues (NO_EFFECT) > > /drivers/clk/clk_kendryte.c: 898 in k210_pll_get_rate() > > 892 static ulong k210_pll_get_rate(struct k210_clk_priv *priv, int = id, > > 893 ulong rate_in) > > 894 { > > 895 u64 r, f, od; > > 896 u32 reg =3D readl(priv->base + k210_plls[id].off); > > 897 > > > > > CID 332929: Integer handling issues (NO_EFFECT) > > > > > This less-than-zero comparison of an unsigned value is never= true. "rate_in < 0UL". > > 898 if (rate_in < 0 || (reg & K210_PLL_BYPASS)) > > 899 return rate_in; > > 900 > > 901 if (!(reg & K210_PLL_PWRD)) > > 902 return 0; > > 903 > >=20 >=20 >=20 > Will send a patch for these. >=20 > > ** CID 332927: (DIVIDE_BY_ZERO) > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > >=20 > >=20 > > _______________________________________________________________________= _________________________________ > > *** CID 332927: (DIVIDE_BY_ZERO) > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In function call "__div64_32", division by expression "__bas= e" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In expression "(u32)_tmp % __base", modulo by expression "__= base" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In function call "__div64_32", division by expression "__bas= e" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In function call "__div64_32", division by expression "__bas= e" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In function call "__div64_32", division by expression "__bas= e" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In function call "__div64_32", division by expression "__bas= e" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In expression "(u32)_tmp % __base", modulo by expression "__= base" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In expression "(u32)_tmp % __base", modulo by expression "__= base" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In expression "(u32)_tmp % __base", modulo by expression "__= base" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In expression "(u32)_tmp % __base", modulo by expression "__= base" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 > > /drivers/clk/clk_kendryte.c: 784 in k210_pll_calc_config() > > 778 } else { > > 779 /* > > 780 * There is no way to only divide once; we need > > 781 * to examine the frequency with and without the > > 782 * effect of od. > > 783 */ > > > > > CID 332927: (DIVIDE_BY_ZERO) > > > > > In expression "(u32)_tmp % __base", modulo by expression "__= base" which may be zero has undefined behavior. > > 784 u64 vco =3D DIV_ROUND_CLOSEST_ULL(rate_in * f, r); > > 785 > > 786 if (vco > 1750000000 || vco < 340000000) > > 787 out_of_spec =3D true; > > 788 } > > 789 >=20 > These are completely safe, but it is relatively non-obvious why. The > only way that r can be 0 is on the very first iteration. When rate > > rate_in, r gets assigned (to a non-zero number) immediately. For the > converse, we only assign to r and od when r * od < goal. goal is > calculated by multiplying f (which is always at least 1) with inv_ratio, > shifted right by 32 bits. In the worst-case (the first iteration), this > is just inv_ratio >> 32. But inv_ratio is rate_in << 32 / rate, and > above we assumed that rate <=3D rate_in. So inv_ratio is always at least 1 > << 32, and we never divide by 0 :) >=20 > In the course of investigating the above, I added some additional test > cases and discovered that we don't always get the best factors in some > cases. I will also send a patch for this. Thanks for looking so quickly! --=20 Tom --GLdS9qjAGFrs7Ts6 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmEAIHEACgkQFHw5/5Y0 tyy2xwv/ZOxSfCZJ40sBYcoOLxKGyYCjbSKRjApzrDNFUmvGSovbbMmF+A6ktL+t yVedjINP5QBXZj5nhZ2mmg6/XcTiDgPsDm3okl8e7rOLSo+gDiuQbWRgFl9hhYzt rBvAidqSI/6LTfxlkCPMG18kIM8CO/QiBSKyxRFLrkvsO2amxuhNEA+cD/AUORyh MwdYkXKePix5DcvWbdWlrxcpmJFjzyjfadCWvfCrcD2ocaYrZ0H2fbpbqWA2agt8 AWY5CWGw4gFsii5v8KKyQ1odQisPl5r6wa28+WLMO3E2ZXpHBszfWVKqH88eQecG 6Pm9gtL9abuX7/1aTdI03TunVnmJmjD4qMnbnBlUlEwcmCNB6Miy3HP7oMj1yrta RVkSsjibE/3Am5pCTFF6grvRWTmbfZPir+CLR6xM/mH5AVhUIzLOD7RjEKq8jkmT gHBmee+/xrEjV+yg02K8QiCbszQVIB764/C21+U9ZN1KCXrkzo317aw4vgyIA1dn 3T8vHetw =m9rR -----END PGP SIGNATURE----- --GLdS9qjAGFrs7Ts6--