Mark variable as not NULL after BOOST_REQUIRE in PVS-Studio

Question

I'm using PVS-Studio to analyze my Testcode. There are often constructs of the form

const noAnimal* animal = dynamic_cast<noAnimal*>(...);
BOOST_REQUIRE(animal);
BOOST_REQUIRE_EQUAL(animal->GetSpecies(), ...);

However I still get a warning V522 There might be dereferencing of a potential null pointer 'animal' for the last line.

I know it is possible, to mark functions as "not returning NULL" but is it also possible to mark a function as a valid NULL check or make PVS-Studio somehow else aware that animal can't be NULL after BOOST_REQUIRE(animal);?

This also happens if the pointer is checked via any assert flavour first.

Answer 1

Thank you for the interesting example. We'll think, what we can do with the BOOST_REQUIRE macro.

At the moment, I can advise you the following solution:

Somewhere after

#include <boost/test/included/unit_test.hpp>

you can write:

#ifdef PVS_STUDIO
  #undef BOOST_REQUIRE
  #define BOOST_REQUIRE(expr) do { if (!(expr)) throw "PVS-Studio"; } while (0)
#endif

This way, you will give a hint to the analyzer, that the false condition causes the abort of the control flow. It is not the most beautiful solution, but I think it was worth telling you about.

Answer 2

Responding to a comment with a large one is a bad idea, so here is my detailed response on the following subject:

Although this is possible it would be a pain to include that define in all testcase files. Also this is not limited to BOOST_REQUIRE only but also applies to assert, SDL_Assert or any other custom macro the user might use.

One should understand that there are three types of test macros and each should be discussed separately.

Macros of the first type simply warn you that something went wrong in the Debug version. A typical example is assert macro. The following code will cause PVS-Studio analyzer to generate a warning:

T* p = dynamic_cast<T *>(x);
assert(p);
p->foo();

The analyzer will point out a possible null-pointer dereferencing here and will be right. A check that uses assert is not sufficient because it will be removed from the Release version. That is, it turns out there’s no check. A better way to implement it is to rewrite the code into something like this:

T* p = dynamic_cast<T *>(x);
if (p == nullptr)
{
  assert(false);
  throw Error;
}
p->foo();

This code won’t trigger the warning.

You may argue that you are 100% sure that dynamic_cast will never return nullptr. I don’t accept this argument. If you are totally sure that the cast is ALWAYS correct, you should use the faster static_cast. If you are not that sure, you must test the pointer before dereferencing it.

Well, OK, I see your point. You are sure that the code is alright, but you need to have that check with dynamic_cast just in case. OK, use the following code then:

assert(dynamic_cast<T *>(x) != nullptr);
T* p = static_cast<T *>(x);
p->foo();

I don’t like it, but at least it’s faster, since the slower dynamic_cast operator will be left out in the Release version, while the analyzer will keep silent.

Moving on to the next type of macros.

Macros of the second type simply warn you that something went wrong in the Debug version and are used in tests. What makes them different from the previous type is that they stop the algorithm under test if the condition is false and generate an error message.

The basic problem with these macros is that the functions are not marked as non-returning. Here’s an example.

Suppose we have a function that generates an error message by throwing an exception. This is what its declaration looks like:

void Error(const char *message);

And this is how the test macro is declared:

#define ENSURE(x) do { if (!x) Error("zzzz"); } while (0)

Using the pointer:

T* p = dynamic_cast<T *>(x);
ENSURE(p);
p->foo();

The analyzer will issue a warning about a possible null-pointer dereferencing, but the code is actually safe. If the pointer is null, the Error function will throw an exception and thus prevent the pointer dereferencing.

We simply need to tell the analyzer about that by using one of the function annotation means, for example:

[[noreturn]] void Error(const char *message);

or:

__declspec(noreturn) void Error(const char *message);

This will help eliminate the false warning. So, as you can see, it’s quite easy to fix things in most cases when using your own macros.

It might be trickier, however, if you deal with carelessly implemented macros from third-party libraries.

This leads us to the third type of macros. You can’t change them, and the analyzer can’t figure out how exactly they work. This is a common situation, as macros may be implemented in quite exotic ways.

There are three options left for you in this case:

1. suppress the warning using one of the false-positive suppression means described in the documentation;
2. use the technique I described in the previous answer;
3. email us.

We are gradually adding support for various tricky macros from popular libraries. In fact, the analyzer is already familiar with most of the specific macros you might encounter, but programmers’ imagination is inexhaustible and we just can’t foresee every possible implementation.