Message ID | 20221020113759.17402-1-jgross@suse.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4242:0:0:0:0:0 with SMTP id s2csp58242wrr; Thu, 20 Oct 2022 04:46:39 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4LXCozsL7RDMq/SIyOI9yfv/kIOO8kP+sdHfCrO4tjAX50dHgrATTWi7FaXIWAwAi4TNAC X-Received: by 2002:a17:906:7308:b0:78e:191e:8389 with SMTP id di8-20020a170906730800b0078e191e8389mr10451122ejc.170.1666266399416; Thu, 20 Oct 2022 04:46:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666266399; cv=none; d=google.com; s=arc-20160816; b=F5d86G14tLWlH/uQeleaUOYpHQukOYAuyZCguNELCWRLcy2q6ZbqShLn1L+OrG6U7b ylt0J6PXghFKZspl3Al82VvuOUaDRrnwAbZnXeV+Yjsby7QAMgVNPkbTHR5KwnolIbCh iDNHEOZFwNmNB6pIHX0bO1xm49oO9eyhZFy/IPhwJKeim9JoXKegQSTzWrwPvPGMD26S J3mMhLp4ZCcUzHTlVmjqGPvlBU2q6VXrBzEa2WGT8gCyTopytOD7jvThyfam97Um9OJg Epj0REe7Pa7g1zyTveAsDnaDN4Y9DTvT6bu373NFBhOYcHhoOcPkNKfXOQTYR3L5V+U+ oiyg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=bOeIQs06Ph1bJ91eHAC9fBVgauw5j59B8/KvO9VWdAY=; b=U6RjxuiOq05Kvy3ky3InkVyXPvu3RFBPipcyIhE+RUcNXqjFMcArwTlkh3rlHw04Y/ L1anLTgJKDFm8Xzhejt6mR0RSlDx+IXYlGevYGiEhJKjzr1Tmdm1GVABu0+4WoJvfxG+ zx+iqFJt/r4FMu+Ba8PUeX0uJvJthLE5TfazNhTIQj/VjY/vyeeTp1zO1PLHLJYS50sw So+9yiTTIdlWWxRYuWc3pAzWqGJAi0sX6WSBZLBulH1QLJU5MQ7wxn05/db3RNhtZVG9 V02mdyJfHrc2gLUqyPoAcZe5jr7tClIzyBN1nBD3cKZophGX78fzzrZYJIPCS7KNcytN XnJg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=PBpvCe+e; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id js22-20020a17090797d600b0078dcfe6a000si19102501ejc.727.2022.10.20.04.46.13; Thu, 20 Oct 2022 04:46:39 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=PBpvCe+e; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231425AbiJTLig (ORCPT <rfc822;ruipengqi7@gmail.com> + 99 others); Thu, 20 Oct 2022 07:38:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42454 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231407AbiJTLia (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 20 Oct 2022 07:38:30 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D8C9D1DEC01 for <linux-kernel@vger.kernel.org>; Thu, 20 Oct 2022 04:38:03 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 92DE21FCFA; Thu, 20 Oct 2022 11:38:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1666265881; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=bOeIQs06Ph1bJ91eHAC9fBVgauw5j59B8/KvO9VWdAY=; b=PBpvCe+eKaNf8bwyjdesNestAeyQ6jmQl9dMvpuhLdQXjqPFo9p9VBZgDru/8YYuiEsJmQ M0a4vZAgkjF5JxMl8e5xw+lL3Gdm19fY3NtK8sbry4qXU0+EZFADAIQWui+2pHnFqOrQMh DHvIn+IF35Oa8nHjrGVNSMG5sLUEBEI= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 4090D13494; Thu, 20 Oct 2022 11:38:01 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id REBlDhkzUWPEBgAAMHmgww (envelope-from <jgross@suse.com>); Thu, 20 Oct 2022 11:38:01 +0000 From: Juergen Gross <jgross@suse.com> To: linux-kernel@vger.kernel.org, x86@kernel.org Cc: Juergen Gross <jgross@suse.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>, "H. Peter Anvin" <hpa@zytor.com>, xen-devel@lists.xenproject.org, Dan Carpenter <dan.carpenter@oracle.com> Subject: [PATCH] x86/xen: silence smatch warning in pmu_msr_chk_emulated() Date: Thu, 20 Oct 2022 13:37:59 +0200 Message-Id: <20221020113759.17402-1-jgross@suse.com> X-Mailer: git-send-email 2.35.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747206956324668288?= X-GMAIL-MSGID: =?utf-8?q?1747206956324668288?= |
Series |
x86/xen: silence smatch warning in pmu_msr_chk_emulated()
|
|
Commit Message
Juergen Gross
Oct. 20, 2022, 11:37 a.m. UTC
Commit 8714f7bcd3c2 ("xen/pv: add fault recovery control to pmu msr
accesses") introduced code resulting in a warning issued by the smatch
static checker, claiming to use an uninitialized variable.
This is a false positive, but work around the warning nevertheless.
Fixes: 8714f7bcd3c2 ("xen/pv: add fault recovery control to pmu msr accesses")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
arch/x86/xen/pmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 20.10.2022 13:37, Juergen Gross wrote: > Commit 8714f7bcd3c2 ("xen/pv: add fault recovery control to pmu msr > accesses") introduced code resulting in a warning issued by the smatch > static checker, claiming to use an uninitialized variable. > > This is a false positive, but work around the warning nevertheless. The risk of introducing a problem might be quite low here, but in general it exists: With the adjustment you remove any chance of the compiler spotting a missing initialization before use. And I'm not convinced using 0 in such a case would actually be ending up sufficiently benign. Jan > --- a/arch/x86/xen/pmu.c > +++ b/arch/x86/xen/pmu.c > @@ -302,7 +302,7 @@ static bool xen_amd_pmu_emulate(unsigned int msr, u64 *val, bool is_read) > static bool pmu_msr_chk_emulated(unsigned int msr, uint64_t *val, bool is_read, > bool *emul) > { > - int type, index; > + int type = 0, index = 0; > > if (is_amd_pmu_msr(msr)) > *emul = xen_amd_pmu_emulate(msr, val, is_read);
On 20.10.22 15:16, Jan Beulich wrote: > On 20.10.2022 13:37, Juergen Gross wrote: >> Commit 8714f7bcd3c2 ("xen/pv: add fault recovery control to pmu msr >> accesses") introduced code resulting in a warning issued by the smatch >> static checker, claiming to use an uninitialized variable. >> >> This is a false positive, but work around the warning nevertheless. > > The risk of introducing a problem might be quite low here, but in general > it exists: With the adjustment you remove any chance of the compiler > spotting a missing initialization before use. And I'm not convinced using > 0 in such a case would actually be ending up sufficiently benign. Hmm, an alternative would be to initialize it to -1 and add a test for the index to be >= 0 before using it. Or to live with the smash warning with the chance, that a compiler might be warning for the same reason in the future. Juergen
On 10/20/22 9:34 AM, Juergen Gross wrote: > On 20.10.22 15:16, Jan Beulich wrote: >> On 20.10.2022 13:37, Juergen Gross wrote: >>> Commit 8714f7bcd3c2 ("xen/pv: add fault recovery control to pmu msr >>> accesses") introduced code resulting in a warning issued by the smatch >>> static checker, claiming to use an uninitialized variable. >>> >>> This is a false positive, but work around the warning nevertheless. >> >> The risk of introducing a problem might be quite low here, but in general >> it exists: With the adjustment you remove any chance of the compiler >> spotting a missing initialization before use. And I'm not convinced using >> 0 in such a case would actually be ending up sufficiently benign. > > Hmm, an alternative would be to initialize it to -1 and add a test for the > index to be >= 0 before using it. > > Or to live with the smash warning with the chance, that a compiler might be > warning for the same reason in the future. Is smatch complaining about both variables or just index? There are two cases in is_intel_pmu_msr() where it returns true but index is not set so perhaps that's what bothers smatch? It shold not complain if is_intel_pmu_msr() returns false. -boris
On 20.10.22 16:22, Boris Ostrovsky wrote: > > On 10/20/22 9:34 AM, Juergen Gross wrote: >> On 20.10.22 15:16, Jan Beulich wrote: >>> On 20.10.2022 13:37, Juergen Gross wrote: >>>> Commit 8714f7bcd3c2 ("xen/pv: add fault recovery control to pmu msr >>>> accesses") introduced code resulting in a warning issued by the smatch >>>> static checker, claiming to use an uninitialized variable. >>>> >>>> This is a false positive, but work around the warning nevertheless. >>> >>> The risk of introducing a problem might be quite low here, but in general >>> it exists: With the adjustment you remove any chance of the compiler >>> spotting a missing initialization before use. And I'm not convinced using >>> 0 in such a case would actually be ending up sufficiently benign. >> >> Hmm, an alternative would be to initialize it to -1 and add a test for the >> index to be >= 0 before using it. >> >> Or to live with the smash warning with the chance, that a compiler might be >> warning for the same reason in the future. > > > Is smatch complaining about both variables or just index? There are two cases in > is_intel_pmu_msr() where it returns true but index is not set so perhaps that's > what bothers smatch? It shold not complain if is_intel_pmu_msr() returns false. I didn't test it myself, so I can only speculate. I guess the problem is when is_intel_pmu_msr() returns true. In the end I don't think we expect much code churn in this area in the future. Its not as if the pmu handling for PV guests is expected to be extended. Juergen
On Thu, Oct 20, 2022 at 10:22:17AM -0400, Boris Ostrovsky wrote: > > On 10/20/22 9:34 AM, Juergen Gross wrote: > > On 20.10.22 15:16, Jan Beulich wrote: > > > On 20.10.2022 13:37, Juergen Gross wrote: > > > > Commit 8714f7bcd3c2 ("xen/pv: add fault recovery control to pmu msr > > > > accesses") introduced code resulting in a warning issued by the smatch > > > > static checker, claiming to use an uninitialized variable. > > > > > > > > This is a false positive, but work around the warning nevertheless. > > > > > > The risk of introducing a problem might be quite low here, but in general > > > it exists: With the adjustment you remove any chance of the compiler > > > spotting a missing initialization before use. And I'm not convinced using > > > 0 in such a case would actually be ending up sufficiently benign. > > > > Hmm, an alternative would be to initialize it to -1 and add a test for the > > index to be >= 0 before using it. > > > > Or to live with the smash warning with the chance, that a compiler might be > > warning for the same reason in the future. > > > Is smatch complaining about both variables or just index? Just "index". > There are two cases in is_intel_pmu_msr() where it returns true but > index is not set so perhaps that's what bothers smatch? Yep. The "index" variable *is* undefined when it's passed so Smatch is correct in what it's saying. But it's is not used on that path inside the function so it's harmless. > It shold not complain if is_intel_pmu_msr() returns false. Correct. I kind of like the patch. We generally say "fix the checker and don't silence the warning" but in this case I feel like the checker is doing the best possible thing and I'm not going to fix it. Trying to silence this warning in Smatch would come with some real downsides. regards, dan carpenter
diff --git a/arch/x86/xen/pmu.c b/arch/x86/xen/pmu.c index 68aff1382872..898a252ed6f1 100644 --- a/arch/x86/xen/pmu.c +++ b/arch/x86/xen/pmu.c @@ -302,7 +302,7 @@ static bool xen_amd_pmu_emulate(unsigned int msr, u64 *val, bool is_read) static bool pmu_msr_chk_emulated(unsigned int msr, uint64_t *val, bool is_read, bool *emul) { - int type, index; + int type = 0, index = 0; if (is_amd_pmu_msr(msr)) *emul = xen_amd_pmu_emulate(msr, val, is_read);