On Tue, Jul 26, 2011 at 5:38 AM, Agner Fog <agner@xxxxxxxxx> wrote: > On 25-07-2011 08:04, Ian Lance Taylor wrote: >> >> It's reasonably straightforward to check for overflow of any operation by >> doing the arithmetic in unsigned types. By definition of the language >> standard, unsigned types wrap rather than overflow. > > I tried some more google search on this problem. It is quite common to check > for overflow after the fact, so obviously many programmers are unaware of > the problem. > > The safe solution is described in a recent CERT paper: > https://www.securecoding.cert.org/confluence/display/seccode/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow > > The CERT paper recommends this check for overflow before adding two signed > integers on a 2's complement machine: > > |if| |( ((si1^si2) | (((si1^(~(si1^si2) & INT_MIN)) + si2)^si2)) >= 0) {| > |||/* handle error condition */| > |} ||else| |{| > |||sum = si1 + si2;| > |}| > > Now, imagine doing something like this before every +, -, *, /, etc. in your > code! If this is necessary then the C/C++ language is useless. We are in > deep trouble now and we desperately need a better solution. Notice CERT does a expr + expr - the intermediate result might be 'undefined' which puts you back in the same boat as before! As I've said before, its ridiculous to test 'a priori' (I wasn't being argumentative). Douglas Gwyn made a similar comment at https://www.securecoding.cert.org/confluence/display/seccode/INT03-C.+Use+a+secure+integer+library. Also, CERT is moving towards the AIR integer model. See http://www.sei.cmu.edu/library/abstracts/reports/10tn008.cfm. > A security conscious programmer will typically replace an array with a > SafeArray class with bounds checking. But even this will not work if the > array index is the result of a calculation that can overflow, which almost > all calculations can. > > Your suggestion to use __attribute__ ((optimize ("-fno-strict-overflow"))); > was an excellent idea. Unfortunately it doesn't work. The compiler says: > warning: āoptimizeā attribute directive ignored. > > We need to be constructive here. We want the optimizations, but we also want > to check if the optimizations go wrong. I think the compiler should be able > to generate warnings for every unsafe optimization, especially when removing > a branch. The compiler generates a warning when optimizing away if (a+5 < > a) but not when optimizing away if (abs(a)<0). > > Some compilers make a warning whenever optimizing away a branch, saying > something like "warning: condition is always false". A branch that cannot be > taken is quite likely to be the symptom of a logical error, so such a > warning will be useful I think, and it would catch an overflow check that is > optimized away. Would that generate too many warnings? That depends on the > programming style, but in most cases I think it would be useful, and of > course you can turn off the warnings. Jeff
