Summary: | Strength reduction, RegExp.exec -> RegExp.test | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gavin Barraclough <barraclough> | ||||
Component: | JavaScriptCore | Assignee: | Gavin Barraclough <barraclough> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | jberlin, ssandela, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Gavin Barraclough
2012-03-17 17:15:47 PDT
Created attachment 132473 [details]
Fix
Attachment 132473 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1
Source/JavaScriptCore/dfg/DFGOperations.h:64: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:65: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:67: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:68: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:70: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:72: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:73: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:78: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:79: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:80: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:81: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:82: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:83: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:84: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:87: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:90: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:91: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/dfg/DFGOperations.h:94: DFG_OPERATION is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Source/JavaScriptCore/runtime/JSFunction.h:57: The parameter name "nativeFunction" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/RegExpObject.h:70: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/RegExpObject.h:71: The parameter name "string" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 21 in 21 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Hi Gavin, The code changes are still resulting in windows build break(as of 111160). The error is same as mentioned by win build bot. Can you please fix it ASAP. I am stalled by this. 3>JSBase.obj : warning LNK4217: locally defined symbol ?staticData@WTFThreadData@WTF@@0PAV?$ThreadSpecific@VWTFThreadData@WTF@@@2@A (private: static class WTF::ThreadSpecific<class WTF::WTFThreadData> * WTF::WTFThreadData::staticData) imported in function "public: __thiscall JSC::APIEntryShim::~APIEntryShim(void)" (??1APIEntryShim@JSC@@QAE@XZ) 3>JavaScriptCore.exp : error LNK2001: unresolved external symbol "public: static class JSC::JSFunction * __cdecl JSC::JSFunction::create(class JSC::ExecState *,class JSC::JSGlobalObject *,int,class JSC::Identifier const &,__int64 (__fastcall*)(class JSC::ExecState *),__int64 (__fastcall*)(class JSC::ExecState *))" (?create@JSFunction@JSC@@SAPAV12@PAVExecState@2@PAVJSGlobalObject@2@HABVIdentifier@2@P6I_J0@Z3@Z) 3>C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\bin\JavaScriptCore.dll : fatal error LNK1120: 1 unresolved externals 3>Build log was saved at "file://C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\obj\JavaScriptCore\BuildLog.htm" 3>JavaScriptCore - 2 error(s), 16 warning(s) http://build.webkit.org/builders/Windows%20Debug%20%28Build%29/builds/47242/steps/compile-webkit/logs/stdio (In reply to comment #5) > 3>JSBase.obj : warning LNK4217: locally defined symbol ?staticData@WTFThreadData@WTF@@0PAV?$ThreadSpecific@VWTFThreadData@WTF@@@2@A (private: static class WTF::ThreadSpecific<class WTF::WTFThreadData> * WTF::WTFThreadData::staticData) imported in function "public: __thiscall JSC::APIEntryShim::~APIEntryShim(void)" (??1APIEntryShim@JSC@@QAE@XZ) > 3>JavaScriptCore.exp : error LNK2001: unresolved external symbol "public: static class JSC::JSFunction * __cdecl JSC::JSFunction::create(class JSC::ExecState *,class JSC::JSGlobalObject *,int,class JSC::Identifier const &,__int64 (__fastcall*)(class JSC::ExecState *),__int64 (__fastcall*)(class JSC::ExecState *))" (?create@JSFunction@JSC@@SAPAV12@PAVExecState@2@PAVJSGlobalObject@2@HABVIdentifier@2@P6I_J0@Z3@Z) > 3>C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\bin\JavaScriptCore.dll : fatal error LNK1120: 1 unresolved externals > 3>Build log was saved at "file://C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\obj\JavaScriptCore\BuildLog.htm" > 3>JavaScriptCore - 2 error(s), 16 warning(s) > > http://build.webkit.org/builders/Windows%20Debug%20%28Build%29/builds/47242/steps/compile-webkit/logs/stdio Fixed in http://trac.webkit.org/changeset/111188 Ooops!- thanks Jessie! Hi, The issue is still present when building WebKit project, it is fixed when building JavaScriptCore though. 5> Creating library C:\cygwin\home\ssandela\new_webkit\WebKit\WebKitBuild\Debug\lib\WebKit.lib and object C:\cygwin\home\ssandela\new_webkit\WebKit\WebKitBuild\Debug\lib\WebKit.exp 5>WebCore.lib(JSBindingsAllInOne.obj) : error LNK2001: unresolved external symbol "public: static class JSC::JSFunction * __cdecl JSC::JSFunction::create(class JSC::ExecState *,class JSC::JSGlobalObject *,int,class JSC::Identifier const &,__int64 (__fastcall*)(class JSC::ExecState *),enum JSC::Intrinsic,__int64 (__fastcall*)(class JSC::ExecState *))" (?create@JSFunction@JSC@@SAPAV12@PAVExecState@2@PAVJSGlobalObject@2@HABVIdentifier@2@P6I_J0@ZW4Intrinsic@2@3@Z) 5>C:\cygwin\home\ssandela\new_webkit\WebKit\WebKitBuild\Debug\bin\WebKit.dll : fatal error LNK1120: 1 unresolved externals (In reply to comment #8) > Hi, The issue is still present when building WebKit project, it is fixed when building JavaScriptCore though. > > 5> Creating library C:\cygwin\home\ssandela\new_webkit\WebKit\WebKitBuild\Debug\lib\WebKit.lib and object C:\cygwin\home\ssandela\new_webkit\WebKit\WebKitBuild\Debug\lib\WebKit.exp > 5>WebCore.lib(JSBindingsAllInOne.obj) : error LNK2001: unresolved external symbol "public: static class JSC::JSFunction * __cdecl JSC::JSFunction::create(class JSC::ExecState *,class JSC::JSGlobalObject *,int,class JSC::Identifier const &,__int64 (__fastcall*)(class JSC::ExecState *),enum JSC::Intrinsic,__int64 (__fastcall*)(class JSC::ExecState *))" (?create@JSFunction@JSC@@SAPAV12@PAVExecState@2@PAVJSGlobalObject@2@HABVIdentifier@2@P6I_J0@ZW4Intrinsic@2@3@Z) > 5>C:\cygwin\home\ssandela\new_webkit\WebKit\WebKitBuild\Debug\bin\WebKit.dll : fatal error LNK1120: 1 unresolved externals This was fixed in http://trac.webkit.org/changeset/111197 Comment on attachment 132473 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=132473&action=review > Source/JavaScriptCore/dfg/DFGAbstractState.cpp:638 > + case RegExpExec: > + case RegExpTest: > + forNode(node.child1()).filter(PredictCell); > + forNode(node.child2()).filter(PredictCell); > + forNode(nodeIndex).makeTop(); > + break; > + This AI rule is wrong because it fails to call clobberStructures(), or clobberWorld() in the modern DFG. This bug has managed to survive until now! The implication is that RegExpExec/RegExpTest could clobber the world if the second argument is an object with a toString method, but AI would incorrectly assume that all of its structure proofs are still intact. Because of this logic, when we wrote clobberize() we also failed to make RegExpExec/RegExpTest clobber the world there. That means we will CSE and LICM around these clobbering operations, too. |