Поделиться через


The many types of asserts and their meaning, and dealing with assert bugs

A good way to get a quick rise out of me is to tell me how you’re going to just delete some noisy assert in your code, or worse still, that some assert is “ignorable” – in fact I think the ignore button on the assert dialog is a crime against humanity.   I think the only buttons that should be on an assert dialog are debug and exit. 

I’m actually surprised how much disagreement I find when discussing how to handle bugs that report assertion failures.  I think part of the problem is that the “assert” macro is used for various different things.  I actually wish it wasn’t.  I wish we had different names for these things so that it was clear which of them is going on. 

So, in the order I deem most interesting, here are assert overloads you should know about: 

 

1. CONTRACT (“valid arguments, or else”)

This is the only context in which you may assert argument constraints to a method with unknown callers (e.g. public methods).  It’s not really an assert at all, if it fails, you’re saying the caller has screwed up, you’re not dealing with bogus arguments, and we are going to exit right here right now, contract failure suck it up.  This is the single best way to ensure that you do not get called with bogus arguments, any bad call patterns will quickly be identified and corrected.   It’s in your face.

If a bug is reported against a contract failure the problem is for sure in the calling code unless the assert is just totally wrong.

If this is your validation strategy then you should be completely comfortable including contracts in release code and in fact all the code is likely to be better if you do because you won’t have to deal with bogus args all over the place.  Things sort themselves out astonishingly quickly when you do this.

Note: if you don’t do contract based validation (lots of code doesn’t) then your contract is “you can call me with ANY args and I will correctly generate a failure code” which means you better validate all the failure paths too because those are part of your obligation.

Note: if you used to use contract based validation and you loosen your semantics to allow more arg types and some error codes (as above) then you could break your callers because they may now run, incorrectly (!) in cases where you previously would have saved the day.  E.g. index validation was happening by lookup in a collection that failed for bogus indexes.  Now you “helpfully” return an unexpected failure status code and the calling code now has an exploitable buffer overflow!

 

2. INVARIANT (aka the “normal” assert)

Normal assertions are about your code explaining to itself, or a reader, what is going on.  What rules have you created in your data structures and algorithms (e.g. I coded it so that there is no more than one outstanding transaction at a time so I can ASSERT(count <= 1).

These assertions inform the reader that the code that follows is practically guaranteed to assume that the invariant is true (e.g. in this case it assumes that count is never 2 or greater) and will most likely BREAK if you proceed.  Now frankly if you don’t need to assume the invariant in question because the code that follows is general enough that (e.g.) any count works then it’s probably not a good idea to add the assert, what’s the point?  It’s not a necessary invariant.

If an assert like this starts to fire it means that the code is not working as intended, perhaps it was never quite right, perhaps someone has made a change during maintenance that makes it so that the former invariant no longer holds.  In all these cases the code that follows the assert is almost certainly WRONG because it assumes the invariant is true.  This is a bug farm, the failure MUST be dealt with swiftly.

OK as always there is also the possibility that the invariant was just stated incorrectly because of a typo or something.

An invariant assert MUST NOT be followed by a redundant test such as:

ASSERT(!foo);

If (foo) { stuff }

This is because the ASSERT itself is claiming that it is impossible for foo to be true, hence you could not possibly create any test of any kind that exercises “stuff” hence stuff is certainly wrong (guilty until proven innocent) and has no business being in any production code.  You may as well write

if (foo) { stuff some hacker might exploit for a security attack but no customer will ever benefit from;}

If it’s not impossible then it’s not an invariant and you have no business asserting it.  So, pick one.  But see #3 below.

The only reason that you might want to exclude invariants from release code is if checking the invariant is prohibitively expensive, or there are so many it would bloat your code ridiculously.  Logically you should be willing to upgrade them to release_assert or some such other macro.

You cannot ever responsibly delete invariants without careful engineering to establish some new correct situation no matter how “noisy” they are.

Bugs involving reports of invariants failing are almost certainly more serious problems than say crashes (because the release code might behave in an exploitable fashion).

 

3. TRIPWIRE (“what I wish was true”)

These ones really need a different name and a different affordance when they fail. 

Sometimes people sprinkle “asserts” in their code because they think something is true, and they want to get data about whether it really is or not.  Maybe they’re considering some change to enforce it via contract, or maybe they want to make an optimization based on the fact that they expect to get ridiculously few reports of the assertion failing.

Tripwires are totally unlike invariants in that it is completely normal to redundantly test the tripwire and to not assume it at all in the code that follows.

When a tripwire gets too noisy you can always call the experiment over and just delete it.  It adds nothing.

These should basically never be upgraded to release_asserts except maybe for very controlled experiments. 

Calling them asserts is very confusing and totally devalues the importance of #1 and #2.

4. EXPECTATION (“my test expects this state”)

Some test frameworks and/or developers use ASSERT in place of an EXPECTATION.  An invariant assert is ALWAYS true for any input; an expectation is a statement that in a particular test case a certain condition or set of conditions should hold.  True asserts that happen in the context of running a test case are in some sense more serious than expectation failures, the latter means that a particular test case is not working right.  The former means that the code is deluded about its own invariants.

If only so that you can easily separate bugs about test cases failing vs. invariants failing it’s good to use a different name for these as well.

 

Conclusion

When faced with bugs related to assertion failures it is vitally important to know what kind of assert you’re dealing with.   The remediation is going to be vastly different depending on the intent of the writer.  Hopefully there are good comments to guide you.

Assertions that are flat out wrong (e.g. obvious typo) should be corrected.  Assertions that describe important invariants must have their failures addressed by suitable corrections that either change the invariant or re-establish its truth.  Dealing with such bugs in the context of an assertion failure is invariably easier than waiting for whatever terrible side-effect is caused code running with broken invariants, many of which are hacker-exploitable.

Simply pushing the ignore button on asserts is rarely the right thing to do and even the presence of an ignore button encourages the wrong sorts of practices.

Letting your asserts “decay” is a recipe for future disasters and seriously erodes confidence in the overall quality of your code.

Comments

  • Anonymous
    October 10, 2015
    This is a good and clean list. Shouldn't there be a RELEASE_INVARIANT as well? Sometimes you want assertions to make it into the production code just to be sure.

  • Anonymous
    October 13, 2015
    The comment has been removed

  • Anonymous
    October 20, 2015
    The comment has been removed