132683 2014-05-08 08:33:56 -0700 [Win] Crash when enabling DFG JIT. 2014-05-09 09:30:14 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 peavo webkit-unassigned bfulgham commit-queue ggaren mark.lam oldest_to_newest 1007624 0 peavo 2014-05-08 08:33:56 -0700 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. 1007629 1 231070 peavo 2014-05-08 08:39:32 -0700 Created attachment 231070 Patch 1007679 2 231070 ggaren 2014-05-08 11:02:01 -0700 Comment on attachment 231070 Patch How does regT0, which is an enumerated constant, alias to void*? 1007715 3 peavo 2014-05-08 12:17:50 -0700 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. 1007763 4 ggaren 2014-05-08 14:03:02 -0700 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*? 1007774 5 peavo 2014-05-08 14:36:16 -0700 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) 1007798 6 231070 ggaren 2014-05-08 16:07:36 -0700 Comment on attachment 231070 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 1007986 7 peavo 2014-05-09 07:51:35 -0700 (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. 1008001 8 231070 commit-queue 2014-05-09 09:30:11 -0700 Comment on attachment 231070 Patch Clearing flags on attachment: 231070 Committed r168535: <http://trac.webkit.org/changeset/168535> 1008002 9 commit-queue 2014-05-09 09:30:14 -0700 All reviewed patches have been landed. Closing bug. 231070 2014-05-08 08:39:32 -0700 2014-05-09 09:30:10 -0700 Patch bug-132683-20140508173908.patch text/plain 3178 peavo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTY4NDcwKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBA CisyMDE0LTA1LTA4ICBwZWF2b0BvdXRsb29rLmNvbSAgPHBlYXZvQG91dGxvb2suY29tPgorCisg ICAgICAgIFtXaW5dIENyYXNoIHdoZW4gZW5hYmxpbmcgREZHIEpJVC4KKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEzMjY4MworCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE9uIHdpbmRvd3MsIHVzaW5nIHJlZ2lz dGVyIEdQUkluZm86OnJlZ1QwIGFzIHBhcmFtZXRlciB0byBlLmcuIEpJVDo6c3RvcmVEb3VibGUo Li4uLCBHUFJJbmZvOjpyZWdUMCkpLAorICAgICAgICByZXN1bHRzIGluIGEgY2FsbCB0byBKSVQ6 OnN0b3JlRG91YmxlKEZQUmVnaXN0ZXJJRCBzcmMsIGNvbnN0IHZvaWQqIGFkZHJlc3MpLAorICAg ICAgICB3aGVyZSB0aGUgYWRkcmVzcyBwYXJhbWV0ZXIgZ2V0cyB0aGUgdmFsdWUgb2YgR1BSSW5m bzo6cmVnVDAsIHdoaWNoIGlzIDAgKGVheCBvbiBXaW5kb3dzKS4KKyAgICAgICAgVGhpcyBjYXVz ZXMgdGhlIHJlZ2lzdGVyIHRvIGJlIHdyaXR0ZW4gdG8gYWRkcmVzcyAwLCBoZW5jZSB0aGUgY3Jh c2guCisKKyAgICAgICAgKiBkZmcvREZHT1NSRXhpdENvbXBpbGVyMzJfNjQuY3BwOgorICAgICAg ICAoSlNDOjpERkc6Ok9TUkV4aXRDb21waWxlcjo6Y29tcGlsZUV4aXQpOiBVc2UgYWRkcmVzcyBp biByZWdUMCBhcyBwYXJhbWV0ZXIuCisgICAgICAgICogZGZnL0RGR09TUkV4aXRDb21waWxlcjY0 LmNwcDoKKyAgICAgICAgKEpTQzo6REZHOjpPU1JFeGl0Q29tcGlsZXI6OmNvbXBpbGVFeGl0KTog RGl0dG8uCisKIDIwMTQtMDUtMDcgIENvbW1pdCBRdWV1ZSAgPGNvbW1pdC1xdWV1ZUB3ZWJraXQu b3JnPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHIxNjg0NTEuCkluZGV4OiBT b3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR09TUkV4aXRDb21waWxlcjMyXzY0LmNwcAo9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR09TUkV4aXRDb21waWxlcjMy XzY0LmNwcAkocmV2aXNpb24gMTY4NDI4KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9E RkdPU1JFeGl0Q29tcGlsZXIzMl82NC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTI4Nyw3ICsyODcs NyBAQCB2b2lkIE9TUkV4aXRDb21waWxlcjo6Y29tcGlsZUV4aXQoY29uc3QKICAgICAgICAgY2Fz ZSBJbkZQUjoKICAgICAgICAgY2FzZSBEb3VibGVEaXNwbGFjZWRJbkpTU3RhY2s6CiAgICAgICAg ICAgICBtX2ppdC5tb3ZlKEFzc2VtYmx5SGVscGVyczo6VHJ1c3RlZEltbVB0cihzY3JhdGNoICsg aW5kZXgpLCBHUFJJbmZvOjpyZWdUMCk7Ci0gICAgICAgICAgICBtX2ppdC5sb2FkRG91YmxlKEdQ UkluZm86OnJlZ1QwLCBGUFJJbmZvOjpmcFJlZ1QwKTsKKyAgICAgICAgICAgIG1faml0LmxvYWRE b3VibGUoTWFjcm9Bc3NlbWJsZXI6OkFkZHJlc3MoR1BSSW5mbzo6cmVnVDApLCBGUFJJbmZvOjpm cFJlZ1QwKTsKICAgICAgICAgICAgIG1faml0LnB1cmlmeU5hTihGUFJJbmZvOjpmcFJlZ1QwKTsK ICAgICAgICAgICAgIG1faml0LnN0b3JlRG91YmxlKEZQUkluZm86OmZwUmVnVDAsIEFzc2VtYmx5 SGVscGVyczo6YWRkcmVzc0ZvcihvcGVyYW5kKSk7CiAgICAgICAgICAgICBicmVhazsKSW5kZXg6 IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHT1NSRXhpdENvbXBpbGVyNjQuY3BwCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHT1NSRXhpdENvbXBpbGVyNjQu Y3BwCShyZXZpc2lvbiAxNjg0MjgpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR09T UkV4aXRDb21waWxlcjY0LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMjE3LDcgKzIxNyw3IEBAIHZv aWQgT1NSRXhpdENvbXBpbGVyOjpjb21waWxlRXhpdChjb25zdAogICAgICAgICBzd2l0Y2ggKHJl Y292ZXJ5LnRlY2huaXF1ZSgpKSB7CiAgICAgICAgIGNhc2UgSW5GUFI6CiAgICAgICAgICAgICBt X2ppdC5tb3ZlKEFzc2VtYmx5SGVscGVyczo6VHJ1c3RlZEltbVB0cihzY3JhdGNoICsgaW5kZXgp LCBHUFJJbmZvOjpyZWdUMCk7Ci0gICAgICAgICAgICBtX2ppdC5zdG9yZURvdWJsZShyZWNvdmVy eS5mcHIoKSwgR1BSSW5mbzo6cmVnVDApOworICAgICAgICAgICAgbV9qaXQuc3RvcmVEb3VibGUo cmVjb3ZlcnkuZnByKCksIE1hY3JvQXNzZW1ibGVyOjpBZGRyZXNzKEdQUkluZm86OnJlZ1QwKSk7 CiAgICAgICAgICAgICBicmVhazsKICAgICAgICAgICAgIAogICAgICAgICBkZWZhdWx0OgpAQCAt Mjk2LDcgKzI5Niw3IEBAIHZvaWQgT1NSRXhpdENvbXBpbGVyOjpjb21waWxlRXhpdChjb25zdAog ICAgICAgICBjYXNlIEluRlBSOgogICAgICAgICBjYXNlIERvdWJsZURpc3BsYWNlZEluSlNTdGFj azoKICAgICAgICAgICAgIG1faml0Lm1vdmUoQXNzZW1ibHlIZWxwZXJzOjpUcnVzdGVkSW1tUHRy KHNjcmF0Y2ggKyBpbmRleCksIEdQUkluZm86OnJlZ1QwKTsKLSAgICAgICAgICAgIG1faml0Lmxv YWREb3VibGUoR1BSSW5mbzo6cmVnVDAsIEZQUkluZm86OmZwUmVnVDApOworICAgICAgICAgICAg bV9qaXQubG9hZERvdWJsZShNYWNyb0Fzc2VtYmxlcjo6QWRkcmVzcyhHUFJJbmZvOjpyZWdUMCks IEZQUkluZm86OmZwUmVnVDApOwogICAgICAgICAgICAgbV9qaXQucHVyaWZ5TmFOKEZQUkluZm86 OmZwUmVnVDApOwogICAgICAgICAgICAgbV9qaXQuYm94RG91YmxlKEZQUkluZm86OmZwUmVnVDAs IEdQUkluZm86OnJlZ1QwKTsKICAgICAgICAgICAgIG1faml0LnN0b3JlNjQoR1BSSW5mbzo6cmVn VDAsIEFzc2VtYmx5SGVscGVyczo6YWRkcmVzc0ZvcihvcGVyYW5kKSk7Cg==