linux-kernel - Re: [PATCH] arc: Fix __fls() const-foldability via __builtin

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ldn644d0.fsf@prevas.dk>
Date: Tue, 26 Aug 2025 15:08:59 +0200
From: Rasmus Villemoes <ravi@...vas.dk>
To: Kees Cook <kees@...nel.org>
Cc: Yury Norov <yury.norov@...il.com>,  kernel test robot <lkp@...el.com>,
  Vineet Gupta <vgupta@...nel.org>,  linux-snps-arc@...ts.infradead.org,
  linux-kernel@...r.kernel.org,  linux-hardening@...r.kernel.org
Subject: Re: [PATCH] arc: Fix __fls() const-foldability via __builtin_clzl()

On Mon, Aug 25 2025, Kees Cook <kees@...nel.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