On Thu, Nov 6, 2025 at 6:22 PM Andrew MacLeod <[email protected]> wrote:
>
> I can add a check in the inferred range manager for atomic loads to
> resolve this PR.
> The IL sequence tends to look like:
>
> _1 = &this_2(D)->b;
> __atomic_load_8 (_1, 0);
>
> Just want to make sure I get this right since memory operations are not
> my strong suit.
>
> The first argument to the atomic load is non-null (so _1), as well as
> the base of the RHS of the address expression that defines _1 are
> non-zero?. (this_2)
>
> The attached patch scavenges a little code from
> fold_using_range::range_of_address that I think works... but perhaps
> there is a more efficient way to do this? Is this likely to work OK and
> be safe? or are there additional checks I need to be doing?
>
> And I suppose there is an entire range of atomic operations this applies
> to? certainly atomic_store should qualify...
>
> Bootstraps on x86_64-pc-linux-gnu with no regressions, but I'm not sure
> that is a really extensive test for this..
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.
Thanks,
Andrew
>
> Andrew
>
