From owner-svn-src-head@freebsd.org Thu Sep 10 18:49:00 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id DBE173E1318; Thu, 10 Sep 2020 18:49:00 +0000 (UTC) (envelope-from agapon@gmail.com) Received: from mail-lj1-f193.google.com (mail-lj1-f193.google.com [209.85.208.193]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BnSbJ0vJwz4Tjm; Thu, 10 Sep 2020 18:48:59 +0000 (UTC) (envelope-from agapon@gmail.com) Received: by mail-lj1-f193.google.com with SMTP id n25so9556858ljj.4; Thu, 10 Sep 2020 11:48:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=LG8mRh5AtYhdkO+fOpvOmSGnwrwY8QCrmTaFrlA76fA=; b=jE3FBq/sQF1XDkvVtkXj9I0tqEdFg/UpjtNmeIca4bA6Ls09+o0Va1UT7Tayqf2wtb UQy7/FEA68VnOP+TK/gRpHk1J9sT1pnyqvKTeWg9z0tsatWBmTvdvHYcYHglo5bFBPlO N0uyPcJateMpoZtQOVjYkVArfoiX2yuVASCok1SWrXjC+a1Wn1MRmb9vcSS+7tyRcDLP U+i7mHH1NtQIbpF4ivGlGeqkE2m5nR4Vt1tRFcX5xGoNRZ6nlxT9krtkUkGwJOsrTg3o 2qQ4NOKOkkph273D/PSLlUilCKEFFD8QEJZBMP3RbJZdXhpa0AcVslrLibaNpjBmkgt0 cVUw== X-Gm-Message-State: AOAM531N0mePk6ixL6E+ywWuyVA04lib6kBQKFgd5ffRoSBMVQVOTCYV L+etzUaGo9RXZ+2AQRWdNsfP9hQx22A= X-Google-Smtp-Source: ABdhPJyifcyIirAMMuWBcXvGQ+lh/T2+j+6XVpqOpXDQlQ3FKdIqt9OHsJc056HnuWk0zwKQ0D08pA== X-Received: by 2002:a2e:9948:: with SMTP id r8mr5017563ljj.126.1599763737667; Thu, 10 Sep 2020 11:48:57 -0700 (PDT) Received: from [192.168.0.88] (east.meadow.volia.net. [93.72.151.96]) by smtp.googlemail.com with ESMTPSA id z8sm1801749ljh.19.2020.09.10.11.48.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 10 Sep 2020 11:48:56 -0700 (PDT) Subject: Re: uninitialized variables [Was: svn commit: r365445 - head/sys/cam/mmc] To: Mark Johnston Cc: Michal Meloun , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <202009080546.0885kAgk006783@repo.freebsd.org> <34826ee7-12a9-d309-1fee-cd2e95744603@FreeBSD.org> <67be7fa5-30dd-b7ee-1076-9c29195d83d3@gmail.com> <20200908124848.GB66031@raichu> <6b18b5ef-a743-3d5e-8dd2-24640614ec88@FreeBSD.org> <20200909134429.GA65588@raichu> From: Andriy Gapon Openpgp: preference=signencrypt Autocrypt: addr=avg@FreeBSD.org; prefer-encrypt=mutual; keydata= mDMEX1iFDhYJKwYBBAHaRw8BAQdAiu8JG/oLFkVkOAJqJc7Dx5KI/Q6C3SBI20EQm+DXnAu0 HkFuZHJpeSBHYXBvbiA8YXZnQEZyZWVCU0Qub3JnPoiWBBMWCAA+FiEEyCHHZM09l0OE3Ir/ 1A1+Gq8+L1EFAl9YhQ4CGwMFCQeEzgAFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQ1A1+ Gq8+L1Fc0wD/ZjmhHfbCJywZU3aOxXIPjcz73FYEGMvqMCCLAWyLbSABALFL+1ZNrjV3BGjq 889cOYFuboA/Yn3eWezS+tfqYBsGuDgEX1iFDhIKKwYBBAGXVQEFAQEHQL6B20Xi600TrkpG P9fWjl7JtHNxqrHKhX6Kg7kgb4ILAwEIB4h+BBgWCAAmFiEEyCHHZM09l0OE3Ir/1A1+Gq8+ L1EFAl9YhQ4CGwwFCQeEzgAACgkQ1A1+Gq8+L1F3cgEAktp4h+IJUJxL1vn6zMOt//znni/J TanKfQuA8wGXcGkBAKpZJhqMkg+pKk7MGvJhgJ6nCpTZ+rMK6vZVZLUWc3QF Message-ID: Date: Thu, 10 Sep 2020 21:48:55 +0300 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:60.0) Gecko/20100101 Firefox/60.0 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20200909134429.GA65588@raichu> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 4BnSbJ0vJwz4Tjm X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of agapon@gmail.com designates 209.85.208.193 as permitted sender) smtp.mailfrom=agapon@gmail.com X-Spamd-Result: default: False [-2.44 / 15.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; TO_DN_SOME(0.00)[]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17:c]; RCPT_COUNT_FIVE(0.00)[5]; RCVD_COUNT_THREE(0.00)[3]; NEURAL_HAM_SHORT(-0.62)[-0.617]; FORGED_SENDER(0.30)[avg@FreeBSD.org,agapon@gmail.com]; RECEIVED_SPAMHAUS_PBL(0.00)[93.72.151.96:received]; MIME_TRACE(0.00)[0:+]; R_DKIM_NA(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; MID_RHS_MATCH_FROM(0.00)[]; FROM_NEQ_ENVFROM(0.00)[avg@FreeBSD.org,agapon@gmail.com]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-0.84)[-0.843]; FROM_HAS_DN(0.00)[]; NEURAL_HAM_LONG(-0.98)[-0.983]; TAGGED_RCPT(0.00)[]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[FreeBSD.org]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCVD_IN_DNSWL_NONE(0.00)[209.85.208.193:from]; RWL_MAILSPIKE_POSSIBLE(0.00)[209.85.208.193:from]; FREEMAIL_CC(0.00)[gmail.com,freebsd.org]; RCVD_TLS_ALL(0.00)[]; MAILMAN_DEST(0.00)[svn-src-head,svn-src-all] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Sep 2020 18:49:00 -0000 On 09/09/2020 16:44, Mark Johnston wrote: > On Wed, Sep 09, 2020 at 08:49:01AM +0300, Andriy Gapon wrote: >> On 08/09/2020 15:48, Mark Johnston wrote: >>> I observed the same thing recently as well: the compiler catches >>> uninitialized variables only in simple cases. In my case, any uses of >>> goto within the function seemed to silence the warning, even if they >>> appeared after the uninitialized reference. >> >> I am running a kernel build now with this addition (for clang): >> CWARNEXTRA+= -Wconditional-uninitialized -Wno-error-conditional-uninitialized >> >> It produces a ton of warnings. >> Some of them are probably false positives, but some look quite reasonable. > > It has a lot of trouble with code patterns of the form: > > for (i = 0; i < 100; i++) { > val = foo(); > } > if (val != 0) /* may be uninitialized!!1 */ > bar(); > > or > > if (foo == bar) > val = baz(); > > if (foo == bar && val == 3) > > > The second example makes some sense to me since it's hard to prove that > foo == bar will not change between the first and second evaluations. I also noted the first pattern as the most common source of false positives. So, it seems that we cannot have what we want. Without -Wconditional-uninitialized clang is too conservative, with the option it's too "loose". I seem to recall that compilers used to be better than that. But maybe it's just false memories ("there used to be more snow in the winter", etc). >> E.g.: >> sys/cam/cam_periph.c:314:19: warning: variable 'p_drv' may be uninitialized when >> used here [-Wconditional-uninitialized] >> TAILQ_REMOVE(&(*p_drv)->units, periph, unit_links); >> >> Indeed, there is a conditional 'goto failure' before a first assignment to p_drv >> and the line is after the label. So, maybe the situation is impossible, but it >> is reasonable to warn about it. >> >> But the number of false positives (and "possible but impossible" situations) is >> too overwhelming. > > Yeah. I looked at maybe 30 warnings (out of hundreds) this morning > and they were all false positives. KMSAN will provide a new tool for > finding such bugs, but they will only be detected at runtime. > -- Andriy Gapon