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

Wrong calling convention #15

Closed
cfis opened this issue Apr 5, 2010 · 5 comments
Closed

Wrong calling convention #15

cfis opened this issue Apr 5, 2010 · 5 comments

Comments

@cfis
Copy link

cfis commented Apr 5, 2010

rb-readline library has a pretty serious bug though that causes an MSVC instrumented build (I find it easiest to find bugs this way) to blow up. Its uses the cdecl calling convention to call the windows api. That causes the stack to get corrupted. Instead, it should use the stdcall calling convention.

diff --git a/lib/rbreadline.rb b/lib/rbreadline.rb
index 43894aa..b215aff 100644
--- a/lib/rbreadline.rb
+++ b/lib/rbreadline.rb
@@ -4288,7 +4288,7 @@ module RbReadline
def initialize(dllname, func, import, export = "0")
@proto = [import].join.tr("VPpNnLlIi", "0SSI").sub(/^(.)0*$/, '\1')
handle = DLL[dllname] ||= DL.dlopen(dllname)

  •           @func = DL::CFunc.new(handle[func], TYPEMAP[export.tr("VPpNnLlIi", "0SSI")], func)
    
  •           @func = DL::CFunc.new(handle[func], TYPEMAP[export.tr("VPpNnLlIi", "0SSI")], func, :stdcall)
         end
    
         def call(*args)
    

    -- 1.7.0.4.360.g11766c.dirty

@jmhodges
Copy link

jmhodges commented Apr 5, 2010

Your patch came up all busted. Why not just make a fork, a new branch and push up the changes?

@cfis
Copy link
Author

cfis commented Apr 5, 2010

Yeah, guess so. Its such a simple change though:

@func = DL::CFunc.new(handle[func], TYPEMAP[export.tr("VPpNnLlIi", "0SSI")], func)

To:

@func = DL::CFunc.new(handle[func], TYPEMAP[export.tr("VPpNnLlIi", "0SSI")], func, :stdcall)

Anyway, sent Luis the patch file. I'll setup a fork if i don't hear from him.

@cfis
Copy link
Author

cfis commented Apr 5, 2010

Not sure how to reopen this...grrr...

@rdp
Copy link
Contributor

rdp commented Jun 7, 2010

appears this is fixed now?

@luislavena
Copy link
Collaborator

This has already been fixed. Closing this out.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants