Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segfault when unsafe_loading inbounds pointer #56265

Closed
jakobnissen opened this issue Oct 21, 2024 · 3 comments
Closed

Segfault when unsafe_loading inbounds pointer #56265

jakobnissen opened this issue Oct 21, 2024 · 3 comments
Labels
compiler:codegen Generation of LLVM IR and native code regression 1.11 Regression in the 1.11 release

Comments

@jakobnissen
Copy link
Member

jakobnissen commented Oct 21, 2024

I'm not sure if this is expected behaviour, but I couldn't find any information about expected alignment in Julia.

Code to reproduce:

julia> using SIMD

julia> s = b"AAAAAAAAAAAAAABBBBBCBCBBBBBBBBCDE";

julia> foo(s) = GC.@preserve s unsafe_load(Ptr{Vec{32, UInt8}}(pointer(s)));

julia> foo(s)
[122821] signal 11 (128): Segmentation fault

I believe this may be a Julia bug, because SIMD doesn't add methods to unsafe_load.

Edit: Looks like the generated code changed between 1.10 and 1.11. In 1.11, it emits a vmovaps, which requires the pointer to be 16-byte aligned, which strings aren't always, whereas in 1.10, it emitted vmovups instead.

This is on Julia 1.11.1 and SIMD 3.6.0

@KristofferC
Copy link
Member

KristofferC commented Oct 21, 2024

FWIW, it "works" on 1.10.

julia> using SIMD

julia> s = b"AAAAAAAAAAAAAABBBBBCBCBBBBBBBBCDE";

julia> foo(s) = GC.@preserve s unsafe_load(Ptr{Vec{32, UInt8}}(pointer(s)));

julia> s_vec = foo(s)
<32 x UInt8>[0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x42, 0x42, 0x42, 0x42, 0x42, 0x43, 0x42, 0x43, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x42, 0x43, 0x44]

julia> transcode(String, collect(Tuple(s_vec)))
"AAAAAAAAAAAAAABBBBBCBCBBBBBBBBCD"

@vtjnash
Copy link
Member

vtjnash commented Oct 21, 2024

unsafe_load is supposed to annotate with align 1 to prevent this issue, but looks like it is picking up the sret alignment instead when creating the memcpy?

julia> foo(s) = GC.@preserve s unsafe_load(Ptr{Tuple{NTuple{32, VecElement{UInt8}}}}(pointer(s)),)
foo (generic function with 4 methods)

julia> @code_llvm optimize=false foo("")
; Function Signature: foo(String)
;  @ REPL[12]:1 within `foo`
define void @julia_foo_5705(ptr noalias nocapture noundef nonnull sret([1 x <32 x i8>]) align 32 dereferenceable(32) %sret_return, ptr noundef nonnull %"s::String") #0 {
top:
  %pointerref = alloca [2 x i128], align 16
  %pgcstack = call ptr @julia.get_pgcstack()
  %current_task = getelementptr inbounds i8, ptr %pgcstack, i32 -144
  %world_age = getelementptr inbounds i8, ptr %current_task, i32 152
  %0 = load i64, ptr %world_age, align 8
  %1 = call token (...) @llvm.julia.gc_preserve_begin(ptr %"s::String")
; ┌ @ strings/string.jl:130 within `pointer`
; │┌ @ pointer.jl:62 within `unsafe_convert`
    %2 = call nonnull ptr @julia.pointer_from_objref(ptr %"s::String") #4
    %string_ptr = getelementptr inbounds i8, ptr %2, i32 8
; └└
; ┌ @ pointer.jl:153 within `unsafe_load` @ pointer.jl:153
   %3 = getelementptr inbounds [1 x <32 x i8>], ptr %string_ptr, i64 0
   call void @llvm.memcpy.p0.p0.i64(ptr align 16 %pointerref, ptr align 16 %3, i64 32, i1 false)
; └
  call void @llvm.julia.gc_preserve_end(token %1)
  call void @llvm.memcpy.p0.p0.i64(ptr align 16 %sret_return, ptr align 16 %pointerref, i64 32, i1 false)
  ret void
}

@vtjnash vtjnash added compiler:codegen Generation of LLVM IR and native code regression 1.11 Regression in the 1.11 release labels Oct 21, 2024
@jakobnissen
Copy link
Member Author

Fixed on #57714, possibly due to #56938.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code regression 1.11 Regression in the 1.11 release
Projects
None yet
Development

No branches or pull requests

3 participants