Can "unspecified order of evaluation" be detected with static analysis?

127 Views Asked by At

For most of my C++ projects, I strongly rely on static analysis to prevent bugprone code getting into the master branch. However, nothing seems to detect bugs caused by unspecified order of evaluation. While some tools may spot suspicious code like foo(++i, --i), none of them seem to detect the problem here:

std::map<int, int> m;
int i = 0;
m[++i] = --i;

Here is the list of analyzers I've tried:

EDIT: as pointed out in the comments, the problem in the snippet above is detected by clang -std=c++14 -Wunsequenced. Here is a less trivial example that closely resembles a bug I've recently discovered in my project, and no warnings is given:

#include <unordered_map>
#include <cassert>
#include <memory>
#include <iostream>

struct Object {
};

struct Wrapper {
    std::unique_ptr<Object> object;
    std::string description;
};

class ObjectRegistry {
public:
    void add_object(Wrapper& w) {
        objects_[w.object.get()] = Wrapper{std::move(w.object), "added from " + w.description};

        // this assertion may or may not fail depending on the order of evaluation in the line above, which is unspecified
        assert(objects_.begin()->first != nullptr);
    }

private:
    // In our application, we need to quickly access the wrapper by raw pointer of its object
    std::unordered_map<Object*, Wrapper> objects_;
};

int main(int argc, char *argv[]) {
    ObjectRegistry registry;
    Wrapper w{std::make_unique<Object>(), "first object"};
    registry.add_object(w);
    return 0;
}

Is there any fundamental problem why this can't be detected with static analysis? If not, does there exist a tool that's able to detect it?

1

There are 1 best solutions below

0
Alexander Kurenev On

As already mentioned, your first code snippet is correct since C++17 and it equals to m[0] = -1;. Before C++17 this code contains undefined behavior. I checked with PVS-Studio, and it issues the V567 warning.

Next example is really complex. Analyzer should do the following:

  • Know all the things about the order of evaluation for every version of the C++ standard. It's quite easy :)
  • Understand that Wrapper { .... } is an aggregate initialization. The first initializer will call move constructor of std::unique_ptr class for .object non-static data member
  • Understand that w.object was moved and the smart pointer becomes nullptr after the aggregate initialization
  • Understand that w.object.get() call will always return nullptr and issue some warning.

This is the behavior since C++17 and this code will always do strange thing. Before C++17 a compiler / static code analyzer could perform evaluation in any order.