From 0d0b853961940cf2fea83a722a36ec5a40fb0f50 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Thu, 21 Mar 2019 14:11:41 -0400 Subject: [PATCH 1/5] Fix llvm-assertion: don't load ghost types As I understand it, LLVM doesn't support 0-sized structs (ghost types), which julia makes significant use of. This fixes an incorrect behavior in the julia compiler accidentally attempting to instantiate such 0-sized structs. When using a debug build of LLVM, this fails with an assertion error, but failed silently on standard LLVM. --- src/intrinsics.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp index 7d7c24cdda0b6..701c865fcbe13 100644 --- a/src/intrinsics.cpp +++ b/src/intrinsics.cpp @@ -600,8 +600,13 @@ static jl_cgval_t emit_pointerref(jl_codectx_t &ctx, jl_cgval_t *argv) bool isboxed; Type *ptrty = julia_type_to_llvm(ety, &isboxed); assert(!isboxed); - Value *thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ); - return typed_load(ctx, thePtr, im1, ety, tbaa_data, nullptr, true, align_nb); + if (!type_is_ghost(ptrty)) { + Value *thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ); + return typed_load(ctx, thePtr, im1, ety, tbaa_data, nullptr, true, align_nb); + } + else { + return mark_julia_const(((jl_datatype_t*)ety)->instance); + } } } @@ -663,8 +668,12 @@ static jl_cgval_t emit_pointerset(jl_codectx_t &ctx, jl_cgval_t *argv) bool isboxed; Type *ptrty = julia_type_to_llvm(ety, &isboxed); assert(!isboxed); - thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ); - typed_store(ctx, thePtr, im1, x, ety, tbaa_data, nullptr, nullptr, align_nb); + if (!type_is_ghost(ptrty)) { + thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ); + typed_store(ctx, thePtr, im1, x, ety, tbaa_data, nullptr, nullptr, align_nb); + } else { + return e; + } } return mark_julia_type(ctx, thePtr, false, aty); } From 8df4de4b543d0057b457324cacad5a88f9464a1c Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 21 Mar 2019 21:42:25 -0400 Subject: [PATCH 2/5] Apply vtjnash's suggested change to src/intrinsics.cpp Co-Authored-By: NHDaly --- src/intrinsics.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp index 701c865fcbe13..b940f167da4bb 100644 --- a/src/intrinsics.cpp +++ b/src/intrinsics.cpp @@ -605,7 +605,7 @@ static jl_cgval_t emit_pointerref(jl_codectx_t &ctx, jl_cgval_t *argv) return typed_load(ctx, thePtr, im1, ety, tbaa_data, nullptr, true, align_nb); } else { - return mark_julia_const(((jl_datatype_t*)ety)->instance); + return ghostValue(ety); } } } From d0b0ca3c58f72a013ff3b9f29fdbfcee523b9fa8 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Thu, 21 Mar 2019 21:44:55 -0400 Subject: [PATCH 3/5] Apply vtjnash's suggestion to src/intrinsics.cpp replace `mark_julia_type()` with `return e` always. --- src/intrinsics.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/intrinsics.cpp b/src/intrinsics.cpp index b940f167da4bb..61b9431177905 100644 --- a/src/intrinsics.cpp +++ b/src/intrinsics.cpp @@ -671,11 +671,9 @@ static jl_cgval_t emit_pointerset(jl_codectx_t &ctx, jl_cgval_t *argv) if (!type_is_ghost(ptrty)) { thePtr = emit_unbox(ctx, ptrty->getPointerTo(), e, e.typ); typed_store(ctx, thePtr, im1, x, ety, tbaa_data, nullptr, nullptr, align_nb); - } else { - return e; } } - return mark_julia_type(ctx, thePtr, false, aty); + return e; } static Value *emit_checked_srem_int(jl_codectx_t &ctx, Value *x, Value *den) From 37250720d910ba1029fbbf775ac1e634dbe89b05 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Thu, 21 Mar 2019 21:51:02 -0400 Subject: [PATCH 4/5] Add test for Issue #29929: `unsafe_store!(::Ptr{Nothing}, ::Nothing)` --- test/intrinsics.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/intrinsics.jl b/test/intrinsics.jl index 993efc63a4c03..0e8b2ec98fc23 100644 --- a/test/intrinsics.jl +++ b/test/intrinsics.jl @@ -100,3 +100,6 @@ let f = Core.Intrinsics.ashr_int @test f(Int32(-1), -10) == -1 @test f(Int32(2), -1) == 0 end + +# issue #29929 +@test unsafe_store!(Ptr{Nothing}(C_NULL), nothing) == Ptr{Nothing}(0) From 33c15d304174d9f05c7944211be5b560427ae8e5 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Thu, 21 Mar 2019 21:54:18 -0400 Subject: [PATCH 5/5] More tests: can load/store any 0-sized struct --- test/intrinsics.jl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/intrinsics.jl b/test/intrinsics.jl index 0e8b2ec98fc23..a5f3308c68639 100644 --- a/test/intrinsics.jl +++ b/test/intrinsics.jl @@ -102,4 +102,7 @@ let f = Core.Intrinsics.ashr_int end # issue #29929 -@test unsafe_store!(Ptr{Nothing}(C_NULL), nothing) == Ptr{Nothing}(0) +@test unsafe_store!(Ptr{Nothing}(C_NULL), nothing) === Ptr{Nothing}(0) +@test unsafe_load(Ptr{Nothing}(0)) === nothing +struct GhostStruct end +@test unsafe_load(Ptr{GhostStruct}(rand(Int))) === GhostStruct()