Guard MODULO against the undefined INT_MIN % -1 (SIGFPE that kills the IOC)#38
Open
physwkim wants to merge 1 commit into
Open
Guard MODULO against the undefined INT_MIN % -1 (SIGFPE that kills the IOC)#38physwkim wants to merge 1 commit into
physwkim wants to merge 1 commit into
Conversation
The calc engines compute '%' as a signed integer remainder on values narrowed from double. Every MODULO case guards a zero divisor but none guards the one remaining case C leaves undefined: INT_MIN % -1 (and INT64_MIN % -1 on LP64). On x86 the idiv instruction that implements '%' traps the quotient overflow as #DE, delivered as SIGFPE, so a single calc/scalc/acalc expression whose dividend narrows to INT_MIN (readily produced by an out-of-range or NaN double, which casts to 0x80000000) against a -1 divisor takes down the whole IOC. Mathematically x % -1 == 0 for every x, so route each remainder through a small safeModulo() helper that returns 0 for a -1 divisor instead of executing the trapping idiv. This closes the family at every site with one rule rather than a per-site patch: sCalcPerform.c numeric path (int) and string path (long) aCalcPerform.c scalar, array%scalar, and array%array paths (int) The zero-divisor guards are unchanged; only the -1 case, previously undefined behaviour, is now defined to 0. Ordinary remainders are unaffected. Proved with a driver linking the real sCalcPerform/aCalcPerform translation units and driving each site through sCalcPostfix/aCalcPostfix + Perform, one fork per case (INT_MIN dividend, -1 divisor): site before after sCalc numeric (:562) SIGFPE 0 sCalc string (:1109) SIGFPE 0 (post0==USES_STRING) aCalc scalar (:701) SIGFPE 0 aCalc arr%sca (:674) SIGFPE 0 aCalc arr%arr (:650) SIGFPE 0 control 17 % 5 2 2
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
1 participant
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.
The calc engines evaluate
%as a signed integer remainder on values narrowed fromdouble. EachMODULOcase guards a zero divisor but none guardsINT_MIN % -1(andINT64_MIN % -1on LP64) — undefined in C, and on x86 theidivbehind%traps the quotient overflow as#DE/SIGFPE. Because an out-of-range or NaNdoublecasts to0x80000000=INT_MIN, an ordinary data-driven expression (a large counter or ADC sum, a NaN) against a-1divisor crashes the whole IOC.Five sites are affected:
sCalcPerform.cnumeric ((int)) and string-stack ((long)) paths, andaCalcPerform.cscalar / array%scalar / array%array paths. Fixed uniformly by routing each remainder through asafeModulo()helper that returns 0 for a-1divisor (x % -1 == 0for all x), leaving the zero-divisor guards and all ordinary remainders unchanged.Verified with a driver linking the real
sCalcPerform/aCalcPerformtranslation units and driving every site through the public postfix+perform API: all five SIGFPE before the change and return 0 after;17 % 5returns 2 unchanged.The same defect exists in epics-base's
calcPerform.cMODULO; a separate fix is being submitted there.