Bug 69058

Summary:

DFG operation calls should be stdcall in Linux JSVALUE32_64 DFG JIT

Product:

WebKit

Reporter:

Yuqiang Xian <yuqiang.xian>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, ggaren

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

69413

Bug Blocks:

Attachments:

Description	Flags
the patch	barraclough: review-
patch	barraclough: review+

Yuqiang Xian

Reported 2011-09-28 20:42:25 PDT

With the STDCALL prefix, on Linux a function pointer is not implicitly converted to a FunctionPtr obj.

Attachments
the patch (26.74 KB, patch) 2011-09-28 20:48 PDT, Yuqiang Xian	barraclough: review-	Details Formatted Diff Diff
patch (2.85 KB, patch) 2011-09-29 05:07 PDT, Yuqiang Xian	barraclough: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Yuqiang Xian

Comment 1 2011-09-28 20:48:33 PDT

Created attachment 109118 [details] the patch

Gavin Barraclough

Comment 2 2011-09-28 23:59:31 PDT

Comment on attachment 109118 [details] the patch Looks like this patch is pre r96293, is there still a problem post-r96293? I don't think we want to take this change. We should be able to give FunctionPtr an appropriate set of constructors. If Linux defaults to stdcall, it is possible that the '#if CPU(X86) && COMPILER(GCC)' check in DFGOperations.h should be '#if CALLING_CONVENTION_IS_CDECL'. We should be able to fix the problem without littering the JIT with explicit constructor calls.

Yuqiang Xian

Comment 3 2011-09-29 00:06:17 PDT

Hi Gavin, Linux also defaults to cdecl. And surely we expects stdcall for JSValue32_64 DFG operations (which was a bug in previous code).

Yuqiang Xian

Comment 4 2011-09-29 05:07:57 PDT

Created attachment 109153 [details] patch This patch fixed the stdcall FunctionPtr constructors issue on Linux.

Geoffrey Garen

Comment 5 2011-09-29 12:09:35 PDT

Comment on attachment 109153 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=109153&action=review > Source/JavaScriptCore/assembler/MacroAssemblerCodeRef.h:55 > +#if CPU(X86) && COMPILER(GCC) I don't think GCC is the right test here. It will definitely be wrong for people building with clang. But I think it may also be wrong for people building with GCC on non-cdecl platforms, since cdecl is a property of the platform ABI, not the compiler. I'd suggest (OS(DARWIN) || OS(LINUX)) here.

Gavin Barraclough

Comment 6 2011-09-29 12:35:32 PDT

I think stdcall is only a convention in windows, so per geoff's comments I'm going to make this !PLATFORM(WINDOWS), r+ & land.

Gavin Barraclough

Comment 7 2011-09-29 12:37:00 PDT

Fixed in r96347

Geoffrey Garen

Comment 8 2011-09-29 12:42:35 PDT

(In reply to comment #6) > I think stdcall is only a convention in windows, so per geoff's comments I'm going to make this !PLATFORM(WINDOWS), r+ & land. But what about Open Watcom C++??? :)

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