From 4b8b4b6bf9c06844e777baa538ef6bf03a45c149 Mon Sep 17 00:00:00 2001 From: Paul Koch Date: Sat, 17 Oct 2026 16:53:21 -0700 Subject: [PATCH 1/6] Resolve nightly native analysis warnings Make existing bounds and allocation invariants visible to analysis, reuse purification and nominal flags consistently, initialize update pointers, and replace the strcpy wrapper. Restore BoosterCore analysis and ignore MSBuild-only unused driver arguments without disabling source diagnostics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a145ac79-0374-48e4-b265-e745cb43bb0d --- build.bat | 4 ++-- shared/libebm/BoosterCore.cpp | 4 ---- shared/libebm/CalcInteractionStrength.cpp | 3 +++ shared/libebm/CutWinsorized.cpp | 2 ++ shared/libebm/DataSetBoosting.cpp | 3 +++ shared/libebm/GenerateTermUpdate.cpp | 6 ++++-- shared/libebm/PartitionOneDimensionalBoosting.cpp | 10 +++++----- shared/libebm/interpretable_numerics.cpp | 11 ++++------- 8 files changed, 23 insertions(+), 20 deletions(-) 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..786daaa38 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 @@ -157,7 +158,7 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, UIntSplit* pSplit = pInnerTermUpdate->GetSplitPointer(iDimension); FloatScore* const aUpdateScore = pInnerTermUpdate->GetTensorScoresPointer(); - FloatScore* pUpdateScore; + FloatScore* pUpdateScore = aUpdateScore; auto* const aBins = pBoosterShell->GetBoostingMainBins()->Specialize(); @@ -165,7 +166,6 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, const Bin* pDregsBin = nullptr; const Bin* const* ppBinCur; if(!bNominal) { - pUpdateScore = aUpdateScore; ppBinCur = nullptr; if(bMissing) { @@ -265,7 +265,7 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, size_t iEdge; FloatScore hess; const GradientPair* pGradientPair; - if(nullptr != ppBinCur) { + if(bNominal) { goto determine_bin; } EBM_ASSERT(!bNominal); @@ -313,7 +313,7 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, ++pGradientPair; } while(pGradientPairEnd != pGradientPair); - if(nullptr == ppBinCur) { + if(!bNominal) { break; } EBM_ASSERT(bNominal); @@ -339,7 +339,7 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, } if(!pTreeNode->DECONSTRUCT_IsRightChildTraversal()) { // we checked earlier that countBins could be converted to a UIntSplit - if(nullptr == ppBinCur) { + if(!bNominal) { EBM_ASSERT(!bNominal); while(true) { // not a real loop diff --git a/shared/libebm/interpretable_numerics.cpp b/shared/libebm/interpretable_numerics.cpp index b98ba6b17..c4ae58d3d 100644 --- a/shared/libebm/interpretable_numerics.cpp +++ b/shared/libebm/interpretable_numerics.cpp @@ -6,7 +6,7 @@ #include // size_t, ptrdiff_t #include // std::numeric_limits -#include // strcpy, strchr, memmove, memcpy +#include // strlen, strchr, memmove, memcpy #include "libebm.h" #include "logging.h" // EBM_ASSERT @@ -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]); + const size_t cExponentChars = strlen(&pStr[k_iExp]) + size_t{1}; + EBM_ASSERT(cExponentChars <= k_cCharsFloatPrint - k_iExp); + memcpy(&strTruncated[iTruncateTextAfter], &pStr[k_iExp], cExponentChars * sizeof(*pStr)); if(PREDICTABLE(nullptr != pLowChopOut)) { *pLowChopOut = StringToFloatWithFixup(strTruncated, iTruncateTextAfter); From 394e234e6fafe8567a53ecafac432001391c06ed Mon Sep 17 00:00:00 2001 From: Paul Koch Date: Sat, 17 Oct 2026 17:28:51 -0700 Subject: [PATCH 2/6] Make purification surface bounds explicit to analysis Keep the zero-dimensional and one-dimensional exits unchanged while recording the nonempty-surface invariant for higher-dimensional randomized purification. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a145ac79-0374-48e4-b265-e745cb43bb0d --- shared/libebm/Purify.cpp | 3 +++ 1 file changed, 3 insertions(+) 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; From ef7312cdce6ef92efeed54b284561991e77e8abb Mon Sep 17 00:00:00 2001 From: Paul Koch Date: Sat, 17 Oct 2026 17:41:15 -0700 Subject: [PATCH 3/6] Bound mantissa traversal by the validated decimal position Use the fixed decimal-point position of the validated float format instead of relying on character contents for the lower traversal bound. Keep the character invariant checked in debug builds. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a145ac79-0374-48e4-b265-e745cb43bb0d --- shared/libebm/interpretable_numerics.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shared/libebm/interpretable_numerics.cpp b/shared/libebm/interpretable_numerics.cpp index c4ae58d3d..a34db7ed2 100644 --- a/shared/libebm/interpretable_numerics.cpp +++ b/shared/libebm/interpretable_numerics.cpp @@ -416,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); From 14aaa3cbc8bc0867350e1656a8d6b30f7d3d00e2 Mon Sep 17 00:00:00 2001 From: Paul Koch Date: Sat, 17 Oct 2026 22:23:48 -0700 Subject: [PATCH 4/6] Preserve pointer-based branching with cross-compiler analysis assertions Restore Flatten's original ppBinCur checks and initialize pUpdateScore only in the non-nominal branch. State the non-nominal invariant with ANALYSIS_ASSERT and use MSVC's analysis-only __analysis_assume intrinsic so C6001 is resolved without changing the runtime branching. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a145ac79-0374-48e4-b265-e745cb43bb0d --- shared/libebm/PartitionOneDimensionalBoosting.cpp | 10 ++++++---- shared/libebm/unzoned/unzoned.h | 4 ++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/shared/libebm/PartitionOneDimensionalBoosting.cpp b/shared/libebm/PartitionOneDimensionalBoosting.cpp index 786daaa38..08c128ab0 100644 --- a/shared/libebm/PartitionOneDimensionalBoosting.cpp +++ b/shared/libebm/PartitionOneDimensionalBoosting.cpp @@ -158,7 +158,7 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, UIntSplit* pSplit = pInnerTermUpdate->GetSplitPointer(iDimension); FloatScore* const aUpdateScore = pInnerTermUpdate->GetTensorScoresPointer(); - FloatScore* pUpdateScore = aUpdateScore; + FloatScore* pUpdateScore; auto* const aBins = pBoosterShell->GetBoostingMainBins()->Specialize(); @@ -166,6 +166,7 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, const Bin* pDregsBin = nullptr; const Bin* const* ppBinCur; if(!bNominal) { + pUpdateScore = aUpdateScore; ppBinCur = nullptr; if(bMissing) { @@ -265,10 +266,11 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, size_t iEdge; FloatScore hess; const GradientPair* pGradientPair; - if(bNominal) { + if(nullptr != ppBinCur) { goto determine_bin; } EBM_ASSERT(!bNominal); + ANALYSIS_ASSERT(!bNominal); iEdge = reinterpret_cast(ppBinLast) - reinterpret_cast(apBins) + 1; @@ -313,7 +315,7 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, ++pGradientPair; } while(pGradientPairEnd != pGradientPair); - if(!bNominal) { + if(nullptr == ppBinCur) { break; } EBM_ASSERT(bNominal); @@ -339,7 +341,7 @@ static ErrorEbm Flatten(BoosterShell* const pBoosterShell, } if(!pTreeNode->DECONSTRUCT_IsRightChildTraversal()) { // we checked earlier that countBins could be converted to a UIntSplit - if(!bNominal) { + if(nullptr == ppBinCur) { EBM_ASSERT(!bNominal); while(true) { // not a real loop diff --git a/shared/libebm/unzoned/unzoned.h b/shared/libebm/unzoned/unzoned.h index 3ab269462..42da5f42d 100644 --- a/shared/libebm/unzoned/unzoned.h +++ b/shared/libebm/unzoned/unzoned.h @@ -177,11 +177,15 @@ extern "C" { INLINE_ALWAYS static void StopClangAnalysis(void) EBM_NOEXCEPT ANALYZER_NORETURN {} +#if defined(_MSC_VER) && !defined(__clang__) +#define ANALYSIS_ASSERT(b) __analysis_assume(b) +#else #define ANALYSIS_ASSERT(b) \ do { \ if(!(b)) \ StopClangAnalysis(); \ } while((void)0, 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 From 097a966dfb83d7eea0b67a0b1d08c958d68c29d6 Mon Sep 17 00:00:00 2001 From: Paul Koch Date: Sat, 17 Oct 2026 22:53:58 -0700 Subject: [PATCH 5/6] Keep the validated exponent copy as a single strcpy call Restore strcpy instead of strlen plus memcpy. Document why the validated exponent fits after truncation and suppress only clang-analyzer-security.insecureAPI.strcpy at that call, without stopping subsequent path analysis. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a145ac79-0374-48e4-b265-e745cb43bb0d --- shared/libebm/interpretable_numerics.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shared/libebm/interpretable_numerics.cpp b/shared/libebm/interpretable_numerics.cpp index a34db7ed2..78382b486 100644 --- a/shared/libebm/interpretable_numerics.cpp +++ b/shared/libebm/interpretable_numerics.cpp @@ -6,7 +6,7 @@ #include // size_t, ptrdiff_t #include // std::numeric_limits -#include // strlen, strchr, memmove, memcpy +#include // strcpy, strchr, memmove, memcpy #include "libebm.h" #include "logging.h" // EBM_ASSERT @@ -401,9 +401,9 @@ static bool StringToFloatChopped(const char* const pStr, iTruncateMantissaTextDigitsAfterFirstDigit + size_t{3}; memcpy(strTruncated, pStr, iTruncateTextAfter * sizeof(*pStr)); - const size_t cExponentChars = strlen(&pStr[k_iExp]) + size_t{1}; - EBM_ASSERT(cExponentChars <= k_cCharsFloatPrint - k_iExp); - memcpy(&strTruncated[iTruncateTextAfter], &pStr[k_iExp], cExponentChars * sizeof(*pStr)); + // 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); From 1039c9023d6c3a305f1e44785f02de5deabae281 Mon Sep 17 00:00:00 2001 From: Paul Koch Date: Sat, 17 Oct 2026 23:11:55 -0700 Subject: [PATCH 6/6] Separate clang, MSVC, and fallback analysis assertions Keep clang first because clang-cl defines both __clang__ and _MSC_VER. Use __analysis_assume only for MSVC and a no-op for compilers without the supported analysis intrinsics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a145ac79-0374-48e4-b265-e745cb43bb0d --- shared/libebm/unzoned/unzoned.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/shared/libebm/unzoned/unzoned.h b/shared/libebm/unzoned/unzoned.h index 42da5f42d..fd95a5561 100644 --- a/shared/libebm/unzoned/unzoned.h +++ b/shared/libebm/unzoned/unzoned.h @@ -177,14 +177,16 @@ extern "C" { INLINE_ALWAYS static void StopClangAnalysis(void) EBM_NOEXCEPT ANALYZER_NORETURN {} -#if defined(_MSC_VER) && !defined(__clang__) -#define ANALYSIS_ASSERT(b) __analysis_assume(b) -#else +#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