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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1ECFAC433F5 for ; Mon, 4 Apr 2022 22:53:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=9qmloq5+owSnn328ohpbIvWM6gGlAMliNY3LY98JoVc=; b=mdzRpv0iEEbqaG 68dT/3nbJLrzihL4DVEOQRRKSSFQ1tmbVeBRN1wN/yasmOB4tUDAWydIm4MWAbbSUp8dN2M/cBtuF wTlh2b87C8amBUq7tyypGJEqDzgZpOaP2gHs4dGLCeAzg1qN4gcpLjTQrQyEVgYkAxtlFqtv1GjU7 b783pZqwHaV34MBBTGIDCz5dbkpUzOVlqHNStmoVE3VZFhj73WI+h6nkMUyS3LgZARlXxb20Ycvhl IwqhmWxoWVpSYZAfvKOVKKOGn0txzub+ccxNJ4HUVpGzE5RcD61w/98gfb3xXgEXGB9qYWIjgJaYh POmwdt/7LcUl1ngk6Fgg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nbVYZ-00Gcle-M7; Mon, 04 Apr 2022 22:52:19 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nbVYW-00GckG-8S for linux-arm-kernel@lists.infradead.org; Mon, 04 Apr 2022 22:52:18 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 07BE0616C1; Mon, 4 Apr 2022 22:52:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CFCC6C2BBE4; Mon, 4 Apr 2022 22:52:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1649112733; bh=54rQNEoc5ehhks896JDl4yq3WCx9/eYdhiBhR9iefCc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=bF3QJRnbcjc8y6fPNoXkVkRgf5gEx3NVFnuts+9skTbjtP9eot8cnBXlFI8pw4fuE GDxPaF9NNC+WKlfDsnXfLMOEPfRc4SbvvzHprSmilCsuJiNcMP9TXgJX0wirB2EJU0 1jt2jv3o/4kcckKRO0ZLqHFrWfTjDbf8Uy9BGbXRYBZHFHKIj0GPlWBKZvOJgZqbm9 5c2ejL+9CkNE5w62Iy313yMgPWNfOLxHEqB+RZKw2gQBiiajiu/lpyCo/x+BlNMkoK HVm2lhF8OwyNiL9R63NGAXx12znc6NTHCGZABalsH/d85DkPbOIyGTj3JVExakaPjy Smyk9vwpAPC8Q== Date: Mon, 4 Apr 2022 15:52:11 -0700 From: Nathan Chancellor To: Sami Tolvanen Cc: Masahiro Yamada , Kees Cook , Catalin Marinas , Mark Rutland , Nick Desaulniers , Will Deacon , linux-arm-kernel , Linux Kernel Mailing List , llvm@lists.linux.dev Subject: Re: [PATCH 1/3] kbuild: Change CFI_CLANG to depend on __builtin_function_start Message-ID: References: <20220401201916.1487500-1-samitolvanen@google.com> <20220401201916.1487500-2-samitolvanen@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220404_155216_428449_D6E9C758 X-CRM114-Status: GOOD ( 28.20 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Apr 04, 2022 at 12:40:46PM -0700, Sami Tolvanen wrote: > On Sat, Apr 2, 2022 at 6:32 AM Masahiro Yamada wrote: > > > > On Sat, Apr 2, 2022 at 5:19 AM Sami Tolvanen wrote: > > > > > > Clang 14 added support for the __builtin_function_start() > > > built-in function, which allows us to implement function_nocfi() > > > without architecture-specific inline assembly. This patch changes > > > CONFIG_CFI_CLANG to depend on the built-in and effectively upgrades > > > the minimum supported compiler version for CFI to Clang 14. > > > > From this description, I think the straight-forward change would be: > > > > depends on CLANG_VERSION >= 120000 > > --> > > depends on CLANG_VERSION >= 140000 > > > > Any reason to avoid this? > > I thought testing for the compiler feature was preferred, but I can > certainly just increase the minimum version number here too. I think we have been somewhat inconsistent with feature versus version checking. It might be nice to hash out when a feature check should be done instead of a version one. Generally, I think we tend to prefer version checks, as they are "cheaper" since we do not have to call the compiler again because we already cached the version code. When adding version checks, our policy has always been use the upstream version of LLVM that the feature in question shipped in, even if it is a top of tree version, as people who are using prereleased versions of LLVM should be frequently updating them. Unfortunately, that does not always match reality. For example, Android's LLVM tracks the main branch but they are almost always behind by a few months. For example, the latest release is 14.0.4, based on a version of LLVM from January 28th: https://android.googlesource.com/platform/prebuilts/clang/host/linux-x86/+/ab73cd180863dbd17fdb8f20e39b33ab38030cf9/clang-r450784b/clang_source_info.md https://github.com/llvm/llvm-project/commits/282c83c32384cb2f37030c28650fef4150a8b67c Normally, I would say "who cares?" but Android's LLVM is used by the Android kernel team both downstream and upstream, so I would argue it is important to take that into account when deciding to do a feature check versus a version check. In other words, by moving to a version check, will we knowingly break a version of clang that is relatively widely used? In this case, 14.0.4 has __builtin_function_start(), so I think it is okay to use a version check instead of a feature one. There are times where feature or problem checking is not always possible: https://lore.kernel.org/r/20220318230747.3900772-1-nathan@kernel.org/ In cases like these, we can work to upgrade the compiler before changing the feature check ot a version one, which helps minimize the window for breakage. Another aspect of feature versus version checks is that it is easier to clean up stale versions checks when we bump the minimum supported version of the compiler (as we can just look for "CLANG_VERSION" across the tree) compared to stale feature checks. We could fix this by adding some sort of common keyword, like Compiler check: like Compiler check: clang <= 14.0.0 Cheers, Nathan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel