On 11/6/25 10:46 PM, Andrew Pinski wrote:
On Thu, Nov 6, 2025 at 6:22 PM Andrew MacLeod <[email protected]> wrote:
Isn't the other route on fixing this is by adding the nonnull
attribute to these builtins? I tried looking to see if anyone had
mentioned why these builtins didn't have the nonnull attribute on them
but I didn't find anything.
The nonnull attribute support for builtins has been there since 2002
and atomic builtins were added afterwards (towards 2011).
DEF_ATTR_TREE_LIST (ATTR_NONNULL_1_NOTHROW_LEAF_LIST, ATTR_NONNULL, \
ATTR_LIST_1, ATTR_NOTHROW_LEAF_LIST)
DEF_ATTR_TREE_LIST (ATTR_NONNULL_1_LEAF_LIST , ATTR_NONNULL, \
ATTR_LIST_1, ATTR_LEAF_LIST )
#define ATTR_NONNULL_1_NOTHROWCALL_LEAF_LIST (flag_non_call_exceptions ? \
ATTR_NONNULL_1_LEAF_LIST : ATTR_NONNULL_1_NOTHROW_LEAF_LIST)
And then use ATTR_NONNULL_1_NOTHROWCALL_LEAF_LIST instead of
ATTR_NOTHROWCALL_LEAF_LIST in DEF_SYNC_BUILTIN for the builtins in
sync-builtins.def that are non null for the 1st argument.
I can only think that nobody has looked into the code generation
enough here to make a big difference. This would also handle isolate
paths too without any extra coding so I suspect special casing these
builtins everywhere is not a route which we want just for nonnullness.
I suspect we just missed that these should have been marked. We should
be doing the right thing with the markup -- meaning we can assume the
the value is non-null after the call point since the "call" would have
trapped on a NULL dereference. We probably also make limited use of the
non-null property on the path to the call, though we have to be a bit
more careful with backpropagating the non-null property.
jeff
