NDPluginStats: value-initialise stats structs so dark frames don't broadcast garbage#597
Merged
Conversation
…oadcast garbage
processCallbacks() declared its working statistics structs as
NDStats_t stats, *pStats=&stats, statsTemp, *pStatsTemp=&statsTemp;
NDStats_t is a POD with no constructor and the locals were never zeroed.
The central-moment fields -- sigmaXY, skewX/Y, kurtosisX/Y, eccentricity
and orientation -- are assigned only inside the if (M00 > 0.) block of
doComputeCentroid(). A frame whose every pixel is below CentroidThreshold
(a dark frame, a closed shutter, below-threshold illumination), or a run
with ComputeCentroid disabled, leaves M00 == 0 and those fields unwritten.
They are then copied unconditionally into the broadcast time-series
NDArray and the corresponding _RBV parameters, so SigmaXY_RBV,
SkewX/Y_RBV, KurtosisX/Y_RBV, Eccentricity_RBV, Orientation_RBV and their
time-series waveforms carry stack garbage -- run-to-run varying, possibly
NaN/inf -- on any dark or below-threshold frame. Dark frames are routine,
so archives are corrupted and alarm thresholds on those PVs fire
spuriously.
Value-initialise both structs (NDStats_t stats={}, statsTemp={}) so every
field defaults to zero and an unassigned central moment reads as 0 rather
than uninitialised memory. The pointer members (profileX/Y, histogram,
totalArray, netArray) are still assigned and freed under their existing
guards; zero-init only makes them null when unused.
Verified with a proof driver: a struct placed on memory pre-filled with
0xAA and processed as a dark frame (M00 == 0) broadcasts the garbage
without the initialiser and 0 with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NDStats_t stats, statsTemp;are POD locals, never zeroed. The central-moment fields (sigmaXY,skewX/Y,kurtosisX/Y,eccentricity,orientation) are written only insideif (M00 > 0.), but copied unconditionally into the broadcast time-series array and the_RBVparams. A dark or below-threshold frame (M00 == 0), orComputeCentroid = 0, publishes stack garbage — possibly NaN/inf — to those PVs and waveforms, corrupting archives and firing spurious alarms. Value-initialise both structs (= {}); pointer members stay allocated/freed under their existing symmetric guards. Proven with a driver running the broadcast path on0xAA-poisoned stack memory with a dark frame: garbage before, 0 after.