Bug 58483

Summary: GC allocate Structure
Product: WebKit Reporter: Oliver Hunt <oliver>
Component: New BugsAssignee: Oliver Hunt <oliver>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, buildbot, eric, gustavo, nlawrence, pnormand, webkit-ews, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch
none
qt fixes
none
windows fixes none

Description Oliver Hunt 2011-04-13 14:55:43 PDT 
GC allocate Structure
Comment 1 Oliver Hunt 2011-04-13 15:08:15 PDT 
Created attachment 89472 [details]
Patch
Comment 2 WebKit Review Bot 2011-04-13 15:12:06 PDT 
Attachment 89472 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSCallbackConstr..." exit_code: 1

Source/JavaScriptCore/interpreter/Interpreter.cpp:2703:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:2958:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:2982:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:3006:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/bytecode/Instruction.h:70:  _stubRoutine is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:70:  _base is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _stubRoutine is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _base is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _proto is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 9 in 182 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Early Warning System Bot 2011-04-13 15:26:26 PDT 
Attachment 89472 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8404415
Comment 4 Geoffrey Garen 2011-04-13 15:42:09 PDT 
Comment on attachment 89472 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=89472&action=review

r=me

> Source/JavaScriptCore/bytecode/Instruction.h:146
> +            u.structure.clear();

No need for clear here.

> Source/JavaScriptCore/runtime/StructureTransitionTable.h:154
> +            HandleHeap::heapFor(slot)->makeWeak(slot, 0, 0);

No need for 0, 0 here.
Comment 5 Build Bot 2011-04-13 15:59:58 PDT 
Attachment 89472 [details] did not build on win:
Build output: http://queues.webkit.org/results/8397434
Comment 6 Oliver Hunt 2011-04-13 16:18:00 PDT 
Created attachment 89487 [details]
Patch
Comment 7 WebKit Review Bot 2011-04-13 16:21:11 PDT 
Attachment 89487 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/dom/gc-10.html', u'Source..." exit_code: 1

Source/JavaScriptCore/interpreter/Interpreter.cpp:2703:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:2958:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:2982:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:3006:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/bytecode/Instruction.h:70:  _stubRoutine is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:70:  _base is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _stubRoutine is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _base is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _proto is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 9 in 184 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Early Warning System Bot 2011-04-13 16:33:36 PDT 
Attachment 89487 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8404433
Comment 9 Build Bot 2011-04-13 16:59:26 PDT 
Attachment 89487 [details] did not build on win:
Build output: http://queues.webkit.org/results/8400486
Comment 10 Oliver Hunt 2011-04-13 17:04:47 PDT 
Created attachment 89494 [details]
qt fixes
Comment 11 WebKit Review Bot 2011-04-13 17:09:35 PDT 
Attachment 89494 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/dom/gc-10.html', u'Source..." exit_code: 1

Source/JavaScriptCore/interpreter/Interpreter.cpp:2703:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:2958:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:2982:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:3006:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/bytecode/Instruction.h:70:  _stubRoutine is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:70:  _base is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _stubRoutine is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _base is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _proto is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 9 in 188 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Build Bot 2011-04-13 17:47:03 PDT 
Attachment 89494 [details] did not build on win:
Build output: http://queues.webkit.org/results/8398476
Comment 13 Oliver Hunt 2011-04-13 18:15:46 PDT 
Created attachment 89510 [details]
windows fixes
Comment 14 WebKit Review Bot 2011-04-13 18:18:22 PDT 
Attachment 89510 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/dom/gc-10.html', u'Source..." exit_code: 1

Source/JavaScriptCore/interpreter/Interpreter.cpp:2703:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:2958:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:2982:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/interpreter/Interpreter.cpp:3006:  Missing spaces around &&  [whitespace/operators] [3]
Source/JavaScriptCore/bytecode/Instruction.h:70:  _stubRoutine is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:70:  _base is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _stubRoutine is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _base is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/bytecode/Instruction.h:78:  _proto is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 9 in 188 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Oliver Hunt 2011-04-13 20:29:08 PDT 
Committed r83808: <http://trac.webkit.org/changeset/83808>
Comment 16 Philippe Normand 2011-04-14 00:28:15 PDT 
This seems to have seriously broken GTK 32-bits Release. 20+ tests failing.
Comment 17 Philippe Normand 2011-04-14 08:52:49 PDT 
(In reply to comment #16)
> This seems to have seriously broken GTK 32-bits Release. 20+ tests failing.

Rolling out these revisions fixes the issue on GTK:

83827
83810
83809
83808

In case no other follow-up commit lands
Comment 18 Oliver Hunt 2011-04-14 09:40:43 PDT 
(In reply to comment #17)
> (In reply to comment #16)
> > This seems to have seriously broken GTK 32-bits Release. 20+ tests failing.
> 
> Rolling out these revisions fixes the issue on GTK:
> 
> 83827
> 83810
> 83809
> 83808
> 
> In case no other follow-up commit lands

What is the build bot on build.webkit.org?  That seemed to be all green once i landed the follow up 32bit fix...
Comment 19 Philippe Normand 2011-04-14 13:41:11 PDT 
(In reply to comment #18)
> (In reply to comment #17)
> > (In reply to comment #16)
> > > This seems to have seriously broken GTK 32-bits Release. 20+ tests failing.
> > 
> > Rolling out these revisions fixes the issue on GTK:
> > 
> > 83827
> > 83810
> > 83809
> > 83808
> > 
> > In case no other follow-up commit lands
> 
> What is the build bot on build.webkit.org?  That seemed to be all green once i landed the follow up 32bit fix...

Simply http://build.webkit.org/builders/GTK%20Linux%2032-bit%20Release
Comment 20 Oliver Hunt 2011-04-14 13:50:37 PDT 
(In reply to comment #19)
> (In reply to comment #18)
> > (In reply to comment #17)
> > > (In reply to comment #16)
> > > > This seems to have seriously broken GTK 32-bits Release. 20+ tests failing.
> > > 
> > > Rolling out these revisions fixes the issue on GTK:
> > > 
> > > 83827
> > > 83810
> > > 83809
> > > 83808
> > > 
> > > In case no other follow-up commit lands
> > 
> > What is the build bot on build.webkit.org?  That seemed to be all green once i landed the follow up 32bit fix...
> 
> Simply http://build.webkit.org/builders/GTK%20Linux%2032-bit%20Release

Why are there no crash logs being reported by that bot?

Is that an interpreter bot? (I tested interpreter, though maybe only 64bit?)
Comment 21 Philippe Normand 2011-04-14 13:57:17 PDT 
That bot doesn't dump stack-traces indeed :/ Gustavo manages that bot, maybe he could install the scripts we use in the Igalia Debug bots ;)

IIRC the crash happens in cti_op_get_by_val() that's the only info I could extract from the stack-trace I got locally :( Being a release build we don't get dbg symbols...
Comment 22 Philippe Normand 2011-04-15 00:44:06 PDT 
Rolled out in http://trac.webkit.org/changeset/83955
Comment 23 Oliver Hunt 2011-04-15 10:12:45 PDT 
(In reply to comment #22)
> Rolled out in http://trac.webkit.org/changeset/83955
Pardon? You rolled it out because it broke a non-core builder, having failed to provide any information that actually helped diagnose the problem, when every other platform was fine with the change?
Comment 24 Oliver Hunt 2011-04-15 10:13:59 PDT 
(In reply to comment #22)
> Rolled out in http://trac.webkit.org/changeset/83955

In future if you feel the desire to rollout a change like this actually ask first, don't blindly roll out with the assumption that that is the right choice, given the absence of further information i'm assuming it's a gtk specific bug, that i cannot fix.
Comment 25 Philippe Normand 2011-04-15 10:18:39 PDT 
It broke a core builder.

And if there was a 64-bits Release builder it would have failed as well. My local 64-bits build was also broken.

I'm willing to test any new version of this patch BTW.
Comment 26 Philippe Normand 2011-04-15 10:59:21 PDT 
gdb -args  WebKitBuild/Release/Programs/DumpRenderTree LayoutTests/editing/selection/find-yensign-and-backslash-with-japanese-fonts.html

(gdb) bt
#0  0x00007ffff75cef44 in cti_op_get_by_val ()
   from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-1.0.so.0
#1  0x00007fff9e4d4f2e in ?? ()
#2  0x00007fff9e0a8150 in ?? ()
#3  0x00007fff9e0ba910 in ?? ()
#4  0xffff00000000000d in ?? ()
#5  0x00007fff9dea8e90 in ?? ()
#6  0x00007fff9e4d53bf in ?? ()
#7  0x00007fffffffca90 in ?? ()
#8  0x0000000000000000 in ?? ()

This is a Release build indeed.
Comment 27 Oliver Hunt 2011-04-15 11:01:05 PDT 
(In reply to comment #26)
> gdb -args  WebKitBuild/Release/Programs/DumpRenderTree LayoutTests/editing/selection/find-yensign-and-backslash-with-japanese-fonts.html
> 
> (gdb) bt
> #0  0x00007ffff75cef44 in cti_op_get_by_val ()
>    from /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Release/.libs/libwebkitgtk-1.0.so.0
> #1  0x00007fff9e4d4f2e in ?? ()
> #2  0x00007fff9e0a8150 in ?? ()
> #3  0x00007fff9e0ba910 in ?? ()
> #4  0xffff00000000000d in ?? ()
> #5  0x00007fff9dea8e90 in ?? ()
> #6  0x00007fff9e4d53bf in ?? ()
> #7  0x00007fffffffca90 in ?? ()
> #8  0x0000000000000000 in ?? ()
> 
> This is a Release build indeed.

needs line number information, can you build without symbols stripped.
Comment 28 Oliver Hunt 2011-04-15 17:00:38 PDT 
Committed r84052
Comment 29 WebKit Review Bot 2011-04-15 18:22:06 PDT 
http://trac.webkit.org/changeset/84052 might have broken WinCairo Debug (Build)
Comment 30 Gavin Barraclough 2011-09-09 22:21:47 PDT 
*** Bug 43641 has been marked as a duplicate of this bug. ***