Bug 210176

Summary: [JSC][32-bits] Build failure after r259676 (Not using strict mode within ClassDeclaration statement)
Product: WebKit Reporter: Pablo Saavedra <psaavedra>
Component: JavaScriptCoreAssignee: Guillaume Emont <guijemont>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ews-watchlist, guijemont, keith_miller, mark.lam, msaboff, pmatos, saam, ticaiolima, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Pablo Saavedra 2020-04-08 00:42:22 PDT 
Buildjob: https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Thumb2%20Release/builds/11260

```
Category	None
Changed by	tzagallo@apple.com
Changed at	Tue 07 Apr 2020 15:32:22
Repository	/var/svn/webkit
Branch	trunk
Revision	259676
Comments
Not using strict mode within ClassDeclaration statement
https://bugs.webkit.org/show_bug.cgi?id=205578
<rdar://problem/58194589>

Reviewed by Yusuke Suzuki.
```

Errors:

```
6303-FAILED: Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/dfg/DFGSpeculativeJIT32_64.cpp.o 
6304-/usr/bin/ccache /home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/x86_64-pokysdk-linux/usr/bin/arm-poky-linux-gnueabi/arm-poky-linux-gnueabi-g++   -march=armv7-a -mthumb -mfpu=neon -mfloat-abi=hard -fstack-protector-strong  -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security --sysroot=/home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/armv7at2hf-neon-poky-linux-gnueabi --sysroot=/home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/armv7at2hf-neon-poky-linux-gnueabi  -DBUILDING_JavaScriptCore -DBUILDING_WITH_CMAKE=1 -DBUILDING_WPE__=1 -DGETTEXT_PACKAGE=\"WPE\" -DHAVE_CONFIG_H=1 -DJSC_COMPILATION -DJSC_GLIB_API_ENABLED -DPKGLIBDIR=\"/usr/local/lib/wpe-webkit-1.0\" -DSVN_REVISION=\"r259680\" -DU_SHOW_CPLUSPLUS_API=0 -IDerivedSources/ForwardingHeaders -I. -I../../Source/JavaScriptCore -I../../Source/JavaScriptCore/API -I../../Source/JavaScriptCore/assembler -I../../Source/JavaScriptCore/b3 -I../../Source/JavaScriptCore/b3/air -I../../Source/JavaScriptCore/bindings -I../../Source/JavaScriptCore/builtins -I../../Source/JavaScriptCore/bytecode -I../../Source/JavaScriptCore/bytecompiler -I../../Source/JavaScriptCore/dfg -I../../Source/JavaScriptCore/disassembler -I../../Source/JavaScriptCore/disassembler/ARM64 -I../../Source/JavaScriptCore/disassembler/udis86 -I../../Source/JavaScriptCore/domjit -I../../Source/JavaScriptCore/ftl -I../../Source/JavaScriptCore/heap -I../../Source/JavaScriptCore/debugger -I../../Source/JavaScriptCore/inspector -I../../Source/JavaScriptCore/inspector/agents -I../../Source/JavaScriptCore/inspector/augmentable -I../../Source/JavaScriptCore/inspector/remote -I../../Source/JavaScriptCore/interpreter -I../../Source/JavaScriptCore/jit -I../../Source/JavaScriptCore/llint -I../../Source/JavaScriptCore/parser -I../../Source/JavaScriptCore/profiler -I../../Source/JavaScriptCore/runtime -I../../Source/JavaScriptCore/tools -I../../Source/JavaScriptCore/wasm -I../../Source/JavaScriptCore/wasm/js -I../../Source/JavaScriptCore/yarr -IDerivedSources/JavaScriptCore -IDerivedSources/JavaScriptCore/inspector -IDerivedSources/JavaScriptCore/runtime -IDerivedSources/JavaScriptCore/yarr -I../../Source/ThirdParty/capstone/Source/include -IDerivedSources/ForwardingHeaders/JavaScriptCore/glib -IDerivedSources/JavaScriptCore/javascriptcorewpe/jsc -I../../Source/JavaScriptCore/API/glib -IDerivedSources -I../../Source/ThirdParty -isystem /home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/armv7at2hf-neon-poky-linux-gnueabi/usr/include/glib-2.0 -isystem /home/bot/toolchain_env_wandboard-mesa-wpe-nightly/sysroots/armv7at2hf-neon-poky-linux-gnueabi/usr/lib/glib-2.0/include -fdiagnostics-color=always -Wextra -Wall -Wno-expansion-to-defined -Wno-noexcept-type -Wno-psabi -Wno-maybe-uninitialized -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align  -O2 -pipe -g -feliminate-unused-debug-types  -fno-strict-aliasing -fno-exceptions -fno-rtti -O3 -DNDEBUG -fPIC   -ffp-contract=off -std=c++17 -MD -MT Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/dfg/DFGSpeculativeJIT32_64.cpp.o -MF Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/dfg/DFGSpeculativeJIT32_64.cpp.o.d -o Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/dfg/DFGSpeculativeJIT32_64.cpp.o -c ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
6305-../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp: In member function ‘void JSC::DFG::SpeculativeJIT::compileContiguousPutByVal(JSC::DFG::Node*, BaseOperandType&, PropertyOperandType&, ValueOperandType&, JSC::GPRReg, TagType)’:
6306:../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:1787:23: error: ‘class JSC::DFG::JITCompiler’ has no member named ‘isStrictModeFor’
6307- 1787 |                 m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValDirectBeyondArrayBoundsStrict : operationPutByValDirectBeyondArrayBoundsNonStrict,
6308-      |                       ^~~~~~~~~~~~~~~
6309:../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:1792:23: error: ‘class JSC::DFG::JITCompiler’ has no member named ‘isStrictModeFor’
6310- 1792 |                 m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValBeyondArrayBoundsStrict : operationPutByValBeyondArrayBoundsNonStrict,
6311-      |                       ^~~~~~~~~~~~~~~
6312-../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp: In member function ‘void JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node*)’:
6313:../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2632:37: error: ‘class JSC::DFG::JITCompiler’ has no member named ‘isStrictModeFor’
6314- 2632 |                 callOperation(m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValDirectCellStrict : operationPutByValDirectCellNonStrict, TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic)), baseGPR, propertyRegs, valueRegs);
6315-      |                                     ^~~~~~~~~~~~~~~
6316:../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2634:37: error: ‘class JSC::DFG::JITCompiler’ has no member named ‘isStrictModeFor’
6317- 2634 |                 callOperation(m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValCellStrict : operationPutByValCellNonStrict, TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic)), baseGPR, propertyRegs, valueRegs);
6318-      |                                     ^~~~~~~~~~~~~~~
6319:../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2745:31: error: ‘class JSC::DFG::JITCompiler’ has no member named ‘isStrictModeFor’
6320- 2745 |                         m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValDirectBeyondArrayBoundsStrict : operationPutByValDirectBeyondArrayBoundsNonStrict,
6321-      |                               ^~~~~~~~~~~~~~~
6322:../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2750:31: error: ‘class JSC::DFG::JITCompiler’ has no member named ‘isStrictModeFor’
6323- 2750 |                         m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValBeyondArrayBoundsStrict : operationPutByValBeyondArrayBoundsNonStrict,
6324-      |                               ^~~~~~~~~~~~~~~
6325:../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:2785:29: error: ‘class JSC::DFG::JITCompiler’ has no member named ‘isStrictModeFor’
6326- 2785 |         callOperation(m_jit.isStrictModeFor(node->origin.semantic) ? operationPutByValWithThisStrict : operationPutByValWithThis,
6327-      |         
```

also:

```
6393:../../Source/JavaScriptCore/jit/CCallHelpers.h:635:60: error: static assertion failed: One last sanity check
6394-  635 |         static_assert(FunctionTraits<OperationType>::arity == numGPRArgs + numFPRArgs, "One last sanity check");
6395-      |                                                      ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
6396-../../Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>) [with OperationType = long long int(JSC::JSGlobalObject*, JSC::CallFrame*, JSC::ECMAMode); unsigned int numGPRArgs = 2; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0]’:
6397-../../Source/JavaScriptCore/jit/CCallHelpers.h:491:9:   required from ‘std::enable_if_t<(sizeof (typename WTF::FunctionTraits<T>::ArgumentType<(numGPRArgs + numFPRArgs)>) <= 4)> JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, JSC::GPRReg, Args ...) [with OperationType = long long int(JSC::JSGlobalObject*, JSC::CallFrame*, JSC::ECMAMode); unsigned int numGPRArgs = 1; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; Args = {}; std::enable_if_t<(sizeof (typename WTF::FunctionTraits<T>::ArgumentType<(numGPRArgs + numFPRArgs)>) <= 4)> = void; JSC::GPRReg = JSC::ARMRegisters::RegisterID]’
6398-../../Source/JavaScriptCore/jit/CCallHelpers.h:579:13:   required from ‘std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::ARMv7Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::ARMv7Assembler>::TrustedImm>::value)> JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, Arg, Args ...) [with OperationType = long long int(JSC::JSGlobalObject*, JSC::CallFrame*, JSC::ECMAMode); unsigned int numGPRArgs = 0; unsigned int numGPRSources = 0; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0; Arg = JSC::DFG::SpeculativeJIT::TrustedImmPtr; Args = {JSC::ARMRegisters::RegisterID}; std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::ARMv7Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::ARMv7Assembler>::TrustedImm>::value)> = void]’
6399-../../Source/JavaScriptCore/jit/CCallHelpers.h:669:9:   required from ‘std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> JSC::CCallHelpers::setupArguments(Args ...) [with OperationType = long long int(JSC::JSGlobalObject*, JSC::CallFrame*, JSC::ECMAMode); Args = {JSC::DFG::SpeculativeJIT::TrustedImmPtr, JSC::ARMRegisters::RegisterID}; std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> = void]’
6400-../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:811:124:   required from here
6401:../../Source/JavaScriptCore/jit/CCallHelpers.h:635:60: error: static assertion failed: One last sanity check
6402-In file included from ../../Source/JavaScriptCore/heap/BlockDirectoryBits.h:30,
6403-                 from ../../Source/JavaScriptCore/heap/BlockDirectory.h:29,
6404-                 from ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:30,
6405-                 from ../../Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:28:
6406-DerivedSources/ForwardingHeaders/wtf/Vector.h: In instantiation of ‘static void WTF::VectorMover<false, T>::move(T*, T*, T*) [with T = JSC::ARMv7Assembler::LinkRecord]’:
6407-DerivedSources/ForwardingHeaders/wtf/Vector.h:257:65:   required from ‘static void WTF::VectorTypeOperations<T>::move(T*, T*, T*) [with T = JSC::ARMv7Assembler::LinkRecord]’
6408-DerivedSources/ForwardingHeaders/wtf/Vector.h:1194:25:   required from ‘void WTF::Vector< <template-parameter-1-1>, <anonymous>, <template-parameter-1-3>, <anonymous>, Malloc>::reserveCapacity(size_t) [with T = JSC::ARMv7Assembler::LinkRecord; unsigned int inlineCapacity = 0; OverflowHandler = WTF::UnsafeVectorOverflow; unsigned int minCapacity = 16; Malloc = WTF::FastMalloc; size_t = unsigned int]’
6409-DerivedSources/ForwardingHeaders/wtf/Vector.h:1047:5:   required from ‘void WTF::Vector< <template-parameter-1-1>, <anonymous>, <template-parameter-1-3>, <anonymous>, Malloc>::expandCapacity(size_t) [with T = JSC::ARMv7Assembler::LinkRecord; unsigned int inlineCapacity = 0; OverflowHandler = WTF::UnsafeVectorOverflow; unsigned int minCapacity = 16; Malloc = WTF::FastMalloc; size_t = unsigned int]’
6410-DerivedSources/ForwardingHeaders/wtf/Vector.h:1054:9:   required from ‘T* WTF::Vector< <template-parameter-1-1>, <anonymous>, <template-parameter-1-3>, <anonymous>, Malloc>::expandCapacity(size_t, T*) [with T = JSC::ARMv7Assembler::LinkRecord; unsigned int inlineCapacity = 0; OverflowHandler = WTF::UnsafeVectorOverflow; unsigned int minCapacity = 16; Malloc = WTF::FastMalloc; size_t = unsigned int]’
6411-DerivedSources/ForwardingHeaders/wtf/Vector.h:1347:9:   required from ‘void WTF::Vector< <template-parameter-1-1>, <anonymous>, <template-parameter-1-3>, <anonymous>, Malloc>::appendSlowCase(U&&) [with U = JSC::ARMv7Assembler::LinkRecord; T = JSC::ARMv7Assembler::LinkRecord; unsigned int inlineCapacity = 0; OverflowHandler = WTF::UnsafeVectorOverflow; unsigned int minCapacity = 16; Malloc = WTF::FastMalloc]’
6412-DerivedSources/ForwardingHeaders/wtf/Vector.h:1309:5:   required from ‘void WTF::Vector< <template-parameter-1-1>, <anonymous>, <template-parameter-1-3>, <anonymous>, Malloc>::append(U&&) [with U = JSC::ARMv7Assembler::LinkRecord; T = 
JSC::ARMv7Assembler::LinkRecord; unsigned int inlineCapacity = 0; OverflowHandler = WTF::UnsafeVectorOverflow; unsigned int minCapacity = 16; Malloc = WTF::FastMalloc]’
6413-DerivedSources/ForwardingHeaders/wtf/Vector.h:773:52:   required from ‘void WTF::Vector< <template-parameter-1-1>, <anonymous>, <template-parameter-1-3>, <anonymous>, Malloc>::append(WTF::Vector< <template-parameter-1-1>, <anonymous>, <template-parameter-1-3>, <anonymous>, Malloc>::ValueType&&) [with T = JSC::ARMv7Assembler::LinkRecord; unsigned int inlineCapacity = 0; OverflowHandler = WTF::UnsafeVectorOverflow; unsigned int minCapacity = 16; Malloc = WTF::FastMalloc; WTF::Vector< <template-parameter-1-1>, <anonymous>, <template-parameter-1-3>, <anonymous>, Malloc>::ValueType = JSC::ARMv7Assembler::LinkRecord]’

```
Comment 1 Guillaume Emont 2020-04-08 04:13:25 PDT 
Created attachment 395789 [details]
Patch

First attempt at the patch. This fixes building on armv7, relying on EWS to run tests
Comment 2 Aakash Jain 2020-04-08 04:21:56 PDT 
*** Bug 210181 has been marked as a duplicate of this bug. ***
Comment 3 Caio Lima 2020-04-08 06:42:48 PDT 
Comment on attachment 395789 [details]
Patch

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

Informal review there. LGTM.

> Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:404
> +    ECMAMode ecmaMode = ECMAMode::strict();

Is there a reason to initialize it? IIUC, load will set a value below based on `bytecode.m_ecmaMode`;
Comment 4 Aakash Jain 2020-04-08 07:08:25 PDT 
Comment on attachment 395789 [details]
Patch

setting r+ and cq+ since it passes EWS and fix the build breakage. Further improvements (if any) can be done in follow-up patch.
Comment 5 EWS 2020-04-08 07:32:19 PDT 
Committed r259715: <https://trac.webkit.org/changeset/259715>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395789 [details].
Comment 6 Radar WebKit Bug Importer 2020-04-08 07:33:13 PDT 
<rdar://problem/61453881>
Comment 7 Guillaume Emont 2020-04-08 07:38:00 PDT 
(In reply to Caio Lima from comment #3)
> Comment on attachment 395789 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=395789&action=review
> 
> Informal review there. LGTM.
> 
> > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:404
> > +    ECMAMode ecmaMode = ECMAMode::strict();
> 
> Is there a reason to initialize it? IIUC, load will set a value below based
> on `bytecode.m_ecmaMode`;

That's a good question I'd like to forward to Tadeu, since I basically cut and paste his code from JITPropertyAccess.cpp:

https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/jit/JITPropertyAccess.cpp#L308

but yeah, it doesn't look like it's needed.
Comment 8 Tadeu Zagallo 2020-04-08 09:52:02 PDT 
(In reply to Guillaume Emont from comment #7)
> (In reply to Caio Lima from comment #3)
> > Comment on attachment 395789 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=395789&action=review
> > 
> > Informal review there. LGTM.
> > 
> > > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:404
> > > +    ECMAMode ecmaMode = ECMAMode::strict();
> > 
> > Is there a reason to initialize it? IIUC, load will set a value below based
> > on `bytecode.m_ecmaMode`;
> 
> That's a good question I'd like to forward to Tadeu, since I basically cut
> and paste his code from JITPropertyAccess.cpp:
> 
> https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/jit/
> JITPropertyAccess.cpp#L308
> 
> but yeah, it doesn't look like it's needed.

The reason is that ECMAMode doesn't have a default initializer. I found it easier to not add one while refactoring so I'd look at all usages of it.