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

"Puts" errors and breaks program when used in a Fiber on Windows #7955

Closed
cfsamson opened this issue Jul 7, 2019 · 1 comment
Closed

"Puts" errors and breaks program when used in a Fiber on Windows #7955

cfsamson opened this issue Jul 7, 2019 · 1 comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows Windows support based on the MSVC toolchain / Win32 API

Comments

@cfsamson
Copy link
Contributor

cfsamson commented Jul 7, 2019

Problem

When puts is used in a context switch on Windows it causes an error which is subtle since it prevents anything from being written to the console at all so there are no visible errors given and the code simply seems to break.

Solving this presumably helps #6955 move along and is the real error causing #7947.

Details

When puts is called after any context switch on Windows, like a fiber, it errors with the message File not open for writing. This relevant part of the source code raising this error is found here. What makes this troublesome is that the error message itself is not being printed either. This might be caused by some kind of lock contention where a lock on stdout is not released as expected.

However if you replace:

puts "Hello"

With:

STDOUT << "Hello"
STDOUT.flush

It works.

To see the actual error you'll have to disassemble the code using a disassembler for Windows and step through the code instruction by instruction. After the message is supposed to be printed you'll see one of the CPU registers pointing at a memory location containing the string "File not open for writing", but the error message never actually gets printed.

How to reproduce

Since Fibers for Windows is not implemented yet (coincidentally since the implementation didn't seem to work because of this very error), I've written an absolute minimum amount of code to reproduce this error on the current compiler (v0.29):

I would suggest you use Windows and WSL to test this

  1. Download Ubuntu 16.04 from Windows Store
  2. Follow Porting to Windows and if you have windows 10 and Visual Studio 2019 follow the updates in Update Wiki page "Porting to Windows" #7932 as well.
  3. In a new folder create a file and paste the code below into it, call it puts_error.cr if you want to copy paste the rest of the commands below
  4. In WSL run crystal build -D gc_none --cross-compile --target x86_64-pc-windows-msvc puts_error.cr
  5. Make sure you move the resulting .o file to the folder where you have pcre.lib and gc.libin the root folder. Open x86_64 Cross Tools Command Promt for vs 2019 and write this command: cl "puts_error.o" "/Feputs_error" pcre.lib gc.lib advapi32.lib libcmt.lib legacy_stdio_definitions.lib
  6. Run puts_error.exe. The program seems to just not work.
  7. Change puts with STDOUT like shown at the bottom an see the message printed.
STACK_SIZE = 1024*1024*8
WORD_SIZE = sizeof(Void*)
SHADOW_SPACE_SIZE = 32

class Test
  property value : UInt64 = 0
  property stack = Array(UInt8).new(STACK_SIZE)
  property func : Proc(Void) = ->{}
  property rsp : UInt64 = 0

  def makecontext(proc : Proc)
    @func = proc

    s_ptr = @stack.to_unsafe.as(UInt8*) + STACK_SIZE
    s_ptr = Pointer(UInt8).new(s_ptr.address & ~15)  

    s_start = s_ptr - SHADOW_SPACE_SIZE 
    @rsp = (s_start - WORD_SIZE).as(Void*).address 

    s_start_ptr = s_start.as(UInt64*)
    s_start_ptr.value = ->start(Test).pointer.address

    first_param = (s_start - 8).as(UInt64*)
    first_param.value = self.as(Void*).address
  end

  def run
    @func.call
    Process.exit(0)
  end
end

def start(f : Test)
  f.run
end

@[NoInline]
@[Naked]
def swap(data)
  {% if flag?(:win32) %}
  asm("
  pushq %rcx # search marker
  movq $0, %rsp
  popq %rcx
  "
  :: "r"(data) )
  
  {% else %}
  asm("
  pushq %rdi # search marker
  movq $0, %rsp
  popq %rdi
  "
  :: "r"(data))
  {% end %}
end

puts "start"
test = Test.new
test.makecontext(->{ 
  puts "Hello from new stack"
 })

swap(test.rsp)

If you replace puts "Hello from new stack with:

STDOUT << "Hello from new stack\n"
STDOUT.flush

It works as expected on both Linux and Windows.

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows Windows support based on the MSVC toolchain / Win32 API labels Jul 23, 2019
@straight-shoota
Copy link
Member

This error no longer reproduces. Probably fixed in #7995

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows Windows support based on the MSVC toolchain / Win32 API
Projects
None yet
Development

No branches or pull requests

2 participants