Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable implicit fallthrough warning #43397

Merged
merged 14 commits into from
Oct 21, 2020
1 change: 1 addition & 0 deletions eng/native/configurecompiler.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ if (CLR_CMAKE_HOST_UNIX)
add_compile_options(-Wno-unused-value)
add_compile_options(-Wno-unused-function)
add_compile_options(-Wno-tautological-compare)
add_compile_options(-Wimplicit-fallthrough)

#These seem to indicate real issues
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-Wno-invalid-offsetof>)
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/ToolBox/superpmi/mcs/verbildump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,7 @@ void DumpILToConsoleBare(unsigned char* ilCode, int len)
LogError("unknown ilCode 0xfe%2x at offset %d in MethodGen::PrettyPrint", ilCode[i], i);
break;
}
break;
default:
LogError("unknown ilCode 0x%02x at offset %d in MethodGen::PrettyPrint", ilCode[i], i);
break;
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/src/classlibnative/bcltype/varargsnative.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,8 @@ VarArgsNative::GetNextArgHelper(
}
#endif

// fall through
__fallthrough;

case ELEMENT_TYPE_CLASS: {
value->type = data->SigPtr.GetTypeHandleThrowing(data->ArgCookie->pModule, &typeContext);

Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/src/debug/di/rstype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ HRESULT CordbType::MkType(CordbAppDomain * pAppDomain,

pClass->SetIsValueClass(true);
pClass->SetIsValueClassKnown(true);
// drop through
__fallthrough;

case ELEMENT_TYPE_CLASS:
{
Expand Down Expand Up @@ -2009,6 +2009,7 @@ void CordbType::TypeToExpandedTypeData(DebuggerIPCE_ExpandedTypeData *data)
}
case ELEMENT_TYPE_END:
_ASSERTE(!"bad element type!");
break;

default:
data->elementType = m_elementType;
Expand Down Expand Up @@ -2389,6 +2390,7 @@ HRESULT CordbType::GetTypeID(COR_TYPEID *pId)
case ELEMENT_TYPE_BYREF:
case ELEMENT_TYPE_FNPTR:
IfFailThrow(CORDBG_E_UNSUPPORTED);
break;
default:
_ASSERTE(!"unexpected element type!");
IfFailThrow(CORDBG_E_UNSUPPORTED);
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/src/debug/ee/amd64/amd64walker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,7 @@ static int opSize(Amd64InstrDecode::InstrForm form, int pp, bool W, bool L, bool
// P_M6B_or_M4B
case Amd64InstrDecode::InstrForm::MOnly_P_M6B_or_M4B:
opSize = P ? 6 : 4;
break;
// M4B
case Amd64InstrDecode::InstrForm::M1st_M4B:
case Amd64InstrDecode::InstrForm::M1st_M4B_I1B:
Expand Down Expand Up @@ -833,7 +834,7 @@ void NativeWalker::DecodeInstructionForPatchSkip(const BYTE *address, Instructio
opCodeMap = Primary;
break;
}
// Fall through
__fallthrough;
case 0xc4: // Vex 3-byte
opCodeMap = (OpcodeMap)(int(address[0]) << 8 | (address[1] & 0x1f));
// W is the top bit of opcode2.
Expand Down Expand Up @@ -965,7 +966,7 @@ void NativeWalker::DecodeInstructionForPatchSkip(const BYTE *address, Instructio
case 2:
case 3:
pInstrAttrib->m_fIsCall = true;
// fall through
__fallthrough;
case 4:
case 5:
pInstrAttrib->m_fIsAbsBranch = true;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/debug/ee/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5884,6 +5884,7 @@ bool DebuggerStepper::TrapStep(ControllerStackInfo *info, bool in)
fIsJump = true;

// fall through...
__fallthrough;

case WALK_CALL:
LOG((LF_CORDB,LL_INFO10000, "DC::TS:Imm:WALK_CALL ip=%p nextip=%p skipip=%p\n", walker.GetIP(), walker.GetNextIP(), walker.GetSkipIP()));
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/src/debug/ee/debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10722,6 +10722,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
//
// For regular (non-jit) attach, fall through to do an async break.
//
__fallthrough;

case DB_IPCE_ASYNC_BREAK:
{
Expand Down Expand Up @@ -12138,7 +12139,7 @@ void Debugger::TypeHandleToExpandedTypeInfo(AreValueTypesBoxed boxed,
case ELEMENT_TYPE_VALUETYPE:
if (boxed == OnlyPrimitivesUnboxed || boxed == AllBoxed)
res->elementType = ELEMENT_TYPE_CLASS;
// drop through
__fallthrough;

case ELEMENT_TYPE_CLASS:
{
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/gc/unix/gcenv.unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -792,8 +792,10 @@ bool ReadMemoryValueFromFile(const char* filename, uint64_t* val)
{
case 'g':
case 'G': multiplier = 1024;
[[fallthrough]];
case 'm':
case 'M': multiplier = multiplier * 1024;
[[fallthrough]];
case 'k':
case 'K': multiplier = multiplier * 1024;
}
Expand Down
10 changes: 10 additions & 0 deletions src/coreclr/src/ilasm/assem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1611,16 +1611,26 @@ unsigned hash(
switch(len) /* all the case statements fall through */
{
case 11: c+=((unsigned)k[10] << 24);
__fallthrough;
case 10: c+=((unsigned)k[9] << 16);
__fallthrough;
case 9 : c+=((unsigned)k[8] << 8);
__fallthrough;
/* the first byte of c is reserved for the length */
case 8 : b+=((unsigned)k[7] << 24);
__fallthrough;
case 7 : b+=((unsigned)k[6] << 16);
__fallthrough;
case 6 : b+=((unsigned)k[5] << 8);
__fallthrough;
case 5 : b+=k[4];
__fallthrough;
case 4 : a+=((unsigned)k[3] << 24);
__fallthrough;
case 3 : a+=((unsigned)k[2] << 16);
__fallthrough;
case 2 : a+=((unsigned)k[1] << 8);
__fallthrough;
case 1 : a+=k[0];
/* case 0: nothing left to add */
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/ilasm/assembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1578,6 +1578,7 @@ void Assembler::EmitInstrVarByName(Instr* instr, __in __nullterminated char* lab
case CEE_STARG:
case CEE_STARG_S:
nArgVarFlag++;
__fallthrough;
case CEE_LDLOCA:
case CEE_LDLOCA_S:
case CEE_LDLOC:
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/ilasm/prebuilt/asmparse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2019,6 +2019,7 @@ YYLOCAL YYNEAR YYPASCAL YYPARSER()

yyerrlab:
++yynerrs;
__fallthrough;

case 1:
case 2: /* incompletely recovered error ... try again */
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/ildasm/dasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2008,6 +2008,7 @@ BYTE* PrettyPrintCABlobValue(PCCOR_SIGNATURE &typePtr,
break;
case ELEMENT_TYPE_CLASS :
typePtr += CorSigUncompressToken(typePtr, &tk); //skip the following token
__fallthrough;
case SERIALIZATION_TYPE_TYPE :
appendStr(out,KEYWORD("type"));
appendStr(out,appendix);
Expand Down Expand Up @@ -2668,6 +2669,7 @@ void DumpDefaultValue(mdToken tok, __inout __nullterminated char* szString, void
break;
}
//else fall thru to default case, to report the error
__fallthrough;

default:
szptr+=sprintf_s(szptr,SZSTRING_REMAINING_SIZE(szptr),ERRORMSG(" /* ILLEGAL CONSTANT type:0x%02X, size:%d bytes, blob: "),MDDV.m_bType,MDDV.m_cbSize);
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/ildasm/dasm_sz.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ unsigned SizeOfField(PCCOR_SIGNATURE *ppSig, ULONG cSig, IMDInternalImport* pImp
case ELEMENT_TYPE_CMOD_OPT :
case ELEMENT_TYPE_CMOD_REQD :
*ppSig += CorSigUncompressToken(*ppSig, &tk);
__fallthrough; // TODO: is this correct?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidwrighton I am not sure if this fallthrough here is intentional, can you please advise me on this one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this looks intentional. I don't think its well written, but intentional, and not functionally wrong... yes.

case ELEMENT_TYPE_PINNED :
case ELEMENT_TYPE_SZARRAY : // uElementNumber doesn't change
if(*ppSig < pSigEnd) Reiterate = TRUE;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,7 @@ void CodeGen::genCodeForBBlist()

case BBJ_EHCATCHRET:
noway_assert(!"Unexpected BBJ_EHCATCHRET"); // not used on x86
break;

case BBJ_EHFINALLYRET:
case BBJ_EHFILTERRET:
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/src/jit/emitarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2133,7 +2133,6 @@ void emitter::emitIns_R_R(
assert(isFloatReg(reg1));
assert(isFloatReg(reg2));
}
__fallthrough;

VCVT_COMMON:
fmt = IF_T2_VFP2;
Expand Down Expand Up @@ -2847,7 +2846,6 @@ void emitter::emitIns_R_R_I(instruction ins,
//
// If we did not find a thumb-1 encoding above
//
__fallthrough;

COMMON_THUMB2_LDST:
assert(fmt == IF_NONE);
Expand Down Expand Up @@ -3070,9 +3068,9 @@ void emitter::emitIns_R_R_R(instruction ins,
assert(!"Instruction cannot be encoded");
}
}
__fallthrough;

#if !defined(USE_HELPERS_FOR_INT_DIV)
__fallthrough;
case INS_sdiv:
case INS_udiv:
#endif // !USE_HELPERS_FOR_INT_DIV
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/jit/emitarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13920,6 +13920,7 @@ emitter::insExecutionCharacteristics emitter::getInsExecutionCharacteristics(ins
perfScoreUnhandledInstruction(id, &result);
break;
}
break;

// ALU, basic immediate
case IF_DI_1A: // cmp, cmn
Expand Down
9 changes: 7 additions & 2 deletions src/coreclr/src/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4968,6 +4968,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
break;
case CEE_RET:
retBlocks++;
break;

default:
break;
Expand Down Expand Up @@ -5299,7 +5300,8 @@ void Compiler::fgLinkBasicBlocks()
BADCODE("Fall thru the end of a method");
}

// Fall through, the next block is also reachable
// Fall through, the next block is also reachable
__fallthrough;

case BBJ_NONE:
curBBdesc->bbNext->bbRefs++;
Expand Down Expand Up @@ -5661,7 +5663,7 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F
the target of a jump.
*/

// fall-through
__fallthrough;

case CEE_JMP:
/* These are equivalent to a return from the current method
Expand Down Expand Up @@ -16628,6 +16630,7 @@ void Compiler::fgDetermineFirstColdBlock()
{
default:
noway_assert(!"Unhandled jumpkind in fgDetermineFirstColdBlock()");
break;

case BBJ_CALLFINALLY:
// A BBJ_CALLFINALLY that falls through is always followed
Expand Down Expand Up @@ -21507,6 +21510,7 @@ void Compiler::fgDebugCheckFlags(GenTree* tree)
break;
case GT_ADDR:
assert(!op1->CanCSE());
break;

case GT_IND:
// Do we have a constant integer address as op1?
Expand Down Expand Up @@ -21578,6 +21582,7 @@ void Compiler::fgDebugCheckFlags(GenTree* tree)
// +--------------+----------------+
}
}
break;

default:
break;
Expand Down
9 changes: 6 additions & 3 deletions src/coreclr/src/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4183,7 +4183,8 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
break;
}

// fall through and set GTF_REVERSE_OPS
// fall through and set GTF_REVERSE_OPS
__fallthrough;

case GT_LCL_VAR:
case GT_LCL_FLD:
Expand Down Expand Up @@ -5689,9 +5690,9 @@ bool GenTree::OperMayThrow(Compiler* comp)
//
return true;
}
break;
}
#endif // FEATURE_HW_INTRINSICS

default:
break;
}
Expand Down Expand Up @@ -11320,6 +11321,7 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack)
case GT_JCMP:
printf(" cond=%s%s", (tree->gtFlags & GTF_JCMP_TST) ? "TEST_" : "",
(tree->gtFlags & GTF_JCMP_EQ) ? "EQ" : "NE");
break;

default:
assert(!"don't know how to display tree leaf node");
Expand Down Expand Up @@ -14581,6 +14583,7 @@ GenTree* Compiler::gtFoldExprConst(GenTree* tree)
#endif
goto DONE;
}
break;

default:
break;
Expand Down Expand Up @@ -17456,8 +17459,8 @@ GenTree* Compiler::gtGetSIMDZero(var_types simdType, var_types baseType, CORINFO
break;
case TYP_UINT:
assert(simdHandle == m_simdHandleCache->Vector64UIntHandle);
break;
#endif // defined(TARGET_ARM64) && defined(FEATURE_HW_INTRINSICS)
break;
default:
break;
}
Expand Down
5 changes: 4 additions & 1 deletion src/coreclr/src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5630,6 +5630,7 @@ void Compiler::verVerifyCall(OPCODE opcode,
}
}
// fall thru to default checks
__fallthrough;
default:
VerifyOrReturn(!(mflags & CORINFO_FLG_ABSTRACT), "method abstract");
}
Expand Down Expand Up @@ -13139,6 +13140,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
case CEE_CONV_OVF_U_UN:
case CEE_CONV_U:
isNative = true;
break;
default:
// leave 'isNative' = false;
break;
Expand Down Expand Up @@ -14108,7 +14110,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
}
}

// fall through
__fallthrough;

case CEE_CALLVIRT:
case CEE_CALL:
Expand Down Expand Up @@ -16346,6 +16348,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)

case 0xCC:
OutputDebugStringA("CLR: Invalid x86 breakpoint in IL stream\n");
__fallthrough;

case CEE_ILLEGAL:
case CEE_MACRO_END:
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/jit/inlinepolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1536,6 +1536,7 @@ void DiscretionaryPolicy::ComputeOpcodeBin(OPCODE opcode)

case CEE_RET:
m_ReturnCount++;
break;

default:
break;
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/src/jit/instr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2231,8 +2231,10 @@ instruction CodeGen::ins_FloatConv(var_types to, var_types from)
{
case TYP_FLOAT:
NYI("long to float");
break;
case TYP_DOUBLE:
NYI("long to double");
break;
default:
unreached();
}
Expand All @@ -2246,6 +2248,7 @@ instruction CodeGen::ins_FloatConv(var_types to, var_types from)
return INS_vcvt_f2u;
case TYP_LONG:
NYI("float to long");
break;
case TYP_DOUBLE:
return INS_vcvt_f2d;
case TYP_FLOAT:
Expand All @@ -2263,6 +2266,7 @@ instruction CodeGen::ins_FloatConv(var_types to, var_types from)
return INS_vcvt_d2u;
case TYP_LONG:
NYI("double to long");
break;
case TYP_FLOAT:
return INS_vcvt_d2f;
case TYP_DOUBLE:
Expand All @@ -2274,6 +2278,7 @@ instruction CodeGen::ins_FloatConv(var_types to, var_types from)
default:
unreached();
}
unreached();
}

#elif defined(TARGET_ARM64)
Expand Down
Loading