211701 – -Wtype-limits warning spam from CCallHelpers.h

Bug 211701 - -Wtype-limits warning spam from CCallHelpers.h

Summary: -Wtype-limits warning spam from CCallHelpers.h

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore (show other bugs)
Version:	WebKit Nightly Build
Hardware:	PC Linux

Importance:	P2 Normal
Assignee:	Michael Catanzaro

URL:
Keywords:	InRadar

Depends on:
Blocks:

Reported:	2020-05-10 13:09 PDT by Michael Catanzaro
Modified:	2020-05-17 19:22 PDT (History)
CC List:	10 users (show)

See Also:	212006

Attachments
Patch (1.96 KB, patch) 2020-05-12 08:52 PDT, Michael Catanzaro	no flags	Details \| Formatted Diff \| Diff
Patch (2.41 KB, patch) 2020-05-12 14:40 PDT, Michael Catanzaro	no flags	Details \| Formatted Diff \| Diff
Patch for landing (2.46 KB, patch) 2020-05-15 11:13 PDT, Michael Catanzaro	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Michael Catanzaro 2020-05-10 13:09:36 PDT

The stable release of GCC 10 has started warning about some preexisting code in CCallHelpers.h. This one is really long, but I'm going to copy the whole thing because the template instantiation is important here. It's four separate warnings; I've added a newline between each one for readability:

[821/2747] Building CXX object Source/JavaScriptCore/CMak...ScriptCore/unified-sources/UnifiedSource-f0a787a9-1.cpp.o
In file included from /home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/bytecode/AccessCase.cpp:31,
                 from DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-f0a787a9-1.cpp:1:
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘std::array<RegType, NumberOfRegisters> JSC::CCallHelpers::clampArrayToSize(std::array<RegType, decltype (JSC::toInfoFromReg((RegType)(-1)))::numberOfRegisters>) [with unsigned int TargetSize = 0; RegType = JSC::X86Registers::XMMRegisterID; decltype (JSC::toInfoFromReg((RegType)(-1))) = JSC::FPRInfo]’:
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:645:85:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 2; unsigned int numGPRSources = 1; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:381:46:   required from ‘void JSC::CCallHelpers::marshallArgumentRegister(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, RegType, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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; RegType = JSC::X86Registers::RegisterID; Args = {}]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:399:48:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, JSC::GPRReg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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 = {}; JSC::GPRReg = JSC::X86Registers::RegisterID]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:579:46:   required from ‘std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, Arg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr; Args = {JSC::X86Registers::RegisterID}; std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> = void]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:669:63:   required from ‘std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> JSC::CCallHelpers::setupArguments(Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); Args = {JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr, JSC::X86Registers::RegisterID}; std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> = void; typename WTF::FunctionTraits<T>::ArgumentType<0> = JSC::VM*]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/bytecode/AccessCase.cpp:1851:161:   required from here
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:316:32: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  316 |         for (unsigned i = 0; i < TargetSize; i++) {
      |                              ~~^~~~~~~~~~~~

/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘void JSC::CCallHelpers::setupStubArgs(std::array<RegType, NumberOfRegisters>, std::array<RegType, NumberOfRegisters>) [with unsigned int NumberOfRegisters = 0; RegType = JSC::X86Registers::XMMRegisterID]’:
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:645:45:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>) [with OperationType = char*(JSC::VM*, JSC::JSObject*); unsigned int numGPRArgs = 2; unsigned int numGPRSources = 1; unsigned int numFPRArgs = 0; unsigned int numFPRSources = 0; unsigned int numCrossSources = 0; unsigned int extraGPRArgs = 0; unsigned int extraPoke = 0]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:381:46:   required from ‘void JSC::CCallHelpers::marshallArgumentRegister(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, RegType, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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; RegType = JSC::X86Registers::RegisterID; Args = {}]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:399:48:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, JSC::GPRReg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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 = {}; JSC::GPRReg = JSC::X86Registers::RegisterID]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:579:46:   required from ‘std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, Arg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr; Args = {JSC::X86Registers::RegisterID}; std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> = void]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:669:63:   required from ‘std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> JSC::CCallHelpers::setupArguments(Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); Args = {JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr, JSC::X86Registers::RegisterID}; std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> = void; typename WTF::FunctionTraits<T>::ArgumentType<0> = JSC::VM*]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/bytecode/AccessCase.cpp:1851:161:   required from here
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:102:32: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  102 |         for (unsigned i = 0; i < NumberOfRegisters; ++i) {
      |                              ~~^~~~~~~~~~~~~~~~~~~

/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘std::array<RegType, NumberOfRegisters> JSC::CCallHelpers::clampArrayToSize(std::array<RegType, decltype (JSC::toInfoFromReg((RegType)(-1)))::numberOfRegisters>) [with unsigned int TargetSize = 0; RegType = JSC::X86Registers::RegisterID; decltype (JSC::toInfoFromReg((RegType)(-1))) = JSC::GPRInfo]’:
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:639:85:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:386:42:   required from ‘void JSC::CCallHelpers::marshallArgumentRegister(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, RegType, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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; RegType = JSC::X86Registers::RegisterID; Args = {}]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:399:48:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, JSC::GPRReg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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 = {}; JSC::GPRReg = JSC::X86Registers::RegisterID]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:579:46:   required from ‘std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, Arg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr; Args = {JSC::X86Registers::RegisterID}; std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> = void]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:669:63:   required from ‘std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> JSC::CCallHelpers::setupArguments(Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); Args = {JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr, JSC::X86Registers::RegisterID}; std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> = void; typename WTF::FunctionTraits<T>::ArgumentType<0> = JSC::VM*]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/bytecode/AccessCase.cpp:1851:161:   required from here
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:316:32: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  316 |         for (unsigned i = 0; i < TargetSize; i++) {
      |                              ~~^~~~~~~~~~~~

/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘void JSC::CCallHelpers::setupStubArgs(std::array<RegType, NumberOfRegisters>, std::array<RegType, NumberOfRegisters>) [with unsigned int NumberOfRegisters = 0; RegType = JSC::X86Registers::RegisterID]’:
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:639:45:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:386:42:   required from ‘void JSC::CCallHelpers::marshallArgumentRegister(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, RegType, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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; RegType = JSC::X86Registers::RegisterID; Args = {}]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:399:48:   required from ‘void JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, JSC::GPRReg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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 = {}; JSC::GPRReg = JSC::X86Registers::RegisterID]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:579:46:   required from ‘std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> JSC::CCallHelpers::setupArgumentsImpl(JSC::CCallHelpers::ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke>, Arg, Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); 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::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr; Args = {JSC::X86Registers::RegisterID}; std::enable_if_t<(std::is_base_of<JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm, Arg>::value || std::is_convertible<Arg, JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImm>::value)> = void]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:669:63:   required from ‘std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> JSC::CCallHelpers::setupArguments(Args ...) [with OperationType = char*(JSC::VM*, JSC::JSObject*); Args = {JSC::AbstractMacroAssembler<JSC::X86Assembler>::TrustedImmPtr, JSC::X86Registers::RegisterID}; std::enable_if_t<(! std::is_same<typename WTF::FunctionTraits<T>::ArgumentType<0>, JSC::CallFrame*>::value)> = void; typename WTF::FunctionTraits<T>::ArgumentType<0> = JSC::VM*]’
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/bytecode/AccessCase.cpp:1851:161:   required from here
/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h:102:32: warning: comparison of unsigned expression in ‘< 0’ is always false [-Wtype-limits]
  102 |         for (unsigned i = 0; i < NumberOfRegisters; ++i) {
      |                              ~~^~~~~~~~~~~~~~~~~~~

So looking at the code, one would think "well, the template should never be instantiated with TargetSize == or NumberOfRegisters == 0, and therefore this must be a false positive." But in fact, the templates really are being instantiated with these values, according to the warning:

/home/mcatanzaro/Projects/WebKit/Source/JavaScriptCore/jit/CCallHelpers.h: In instantiation of ‘void JSC::CCallHelpers::setupStubArgs(std::array<RegType, NumberOfRegisters>, std::array<RegType, NumberOfRegisters>) [with unsigned int NumberOfRegisters = 0; RegType = JSC::X86Registers::XMMRegisterID]’:

Seems that's happening here:

    template<typename OperationType, typename... Args>
    ALWAYS_INLINE std::enable_if_t<std::is_same<FIRST_ARGUMENT_TYPE, CallFrame*>::value> setupArguments(Args... args)
    {
#if USE(JSVALUE64)
        // This only really works for 64-bit since jsvalue regs mess things up for 32-bit...
        static_assert(FunctionTraits<OperationType>::cCallArity() == sizeof...(Args) + 1, "Basic sanity check");
#endif
        setupArgumentsImpl<OperationType, 0, 0, 0, 0, 0, 0, 0>(ArgCollection<0, 0, 0, 0, 0, 0, 0>(), GPRInfo::callFrameRegister, args...);
    }

    template<typename OperationType, typename... Args>
    ALWAYS_INLINE std::enable_if_t<!std::is_same<FIRST_ARGUMENT_TYPE, CallFrame*>::value> setupArguments(Args... args)
    {
#if USE(JSVALUE64)
        // This only really works for 64-bit since jsvalue regs mess things up for 32-bit...
        static_assert(FunctionTraits<OperationType>::cCallArity() == sizeof...(Args), "Basic sanity check");
#endif
        setupArgumentsImpl<OperationType, 0, 0, 0, 0, 0, 0, 0>(ArgCollection<0, 0, 0, 0, 0, 0, 0>(), args...);
    }

I tried this naive patch to block that case from being reached at runtime, but it's not enough to avoid the warnings:

diff --git a/Source/JavaScriptCore/jit/CCallHelpers.h b/Source/JavaScriptCore/jit/CCallHelpers.h
index 35d509bc3f54..2c3425edd9f8 100644
--- a/Source/JavaScriptCore/jit/CCallHelpers.h
+++ b/Source/JavaScriptCore/jit/CCallHelpers.h
@@ -99,9 +99,11 @@ private:
         typedef std::pair<RegType, RegType> RegPair;
         Vector<RegPair, NumberOfRegisters> pairs;
 
-        for (unsigned i = 0; i < NumberOfRegisters; ++i) {
-            if (sources[i] != destinations[i])
-                pairs.append(std::make_pair(sources[i], destinations[i]));
+        if (NumberOfRegisters > 0) {
+            for (unsigned i = 0; i < NumberOfRegisters; ++i) {
+                if (sources[i] != destinations[i])
+                    pairs.append(std::make_pair(sources[i], destinations[i]));
+            }
         }
 
 #if ASSERT_ENABLED
@@ -313,9 +315,11 @@ private:
 
         std::array<RegType, TargetSize> result { };
 
-        for (unsigned i = 0; i < TargetSize; i++) {
-            ASSERT(sourceArray[i] != static_cast<int32_t>(InfoTypeForReg<RegType>::InvalidIndex));
-            result[i] = sourceArray[i];
+        if (TargetSize > 0) {
+            for (unsigned i = 0; i < TargetSize; i++) {
+                ASSERT(sourceArray[i] != static_cast<int32_t>(InfoTypeForReg<RegType>::InvalidIndex));
+                result[i] = sourceArray[i];
+            }
         }
 
         return result;

I don't pretend to understand the code and I don't know what to do with them. I guess as long as we are instantiating these templates with TargetSize or NumberOfRegisters set to 0, we'll need to create separate template specializations to avoid the warning? I'd sooner just disable the warning with pragmas if it doesn't look like an actual bug.

Comment 1 Michael Catanzaro 2020-05-12 08:52:47 PDT

I'm going to add IGNORE_WARNINGS macros, that's all I can really do here.

Comment 2 Michael Catanzaro 2020-05-12 08:52:53 PDT

Created attachment 399130 [details]
Patch

Comment 3 Mark Lam 2020-05-12 09:21:56 PDT

Can you try adding if constexpr checks around the for statements like so:

        if constexpr (!!NumberOfRegisters) {
            for (unsigned i = 0; i < NumberOfRegisters; ++i) {
                if (sources[i] != destinations[i])
                    pairs.append(std::make_pair(sources[i], destinations[i]));
            }
        }

        if constexpr (!!TargetSize) {
            for (unsigned i = 0; i < TargetSize; i++) {
                ASSERT(sourceArray[i] != static_cast<int32_t>(InfoTypeForReg<RegType>::InvalidIndex));
                result[i] = sourceArray[i];
            }
        }

See if that helps.  If it does, add a comment about why the if constexpr was added.

Comment 4 Mark Lam 2020-05-12 09:33:04 PDT

Are any of the EWS using GCC10 yet?  If not, are there plans to migrate that way?   It's going to be difficult to keep these from continuing to manifest without the EWS.

Comment 5 Darin Adler 2020-05-12 10:31:37 PDT

Comment on attachment 399130 [details]
Patch

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

> Source/JavaScriptCore/jit/CCallHelpers.h:103
>          for (unsigned i = 0; i < NumberOfRegisters; ++i) {

Is this warning just about when NumberOfRegisters is 0?

Comment 6 Darin Adler 2020-05-12 10:32:08 PDT

(In reply to Mark Lam from comment #3)
> Can you try adding if constexpr checks

We should try this!

Comment 7 Michael Catanzaro 2020-05-12 14:20:04 PDT

Thanks for being responsive to my attempts to keep the GCC build nice and clean. :)

(In reply to Mark Lam from comment #4)
> Are any of the EWS using GCC10 yet?  If not, are there plans to migrate that
> way?   It's going to be difficult to keep these from continuing to manifest
> without the EWS.

I'll let Igalia answer that. afaik the current EWS just uses whatever GCC is provided by Debian stable. Having more EWS would certainly be nice.

Also note that EWS does not build with -Werror, so there's no way to notice warnings even on current compiler versions. I think changing that would catch probably 90% of GCC warnings before they're committed to trunk, since relatively few warnings are new warnings.

(In reply to Darin Adler from comment #5)
> > Source/JavaScriptCore/jit/CCallHelpers.h:103
> >          for (unsigned i = 0; i < NumberOfRegisters; ++i) {
> 
> Is this warning just about when NumberOfRegisters is 0?

Right.

(In reply to Mark Lam from comment #3)
> Can you try adding if constexpr checks around the for statements like so:

Oooh, didn't know about constexpr if. I think that worked, nice! Will let it continue building a while longer just to be certain.

Comment 8 Michael Catanzaro 2020-05-12 14:40:21 PDT

Created attachment 399179 [details]
Patch

Comment 9 Darin Adler 2020-05-12 14:42:26 PDT

Comment on attachment 399179 [details]
Patch

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

> Source/JavaScriptCore/jit/CCallHelpers.h:103
> +        if constexpr (!!NumberOfRegisters) {

I don’t think we need the !!

> Source/JavaScriptCore/jit/CCallHelpers.h:320
> +        if constexpr (!!TargetSize) {

Ditto.

Comment 10 Mark Lam 2020-05-12 15:00:03 PDT

Comment on attachment 399179 [details]
Patch

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

>> Source/JavaScriptCore/jit/CCallHelpers.h:103
>> +        if constexpr (!!NumberOfRegisters) {
> 
> I don’t think we need the !!

I already tried without the !!.  Clang complains about ints not matching bools.  That's why I added it.

Comment 11 Michael Catanzaro 2020-05-12 16:01:27 PDT

How about if constexpr (NumberOfRegisters > 0)? That's entirely equivalent and complies with WebKit code style.

The clearest thing to do would be to write '== 0', but our code style doesn't allow that.

Comment 12 Mark Lam 2020-05-15 09:25:34 PDT

(In reply to Michael Catanzaro from comment #11)
> How about if constexpr (NumberOfRegisters > 0)? That's entirely equivalent
> and complies with WebKit code style.
> 
> The clearest thing to do would be to write '== 0', but our code style
> doesn't allow that.

IMHO, (NumberOfRegisters > 0) is fine or you can just leave it as (!!NumberOfRegisters).

Comment 13 Michael Catanzaro 2020-05-15 11:13:06 PDT

Created attachment 399498 [details]
Patch for landing

Comment 14 Michael Catanzaro 2020-05-15 11:25:17 PDT

Apparently the > 0 construction doesn't avoid the build warning...? Huh....

Comment 15 Michael Catanzaro 2020-05-15 11:29:55 PDT

(In reply to Michael Catanzaro from comment #14)
> Apparently the > 0 construction doesn't avoid the build warning...? Huh....

Ah no, it probably does, I just reverted my local changes out of habit before starting a test build to check it. :P

Comment 16 EWS 2020-05-15 12:25:05 PDT

Committed r261754: <https://trac.webkit.org/changeset/261754>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399498 [details].

Comment 17 Radar WebKit Bug Importer 2020-05-15 12:26:17 PDT

<rdar://problem/63284459>