From 578466d506dab0f23f6df78034eeafe5e911d599 Mon Sep 17 00:00:00 2001 From: Sam Schweigel Date: Tue, 4 Feb 2025 14:22:25 -0800 Subject: [PATCH] Taint :noub on pointerref/pointerset, getfield with no bounds check Also document the bounds check disabling flag on getfield, add docs for pointerref/pointerset/memorynew. getfield with symbol will throw even if boundschecking is turned off. --- Compiler/src/tfuncs.jl | 39 ++++++++++++++++++++----------------- Compiler/test/effects.jl | 33 ++++++++++++++++++++++++++++++- Compiler/test/inference.jl | 8 ++++---- base/docs/basedocs.jl | 21 ++++++++++++-------- base/docs/intrinsicsdocs.jl | 38 ++++++++++++++++++++++++++++++++++++ doc/src/devdocs/builtins.md | 34 +++++++++++++++++++++++--------- 6 files changed, 133 insertions(+), 40 deletions(-) diff --git a/Compiler/src/tfuncs.jl b/Compiler/src/tfuncs.jl index 50b88bb0222ce8..85bdced519fd06 100644 --- a/Compiler/src/tfuncs.jl +++ b/Compiler/src/tfuncs.jl @@ -1033,7 +1033,7 @@ end # we may assume that we don't throw @assert !boundscheck ismod && return false - name ⊑ Int || name ⊑ Symbol || return false + name ⊑ Int || return false sty.name.n_uninitialized == 0 && return true nflds === nothing && return false for i = (datatype_min_ninitialized(sty)+1):nflds @@ -1053,10 +1053,10 @@ end if isa(s, DataType) # Can't say anything about abstract types isabstracttype(s) && return false - # If all fields are always initialized, and bounds check is disabled, - # we can assume we don't throw + # If all fields are always initialized, bounds check is disabled, and + # name is an integer, we can assume we don't throw. if !boundscheck && s.name.n_uninitialized == 0 - name ⊑ Int || name ⊑ Symbol || return false + name ⊑ Int || return false return true end # Else we need to know what the field is @@ -2429,7 +2429,7 @@ const _ARGMEM_BUILTINS = Any[ ] const _INCONSISTENT_INTRINSICS = Any[ - Intrinsics.pointerref, # this one is volatile + Intrinsics.pointerref, Intrinsics.sqrt_llvm_fast, # this one may differ at runtime (by a few ulps) Intrinsics.have_fma, # this one depends on the runtime environment Intrinsics.cglobal, # cglobal lookup answer changes at runtime @@ -2486,6 +2486,7 @@ end function getfield_effects(𝕃::AbstractLattice, argtypes::Vector{Any}, @nospecialize(rt)) length(argtypes) < 2 && return EFFECTS_THROWS obj = argtypes[1] + name = argtypes[2] if isvarargtype(obj) return Effects(EFFECTS_TOTAL; consistent=CONSISTENT_IF_INACCESSIBLEMEMONLY, @@ -2499,18 +2500,16 @@ function getfield_effects(𝕃::AbstractLattice, argtypes::Vector{Any}, @nospeci noub = ALWAYS_TRUE bcheck = getfield_boundscheck(argtypes) nothrow = getfield_nothrow(𝕃, argtypes, bcheck) - if !nothrow - if bcheck !== :on - # If we cannot independently prove inboundsness, taint `:noub`. - # The inbounds-ness assertion requires dynamic reachability, - # while `:noub` needs to be true for all input values. - # However, as a special exception, we do allow literal `:boundscheck`. - # `:noub` will be tainted in any caller using `@inbounds` - # based on the `:noinbounds` effect. - # N.B. We do not taint for `--check-bounds=no` here. - # That is handled in concrete evaluation. - noub = ALWAYS_FALSE - end + if !(bcheck === :on || isdefined_tfunc(𝕃, obj, name) === Const(true)) + # If we cannot independently prove inboundsness, taint `:noub`. + # The inbounds-ness assertion requires dynamic reachability, + # while `:noub` needs to be true for all input values. + # However, as a special exception, we do allow literal `:boundscheck`. + # `:noub` will be tainted in any caller using `@inbounds` + # based on the `:noinbounds` effect. + # N.B. We do not taint for `--check-bounds=no` here. + # That is handled in concrete evaluation. + noub = ALWAYS_FALSE end if hasintersect(widenconst(obj), Module) # Modeled more precisely in abstract_eval_getglobal @@ -2940,7 +2939,11 @@ function intrinsic_effects(f::IntrinsicFunction, argtypes::Vector{Any}) effect_free = !(f === Intrinsics.pointerset) ? ALWAYS_TRUE : ALWAYS_FALSE nothrow = intrinsic_nothrow(f, argtypes) inaccessiblememonly = ALWAYS_TRUE - return Effects(EFFECTS_TOTAL; consistent, effect_free, nothrow, inaccessiblememonly) + noub = ALWAYS_TRUE + if f === Intrinsics.pointerref || f === Intrinsics.pointerset + noub = ALWAYS_FALSE + end + return Effects(EFFECTS_TOTAL; consistent, effect_free, nothrow, inaccessiblememonly, noub) end # TODO: this function is a very buggy and poor model of the return_type function diff --git a/Compiler/test/effects.jl b/Compiler/test/effects.jl index 082d954b4a9bca..b03f201d30c442 100644 --- a/Compiler/test/effects.jl +++ b/Compiler/test/effects.jl @@ -443,13 +443,35 @@ end |> Compiler.is_consistent @test Base.infer_effects((Tuple{Int,Int},Int)) do t, i getfield(t, i, false) end |> Compiler.is_nothrow + +# getfield can throw when bounds checking is turned off when name is a symbol @test Base.infer_effects((Tuple{Int,Int},Symbol)) do t, i getfield(t, i, false) -end |> Compiler.is_nothrow +end |> !Compiler.is_nothrow @test Base.infer_effects((Tuple{Int,Int},String)) do t, i getfield(t, i, false) # invalid name type end |> !Compiler.is_nothrow +# check for :noub tainted when bounds checking off +let e = Base.infer_effects((Tuple{Int, Int}, Int)) do t, i + getfield(t, i) + end + @test !Compiler.is_nothrow(e) + @test Compiler.is_noub(e) +end +let e = Base.infer_effects((Tuple{Int, Int}, Int)) do t, i + getfield(t, i, false) + end + @test Compiler.is_nothrow(e) + @test !Compiler.is_noub(e) +end +let e = Base.infer_effects((Tuple{Int, Int}, Int)) do t, i + getfield(t, 2, false) + end + @test Compiler.is_nothrow(e) + @test Compiler.is_noub(e) +end + @test Base.infer_effects((Some{Any},)) do some getfield(some, 1, :not_atomic) end |> Compiler.is_nothrow @@ -1349,6 +1371,15 @@ let; Base.Experimental.@force_compile; func52843(); end @test !Compiler.intrinsic_nothrow(Core.Intrinsics.pointerref, Any[Type{Ptr{Vector{Int64}}}, Int, Int]) @test !Compiler.intrinsic_nothrow(Core.Intrinsics.pointerref, Any[Type{Ptr{T}} where T, Int, Int]) +let e = Base.infer_effects(Core.Intrinsics.pointerref, (Ptr{Int}, Int, Int)) + @test !Compiler.is_noub(e) + @test !Compiler.is_consistent(e) +end +let e = Base.infer_effects(unsafe_load, (Ptr{Int},)) + @test !Compiler.is_noub(e) + @test !Compiler.is_consistent(e) +end + # post-opt :consistent-cy analysis correctness # https://github.com/JuliaLang/julia/issues/53508 @test !Compiler.is_consistent(Base.infer_effects(getindex, (UnitRange{Int},Int))) diff --git a/Compiler/test/inference.jl b/Compiler/test/inference.jl index 563828ac77296b..5bfbc0cd9ee7c2 100644 --- a/Compiler/test/inference.jl +++ b/Compiler/test/inference.jl @@ -5915,25 +5915,25 @@ end == Val @test Base.infer_effects((Any,Any); optimize=false) do a, b getfield(PartiallyInitialized1(a, b), :b) end |> Compiler.is_nothrow -@test Base.infer_effects((Any,Any,Symbol,); optimize=false) do a, b, f +@test Base.infer_effects((Any,Any,Int,); optimize=false) do a, b, f getfield(PartiallyInitialized1(a, b), f, #=boundscheck=#false) end |> !Compiler.is_nothrow @test Base.infer_effects((Any,Any,Any); optimize=false) do a, b, c getfield(PartiallyInitialized1(a, b, c), :c) end |> Compiler.is_nothrow -@test Base.infer_effects((Any,Any,Any,Symbol); optimize=false) do a, b, c, f +@test Base.infer_effects((Any,Any,Any,Int); optimize=false) do a, b, c, f getfield(PartiallyInitialized1(a, b, c), f, #=boundscheck=#false) end |> Compiler.is_nothrow @test Base.infer_effects((Any,Any); optimize=false) do a, b getfield(PartiallyInitialized2(a, b), :b) end |> Compiler.is_nothrow -@test Base.infer_effects((Any,Any,Symbol,); optimize=false) do a, b, f +@test Base.infer_effects((Any,Any,Int,); optimize=false) do a, b, f getfield(PartiallyInitialized2(a, b), f, #=boundscheck=#false) end |> !Compiler.is_nothrow @test Base.infer_effects((Any,Any,Any); optimize=false) do a, b, c getfield(PartiallyInitialized2(a, b, c), :c) end |> Compiler.is_nothrow -@test Base.infer_effects((Any,Any,Any,Symbol); optimize=false) do a, b, c, f +@test Base.infer_effects((Any,Any,Any,Int); optimize=false) do a, b, c, f getfield(PartiallyInitialized2(a, b, c), f, #=boundscheck=#false) end |> Compiler.is_nothrow diff --git a/base/docs/basedocs.jl b/base/docs/basedocs.jl index 88ed34de02b64b..bed02a3d7427b6 100644 --- a/base/docs/basedocs.jl +++ b/base/docs/basedocs.jl @@ -2435,14 +2435,19 @@ julia> Tuple(Real[1, 2, pi]) # takes a collection tuple """ - getfield(value, name::Symbol, [order::Symbol]) - getfield(value, i::Int, [order::Symbol]) - -Extract a field from a composite `value` by name or position. Optionally, an -ordering can be defined for the operation. If the field was declared `@atomic`, -the specification is strongly recommended to be compatible with the stores to -that location. Otherwise, if not declared as `@atomic`, this parameter must be -`:not_atomic` if specified. + getfield(value, name::Symbol, [boundscheck::Bool=true], [order::Symbol]) + getfield(value, i::Int, [boundscheck::Bool=true], [order::Symbol]) + +Extract a field from a composite `value` by name or position. + +Optionally, an ordering can be defined for the operation. If the field was +declared `@atomic`, the specification is strongly recommended to be compatible +with the stores to that location. Otherwise, if not declared as `@atomic`, this +parameter must be `:not_atomic` if specified. + +The bounds check may be disabled, in which case the behavior of this function is +undefined if `i` is out of bounds. + See also [`getproperty`](@ref Base.getproperty) and [`fieldnames`](@ref). # Examples diff --git a/base/docs/intrinsicsdocs.jl b/base/docs/intrinsicsdocs.jl index c9538ea74ab26a..476f9548ff5522 100644 --- a/base/docs/intrinsicsdocs.jl +++ b/base/docs/intrinsicsdocs.jl @@ -22,6 +22,15 @@ The `Core.Intrinsics` module holds the `Core.IntrinsicFunction` objects. """ Core.Intrinsics +""" + Core.memorynew(::Type{T} where T <: GenericMemory, n::Int) + +Construct an uninitialized [`GenericMemory`](@ref) of length `n`. + +See also [`Memory`](@ref Base.Memory). +""" +Core.memorynew + """ Core.memoryrefnew(::GenericMemory) Core.memoryrefnew(::GenericMemoryRef, index::Int, [boundscheck::Bool]) @@ -129,6 +138,35 @@ See also [`setpropertyonce!`](@ref Base.replaceproperty!) and [`Core.memoryrefse """ Core.memoryrefsetonce! + +""" + Core.Intrinsics.pointerref(p::Ptr{T}, i::Int, align::Int) + +Load a value of type `T` from the address of the `i`th element (1-indexed) +starting at `p`. This is equivalent to the C expression `p[i-1]`. + +The alignment must be a power of two, or 0, indicating the default alignment +for `T`. If `p[i-1]` is out of bounds, invalid, or is not aligned, the behavior +is undefined. An alignment of 1 is always safe. + +See also [`unsafe_load`](@ref). +""" +Core.Intrinsics.pointerref + +""" + Core.Intrinsics.pointerset(p::Ptr{T}, x::T, i::Int, align::Int) + +Store a value of type `T` to the address of the `i`th element (1-indexed) +starting at `p`. This is equivalent to the C expression `p[i-1] = x`. + +The alignment must be a power of two, or `0`, indicating the default alignment +for `T`. If `p[i-1]` is out of bounds, invalid, or is not aligned, the behavior +is undefined. An alignment of 1 is always safe. + +See also [`unsafe_store`](@ref). +""" +Core.Intrinsics.pointerset + """ Core.Intrinsics.atomic_pointerref(pointer::Ptr{T}, order::Symbol) --> T diff --git a/doc/src/devdocs/builtins.md b/doc/src/devdocs/builtins.md index e53321f3e70a06..bb6b1ae0d82a29 100644 --- a/doc/src/devdocs/builtins.md +++ b/doc/src/devdocs/builtins.md @@ -1,12 +1,25 @@ # [Core.Builtins](@id lib-builtins) -## Builtin Function APIs +The following builtin functions are considered unstable, but provide the basic +definitions for what defines the abilities and behaviors of a Julia +program. They are typically accessed through a higher level generic API. -The following Builtin function APIs are considered unstable, but provide the basic -definitions for what defines the abilities and behaviors of a Julia program. They are -typically accessed through a higher level generic API. +## Raw access to memory ```@docs +Core.Intrinsics.pointerref +Core.Intrinsics.pointerset +Core.Intrinsics.atomic_pointerref +Core.Intrinsics.atomic_pointerset +Core.Intrinsics.atomic_pointerswap +Core.Intrinsics.atomic_pointermodify +Core.Intrinsics.atomic_pointerreplace +``` + +## Managed memory + +```@docs +Core.memorynew Core.memoryrefnew Core.memoryrefoffset Core.memoryrefget @@ -16,12 +29,15 @@ Core.memoryrefswap! Core.memoryrefmodify! Core.memoryrefreplace! Core.memoryrefsetonce! -Core.Intrinsics.atomic_pointerref -Core.Intrinsics.atomic_pointerset -Core.Intrinsics.atomic_pointerswap -Core.Intrinsics.atomic_pointermodify -Core.Intrinsics.atomic_pointerreplace +``` + +## Module bindings + Core.get_binding_type + +## Other + +```@docs Core.IntrinsicFunction Core.Intrinsics Core.IR