make Type{Union{}} ismutationfree

[gbaraldi]

Type{Union{}} and Core.TypeofBottom are the same type but not the same object, and Type{Union{}} has different flags flags, normalize them so we don't taint functions that return Union{}. This was causing exp to not be inacessiblememonly.

It seems this regressed on #48220

[Keno]

The ismutationfree flag is supposed to be set for all Type instantiations. This seems like a bug.

[Keno]

Try this patch:

diff --git a/src/jltypes.c b/src/jltypes.c
index 9428bf6a91..d9f50d67d3 100644
--- a/src/jltypes.c
+++ b/src/jltypes.c
@@ -2204,6 +2204,7 @@ void jl_init_types(void) JL_GC_DISABLED
     ((jl_datatype_t*)jl_type_type)->cached_by_hash = 0;
     jl_type_typename->wrapper = jl_new_struct(jl_unionall_type, tttvar, (jl_value_t*)jl_type_type);
     jl_type_type = (jl_unionall_t*)jl_type_typename->wrapper;
+    ((jl_datatype_t*)jl_type_type->body)->ismutationfree = 1;

     jl_typeofbottom_type->super = jl_wrap_Type(jl_bottom_type);

@@ -2807,7 +2808,6 @@ void jl_init_types(void) JL_GC_DISABLED
     jl_symbol_type->ismutationfree = jl_symbol_type->isidentityfree = 1;
     jl_simplevector_type->ismutationfree = jl_simplevector_type->isidentityfree = 1;
     jl_datatype_type->ismutationfree = 1;
-    ((jl_datatype_t*)jl_type_type->body)->ismutationfree = 1;

     // Technically not ismutationfree, but there's a separate system to deal
     // with mutations for global state.

[vtjnash]

But there is a lot of mutable memory reachable via DataType?

[Keno]

We currently have DataType be ismutationfree, otherwise @gbaraldi's issue would be much larger. If we think that's wrong, because Julia can observe it being mutated, then we need to come up with something better. Note that this predates the introduction of the flag though.

[vtjnash]

It seems awkward to need to lie quite so much about it. Unless this predicate (inaccessiblememonly) means that it won't change anything directly through that pointer, which is usually true.

[gbaraldi]

Llvm is changing these attributes right now, but it basically means that it doesn't read or write to memory other functions can see.

[vtjnash]

s/see/mutate/, but that is at odds then with DataType, which generally does reach mutable memory

[gbaraldi]

Then we might have to do this in a case by case level.

[aviatesk]

Making !ismutationfree(T::DataType) would significantly decrease the precision of the :inaccessiblememonly analysis. To mitigate this, we could potentially introduce a new bit to :inaccessiblememonly to mark access to DataType itself as "almost :inaccessiblememonly" but taint it immediately when accessing its properties (such as T.name).

[gbaraldi]

Do you folks want to close this and think of a longer term solution, or merge this as a partial fix and work on something that gives more precise information. I will say that it seems most instances of Type{whatever} do have the bit set, and Union{} was the odd one out.

[aviatesk]

I'd like to go with Keno's patch to fix the precision for now. And then fix the correctness issue in a separate PR.