From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from pegase2.c-s.fr (pegase2.c-s.fr [93.17.235.10]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 4A53C250BF6; Wed, 19 Mar 2025 08:20:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=93.17.235.10 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742372406; cv=none; b=ny9fw6RmHy/pgwviwLs9LDxYKL8YYHB2XvZzOllM4ipdXUSw1UQGb7HaOcSEAefNCJMbR5HS0eLpi4vb1jyGRmJiyJGdP1hv3pRlmn+kJutOyU3XnFZFnrmMhn6igENy5KeCLNSihus0YpP0igameG5Fx4E9qi0iKYCMH/YKwCU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742372406; c=relaxed/simple; bh=9OlvM4XUWWvrHJOd2iYbZSXPoeKrvur3cTL1QmV2LNQ=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=jGPJqRUmdTHdWfAUsr7ntxP/W8arq3G/Iffg08MrASUp/VinXPonljuAYzkK09NwFa5LW256RcQKiQlbpa3nO9gVg/akVdb+kbdE6zGg2iHxeMZU/vNdIWv3DZnmvfggYrbvm7+1mN8FpBwV1uXHo4CO0bx/W3CS98Eh0Msc7N8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=csgroup.eu; spf=pass smtp.mailfrom=csgroup.eu; arc=none smtp.client-ip=93.17.235.10 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=csgroup.eu Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=csgroup.eu Received: from localhost (mailhub3.si.c-s.fr [172.26.127.67]) by localhost (Postfix) with ESMTP id 4ZHh8p3p47z9sSY; Wed, 19 Mar 2025 09:05:30 +0100 (CET) X-Virus-Scanned: amavisd-new at c-s.fr Received: from pegase2.c-s.fr ([172.26.127.65]) by localhost (pegase2.c-s.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 5Pdv_1f5SWZW; Wed, 19 Mar 2025 09:05:30 +0100 (CET) Received: from messagerie.si.c-s.fr (messagerie.si.c-s.fr [192.168.25.192]) by pegase2.c-s.fr (Postfix) with ESMTP id 4ZHh8p2Mb8z9sSX; Wed, 19 Mar 2025 09:05:30 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 2617E8B78F; Wed, 19 Mar 2025 09:05:30 +0100 (CET) X-Virus-Scanned: amavisd-new at c-s.fr Received: from messagerie.si.c-s.fr ([127.0.0.1]) by localhost (messagerie.si.c-s.fr [127.0.0.1]) (amavisd-new, port 10023) with ESMTP id K6S1MuFrSThd; Wed, 19 Mar 2025 09:05:30 +0100 (CET) Received: from [192.168.235.99] (unknown [192.168.235.99]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 585CB8B763; Wed, 19 Mar 2025 09:05:28 +0100 (CET) Message-ID: Date: Wed, 19 Mar 2025 09:05:27 +0100 Precedence: bulk X-Mailing-List: linux-s390@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 07/14] arm64: Add support for suppressing warning backtraces To: Will Deacon , Alessandro Carminati Cc: linux-kselftest@vger.kernel.org, David Airlie , Arnd Bergmann , =?UTF-8?Q?Ma=C3=ADra_Canal?= , Dan Carpenter , Kees Cook , Daniel Diaz , David Gow , Arthur Grillo , Brendan Higgins , Naresh Kamboju , Maarten Lankhorst , Andrew Morton , Maxime Ripard , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Daniel Vetter , Thomas Zimmermann , Guenter Roeck , Alessandro Carminati , Jani Nikula , dri-devel@lists.freedesktop.org, kunit-dev@googlegroups.com, linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, loongarch@lists.linux.dev, x86@kernel.org, Linux Kernel Functional Testing , Catalin Marinas References: <20250313114329.284104-1-acarmina@redhat.com> <20250313114329.284104-8-acarmina@redhat.com> <20250313122503.GA7438@willie-the-truck> <20250318155946.GC13829@willie-the-truck> Content-Language: fr-FR From: Christophe Leroy In-Reply-To: <20250318155946.GC13829@willie-the-truck> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Le 18/03/2025 à 16:59, Will Deacon a écrit : > On Thu, Mar 13, 2025 at 05:40:59PM +0100, Alessandro Carminati wrote: >> On Thu, Mar 13, 2025 at 1:25 PM Will Deacon wrote: >>> >>> On Thu, Mar 13, 2025 at 11:43:22AM +0000, Alessandro Carminati wrote: >>>> diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h >>>> index 28be048db3f6..044c5e24a17d 100644 >>>> --- a/arch/arm64/include/asm/bug.h >>>> +++ b/arch/arm64/include/asm/bug.h >>>> @@ -11,8 +11,14 @@ >>>> >>>> #include >>>> >>>> +#ifdef HAVE_BUG_FUNCTION >>>> +# define __BUG_FUNC __func__ >>>> +#else >>>> +# define __BUG_FUNC NULL >>>> +#endif >>>> + >>>> #define __BUG_FLAGS(flags) \ >>>> - asm volatile (__stringify(ASM_BUG_FLAGS(flags))); >>>> + asm volatile (__stringify(ASM_BUG_FLAGS(flags, %c0)) : : "i" (__BUG_FUNC)); >>> >>> Why is 'i' the right asm constraint to use here? It seems a bit odd to >>> use that for a pointer. >> >> I received this code as legacy from a previous version. >> In my review, I considered the case when HAVE_BUG_FUNCTION is defined: >> Here, __BUG_FUNC is defined as __func__, which is the name of the >> current function as a string literal. >> Using the constraint "i" seems appropriate to me in this case. >> >> However, when HAVE_BUG_FUNCTION is not defined: >> __BUG_FUNC is defined as NULL. Initially, I considered it literal 0, >> but after investigating your concern, I found: >> >> ``` >> $ echo -E "#include \n#include \nint main() >> {\nreturn 0;\n}" | aarch64-linux-gnu-gcc -E -dM - | grep NULL >> #define NULL ((void *)0) >> ``` >> >> I realized that NULL is actually a pointer that is not a link time >> symbol, and using the "i" constraint with NULL may result in undefined >> behavior. >> >> Would the following alternative definition for __BUG_FUNC be more convincing? >> >> ``` >> #ifdef HAVE_BUG_FUNCTION >> #define __BUG_FUNC __func__ >> #else >> #define __BUG_FUNC (uintptr_t)0 >> #endif >> ``` >> Let me know your thoughts. > > Thanks for the analysis; I hadn't noticed this specific issue, it just > smelled a bit fishy. Anyway, the diff above looks better, thanks. That propably deserves a comment. Doesn't sparse and/or checkpatch complain about 0 being used in lieu of NULL ? By the way I had similar problem in the past with GCC not seeing NULL as a __builtin_constant_p(), refer commit 1d8f739b07bd ("powerpc/kuap: Fix set direction in allow/prevent_user_access()") Christophe