Bug 84210 - __ubsan_handle_builtin_unreachable shoun't be const
Summary: __ubsan_handle_builtin_unreachable shoun't be const
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-05 10:29 UTC by Andrey Ryabinin
Modified: 2024-04-03 18:51 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2024-04-03 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Ryabinin 2018-02-05 10:29:59 UTC
Compiling the linux kernel with gcc-8 gives this warning:

lib/ubsan.c:432:1: error: ignoring attribute 'noreturn' in declaration of a built-in function '__ubsan_handle_builtin_unreachable' because it conflicts with attribute 'const' [-Werror=attributes]


It seems that GCC defines __ubsan_handle_builtin_unreachable with noreturn and const attributes:

DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE,
                      "__ubsan_handle_builtin_unreachable",
                      BT_FN_VOID_PTR,
                      ATTR_COLD_CONST_NORETURN_NOTHROW_LEAF_LIST)

And const doesn't make any sense for function that returns void or doesn't return at all.
Shouldn't it be just ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST ?
Comment 1 Richard Biener 2018-02-05 11:35:06 UTC
Confirmed.  Note that I'm not sure it makes no sense - it just means the function has no side-effect besides not returning ;)

lib/ubsan.c is from the kernel I guess where you "copy" the attributes from
sanitizer.def?

GCC internally could also use "no vops" but you couldn't replicate that.

Not sure if it is/was important to have the function 'const'.
Comment 2 Andrey Ryabinin 2018-02-05 12:21:14 UTC
(In reply to Richard Biener from comment #1)
> Confirmed.  Note that I'm not sure it makes no sense - it just means the
> function has no side-effect besides not returning ;)
> 

Well, GCC docs say that const functions "have no effects except to return a value" and "It does not make sense for a const function to return void."
I suppose that noreturn functions fells into the same category here as return void functions.

Besides, __ubsan_handle_builtin_unreachable() does have side-effect, it prints error.


> lib/ubsan.c is from the kernel I guess where you "copy" the attributes from
> sanitizer.def?
> 

Yes, lib/ubsan.c is kernel's implementation of libubsan. We don't exactly 'copy' attributes, but __ubsan_handle_builtin_unreachable() does have noreturn.
I've tried to add const to be consistent with GCC, but it become only worse:

../lib/ubsan.c:432:1: warning: ignoring attribute ‘noreturn’ in declaration of a built-in function ‘__ubsan_handle_builtin_unreachable’ because it conflicts with attribute ‘const’ [-Wattributes]
 {
 ^
<built-in>: note: previous declaration here
../lib/ubsan.c:432:1: warning: ‘const’ attribute on function returning ‘void’ [-Wattributes]
../lib/ubsan.c:432:1: warning: ignoring attribute ‘const’ in declaration of a built-in function ‘__ubsan_handle_builtin_unreachable’ because it conflicts with attribute ‘noreturn’ [-Wattributes]
<built-in>: note: previous declaration here


> GCC internally could also use "no vops" but you couldn't replicate that.
> 

We probably don't have to replicate all attributes. At least userspace libubsan doesn't do that at all.

In fact we could workaround this warning simply by dropping noreturn: http://lkml.kernel.org/r/20180202154813.1625742-1-arnd@arndb.de

But, I'm thinking that there is something wrong in GCC, hence filed this bug.

> Not sure if it is/was important to have the function 'const'.
Comment 3 Martin Liška 2018-02-05 12:28:37 UTC
The const for the builtin was added by Marek as fix for PR63839.
Adding him to CC.
Comment 4 Marek Polacek 2018-02-05 13:23:36 UTC
I don't remember if the const was needed.  I guess if the testcases added in r217553 still pass even without the const then we can get rid of it.
Comment 5 Andrew Pinski 2024-04-03 18:51:34 UTC
And then there was an issue where we convert __builtin_unreachable to __builtin_trap which had the similar issue so we added:
DEF_GCC_BUILTIN        (BUILT_IN_UNREACHABLE_TRAP, "unreachable trap", BT_FN_VOID, ATTR_CONST_NORETURN_NOTHROW_LEAF_COLD_LIST)

instead, see PR 107300 for the issue there. 

So the question becomes why was __builtin_unreachable marked as const, I have no idea.
Anyways reconfirmed.