From owner-svn-src-all@freebsd.org Sat Feb 22 19:13:15 2020 Return-Path: Delivered-To: svn-src-all@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 485C3249977 for ; Sat, 22 Feb 2020 19:13:15 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from outbound2m.ore.mailhop.org (outbound2m.ore.mailhop.org [54.149.155.156]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 48Pyf26DGzz3Ljk for ; Sat, 22 Feb 2020 19:13:14 +0000 (UTC) (envelope-from ian@freebsd.org) ARC-Seal: i=1; a=rsa-sha256; t=1582398792; cv=none; d=outbound.mailhop.org; s=arc-outbound20181012; b=NvvofwJAqnCqiu6pCZyqi7dCZ5C2Of38pB+6+TbbUYvumljM7Tj/lPcBtY3PtJ8NldhDBZ0QQcBYP 2pVTeUSDHCMVHJPjkH3VpXAZCPQ3IX6OchJARec2ZwVXG0qnW8KhfHvu2KS/v7+1SdwVw3Jg8HugfT o9Oc1e8IPVSkh7yBCRuj6k0RJBTiwmE2CSruqVEssyWLMzNmWmG8pICswJJX0soColv0DriRIPu9Ob jkO1mUaqFIXABL78oMWRCV1BGD5l78SQp1q3oncz4euT6IJcwWkh9Dw8jb8J/gcEkGj7Uja9cp6LVT yhqiLw7iNnDbLZL07/O/iNXV/tCN2tA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=outbound.mailhop.org; s=arc-outbound20181012; h=content-transfer-encoding:mime-version:content-type:references:in-reply-to: date:cc:to:from:subject:message-id:dkim-signature:from; bh=AhSOT+Vz32L60NzVlHTOC3q9jxCgsaA9hNu6oyNhre4=; b=uFnH8YPR08CKm/G3JQt7YDRO6ajKIh3zc6ifySsbpELGQgg7iSXwfbG6DVwBcqwX0Ws0aKUFW/AqI LfsHeHS0ze/mi9ZmPk+qSO/vFdhMOwHZE86Z04U5CFfBStnGkn2l1MCR//GCLEMPR7CRTLYK73R5uN KVXX51Nam/Vj+XVHz1fgwQhkjSzj4ifzsWAt4Z4FUBYdXLHRSvh7shWv7OUBKwXlP7mVrEnGGjTM3k mMcY+dGBi0qEtlanVBuW8s3KO60908Yl6cEiODqgWakiaLdrAsm04BnADx3tj9XG4UdejPkGC24HP9 HMl8A+ScyKXltlJk6hPStZybUCDGHKw== ARC-Authentication-Results: i=1; outbound4.ore.mailhop.org; spf=softfail smtp.mailfrom=freebsd.org smtp.remote-ip=67.177.211.60; dmarc=none header.from=freebsd.org; arc=none header.oldest-pass=0; DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outbound.mailhop.org; s=dkim-high; h=content-transfer-encoding:mime-version:content-type:references:in-reply-to: date:cc:to:from:subject:message-id:from; bh=AhSOT+Vz32L60NzVlHTOC3q9jxCgsaA9hNu6oyNhre4=; b=RBeGHWhU3ieGbQChr1UijGguVoagHYGcUEgYeGnk1WEomdKOqqtKaOzEb5UUU3kZrqEka+iv49GcJ jELa8d2g9Mig/iwzhM6Ny7wp+JA873DUfSp8/ppr769mZ/QS5/bftD0I6sOO58oaOOPclxVtvfKlqg B5gM8ZAwrU2hXmJG6QJHpyNRwxFWZ5UUpDQcmANmGb1fuyo3Wgs1fNRLMkbnwwkj/GnTiTn9RziDjF EuozX8tlLPsvANnWrqUkSzPolf2VxlMWyPHUJcN0PHstXDCcZhcjXuIOBenAGI3VDGJUZs+0FmLFH3 nC/0ZjCeGlsFY5RUuXUocG5NX57c5sw== X-MHO-RoutePath: aGlwcGll X-MHO-User: 5d14ce98-55a7-11ea-9eb3-25e2dfa9fa8d X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 67.177.211.60 X-Mail-Handler: DuoCircle Outbound SMTP Received: from ilsoft.org (unknown [67.177.211.60]) by outbound4.ore.mailhop.org (Halon) with ESMTPSA id 5d14ce98-55a7-11ea-9eb3-25e2dfa9fa8d; Sat, 22 Feb 2020 19:13:11 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id 01MJD9uS016299; Sat, 22 Feb 2020 12:13:09 -0700 (MST) (envelope-from ian@freebsd.org) Message-ID: Subject: Re: svn commit: r358248 - head/sys/vm From: Ian Lepore To: Dimitry Andric , Mateusz Guzik Cc: Kyle Evans , svn-src-head , svn-src-all , src-committers Date: Sat, 22 Feb 2020 12:13:09 -0700 In-Reply-To: <6D39FAD8-E581-42A8-97B4-EE63800D78A4@andric.com> References: <202002221620.01MGK46E072303@repo.freebsd.org> <6D39FAD8-E581-42A8-97B4-EE63800D78A4@andric.com> Content-Type: text/plain; charset="ASCII" X-Mailer: Evolution 3.28.5 FreeBSD GNOME Team Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 48Pyf26DGzz3Ljk X-Spamd-Bar: - Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-1.96 / 15.00]; local_wl_from(0.00)[freebsd.org]; NEURAL_HAM_MEDIUM(-0.96)[-0.963,0]; ASN(0.00)[asn:16509, ipnet:54.148.0.0/15, country:US]; NEURAL_HAM_LONG(-1.00)[-0.997,0] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 22 Feb 2020 19:13:15 -0000 On Sat, 2020-02-22 at 20:01 +0100, Dimitry Andric wrote: > On 22 Feb 2020, at 17:44, Mateusz Guzik wrote: > > > > On 2/22/20, Kyle Evans wrote: > > > On Sat, Feb 22, 2020 at 10:25 AM Ian Lepore > > > wrote: > > > > > > > > On Sat, 2020-02-22 at 16:20 +0000, Kyle Evans wrote: > > > > > Author: kevans > > > > > Date: Sat Feb 22 16:20:04 2020 > > > > > New Revision: 358248 > > > > > URL: https://svnweb.freebsd.org/changeset/base/358248 > > > > > > > > > > Log: > > > > > vm_radix: prefer __builtin_unreachable() to an unreachable > > > > > panic() > > > > > > > > > > This provides the needed hint to GCC and offers an > > > > > annotation for > > > > > readers to > > > > > observe that it's in-fact impossible to hit this point. > > > > > We'll get hit > > > > > with a > > > > > a -Wswitch error if the enum applicable to the switch above > > > > > were to > > > > > get > > > > > expanded without the new value(s) being handled. > > > > > > > > > > Modified: > > > > > head/sys/vm/vm_radix.c > > > > > > > > > > Modified: head/sys/vm/vm_radix.c > > > > > ============================================================= > > > > > ================= > > > > > --- head/sys/vm/vm_radix.c Sat Feb 22 13:23:27 > > > > > 2020 (r358247) > > > > > +++ head/sys/vm/vm_radix.c Sat Feb 22 16:20:04 > > > > > 2020 (r358248) > > > > > @@ -208,8 +208,7 @@ vm_radix_node_load(smrnode_t *p, enum > > > > > vm_radix_access > > > > > case SMR: > > > > > return (smr_entered_load(p, vm_radix_smr)); > > > > > } > > > > > - /* This is unreachable, silence gcc. */ > > > > > - panic("vm_radix_node_get: Unknown access type"); > > > > > + __unreachable(); > > > > > } > > > > > > > > > > static __inline void > > > > > > > > What does __unreachable() do if the code ever becomes > > > > reachable? Like > > > > if a new enum value is added and this switch doesn't get > > > > updated? > > > > > > > > > > __unreachable doesn't help here, but the compiler will error out > > > on > > > the switch() if all enum values aren't addressed and there's no > > > default: case. > > > > > > IMO, compilers could/should become smart enough to error if > > > there's an > > > explicit __builtin_unreachable() and they can trivially determine > > > that > > > all paths will terminate before this, independent of > > > -Werror=switch*. > > > _______________________________________________ > > > > I think this is way too iffy, check this program: > > > > > > #include > > > > int > > main(void) > > { > > > > __builtin_unreachable(); > > printf("test\n"); > > } > > > > Neither clang nor gcc warn about this and both stop code generation > > past the statement. > > Indeed, that is exactly the intent. See: > > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable > > "If control flow reaches the point of the __builtin_unreachable, the > program is undefined. It is useful in situations where the compiler > cannot deduce the unreachability of the code." > > E.g. this is *not* meant as a way to enforce the program to abort at > runtime, if the supposedly unreachable part is actually reached. > > For this purpose, one should use an abort() or panic() function call, > with such functions being annotated to never return. > > -Dimitry > The problem is, people will see usages such as what Kyle did, where the code truly is unreachable (due to -Werror=switch), and not realizing that's why it's valid there, they'll assume it's a type of assert- unreachable and copy it/use it in other places as if that's what it was for. So, IMO, using it should be exceedingly rare and there should be a comment nearby about why it's valid in that context, or our __unreachable cover for it should panic on INVARIANTS, as Kyle proposed in an earlier reply. -- Ian