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 in hvcat #18399

Closed
andreasnoack opened this issue Sep 8, 2016 · 9 comments · Fixed by #18428
Closed

Segfault in hvcat #18399

andreasnoack opened this issue Sep 8, 2016 · 9 comments · Fixed by #18428
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version types and dispatch Types, subtyping and method dispatch
Milestone

Comments

@andreasnoack
Copy link
Member

This is a slight modification of @maleadt's example in #18398 but this version doesn't call the method that the PR modifies. It is a regression from 0.4.

immutable TSlow{T,N} <: AbstractArray{T,N}
    data::Dict{NTuple{N,Int}, T}
    dims::NTuple{N,Int}
end
TSlow{T,N}(::Type{T}, dims::NTuple{N,Int}) = TSlow{T,N}(Dict{NTuple{N,Int}, T}(), dims)

Base.convert{T,N  }(::Type{TSlow     }, X::AbstractArray{T,N}) = convert(TSlow{T,N}, X)
Base.convert{T,N  }(::Type{TSlow{T,N}}, X::AbstractArray     ) = TSlow(T, size(X))

Base.size(A::TSlow) = A.dims
Base.linearindexing{A<:TSlow}(::Type{A}) = Base.LinearSlow()
Base.getindex{T}(A::TSlow{T,3}, i1::Int, i2::Int, i3::Int) = zero(T)
Base.setindex!{T}(A::TSlow{T,3}, v, i1::Int, i2::Int, i3::Int) = return nothing

function test_cat()
    B = TSlow(reshape([1:27...], 3, 3, 3))
    C = TSlow([1 2; 3 4])

    Base.typed_hcat(Float64, B)
    Base.typed_hcat(Float64, B, B, B)
    try
        hvcat((2), C, C)
    end
    hvcat((2,2), 'a', 5, 2//3, 1)
end

test_cat()

gives

signal (11): Segmentation fault: 11
while loading /Users/andreasnoack/Desktop/hvcatbug.jl, in expression starting on line 27
collect at ./array.jl:304
getindex at ./tuple.jl:10
unknown function (ip: 0x313c086e6)
jl_call_method_internal at /Users/andreasnoack/julia/src/./julia_internal.h:189 [inlined]
jl_apply_generic at /Users/andreasnoack/julia/src/gf.c:1930
hvcat at ./abstractarray.jl:1358
test_cat at /Users/andreasnoack/Desktop/hvcatbug.jl:24
unknown function (ip: 0x313c016ff)
jl_call_method_internal at /Users/andreasnoack/julia/src/./julia_internal.h:189 [inlined]
jl_apply_generic at /Users/andreasnoack/julia/src/gf.c:1930
do_call at /Users/andreasnoack/julia/src/interpreter.c:66
eval at /Users/andreasnoack/julia/src/interpreter.c:190
jl_toplevel_eval_flex at /Users/andreasnoack/julia/src/toplevel.c:558
jl_parse_eval_all at /Users/andreasnoack/julia/src/ast.c:717
jl_load at /Users/andreasnoack/julia/src/toplevel.c:596 [inlined]
jl_load_ at /Users/andreasnoack/julia/src/toplevel.c:605
include_from_node1 at ./loading.jl:426
jlcall_include_from_node1_20195 at /Users/andreasnoack/julia/usr/lib/julia/sys.dylib (unknown line)
jl_call_method_internal at /Users/andreasnoack/julia/src/./julia_internal.h:189 [inlined]
jl_apply_generic at /Users/andreasnoack/julia/src/gf.c:1930
process_options at ./client.jl:262
_start at ./client.jl:318
jlcall__start_21540 at /Users/andreasnoack/julia/usr/lib/julia/sys.dylib (unknown line)
jl_call_method_internal at /Users/andreasnoack/julia/src/./julia_internal.h:189 [inlined]
jl_apply_generic at /Users/andreasnoack/julia/src/gf.c:1930
true_main at /Users/andreasnoack/bin/julia (unknown line)
main at /Users/andreasnoack/bin/julia (unknown line)
Allocations: 2009974 (Pool: 2009090; Big: 884); GC: 1
[1]    36411 segmentation fault  julia ~/Desktop/hvcatbug.jl
@andreasnoack andreasnoack added bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version labels Sep 8, 2016
@andreasnoack
Copy link
Member Author

First bad commit is cd35df5

@JeffBezanson
Copy link
Member

This might be #17288; it seems to go away if the try is removed.

@maleadt
Copy link
Member

maleadt commented Sep 8, 2016

It seemed similar, but after reverting to the old model (by undeffing JL_ELF_TLS_VARIANT) the issue still triggers... Also, didn't @yuyichao push a temporary workaround for that issue?

@yuyichao
Copy link
Contributor

yuyichao commented Sep 8, 2016

I can reproduce this on AArch64 so either this is the first example that triggers #17288 on AArch64 or this is not #17288.

@yuyichao
Copy link
Contributor

yuyichao commented Sep 8, 2016

Seems like codegen thinks the object have the wrong layout. The first LLVM function below is

function next(g::Generator, s)
    @_inline_meta
    v, s2 = next(g.iter, s)
    g.f(v), s2
end

with g being of type Base.Generator{Base.UnitRange{Int64}, Base.##21#22{Tuple{Char, Int64, Base.Rational{Int64}, Int64}}} where the function type Base.##21#22{Tuple{Char, Int64, Base.Rational{Int64}, Int64}} is a bitstype inlined in g. However, both functions below believes that f should be stored boxed in g so next loads a pointer and passed it to #21 causing it to segfault.

Not sure how that happened yet.

; Function Attrs: sspreq
define internal %jl_value_t* @julia_next_68714(%jl_value_t*, i64) #4 !dbg !3410 {
top:
  %2 = alloca %UnitRange
  %g = alloca %jl_value_t*
  %3 = alloca [2 x i64]
  %4 = alloca [2 x i64]
  %5 = alloca [2 x i64]
  %6 = call %jl_value_t** @julia.gc_root_decl()
  %7 = call %jl_value_t** @julia.gc_root_decl()
  %8 = call %jl_value_t** @julia.gc_root_decl()
  %9 = call %jl_value_t*** @jl_get_ptls_states()
  %10 = bitcast %jl_value_t*** %9 to %jl_value_t**
  %11 = getelementptr %jl_value_t*, %jl_value_t** %10, i64 2
  %12 = bitcast %jl_value_t** %11 to i64**
  %13 = load i64*, i64** %12, !tbaa !3634
  call void @llvm.dbg.declare(metadata %jl_value_t** %g, metadata !3413, metadata !3650), !dbg !4109
  %v = alloca i64
  call void @llvm.dbg.declare(metadata i64* %v, metadata !3415, metadata !3636), !dbg !4109
  %s2 = alloca i64
  call void @llvm.dbg.declare(metadata i64* %s2, metadata !3416, metadata !3636), !dbg !4109
  %"#temp#" = alloca i64
  store %jl_value_t* %0, %jl_value_t** %g
  %s = alloca i64
  store i64 %1, i64* %s
  call void @llvm.dbg.declare(metadata i64* %s, metadata !3414, metadata !3636), !dbg !4109
  %14 = load %jl_value_t*, %jl_value_t** %g, !dbg !4110
  %15 = bitcast %jl_value_t* %14 to i8*, !dbg !4110
  %16 = getelementptr i8, i8* %15, i64 8, !dbg !4110
  %17 = bitcast i8* %16 to %UnitRange*, !dbg !4110
  %18 = getelementptr %UnitRange, %UnitRange* %17, i64 0, !dbg !4110
  %19 = load %UnitRange, %UnitRange* %18, align 8, !dbg !4110, !tbaa !3640
  store %UnitRange %19, %UnitRange* %2, !dbg !4110
  call void @jlsys_next_57739([2 x i64]* sret %3, %UnitRange* %2, i64 %1), !dbg !4110
  store i64 1, i64* %"#temp#", !dbg !4110
  %20 = load i64, i64* %"#temp#", !dbg !4110, !tbaa !3653
  call void @jlsys_indexed_next_52566([2 x i64]* sret %4, [2 x i64]* %3, i64 1, i64 %20), !dbg !4110
  %21 = getelementptr inbounds [2 x i64], [2 x i64]* %4, i32 0, i32 0, !dbg !4110
  %22 = bitcast i64* %v to i8*, !dbg !4110
  %23 = bitcast i64* %21 to i8*, !dbg !4110
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %22, i8* %23, i64 8, i32 0, i1 false), !dbg !4110, !tbaa !3653
  %24 = getelementptr inbounds [2 x i64], [2 x i64]* %4, i32 0, i32 1, !dbg !4110
  %25 = bitcast i64* %"#temp#" to i8*, !dbg !4110
  %26 = bitcast i64* %24 to i8*, !dbg !4110
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %25, i8* %26, i64 8, i32 0, i1 false), !dbg !4110, !tbaa !3653
  %27 = load i64, i64* %"#temp#", !dbg !4110, !tbaa !3653
  call void @jlsys_indexed_next_52566([2 x i64]* sret %5, [2 x i64]* %3, i64 2, i64 %27), !dbg !4110
  %28 = getelementptr inbounds [2 x i64], [2 x i64]* %5, i32 0, i32 0, !dbg !4110
  %29 = bitcast i64* %s2 to i8*, !dbg !4110
  %30 = bitcast i64* %28 to i8*, !dbg !4110
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %29, i8* %30, i64 8, i32 0, i1 false), !dbg !4110, !tbaa !3653
  %31 = getelementptr inbounds [2 x i64], [2 x i64]* %5, i32 0, i32 1, !dbg !4110
  %32 = bitcast i64* %"#temp#" to i8*, !dbg !4110
  %33 = bitcast i64* %31 to i8*, !dbg !4110
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %32, i8* %33, i64 8, i32 0, i1 false), !dbg !4110, !tbaa !3653
  %34 = load %jl_value_t*, %jl_value_t** %g, !dbg !4111
  %35 = bitcast %jl_value_t* %34 to i8*, !dbg !4111
  %36 = getelementptr i8, i8* %35, i64 0, !dbg !4111
  %37 = bitcast i8* %36 to %jl_value_t**, !dbg !4111
  %38 = load %jl_value_t*, %jl_value_t** %37, !dbg !4111, !tbaa !3640
  %39 = load i64, i64* %v, !dbg !4111, !tbaa !3653
  %40 = call %jl_value_t* @"julia_#21_68715"(%jl_value_t* %38, i64 %39) #5, !dbg !4111
  store %jl_value_t* %40, %jl_value_t** %6, !dbg !4111
  %41 = bitcast %jl_value_t*** %9 to i8*, !dbg !4111
  %42 = call %jl_value_t* @jl_gc_pool_alloc(i8* %41, i32 1456, i32 32), !dbg !4111
  %43 = bitcast %jl_value_t* %42 to %jl_value_t**, !dbg !4111
  %44 = getelementptr %jl_value_t*, %jl_value_t** %43, i64 -1, !dbg !4111
  store %jl_value_t* inttoptr (i64 140227156034032 to %jl_value_t*), %jl_value_t** %44, !dbg !4111, !tbaa !3638
  store %jl_value_t* %42, %jl_value_t** %7, !dbg !4111
  store %jl_value_t* %40, %jl_value_t** %8, !dbg !4111
  %45 = bitcast %jl_value_t* %42 to i8*, !dbg !4111
  %46 = getelementptr i8, i8* %45, i64 0, !dbg !4111
  %47 = bitcast i8* %46 to %jl_value_t**, !dbg !4111
  store %jl_value_t* %40, %jl_value_t** %47, !dbg !4111, !tbaa !3640
  call void @julia.gc_root_kill(%jl_value_t** %7), !dbg !4111
  %48 = bitcast %jl_value_t* %42 to i8*, !dbg !4111
  %49 = getelementptr i8, i8* %48, i64 8, !dbg !4111
  %50 = load i64, i64* %s2, !dbg !4111, !tbaa !3653
  %51 = bitcast i8* %49 to i64*, !dbg !4111
  %52 = getelementptr i64, i64* %51, i64 0, !dbg !4111
  store i64 %50, i64* %52, align 8, !dbg !4111, !tbaa !3640
  ret %jl_value_t* %42, !dbg !4111
}
; Function Attrs: sspreq
define internal %jl_value_t* @"julia_#21_68715"(%jl_value_t*, i64) #4 !dbg !3388 {
top:
  %"#self#" = alloca %jl_value_t*
  %2 = call %jl_value_t** @julia.gc_root_decl()
  %3 = call %jl_value_t*** @jl_get_ptls_states()
  %4 = bitcast %jl_value_t*** %3 to %jl_value_t**
  %5 = getelementptr %jl_value_t*, %jl_value_t** %4, i64 2
  %6 = bitcast %jl_value_t** %5 to i64**
  %7 = load i64*, i64** %6, !tbaa !3634
  call void @llvm.dbg.declare(metadata %jl_value_t** %"#self#", metadata !3392, metadata !3650), !dbg !4108
  store %jl_value_t* %0, %jl_value_t** %"#self#"
  %ri = alloca i64
  store i64 %1, i64* %ri
  call void @llvm.dbg.declare(metadata i64* %ri, metadata !3393, metadata !3636), !dbg !4108
  %8 = load %jl_value_t*, %jl_value_t** %"#self#", !dbg !4108
  %9 = bitcast %jl_value_t* %8 to i8*, !dbg !4108
  %10 = getelementptr i8, i8* %9, i64 0, !dbg !4108
  %11 = bitcast i8* %10 to %jl_value_t**, !dbg !4108
  %12 = load %jl_value_t*, %jl_value_t** %11, !dbg !4108, !tbaa !3640
  %13 = call %jl_value_t* @julia_getindex_68641(%jl_value_t* %12, i64 %1) #5, !dbg !4108
  store %jl_value_t* %13, %jl_value_t** %2, !dbg !4108
  ret %jl_value_t* %13, !dbg !4108
}

@yuyichao
Copy link
Contributor

yuyichao commented Sep 8, 2016

Further reduction seems like a type inference/cache issue

abstract AbstractA{T,N}
typealias AbstractM{T} AbstractA{T,2}

immutable TSlow{T,N} <: AbstractA{T,N}
    data
end
TSlow{T,N}(::AbstractArray{T,N}) = TSlow{T,N}(1)

typed_hcat2{T}(::Type{T}, A...) = nothing
function hcat2{T}(A::AbstractM{T}...)
    typed_hcat2(T, A...)
end

function hvcat2(as...)
    hcat2(as[1:1]...)
end

function cat_t2(catdims, typeC::Type, X...)
    for i = 2:1
        X[i]
    end
    i = 1
    [i for d = 1:3]
    return ([], i)
end

C = TSlow([1 2; 3 4])

function test_cat(C)
    B = TSlow(reshape([1:8...], 2, 2, 2))
    cat_t2(2, Float64, B)
    cat_t2(2, Float64, B, B, B)
    hvcat2(C, C)
    hvcat2('a', 5, 2//3, 1)
end

test_cat(C)

@yuyichao
Copy link
Contributor

yuyichao commented Sep 8, 2016

Even further reduction.

type TSlow{T}
    x::T
end

function hvcat2(as)
    cb = ri->as[ri]
    g = Base.Generator(cb, 1)
    g.f(1)
end

function cat_t2(X...)
    for i = 2:1
        X[i]
        d->i
    end
end

C = TSlow{Int}(1)
GB = TSlow{Int}(1)

function test_cat(C)
    B = GB::Union{TSlow{Int},TSlow{Any}}
    cat_t2()
    cat_t2(B, B, B)
    hvcat2((C,))
    hvcat2(((2, 3),))
end

test_cat(C)

Edit: reduced again.

@yuyichao yuyichao added the types and dispatch Types, subtyping and method dispatch label Sep 8, 2016
yuyichao added a commit to yuyichao/explore that referenced this issue Sep 8, 2016
@vtjnash
Copy link
Member

vtjnash commented Sep 9, 2016

adding @nolinine and @show we get:

g = Base.Generator(cb,1) = Base.Generator{Int64,##3#4{Tuple{Tuple{Int64,Int64}}}}(#3,1)
typeof(g) = Base.Generator{Int64,##3#4{Tuple{TSlow{Int64}}}}

and also can get p jl_gdblookuplinfo((void*)$rip); p jl_(jl_uncompress_ast($0, (jl_array_t*)$0->code))) and see that:

SSAValue(4) = Expr(:invoke, Base.Type(Type{Base.Generator},
  Main.##3#4{Tuple{Tuple{Int64, Int64}}}, Int64),
  Base.Generator, SlotNumber(id=4), 1)
  ::Base.Generator{Int64, Main.##3#4{Tuple{Main.TSlow{Int64}}}},

So this is probably an inference bug, as it is sufficient to call code_typed to get the wrong result:

julia> @code_typed test_cat(C)
julia> @code_warntype hvcat2(((2,3),))
...
  g::Base.Generator{Int64,##3#4{Tuple{TSlow{Int64}}}}
...

@vtjnash
Copy link
Member

vtjnash commented Sep 9, 2016

this should patch it, although I may need some help to write the test case for this. It also would be good if perhaps @JeffBezanson could suggest a less pessimistic version of this. This also may be a very strong candidate for being the real fix for the subarray test failure from the beginning of the year?

diff --git a/base/inference.jl b/base/inference.jl
index 885ec23..1940783 100644
--- a/base/inference.jl
+++ b/base/inference.jl
@@ -779,6 +779,7 @@ function abstract_call_gf_by_type(f::ANY, argtype::ANY, sv)
     for (m::SimpleVector) in x
         sig = m[1]
         method = m[3]::Method
+        sparams = m[2]::SimpleVector

         # limit argument type tuple growth
         lsig = length(m[3].sig.parameters)
@@ -807,6 +808,7 @@ function abstract_call_gf_by_type(f::ANY, argtype::ANY, sv)
                     # impose limit if we recur and the argument types grow beyond MAX_TYPE_DEPTH
                     if td > MAX_TYPE_DEPTH
                         sig = limit_type_depth(sig, 0, true, [])
+                        sparams = svec()
                         break
                     else
                         p1, p2 = sig.parameters, infstate.linfo.specTypes.parameters
@@ -827,6 +829,7 @@ function abstract_call_gf_by_type(f::ANY, argtype::ANY, sv)
                             end
                             if limitdepth
                                 sig = Tuple{newsig...}
+                                sparams = svec()
                                 break
                             end
                         end
@@ -859,11 +862,12 @@ function abstract_call_gf_by_type(f::ANY, argtype::ANY, sv)
                 end
                 if !allsame
                     sig = limit_tuple_type_n(sig, lsig+1)
+                    sparams = svec()
                 end
             end
         end
         #print(m,"\n")
-        (_tree, rt) = typeinf_edge(method, sig, m[2], sv)
+        (_tree, rt) = typeinf_edge(method, sig, sparams, sv)
         rettype = tmerge(rettype, rt)
         if is(rettype,Any)
             break

@vtjnash vtjnash added this to the 0.5.x milestone Sep 9, 2016
vtjnash added a commit that referenced this issue Sep 9, 2016
also avoid an expensive loop in the common case of the hotpath

fix #18399
vtjnash added a commit that referenced this issue Sep 9, 2016
also avoid an expensive loop in the common case of the hotpath

fix #18399
vtjnash added a commit that referenced this issue Sep 9, 2016
also avoid an expensive loop in the common case of the hotpath

fix #18399
tkelman pushed a commit that referenced this issue Sep 16, 2016
also avoid an expensive loop in the common case of the hotpath

fix #18399

(cherry picked from commit a188f95)
ref #18428
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants