diff --git a/build.bat b/build.bat index d306aef8d..579c0d89f 100644 --- a/build.bat +++ b/build.bat @@ -36,8 +36,8 @@ for %%x in (%*) do ( ) IF "%%x"=="-analysis" ( - REM MSBuild passes /c to clang-tidy; do not let /WX make this unused argument fatal. - SET extra_analysis=/p:EnableClangTidyCodeAnalysis=True /p:ClangTidyChecks=clang-analyzer-* /p:ClangTidyAdditionalOptions=-Wno-error=unused-command-line-argument /p:RunCodeAnalysis=True + REM MSBuild passes /c to clang-tidy, which does not compile. Ignore only unused driver arguments. + SET extra_analysis=/p:EnableClangTidyCodeAnalysis=True /p:ClangTidyChecks=clang-analyzer-* /p:ClangTidyAdditionalOptions=-Wno-unused-command-line-argument /p:RunCodeAnalysis=True ) ) diff --git a/shared/libebm/BoosterCore.cpp b/shared/libebm/BoosterCore.cpp index 0ca7e7931..fcef7bd2c 100644 --- a/shared/libebm/BoosterCore.cpp +++ b/shared/libebm/BoosterCore.cpp @@ -500,10 +500,6 @@ ErrorEbm BoosterCore::Create(void* const rng, EBM_ASSERT(1 <= cFeatures); // since our iFeature is valid and index 0 would mean cFeatures == 1 EBM_ASSERT(nullptr != pBoosterCore->m_aFeatures); - // Clang does not seems to understand that iFeature is bound to the legal - // range of m_aFeatures through the check "cFeatures <= iFeature" above - StopClangAnalysis(); - const FeatureBoosting* const pInputFeature = &pBoosterCore->m_aFeatures[iFeature]; pTermFeature->m_pFeature = pInputFeature; pTermFeature->m_cStride = cTensorBins; diff --git a/shared/libebm/CalcInteractionStrength.cpp b/shared/libebm/CalcInteractionStrength.cpp index c6ee49d53..c674ba199 100644 --- a/shared/libebm/CalcInteractionStrength.cpp +++ b/shared/libebm/CalcInteractionStrength.cpp @@ -620,6 +620,9 @@ EBM_API_BODY ErrorEbm EBM_CALLING_CONVENTION CalcInteractionStrength(Interaction } } + // The checked tree-node size and the nonempty tensor make this allocation nonzero. + EBM_ASSERT(0 != cBytes); + ANALYSIS_ASSERT(0 != cBytes); pTreeNodesTemp = malloc(cBytes); if(nullptr == pTreeNodesTemp) { free(aWeights); diff --git a/shared/libebm/CutWinsorized.cpp b/shared/libebm/CutWinsorized.cpp index 0a121d836..b2ac77f0a 100644 --- a/shared/libebm/CutWinsorized.cpp +++ b/shared/libebm/CutWinsorized.cpp @@ -174,8 +174,10 @@ EBM_API_BODY ErrorEbm EBM_CALLING_CONVENTION CutWinsorized( ++pHigh; EBM_ASSERT(aFeatureVals <= pLow && pLow < aFeatureVals + cSamples); + ANALYSIS_ASSERT(aFeatureVals <= pLow && pLow < aFeatureVals + cSamples); lowCur = *pLow; EBM_ASSERT(aFeatureVals <= pHigh && pHigh < aFeatureVals + cSamples); + ANALYSIS_ASSERT(aFeatureVals <= pHigh && pHigh < aFeatureVals + cSamples); highCur = *pHigh; // since valMin != valMax per above, we know there is a transition SOMEWHERE, which will exit us } while(LIKELY(lowCur == highCur)); diff --git a/shared/libebm/DataSetBoosting.cpp b/shared/libebm/DataSetBoosting.cpp index 3b53de649..a80f9f824 100644 --- a/shared/libebm/DataSetBoosting.cpp +++ b/shared/libebm/DataSetBoosting.cpp @@ -904,6 +904,9 @@ ErrorEbm DataSetBoosting::InitBags(const bool bAllocateCachedTensors, double weight = double{1}; if(nullptr != pOccurrencesFrom) { EBM_ASSERT(size_t{0} != cInnerBags); + // InitDataSetBoosting partitions exactly cIncludedSamples across the subsets. + EBM_ASSERT(pOccurrencesFrom < aOccurrencesFrom + cIncludedSamples); + ANALYSIS_ASSERT(pOccurrencesFrom < aOccurrencesFrom + cIncludedSamples); const uint8_t cOccurrences = *pOccurrencesFrom; ++pOccurrencesFrom; weight = static_cast(cOccurrences); diff --git a/shared/libebm/GenerateTermUpdate.cpp b/shared/libebm/GenerateTermUpdate.cpp index 8f9092307..0fefe674d 100644 --- a/shared/libebm/GenerateTermUpdate.cpp +++ b/shared/libebm/GenerateTermUpdate.cpp @@ -394,7 +394,9 @@ static ErrorEbm BoostMultiDimensional(BoosterShell* const pBoosterShell, double* aWeights = nullptr; double* pGradient = nullptr; double* pHessian = nullptr; - if(0 != ((TermBoostFlags_PurifyUpdate | TermBoostFlags_PurifyGain) & flags)) { + const bool bPurifyUpdate = 0 != (TermBoostFlags_PurifyUpdate & flags); + const bool bPurifyGain = 0 != (TermBoostFlags_PurifyGain & flags); + if(bPurifyUpdate || bPurifyGain) { // allocate the biggest tensor that is possible to split into // TODO: cache this memory allocation so that we don't do it each time @@ -537,7 +539,7 @@ static ErrorEbm BoostMultiDimensional(BoosterShell* const pBoosterShell, EBM_ASSERT(!std::isnan(*pTotalGain)); EBM_ASSERT(0 <= *pTotalGain); - if(0 != (TermBoostFlags_PurifyUpdate & flags) && 0 == (TermBoostFlags_PurifyGain & flags)) { + if(bPurifyUpdate && !bPurifyGain) { Tensor* const pTensor = pBoosterShell->GetInnerTermUpdate(); size_t cDimensions = pTerm->GetCountDimensions(); diff --git a/shared/libebm/PartitionOneDimensionalBoosting.cpp b/shared/libebm/PartitionOneDimensionalBoosting.cpp index 441559ca3..08c128ab0 100644 --- a/shared/libebm/PartitionOneDimensionalBoosting.cpp +++ b/shared/libebm/PartitionOneDimensionalBoosting.cpp @@ -123,6 +123,7 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, EBM_ASSERT(nullptr != pBoosterShell); EBM_ASSERT(iDimension <= k_cDimensionsMax); EBM_ASSERT(nullptr != apBins); + ANALYSIS_ASSERT(nullptr != apBins); EBM_ASSERT(nullptr != apBinsEnd); EBM_ASSERT(apBins < apBinsEnd); // if zero bins then we should have handled elsewhere EBM_ASSERT(2 <= cSlices); // if there were no cuts then we wouldn't satisfy the minimum gain @@ -269,6 +270,7 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, goto determine_bin; } EBM_ASSERT(!bNominal); + ANALYSIS_ASSERT(!bNominal); iEdge = reinterpret_cast(ppBinLast) - reinterpret_cast(apBins) + 1; diff --git a/shared/libebm/Purify.cpp b/shared/libebm/Purify.cpp index baa73d3a1..585196ca0 100644 --- a/shared/libebm/Purify.cpp +++ b/shared/libebm/Purify.cpp @@ -2089,6 +2089,9 @@ EBM_API_BODY ErrorEbm EBM_CALLING_CONVENTION Purify(double tolerance, ++iExclude; } while(cDimensions != iExclude); + // Nonempty tensors with at least two dimensions have nonempty surfaces. + EBM_ASSERT(0 != cSurfaceBins); + ANALYSIS_ASSERT(0 != cSurfaceBins); if(IsMultiplyError(cScoresArrayBytes, cSurfaceBins)) { LOG_0(Trace_Error, "ERROR Purify IsMultiplyError(cScoresArrayBytes, cSurfaceBins)"); return Error_IllegalParamVal; diff --git a/shared/libebm/interpretable_numerics.cpp b/shared/libebm/interpretable_numerics.cpp index b98ba6b17..78382b486 100644 --- a/shared/libebm/interpretable_numerics.cpp +++ b/shared/libebm/interpretable_numerics.cpp @@ -378,11 +378,6 @@ static double StringToFloatWithFixup(const char* const str, const size_t iIdenti return ret; } -INLINE_ALWAYS static char* strcpy_NO_WARNINGS(char* const dest, const char* const src) EBM_NOEXCEPT { - StopClangAnalysis(); - return strcpy(dest, src); -} - static bool StringToFloatChopped(const char* const pStr, size_t iTruncateMantissaTextDigitsAfterFirstDigit, double* const pLowChopOut, @@ -406,7 +401,9 @@ static bool StringToFloatChopped(const char* const pStr, iTruncateMantissaTextDigitsAfterFirstDigit + size_t{3}; memcpy(strTruncated, pStr, iTruncateTextAfter * sizeof(*pStr)); - strcpy_NO_WARNINGS(&strTruncated[iTruncateTextAfter], &pStr[k_iExp]); + // FloatToFullString bounds the source; truncation moves its exponent earlier in an equally sized buffer. + // NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.strcpy) + strcpy(&strTruncated[iTruncateTextAfter], &pStr[k_iExp]); if(PREDICTABLE(nullptr != pLowChopOut)) { *pLowChopOut = StringToFloatWithFixup(strTruncated, iTruncateTextAfter); @@ -419,7 +416,8 @@ static bool StringToFloatChopped(const char* const pStr, } while(true) { ch = *pDigit; - if('.' == ch) { + if(strTruncated + size_t{2} == pDigit) { + EBM_ASSERT('.' == ch); --pDigit; start_at_top:; EBM_ASSERT(strTruncated + size_t{1} == pDigit); diff --git a/shared/libebm/unzoned/unzoned.h b/shared/libebm/unzoned/unzoned.h index 3ab269462..fd95a5561 100644 --- a/shared/libebm/unzoned/unzoned.h +++ b/shared/libebm/unzoned/unzoned.h @@ -177,11 +177,17 @@ extern "C" { INLINE_ALWAYS static void StopClangAnalysis(void) EBM_NOEXCEPT ANALYZER_NORETURN {} +#if defined(__clang__) #define ANALYSIS_ASSERT(b) \ do { \ if(!(b)) \ StopClangAnalysis(); \ } while((void)0, 0) +#elif defined(_MSC_VER) +#define ANALYSIS_ASSERT(b) __analysis_assume(b) +#else +#define ANALYSIS_ASSERT(b) ((void)0) +#endif // Disable this for now. Using fast reciprocals seems to change the results quite a bit // and make them worse. Also, the reciprocal algorithm is different between Intel and AMD