Merge pull request #25362 from JuliaLang/jb/pointer_from_objref

disallow `pointer_from_objref` on immutable values. fixes #15857

Author: JeffBezanson

base/essentials.jl

@@ -533,7 +533,7 @@ function getindex(v::SimpleVector, i::Int)
         throw(BoundsError(v,i))
     end
     t = @_gc_preserve_begin v
-    x = unsafe_load(convert(Ptr{Ptr{Cvoid}},data_pointer_from_objref(v)) + i*sizeof(Ptr))
+    x = unsafe_load(convert(Ptr{Ptr{Cvoid}},pointer_from_objref(v)) + i*sizeof(Ptr))
     x == C_NULL && throw(UndefRefError())
     o = unsafe_pointer_to_objref(x)
     @_gc_preserve_end t
@@ -542,7 +542,7 @@ end
 
 function length(v::SimpleVector)
     t = @_gc_preserve_begin v
-    l = unsafe_load(convert(Ptr{Int},data_pointer_from_objref(v)))
+    l = unsafe_load(convert(Ptr{Int},pointer_from_objref(v)))
     @_gc_preserve_end t
     return l
 end
@@ -598,7 +598,7 @@ function isassigned end
 function isassigned(v::SimpleVector, i::Int)
     @boundscheck 1 <= i <= length(v) || return false
     t = @_gc_preserve_begin v
-    x = unsafe_load(convert(Ptr{Ptr{Cvoid}},data_pointer_from_objref(v)) + i*sizeof(Ptr))
+    x = unsafe_load(convert(Ptr{Ptr{Cvoid}},pointer_from_objref(v)) + i*sizeof(Ptr))
     @_gc_preserve_end t
     return x != C_NULL
 end

base/inference.jl

@@ -1331,6 +1331,7 @@ const DataType_name_fieldindex = fieldindex(DataType, :name)
 const DataType_parameters_fieldindex = fieldindex(DataType, :parameters)
 const DataType_types_fieldindex = fieldindex(DataType, :types)
 const DataType_super_fieldindex = fieldindex(DataType, :super)
+const DataType_mutable_fieldindex = fieldindex(DataType, :mutable)
 
 const TypeName_name_fieldindex = fieldindex(TypeName, :name)
 const TypeName_module_fieldindex = fieldindex(TypeName, :module)
@@ -1340,7 +1341,8 @@ function const_datatype_getfield_tfunc(sv, fld)
     if (fld == DataType_name_fieldindex ||
             fld == DataType_parameters_fieldindex ||
             fld == DataType_types_fieldindex ||
-            fld == DataType_super_fieldindex)
+            fld == DataType_super_fieldindex ||
+            fld == DataType_mutable_fieldindex)
         return abstract_eval_constant(getfield(sv, fld))
     end
     return nothing
@@ -2713,7 +2715,7 @@ function abstract_eval(@nospecialize(e), vtypes::VarTable, sv::InferenceState)
             spsig = sv.linfo.def.sig
             if isa(spsig, UnionAll)
                 if !isempty(sv.linfo.sparam_vals)
-                    env = data_pointer_from_objref(sv.linfo.sparam_vals) + sizeof(Ptr{Cvoid})
+                    env = pointer_from_objref(sv.linfo.sparam_vals) + sizeof(Ptr{Cvoid})
                     rt = ccall(:jl_instantiate_type_in_env, Any, (Any, Any, Ptr{Any}), e.args[2], spsig, env)
                 else
                     rt = rewrap_unionall(e.args[2], spsig)

base/pointer.jl

@@ -133,9 +133,15 @@ unsafe_pointer_to_objref(x::Ptr) = ccall(:jl_value_ptr, Any, (Ptr{Cvoid},), x)
 Get the memory address of a Julia object as a `Ptr`. The existence of the resulting `Ptr`
 will not protect the object from garbage collection, so you must ensure that the object
 remains referenced for the whole time that the `Ptr` will be used.
+
+This function may not be called on immutable objects, since they do not have
+stable memory addresses.
 """
-pointer_from_objref(@nospecialize(x)) = ccall(:jl_value_ptr, Ptr{Cvoid}, (Any,), x)
-data_pointer_from_objref(@nospecialize(x)) = pointer_from_objref(x)::Ptr{Cvoid}
+function pointer_from_objref(@nospecialize(x))
+    @_inline_meta
+    typeof(x).mutable || error("pointer_from_objref cannot be used on immutable objects")
+    ccall(:jl_value_ptr, Ptr{Cvoid}, (Any,), x)
+end
 
 eltype(::Type{Ptr{T}}) where {T} = T
 

base/reflection.jl

@@ -275,7 +275,8 @@ julia> isimmutable([1,2])
 false
 ```
 """
-isimmutable(@nospecialize(x)) = (@_pure_meta; (isa(x,Tuple) || !typeof(x).mutable))
+isimmutable(@nospecialize(x)) = (@_pure_meta; !typeof(x).mutable)
+
 isstructtype(t::DataType) = (@_pure_meta; length(t.types) != 0 || (t.size==0 && !t.abstract))
 isstructtype(x) = (@_pure_meta; false)
 

base/refpointer.jl

@@ -61,7 +61,7 @@ Ref(x::Ptr{T}, i::Integer) where {T} = x + (i - 1) * Core.sizeof(T)
 function unsafe_convert(P::Type{Ptr{T}}, b::RefValue{T}) where T
     if isbits(T) || isbitsunion(T)
         return convert(P, pointer_from_objref(b))
-    elseif _isleaftype(T)
+    elseif _isleaftype(T) && T.mutable
         return convert(P, pointer_from_objref(b.x))
     else
         # If the slot is not leaf type, it could be either isbits or not.
@@ -73,7 +73,7 @@ function unsafe_convert(P::Type{Ptr{T}}, b::RefValue{T}) where T
     end
 end
 function unsafe_convert(P::Type{Ptr{Any}}, b::RefValue{Any})
-    return convert(P, data_pointer_from_objref(b))
+    return convert(P, pointer_from_objref(b))
 end
 unsafe_convert(::Type{Ptr{Cvoid}}, b::RefValue{T}) where {T} = convert(Ptr{Cvoid}, unsafe_convert(Ptr{T}, b))
 
@@ -93,7 +93,7 @@ function unsafe_convert(P::Type{Ptr{T}}, b::RefArray{T}) where T
     if isbits(T)
         convert(P, pointer(b.x, b.i))
     else
-        convert(P, data_pointer_from_objref(b.x[b.i]))
+        convert(P, pointer_from_objref(b.x[b.i]))
     end
 end
 function unsafe_convert(P::Type{Ptr{Any}}, b::RefArray{Any})

base/serialize.jl

@@ -79,7 +79,9 @@ const ser_version = 7 # do not make changes without bumping the version #!
 const NTAGS = length(TAGS)
 
 function sertag(@nospecialize(v))
-    ptr = pointer_from_objref(v)
+    # NOTE: we use jl_value_ptr directly since we know at least one of the arguments
+    # in the comparison below is a singleton.
+    ptr = ccall(:jl_value_ptr, Ptr{Cvoid}, (Any,), v)
     ptags = convert(Ptr{Ptr{Cvoid}}, pointer(TAGS))
     # note: constant ints & reserved slots never returned here
     @inbounds for i in 1:(NTAGS-(n_reserved_slots+2*n_int_literals))

base/show.jl

@@ -159,9 +159,12 @@ function show_default(io::IO, @nospecialize(x))
         end
     else
         print(io, "0x")
-        p = data_pointer_from_objref(x)
-        for i in (nb - 1):-1:0
-            print(io, hex(unsafe_load(convert(Ptr{UInt8}, p + i)), 2))
+        r = Ref(x)
+        @gc_preserve r begin
+            p = unsafe_convert(Ptr{Cvoid}, r)
+            for i in (nb - 1):-1:0
+                print(io, hex(unsafe_load(convert(Ptr{UInt8}, p + i)), 2))
+            end
         end
     end
     print(io,')')

base/summarysize.jl

@@ -65,7 +65,9 @@ end
 (ss::SummarySize)(@nospecialize obj) = _summarysize(ss, obj)
 # define the general case separately to make sure it is not specialized for every type
 @noinline function _summarysize(ss::SummarySize, @nospecialize obj)
-    key = pointer_from_objref(obj)
+    # NOTE: this attempts to discover multiple copies of the same immutable value,
+    # and so is somewhat approximate.
+    key = ccall(:jl_value_ptr, Ptr{Cvoid}, (Any,), obj)
     haskey(ss.seen, key) ? (return 0) : (ss.seen[key] = true)
     if _nfields(obj) > 0
         push!(ss.frontier_x, obj)

test/core.jl

@@ -1164,7 +1164,10 @@ end
 
 @test unsafe_pointer_to_objref(ccall(:jl_call1, Ptr{Cvoid}, (Any,Any),
                                      x -> x+1, 314158)) == 314159
-@test unsafe_pointer_to_objref(pointer_from_objref(ℯ+pi)) == ℯ+pi
+let x = [1,2,3]
+    @test unsafe_pointer_to_objref(pointer_from_objref(x)) == x
+    @test unsafe_pointer_to_objref(pointer_from_objref(x)) === x
+end
 
 let
     local a, aa
@@ -3467,11 +3470,12 @@ end
 struct HasHasPadding
     x::HasPadding
 end
-hashaspadding = HasHasPadding(HasPadding(true,1))
-hashaspadding2 = HasHasPadding(HasPadding(true,1))
-unsafe_store!(convert(Ptr{UInt8},pointer_from_objref(hashaspadding)), 0x12, 2)
-unsafe_store!(convert(Ptr{UInt8},pointer_from_objref(hashaspadding2)), 0x21, 2)
-@test object_id(hashaspadding) == object_id(hashaspadding2)
+let hashaspadding = Ref(HasHasPadding(HasPadding(true,1))),
+    hashaspadding2 = Ref(HasHasPadding(HasPadding(true,1)))
+    unsafe_store!(convert(Ptr{UInt8},pointer_from_objref(hashaspadding)), 0x12, 2)
+    unsafe_store!(convert(Ptr{UInt8},pointer_from_objref(hashaspadding2)), 0x21, 2)
+    @test object_id(hashaspadding[]) == object_id(hashaspadding2[])
+end
 
 # issue #12517
 let x = (1,2)
@@ -5638,10 +5642,11 @@ let
     @eval @noinline constant23367(a, b) = (a ? b : $b)
     b2 = Ref(b)[] # copy b via field assignment
     b3 = B23367[b][1] # copy b via array assignment
-    @test pointer_from_objref(b) == pointer_from_objref(b)
-    @test pointer_from_objref(b) != pointer_from_objref(b2)
-    @test pointer_from_objref(b) != pointer_from_objref(b3)
-    @test pointer_from_objref(b2) != pointer_from_objref(b3)
+    addr(@nospecialize x) = ccall(:jl_value_ptr, Ptr{Cvoid}, (Any,), x)
+    @test addr(b)  == addr(b)
+    @test addr(b)  != addr(b2)
+    @test addr(b)  != addr(b3)
+    @test addr(b2) != addr(b3)
 
     @test b === b2 === b3
     @test compare(b, b2)

test/inference.jl

@@ -1300,3 +1300,8 @@ g_test_constant() = (f_constant(3) == 3 && f_constant(4) == 4 ? true : "BAD")
 
 f_pure_add() = (1 + 1 == 2) ? true : "FAIL"
 @test @inferred f_pure_add()
+
+# inference of `T.mutable`
+@test Core.Inference.getfield_tfunc(Const(Int), Const(:mutable)) == Const(false)
+@test Core.Inference.getfield_tfunc(Const(Vector{Int}), Const(:mutable)) == Const(true)
+@test Core.Inference.getfield_tfunc(DataType, Const(:mutable)) == Bool