Re: [PATCH] arc: Fix __fls() const-foldability via __builtin_clzl()

From: Rasmus Villemoes <ravi@prevas.dk>
To: Kees Cook <kees@kernel.org>
Cc: Yury Norov <yury.norov@gmail.com>,
	 kernel test robot <lkp@intel.com>,
	Vineet Gupta <vgupta@kernel.org>,
	 linux-snps-arc@lists.infradead.org,
	linux-kernel@vger.kernel.org,  linux-hardening@vger.kernel.org
Subject: Re: [PATCH] arc: Fix __fls() const-foldability via __builtin_clzl()
Date: Tue, 26 Aug 2025 15:08:59 +0200	[thread overview]
Message-ID: <87ldn644d0.fsf@prevas.dk> (raw)
In-Reply-To: <20250826034354.work.684-kees@kernel.org> (Kees Cook's message of "Mon, 25 Aug 2025 20:43:58 -0700")

On Mon, Aug 25 2025, Kees Cook <kees@kernel.org> wrote:

> While tracking down a problem where constant expressions used by
> BUILD_BUG_ON() suddenly stopped working[1], we found that an added static
> initializer was convincing the compiler that it couldn't track the state
> of the prior statically initialized value. Tracing this down found that
> ffs() was used in the initializer macro, but since it wasn't marked with
> __attribute__const__, the compiler had to assume the function might
> change variable states as a side-effect (which is not true for ffs(),
> which provides deterministic math results).
>
> For arc architecture with CONFIG_ISA_ARCV2=y, the __fls() function
> uses __builtin_arc_fls() which lacks GCC's const attribute, preventing
> compile-time constant folding[2].

That sounds like a compiler bug, or "missed optimization". Looking into
the gcc source code, it seems that most "generic" builtins (well, those
eligible because they're just math) are internally declared with that
const attribute. E.g. there's

gcc/builtins.def:DEF_GCC_BUILTIN        (BUILT_IN_CLZL, "clzl", BT_FN_INT_ULONG, ATTR_CONST_NOTHROW_LEAF_LIST)

The DEF_BUILTIN macro for e.g. avr seems to support providing such
attributes, e.g. there's

gcc/config/avr/builtins.def:DEF_BUILTIN (ABSHR,   1, hr_ftype_hr,   ssabsqq2, "__ssabs_1", attr_const)

But the arc-specific ones in gcc/config/arc/builtins.def have no such
infrastructure readily available.

Of course, I also don't know if __builtin_arc_fls() is even eligible for
the const attribute (I don't know if it might have some side effects on
a flags register or something). But if it is, perhaps it's possible to
amend gcc's builtin declaration by explicitly (re)declaring it

int __builtin_arc_fls(int x) __attribute_const;

?

If __builtin_arc_fls() simply doesn't qualify for attr_const for
$reason, I think it would be good to have that documented in the commit
msg. If it does, I think a gcc ticket and link to that would be in order.

Rasmus