How I Learned to Start Worrying and Distrust the Bomb

Steve Hawley — Tue, 07 Jul 2009 15:07:00 GMT

This is a post about a serious bug I turned up in the Microsoft C++ compilers that target CLI (both Managed C++ and C++/CLI).

One of the key concepts in software engineering is Design by Contract. Design by Contract boils down to “say what you do and do what you say.” Essentially, when I make a semantic definition, I would like it to be abided by the providers and when applicable the clients of the definition. For example, if I define a language runtime I can define arrays such that indexing into the array must always be range checked. I can enforce this limitation at a number of levels. For example, I can provide a runtime array API which includes range checking in all its members. This is the simplest approach, but probably the most inefficient. I could also leave it up to the compiler writers to check before calling the API – this is potentially more efficient because it allows loop invariant optimization. If I’m looping over all elements in an array, I know that if the loop extents are within the range of the given array, then all indexing based on the unchanged loop variable(s) will also be in range.

The problem with putting the onus of checking on the compiler writers is that the adherence to the contract is only as good as the diligence of the compiler writers and the verifiers of the compiler (unit tests, qa, outside certification).

I discovered early on that there was a certain amount of looseness in what appear to be iron rules. For example, from the point of view of Managed C++, there is no difference between out and ref parameters, whereas C# demands that assignment is done to an out parameter in all code paths.

So let’s consider one of the nicest conventions added to .NET for those of us (like Atalasoft) who write class libraries for others to use: ObsoleteAttribute.

By added an Obsolete to a class or method, I can signal my clients that a particular feature is no longer considered to be the “right” way to do something. I may have written a more flexible way to get the same feature. Obsolete lets me communicate that to my clients. In the message that will be passed on by the compiler, I can let my clients know that there is a better way. And it, heaven forbid, I have to revoke access to a feature, I can force an error at compile time. I like this – especially at the piecemeal level of attributes that I can tack onto classes, methods, properties, fields, etc. In C, there isn’t a really good way to do this except via #error and #warning, which can’t be used with the same granularity without some heavy preprocessor lifting.

So let’s consider this awful C# class:

using System;
namespace Helpers
{
    public class Utilities
    {
        [Obsolete("This is really not needed.", true)]
        public static int Add(int a, int b)
        {
            return a + b;
        }
        [Obsolete("This is a horrible constant.", true)]
        public int Pi { get { return 3; } }
    }
}

This is a class with a static method and a property. Both are marked as obsolete and both are marked to fail when used.

If I try to use either of these in C#, the compiler fails with an error. Good. Now comes the problem: C++. Consider this chunk of test code:

#include "stdafx.h"
using namespace System;
#if MANAGED_CPP
int main(System::String __gc *args __gc[])
#else
int main(array<System::String ^> ^args)
#endif
{
// Fails at compile time
//    int sum = Helpers::Utilities::Add(3, 4);
    int sum = 0;
#if MANAGED_CPP
    Helpers::Utilities *u = __gc new Helpers::Utilities();
    sum += u->get_Pi();
#else
    Helpers::Utilities ^u = gcnew Helpers::Utilities();
    sum += u->Pi::get();
#endif
    sum += u->Pi;
    return 0;
}

This code is a little sloppy because I wrote it to be testable from either managed C++ or C++/CLI. Even though the syntax is different, the result is the same. If I try to call Add, I’ll get a compilation error. Good. The problem comes with accessing the property Pi. The C# class has explicitly forbidden the use of the property, yet when I compile this code with either compiler it blissfully succeeds (“========== Rebuild All: 2 succeeded, 0 failed, 0 skipped ==========”). Not only that, it runs. Yikes.

Take the Obsolete attribute with a grain of salt. It is a contract, yes, but it only works as well as those who adhere to it. If you create a property that you want to be truly obsolete and cause compile failures, consider removing it entirely. I don’t like this – it fails compilation everywhere, but we also lose the ability to tell our clients what they should do instead. You could also change the behavior to a runtime throw with an appropriate message in addition to the obsolete attribute. I don’t like this either as I’m creating time bombs for my C++ clients, but at least they can’t cause other problems which could be much worse.

Steve's Tech Talk : design, compiler, C++, C#, code

How I Learned to Start Worrying and Distrust the Bomb