Bug 132683 - [Win] Crash when enabling DFG JIT.
Summary: [Win] Crash when enabling DFG JIT.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-08 08:33 PDT by peavo
Modified: 2014-05-09 09:30 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.10 KB, patch)
2014-05-08 08:39 PDT, peavo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description peavo 2014-05-08 08:33:56 PDT
I'm getting some crashes when running with DFG enabled.

See bug 121001 for further details, the cause and type of fix are the same.
Comment 1 peavo 2014-05-08 08:39:32 PDT
Created attachment 231070 [details]
Patch
Comment 2 Geoffrey Garen 2014-05-08 11:02:01 PDT
Comment on attachment 231070 [details]
Patch

How does regT0, which is an enumerated constant, alias to void*?
Comment 3 peavo 2014-05-08 12:17:50 PDT
I've tested the following code with MSVC which demonstrates the problem:

class A
{
public:
    A(int i) {}
};

void testfunc(void* p)
{
}

void testfunc(A a)
{
}

testfunc(0); // Invokes testfunc(void* p)
testfunc(1); // Invokes testfunc(A a)

When calling testfunc with 0, testfunc(void* p) is called.
When calling testfunc with 1, testfunc(A a) is called.

I'm not sure what the standard says here, it might be a compiler bug.
I assume GCC will call testfunc(A a) in both cases here.
Comment 4 Geoffrey Garen 2014-05-08 14:03:02 PDT
Answering my own question, a RegisterID on x86 is an enumerated constant:

namespace X86Registers {
    typedef enum {
        eax,
        ecx,
        edx,
        ebx,
        esp,
        ebp,
        esi,
        edi,

#if CPU(X86_64)
        r8,
        r9,
        r10,
        r11,
        r12,
        r13,
        r14,
        r15,
#endif
    } RegisterID;

Can you test an enum? Do enumerated constants actually alias to void*?
Comment 5 peavo 2014-05-08 14:36:16 PDT
Tried to replicate the actual code more accurately, and tested with an enum.

I got the same results, with argument regT0, testfunc(void* p) is called, and with argument regT1, testfunc(A a) is called.


namespace X86Registers {
    typedef enum {
        eax,
        ecx,
        edx,
        ebx,
        esp,
        ebp,
        esi,
        edi,
    } RegisterID;
}

typedef X86Registers::RegisterID GPRReg;

typedef X86Registers::RegisterID RegisterID;

static const GPRReg regT0 = X86Registers::eax;
static const GPRReg regT1 = X86Registers::edx;

class A
{
public:
    A(RegisterID i) {}
};

void testfunc(void* p)
{
}

void testfunc(A a)
{
}

testfunc(regT0); // Invokes testfunc(void* p)
testfunc(regT1); // Invokes testfunc(A a)
Comment 6 Geoffrey Garen 2014-05-08 16:07:36 PDT
Comment on attachment 231070 [details]
Patch

r=me

It's kind of a shame that an enum aliases to void* -- that's pretty easy to get wrong.

Perhaps you can fix this in a follow-up patch by changing the void* inputs to ImmPtr inputs, or similar.

I think you should pursue a follow-up patch that changes void* input to
Comment 7 peavo 2014-05-09 07:51:35 PDT
(In reply to comment #6)
> (From update of attachment 231070 [details])
> r=me
> 

Thanks!

> It's kind of a shame that an enum aliases to void* -- that's pretty easy to get wrong.
> 
> Perhaps you can fix this in a follow-up patch by changing the void* inputs to ImmPtr inputs, or similar.
> 
> I think you should pursue a follow-up patch that changes void* input to

Sounds good, I will look into that.
Comment 8 WebKit Commit Bot 2014-05-09 09:30:11 PDT
Comment on attachment 231070 [details]
Patch

Clearing flags on attachment: 231070

Committed r168535: <http://trac.webkit.org/changeset/168535>
Comment 9 WebKit Commit Bot 2014-05-09 09:30:14 PDT
All reviewed patches have been landed.  Closing bug.