Jaa


Why isn't __assume on for asserts in retail mode?

__assume and __forceinline were the first two features I implemented as a developer out of college.  I just resolved a bug as 'By Design' that asked the question posed in the title.  Here's a brief explanation:

First, __assume(n==x) basically tells the expression optimizer & the global optimizer in the compiler that n is being assigned the value of x.  So, if you say __assume(n==5);y=n*n; the compiler will automagically turn that into y=25.  Nifty, right?  It's also used to flag dead code:

 switch(i) {
case 1:
case 2:
    printf("Hello\n");
    break;
default:
    __assume(0);
}

For that switch, there will be no checks for any values other than 1 or 2 - no range check.  Again, in performance sensitive code, this can be a big win.

However, the transformation of assert to __assume could have potentially negative impact on security, so it's not defined by default.  Individual customers should determine if it's an acceptable optimization.

The trouble lies in the potential lack of code coverage testing.  Here's a simplistic case to illustrate the problem:

 
void untestedFunctionFoo()
{
    assert(passwordAlreadyChecked == true);
    // Assert is actually false,
    // but we don't realize that,
    // because we haven't tested it
    DoSomethingThatChecksSecurity();
}

void DoSomethingThatChecksSecurity()
{
    if (!passwordAlreadyChecked)
        GetPassword();
    doSomethingSecure();
}

If the compiler inlines DoSomethingThatChecksSecurity into untestedFunctionFoo, and assert is turned into __assume(passwordAlreadyChecked == true) the optimizer will remove the call to GetPassword(), and then untestedFunctionFoo, which wasn't tested, no longer requires a password to get it.  Your visual code reviews don't pick up that doSomethingSecure might occur without a password, because logically, you're assertting that the password has already been checked, or you're calling GetPassword.

Hopefully, that explains the issue.  Please use __assume with caution.  It can be a performance win, but if you're authoring code with severe security concerns, you're better off without it.

-Kev

Comments

  • Anonymous
    January 30, 2006
    This came up on my team a while ago, and we hit some additional issues:
    1. Asserts may be accessing debug-only fields, so the expression wouldn't even be vaild in retail.
    2. the assert may make a function call that we can't place in the assume.
  • Anonymous
    February 01, 2006
    True, but that's a fairly easy work around by simply having 2 classes of asserts - a Debug only assert, and an __assume'ing assert. This is how the VC++ compiler back end & the 64 bit JIT do it.
  • Anonymous
    March 16, 2006
    You're responsible for this? __assume is by far my favorite compiler extension. Do you have a list of things the compiler can currently do with this because the documentation tells you nothing except that __assume(0) means code is unreachable?

    One other thing I wished you guys implemented was GCC's __builtin_expect (http://gcc.gnu.org/onlinedocs/gcc-4.0.2/gcc/Other-Builtins.html) or something similar to suggest to the compiler that if statements are rarely hit (but I really like what GCC did because it's like a probabalistic __assume).