In Visual Studio version 16.8 Preview 3, we have added a few safety rules to C++ Code Analysis that can find some common mistakes, which can lead to bugs ranging from simple broken features to costly security vulnerabilities. These new rules are developed around issues discovered in production software via security reviews and incidents requiring costly servicing. Every shipping piece of software in Microsoft runs these rules as part of security and compliance requirements.

The first part of this blog series, New Safety Rules in C++ Code Analysis, introduced new rules related to the misuse of VARIANT and its sibling types – such as VARIANTARG, or PROPVARIANT.

This second part of the series introduces new rules about “use of enumerations as index” and “use of Boolean as HRESULT”. To help with these new rules, we have built two code analysis extensions, called EnumIndex and HResultCheck that detect violations of these new rules in code.

Using enum as index

An enumeration or enum is a user-defined integral type that consists of an optional set of named integral constants that are known as enumerators (also called enumeration-constants). Usually, an enumeration provides context to describe a range of values (called enumerators) which are represented as named constants.

An enum can be made scoped by specifying class or struct keyword after the enum keyword, for example:

enum class Suit { Diamonds, Hearts, Clubs, Spades };

Without the class or struct keyword, an enum becomes unscoped.

Using /std:c++17, an enum (regular or scoped) can be defined with an explicit underlying type and no enumerators, which in effect introduces a new integral type that has no implicit conversion to any other type.

Unscoped enumerators can be implicitly converted to int. Scoped enumerators cannot be implicitly converted to int. A cast is required to convert a scoped enumerator to int. Likewise, a cast is required to convert an int to a scoped or unscoped enumerator.

The fact that an enumeration is an integral type that usually consists of a finite set of named constant values (enumerators) which can be converted implicitly or explicitly to int makes it very common to use enumerators as index values. For example:

const auto& colorInfo = ColorTable[color];

You will find lots of discussions online on using enum values as array indices. It really makes sense in many situations.

Frequently, when developers use enumerators of an enum type as indices for an array, they know that the enumerators of the enum type have values starting from zero to a known maximum value, with an increment of one and with no gap between any pair of consecutive enumerators. Thus, most of developers think checking the enumerator value received against the known maximum value would ensure validity of it.

However, using enumerators as array indices is not very safe. Unfortunately, it seems that there are not many discussions on why it can be dangerous.

Let’s look at an example. Consider the following enum and a table of function pointers for which we want to use the enum value as the index:

// MyHeader.h 

#pragma once 

#include <iostream> 

typedef int (*FP)(); 

enum FunctionId 
{ 
    Function1, 
    Function2, 
    Function3, 
    FunctionCount 
}; 

template <int val> 
int GetValue() { return val; }; 

int DoNotCallMe() 
{ 
    std::cout << "This shouldn't be called!\n"; 
    return -1; 
} 

FP fp = DoNotCallMe; 

FP Functions[] 
{ 
    GetValue<0>, 
    GetValue<1>, 
    GetValue<2> 
};

Now, in a source file, let’s define a function to select a function from the table, using an enumerator of the enum as the index for the function pointer table:

#include "MyHeader.h" 

FP GetFunction(FunctionId funcId) 
{ 
    if (funcId < FunctionId::FunctionCount) 
        return Functions[funcId]; 
    return nullptr; 
} 

Neat, isn’t it? To protect from rogue or faulty callers, I check the enumerator value against the known maximum value for FunctionId, so that it doesn’t cause the function to access the table beyond its bound. I know the enumerators of FunctionId enum type will start from zero, incremented by one, and end at FunctionId::FunctionCount – 1, FunctionCount being the last enumerator in the enum.

Let’s continue to add more code that uses this function. Our customer code will have integer value as the selector of a function, and want us to return an integer value through the function:

int GetValue(int funcIdx) 
{ 
    const auto fp = GetFunction(static_cast<FunctionId>(funcIdx)); 
    return fp ? fp() : -1; 
}

As explained above, I needed a cast to convert the integer value for the function table index to the enum type to pass to GetFunction. That will make sure that the int value is properly converted to an enumerator of the FunctionId enum. So far, so good, I hope.

#c++ #diagnostics #writing code #code analysis #static analysis