Bug 105696 - JITThunks should not compile only because of luck
Summary: JITThunks should not compile only because of luck
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on: 105744
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-23 12:28 PST by Filip Pizlo
Modified: 2013-02-11 10:18 PST (History)
19 users (show)

See Also:


Attachments
the patch (141.96 KB, patch)
2012-12-23 12:34 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
another attempt (184.79 KB, patch)
2012-12-23 13:12 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
patch for landing (201.01 KB, patch)
2012-12-23 21:25 PST, Filip Pizlo
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
patch for landing (201.42 KB, patch)
2012-12-24 10:21 PST, Filip Pizlo
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
patch for landing (201.85 KB, patch)
2012-12-24 12:09 PST, Filip Pizlo
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
patch for landing (206.90 KB, patch)
2012-12-24 13:26 PST, Filip Pizlo
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
patch for landing (209.58 KB, patch)
2012-12-24 14:02 PST, Filip Pizlo
eflews.bot: commit-queue-
Details | Formatted Diff | Diff
trying to touch Platform.h (210.07 KB, patch)
2012-12-24 14:26 PST, Filip Pizlo
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
trying more things (210.42 KB, patch)
2012-12-24 19:28 PST, Filip Pizlo
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
patch for landing (168.34 KB, patch)
2013-01-10 14:31 PST, Filip Pizlo
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
patch for landing (169.49 KB, patch)
2013-01-11 13:31 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2012-12-23 12:28:52 PST
Patch forthcoming.
Comment 1 Filip Pizlo 2012-12-23 12:34:40 PST
Created attachment 180628 [details]
the patch

This will probably still have random build errors.  I'm still checking if I need to add #include Operations.h into WebCore and elsewhere.
Comment 2 Sam Weinig 2012-12-23 12:37:49 PST
Comment on attachment 180628 [details]
the patch

rs=me
Comment 3 WebKit Review Bot 2012-12-23 12:38:42 PST
Attachment 180628 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSCallbackObject..." exit_code: 1
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:337:  The parameter name "plan" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:338:  The parameter name "plan" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 3 in 56 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Early Warning System Bot 2012-12-23 12:42:23 PST
Comment on attachment 180628 [details]
the patch

Attachment 180628 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15492228
Comment 5 Early Warning System Bot 2012-12-23 12:42:35 PST
Comment on attachment 180628 [details]
the patch

Attachment 180628 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15492224
Comment 6 EFL EWS Bot 2012-12-23 12:44:23 PST
Comment on attachment 180628 [details]
the patch

Attachment 180628 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15513025
Comment 7 Filip Pizlo 2012-12-23 13:12:29 PST
Created attachment 180629 [details]
another attempt

I think I still have a lot of work to do adding #include "Operations.h" in WebCore.
Comment 8 Filip Pizlo 2012-12-23 21:25:14 PST
Created attachment 180641 [details]
patch for landing

Hopefully this will make all of the things happy.
Comment 9 WebKit Review Bot 2012-12-23 21:28:33 PST
Attachment 180641 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSBase.cpp', u'S..." exit_code: 1
Source/JavaScriptCore/runtime/RegExpCachedResult.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/SmallStrings.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSSymbolTableObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 4 in 164 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Early Warning System Bot 2012-12-23 21:37:04 PST
Comment on attachment 180641 [details]
patch for landing

Attachment 180641 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15503295
Comment 11 Early Warning System Bot 2012-12-23 21:39:19 PST
Comment on attachment 180641 [details]
patch for landing

Attachment 180641 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15498328
Comment 12 EFL EWS Bot 2012-12-23 22:13:36 PST
Comment on attachment 180641 [details]
patch for landing

Attachment 180641 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15511197
Comment 13 Filip Pizlo 2012-12-24 10:21:40 PST
Created attachment 180679 [details]
patch for landing

Try to fix Qt build.  Hopefully it also fixes EFL build.
Comment 14 WebKit Review Bot 2012-12-24 10:24:47 PST
Attachment 180679 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSBase.cpp', u'S..." exit_code: 1
Source/JavaScriptCore/runtime/RegExpCachedResult.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/SmallStrings.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSSymbolTableObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 4 in 165 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 EFL EWS Bot 2012-12-24 11:59:34 PST
Comment on attachment 180679 [details]
patch for landing

Attachment 180679 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15507533
Comment 16 EFL EWS Bot 2012-12-24 12:04:03 PST
Comment on attachment 180679 [details]
patch for landing

Attachment 180679 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15512476
Comment 17 Filip Pizlo 2012-12-24 12:09:02 PST
Created attachment 180685 [details]
patch for landing
Comment 18 WebKit Review Bot 2012-12-24 12:13:09 PST
Attachment 180685 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSBase.cpp', u'S..." exit_code: 1
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 166 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 19 EFL EWS Bot 2012-12-24 12:38:29 PST
Comment on attachment 180685 [details]
patch for landing

Attachment 180685 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15491629
Comment 20 Filip Pizlo 2012-12-24 13:26:36 PST
Created attachment 180689 [details]
patch for landing

And try again.
Comment 21 WebKit Review Bot 2012-12-24 13:29:28 PST
Attachment 180689 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSBase.cpp', u'S..." exit_code: 1
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 178 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 EFL EWS Bot 2012-12-24 13:39:19 PST
Comment on attachment 180689 [details]
patch for landing

Attachment 180689 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15492607
Comment 23 Filip Pizlo 2012-12-24 14:02:53 PST
Created attachment 180690 [details]
patch for landing

Trying again.
Comment 24 WebKit Review Bot 2012-12-24 14:07:12 PST
Attachment 180690 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSBase.cpp', u'S..." exit_code: 1
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 184 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 25 EFL EWS Bot 2012-12-24 14:14:23 PST
Comment on attachment 180690 [details]
patch for landing

Attachment 180690 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15527125
Comment 26 Filip Pizlo 2012-12-24 14:26:57 PST
Created attachment 180691 [details]
trying to touch Platform.h

Trying to see if the build error is due to EFL's build system not doing dependencies correctly.
Comment 27 WebKit Review Bot 2012-12-24 14:30:31 PST
Attachment 180691 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSBase.cpp', u'S..." exit_code: 1
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 185 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 28 Early Warning System Bot 2012-12-24 14:40:13 PST
Comment on attachment 180691 [details]
trying to touch Platform.h

Attachment 180691 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15495588
Comment 29 Early Warning System Bot 2012-12-24 14:43:51 PST
Comment on attachment 180691 [details]
trying to touch Platform.h

Attachment 180691 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15496609
Comment 30 EFL EWS Bot 2012-12-24 15:05:59 PST
Comment on attachment 180691 [details]
trying to touch Platform.h

Attachment 180691 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15490715
Comment 31 Filip Pizlo 2012-12-24 19:28:09 PST
Created attachment 180696 [details]
trying more things
Comment 32 WebKit Review Bot 2012-12-24 19:32:55 PST
Attachment 180696 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSBase.cpp', u'S..." exit_code: 1
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 186 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 33 Early Warning System Bot 2012-12-24 19:41:42 PST
Comment on attachment 180696 [details]
trying more things

Attachment 180696 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15497652
Comment 34 Early Warning System Bot 2012-12-24 19:44:32 PST
Comment on attachment 180696 [details]
trying more things

Attachment 180696 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15496679
Comment 35 EFL EWS Bot 2012-12-24 20:01:09 PST
Comment on attachment 180696 [details]
trying more things

Attachment 180696 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15492693
Comment 36 Chris Dumez 2012-12-25 00:07:37 PST
FYI, the patch is building just fine on my machine for EFL port (despite the EWS saying otherwise). It probably just requires a clean build?
Comment 37 Gyuyoung Kim 2012-12-25 20:05:38 PST
(In reply to comment #36)
> FYI, the patch is building just fine on my machine for EFL port (despite the EWS saying otherwise). It probably just requires a clean build?

Hmm, there are still linking errors on my PC though I build this patch from scratch.

Linking CXX executable ../../../bin/jsc
../../../lib/libjavascriptcore_efl.so.0.1.0: undefined reference to `JSC::JSValue::isGetterSetter() const'
../../../lib/libjavascriptcore_efl.so.0.1.0: undefined reference to `JSC::JSValue::JSValue(JSC::JSCell*)'
../../../lib/libjavascriptcore_efl.so.0.1.0: undefined reference to `JSC::JSValue::asCell() const'
../../../lib/libjavascriptcore_efl.so.0.1.0: undefined reference to `JSC::JSValue::decode(long)'
../../../lib/libjavascriptcore_efl.so.0.1.0: undefined reference to `JSC::JSCell::structure() const'
collect2: ld returned 1 exit status
make[2]: *** [bin/jsc] Error 1
make[1]: *** [Source/JavaScriptCore/shell/CMakeFiles/jsc.dir/all] Error 2
Comment 38 Csaba Osztrogonác 2012-12-27 06:48:39 PST
(In reply to comment #34)
> (From update of attachment 180696 [details])
> Attachment 180696 [details] did not pass qt-wk2-ews (qt):
> Output: http://queues.webkit.org/results/15496679

There are many conflicts and I can't apply the patch on ToT:
Source/JavaScriptCore/CMakeLists.txt
Source/JavaScriptCore/GNUmakefile.list.am
Source/JavaScriptCore/Target.pri
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/jit/JITStubs.cpp

Let's see an earlier revision - r138446. The build failed for me 
as the EWS said. I tracked down in which objects are these missing 
symbols an then I added #include "Operations.h" to the following 
files and now Qt build is happy on r138446:
- ErrorInstance.cpp
- NameInstance.cpp
- JSVariableObject.cpp
Comment 39 Eric Seidel (no email) 2013-01-04 00:41:20 PST
Comment on attachment 180628 [details]
the patch

Cleared Sam Weinig's review+ from obsolete attachment 180628 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 40 Filip Pizlo 2013-01-10 14:31:43 PST
Created attachment 182210 [details]
patch for landing
Comment 41 WebKit Review Bot 2013-01-10 14:43:50 PST
Attachment 182210 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSBase.cpp', u'S..." exit_code: 1
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 178 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 42 Early Warning System Bot 2013-01-10 14:45:17 PST
Comment on attachment 182210 [details]
patch for landing

Attachment 182210 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15806116
Comment 43 Early Warning System Bot 2013-01-10 14:46:52 PST
Comment on attachment 182210 [details]
patch for landing

Attachment 182210 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15809079
Comment 44 Build Bot 2013-01-10 14:56:12 PST
Comment on attachment 182210 [details]
patch for landing

Attachment 182210 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15806117
Comment 45 Build Bot 2013-01-10 15:17:06 PST
Comment on attachment 182210 [details]
patch for landing

Attachment 182210 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15775910
Comment 46 WebKit Review Bot 2013-01-10 15:44:53 PST
Attachment 182210 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSBase.cpp', u'S..." exit_code: 1
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 178 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 47 Early Warning System Bot 2013-01-10 15:46:05 PST
Comment on attachment 182210 [details]
patch for landing

Attachment 182210 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15795388
Comment 48 Early Warning System Bot 2013-01-10 15:50:15 PST
Comment on attachment 182210 [details]
patch for landing

Attachment 182210 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15811051
Comment 49 EFL EWS Bot 2013-01-10 16:47:29 PST
Comment on attachment 182210 [details]
patch for landing

Attachment 182210 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15803142
Comment 50 EFL EWS Bot 2013-01-10 17:03:00 PST
Comment on attachment 182210 [details]
patch for landing

Attachment 182210 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15805138
Comment 51 Filip Pizlo 2013-01-11 13:31:41 PST
Created attachment 182406 [details]
patch for landing
Comment 52 WebKit Review Bot 2013-01-11 13:34:15 PST
Attachment 182406 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSBase.cpp', u'S..." exit_code: 1
Source/JavaScriptCore/runtime/NumberConstructor.cpp:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 181 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 53 Filip Pizlo 2013-01-11 20:51:10 PST
Landed in http://trac.webkit.org/changeset/139541
Comment 54 Filip Pizlo 2013-01-11 20:56:06 PST
Fixed changelogs in http://trac.webkit.org/changeset/139542
Comment 55 Csaba Osztrogonác 2013-02-08 05:42:17 PST
(In reply to comment #53)
> Landed in http://trac.webkit.org/changeset/139541
After this one https://trac.webkit.org/changeset/141185/trunk/Source/JavaScriptCore/runtime/JSGlobalData.h fixed the !ENABLE_JIT build.
Comment 56 Csaba Osztrogonác 2013-02-11 10:18:43 PST
(In reply to comment #53)
> Landed in http://trac.webkit.org/changeset/139541

One more buildfix for !ENABLE(JIT) case - http://trac.webkit.org/changeset/142489