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

Invalid write(io::IO, s::String) method #23520

Closed
yuyichao opened this issue Aug 31, 2017 · 1 comment · Fixed by #23914
Closed

Invalid write(io::IO, s::String) method #23520

yuyichao opened this issue Aug 31, 2017 · 1 comment · Fixed by #23914
Labels
GC Garbage collector

Comments

@yuyichao
Copy link
Contributor

The method definition

write(io::IO, s::String) = unsafe_write(io, pointer(s), reinterpret(UInt, sizeof(s)))

is invalid since it is not guaranteed that the s will be kept alive when this function is inlined.

In particular, this cases @code_llvm show(IOBuffer(), 4) to be invalid

; Function show
; Location: show.jl
define void @jlsys_show_37268(%jl_value_t addrspace(10)* dereferenceable(48), i64) #0 {
top:
; Location: show.jl:353
  %2 = ashr i64 %1, 63
  %3 = add i64 %2, %1
  %4 = xor i64 %3, %2
  %.lobit = lshr i64 %1, 63
  %5 = trunc i64 %.lobit to i8
  %6 = call %jl_value_t addrspace(10)* @jlsys_dec_38356(i64 %4, i64 1, i8 %5)
  %7 = addrspacecast %jl_value_t addrspace(10)* %6 to %jl_value_t*
  %8 = ptrtoint %jl_value_t* %7 to i64
  %9 = add i64 %8, 8
  %10 = inttoptr i64 %9 to i8*
  %11 = bitcast %jl_value_t addrspace(10)* %6 to i64 addrspace(10)*
  %12 = load i64, i64 addrspace(10)* %11, align 8
  %13 = call i64 @jlsys_unsafe_write_37226(%jl_value_t addrspace(10)* %0, i8* %10, i64 %12)
  ret void
}

(Note that the return value of dec is passed to unsafe_write without being rooted).

This may show up as intermittent test failure with UndefVarError(t_...) since it can cause the "t_$i" to return the wrong value and messes up lowering. (Not sure if this shows up on CI but I've hit it multiple times in ~1 day of GC stress testing)

We can workaround this for now by adding @noinline although it'll be nice if we can fix this with some kind of GC preserve scope for a certain object.

@yuyichao yuyichao added the GC Garbage collector label Aug 31, 2017
@StefanKarpinski
Copy link
Member

We should probably audit all uses of pointer in the code base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GC Garbage collector
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants