Bug 58483 – GC allocate Structure

• Component: New Bugs
• Status: RESOLVED FIXED

[Oliver Hunt]

GC allocate Structure

[Oliver Hunt]

Created attachment 89472 [details]
Patch

[WebKit Review Bot]

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.

[Early Warning System Bot]

Attachment 89472 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8404415

[Geoffrey Garen]

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.

[Build Bot]

Attachment 89472 [details] did not build on win:
Build output: http://queues.webkit.org/results/8397434

[Oliver Hunt]

Created attachment 89487 [details]
Patch

[WebKit Review Bot]

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.

[Early Warning System Bot]

Attachment 89487 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8404433

[Build Bot]

Attachment 89487 [details] did not build on win:
Build output: http://queues.webkit.org/results/8400486

[Oliver Hunt]

Created attachment 89494 [details]
qt fixes

[WebKit Review Bot]

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.

[Build Bot]

Attachment 89494 [details] did not build on win:
Build output: http://queues.webkit.org/results/8398476

[Oliver Hunt]

Created attachment 89510 [details]
windows fixes

[WebKit Review Bot]

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.

[Oliver Hunt]

Committed r83808: <http://trac.webkit.org/changeset/83808>

[Philippe Normand]

This seems to have seriously broken GTK 32-bits Release. 20+ tests failing.

[Philippe Normand]

(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

[Oliver Hunt]

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

[Philippe Normand]

(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

[Oliver Hunt]

(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?)

[Philippe Normand]

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

[Philippe Normand]

Rolled out in http://trac.webkit.org/changeset/83955

[Oliver Hunt]

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

[Oliver Hunt]

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

[Philippe Normand]

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.

[Philippe Normand]

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.

[Oliver Hunt]

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

[Oliver Hunt]

Committed r84052

[WebKit Review Bot]

http://trac.webkit.org/changeset/84052 might have broken WinCairo Debug (Build)

[Gavin Barraclough]

*** Bug 43641 has been marked as a duplicate of this bug. ***