Last Comment Bug 648871 - Mismatched calling convention for CanvasStyleSetterType
: Mismatched calling convention for CanvasStyleSetterType
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla6
Assigned To: Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
:
:
Mentors:
Depends on:
Blocks: clang 648872
  Show dependency treegraph
 
Reported: 2011-04-10 09:52 PDT by Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
Modified: 2011-04-15 06:58 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Change the calling convention (1.66 KB, patch)
2011-04-10 09:52 PDT, Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
no flags Details | Diff | Splinter Review
Change the calling convention (872 bytes, patch)
2011-04-10 09:53 PDT, Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
no flags Details | Diff | Splinter Review
NS_STDCALL_FUNCPROTO (999 bytes, patch)
2011-04-10 10:30 PDT, Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
bzbarsky: review+
Details | Diff | Splinter Review
hg generated patch (1.48 KB, patch)
2011-04-14 08:03 PDT, Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
no flags Details | Diff | Splinter Review

Description Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-04-10 09:52:33 PDT
Created attachment 524963 [details] [diff] [review]
Change the calling convention

CanvasStyleSetterType and CanvasStyleSetterType are declared with NS_STDCALL but are used with methods declared with NS_DEFCALL. Clang complains with:

In file included from dom_quickstubs.cpp:241:../../../../dist/include/CustomQS_Canvas2D.h:139:12: error: no matching function for call to 'Canvas2D_SetStyleHelper'
    return Canvas2D_SetStyleHelper(cx, obj, id, vp, &nsIDOMCanvasRenderingContext2D::SetStrokeStyle_multi);
           ^~~~~~~~~~~~~~~~~~~~~~~
../../../../dist/include/CustomQS_Canvas2D.h:51:1: note: candidate function not viable: no known conversion from 'nsresult (nsIDOMCanvasRenderingContext2D::*)(const nsAString_internal &, nsISupports *) __attr
ibute__((cdecl))' to 'CanvasStyleSetterType' (aka 'nsresult (nsIDOMCanvasRenderingContext2D::*)(const nsAString_internal &, nsISupports *)') for 5th argument
Canvas2D_SetStyleHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
^
Comment 1 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-04-10 09:53:39 PDT
Created attachment 524965 [details] [diff] [review]
Change the calling convention
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2011-04-10 09:58:27 PDT
Uh...  nsCanvasRenderingContext2D::SetStrokeStyle_multi is stdcall, no?  At least in all situations in which the typedef is stdcall.

Or put another way, does your patch compile on Windows?
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2011-04-10 10:03:52 PDT
Oh, I see what's going on here.  SetStrokeStyle_multi is defined as NS_IMETHOD, which means one of the following things:

  __stdcall (on Windows)
  nothing special (on OS/2)
  NS_DEFCALL (elsewhere)

When __GNUC__ is defined on i386, NS_DEFCALL expands to:

  __attribute__ ((regparm (0), cdecl))

and otherwise it means nothing special. 

The typedef is using NS_STDCALL, which means the following:

  __stdcall on windows
  nothing special elsewhere

So the current code is correct on Windows (and your new code is wrong there), but broken on a compiler that defines __GNUC__.

The right thing to do here seems to be to use the NS_STDCALL_FUNCPROTO macros to define the typedefs, right?  At least assuming the __GNUC__ bit of those works in clang.
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2011-04-10 10:05:15 PDT
Also, you should be requesting review from specific people if you want the review request to be seen.  :(
Comment 5 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-04-10 10:30:07 PDT
Created attachment 524970 [details] [diff] [review]
NS_STDCALL_FUNCPROTO
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2011-04-12 22:30:48 PDT
Comment on attachment 524970 [details] [diff] [review]
NS_STDCALL_FUNCPROTO

r=me.  Sorry for the lag here...
Comment 7 :Ms2ger (⌚ UTC+1/+2) 2011-04-13 12:39:24 PDT
Rafael, could you please prepare patches as described on <https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3f>? That would make it much easier to push them.
Comment 8 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-04-14 08:03:40 PDT
Created attachment 525999 [details] [diff] [review]
hg generated patch

Note You need to log in before you can comment on or make changes to this bug.