Bug 132683 – [Win] Crash when enabling DFG JIT.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

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.

Created attachment 231070 [details]
Patch

[Geoffrey Garen]

Comment on attachment 231070 [details]
Patch

How does regT0, which is an enumerated constant, alias to void*?

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.

[Geoffrey Garen]

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*?

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)

[Geoffrey Garen]

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

(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.

[WebKit Commit Bot]

Comment on attachment 231070 [details]
Patch

Clearing flags on attachment: 231070

Committed r168535: <http://trac.webkit.org/changeset/168535>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.