[C++] Sonar not detecting RSPEC-836

SonarQube Cloud
,
1
  • ALM used : GitHub
  • CI system used: GitHub actions
  • Scanner command used when applicable: sonar-scanner -Dsonar.projectVersion=1.35.10 -X -Dsonar.pullrequest.key=XXXX -Dsonar.pullrequest.base=XXXX -Dsonar.pullrequest.branch=XXXX -Dsonar.cfamily.gcov.reportsPath=XXXX -Dsonar.cfamily.compile-commands=XXXX/compile_commands.json
  • Languages of the repository: C++
  • Error observed: we have a case similar to this:
#include <iostream>

struct Foo
{
    std::string m_a;

    template <class T, class... Args>
    void parseArgs(T &&t, Args &&...args) {
        m_a += std::to_string(std::forward<T>(t));
        parseArgs(std::forward<Args>(args)...);
    }

    void parseArgs() const {
        // The end of template recursion
    }

    template <class... Args>
    explicit Foo(Args &&...args)
    {
        parseArgs(std::forward<Args>(args)...);
    }
};

struct Boo : public Foo
{
    int m_b;
    explicit Boo(int b) : Foo{m_b, b}, m_b{b} {}
};

int main()
{
    Boo boo(12);
    std::cout << " " << boo.m_b << std::endl;
    std::cout << " " << boo.m_a << std::endl;
    return 0;
}

where Boo inherits from a class that has a generic constructor, and Boo uses a member variable that has not been initialized to build a string inside Foo (in this case m_b), and Sonar is not able to detect this issue.

Notice that if Foo uses the variable to initialize a variable directly, Sonar will complain about assigning a variable to garbage or undefined:

#include <iostream>

struct Foo
{
    int m_c;

    template <class... Args>
    explicit Foo(Args &&...args): m_c(std::forward<Args>(args)...)
    {
    }
};

struct Boo : public Foo
{
    int m_b;
    explicit Boo(int b) : Foo{m_b}, m_b{b} {}
};

int main()
{
    Boo boo(12);
    return 0;
}

Is there a workaround or a better way so that Sonar detects the issue?

You can see the example in godbolt: Compiler Explorer

6

Hello, @Alberto-Izquierdo and welcome!
Thank you for the interesting case. First of all, I agree, it is a false negative (i.e., there should’ve been a bug reported). I created [CPP-5269] - Jira, which you can refer to keep track of the false negative.

Here is what’s happening. I simplified your code example but preserved the problem:

struct S { S(); }; // S has unknown constructor
struct Foo
{
    S m_a;
    int m_c;
    Foo(int &args) { // Here m_a is implicitly initialized first - S::S()
      m_c = args;
    }
};

struct Boo : public Foo
{
    int m_b;
    Boo() : Foo{m_b}, m_b{0} {}
};

int main()
{
    Boo boo;
    return 0;
}

When Foo base struct is initialized, it first calls the default constructor of m_a - S::S().
The S::S() constructor is a function that takes a this pointer to the object being constructed, in this case Boo::m_a. However, m_a is the first field in Foo, which is the first base of Boo, so this pointer technically points to the entire Boo object.

S::S() could, therefore, initialize the Boo::m_b field, like this:

S::S() {
    reinterpret_cast<Boo*>(this)->m_b = 0;
}

and it could do that before the reference args to boo.m_b is assigned to Foo::m_c in the body of Foo::Foo.

We try our best to avoid false positive reports. For example, in this case, given that we do not see the definition of B::B(), we assume the worst: that it can change anything it has access to, including the entire boo object.

While, technically, the reasoning is sound, it is impractical: people do not write this kind of code normally.

Moreover, in your example, it is not some custom S::S() constructor that could initialize boo.m_b, but std::string::string(), which we know for a fact does not do this (we know, but the analyzer does not).

1 Like