Custom assert not beign detected by sonar

SonarQube Cloud
1

Hi guys,

I’m getting sonar errors that describe out of bound memory access. My code checks for the validity of the index with a custom made assert function. If the assert doesn’t pass, the execution of the code is blocked in an infinite while loop.

For example:

#define TEXT_SIZE = 4

char text[TEXT_SIZE] = { };
uint8_t i = 4;

ASSERT(i<4, "Index out of range");
char dummy = text[i];

In the above example, assuming that the assert calls an infinite loop, the last line will never be executed.

The sonar scanner doesn’t understand this situation and gives me a cpp:S3519 out of bound memory error.

The ASSERT macro in my codebase calls a function that saves some data and then enters in a while(true) loop.

Can I do something about it?

Environment

  • Bitbucket Cloud
  • Bamboo
  • C++
  • embedded
5

Hi @MCMattia and welcome to our community!

To understand your situation I will need a little more context. I have tried to encode what you are telling me into a complete compilable code example:

#define TEXT_SIZE 4
#define ASSERT(cond, msg) if (!(cond)) while (true)

void top() {
    char text[TEXT_SIZE] = { };
    int i = 4;

    ASSERT(i<4, "Index out of range");
    char dummy = text[i];
}

The issue of rule S3519 is correctly suppressed. So to understand why you see a S3519 issue report, and what you can do about it, let’s do the following:

  • Search in the analysis log for the full path of the source file for which you want to create a reproducer - the file that contains the false-positive. You will have to use exactly this name (same case, / or \…)
  • Add the reproducer option to the scanner configuration:
    sonar.cfamily.reproducer=“Full path to the .cpp”
  • Re-run the scanner to generate a file named sonar-cfamily.reproducer in the project folder.
  • Share this file privately by replying to the private message I’ve sent you.

P.S. in case of an issue in a header file, you want to generate the reproducer of the source file that includes that header.

1 Like
6

@MCMattia provided me with more details privately and here is the conclusion:

Our analyzer reads each translation unit (.cpp file with all the headers it #includes) separately and independently of other translation units. It shares almost no information between the translation units. If a function void my_infinite_loop() is defined in a.cpp and used in b.cpp, when analyzing b.cpp, the analyzer will not know anything about f except its signature.

Simplified the situation can be represented as the following. Two files a.cpp and b.cpp.
a.cpp defines my_infinite_loop like this:

// a.cpp
void my_infinite_loop() {
  while (true) {
     // infinite loop
  }
}

b.cpp uses it in an ASSERT macro like this:

// b.cpp
void my_infinite_loop();

#define TEXT_SIZE 4
#define ASSERT(cond, msg) if (!(cond)) my_infinite_loop()

void top() {
    char text[TEXT_SIZE] = { };
    int i = 4;

    ASSERT(i<4, "Index out of range");
    char dummy = text[i]; // False positive
}

The analyzer sees b.cpp independently of a.cpp. The vast majority of C++ functions do return eventually, so it assumes my_infinite_loop will return. So it assumes that cond might be false and discovers that in this case text[i] will overflow.

CPP-4723 is aimed at addressing this limitation.

In the meantime, you have two options:

  • Define my_infinite_loop in b.cpp:
// b.cpp
void my_infinite_loop() {
  while (true) {
     // infinite loop
  }
}

#define TEXT_SIZE 4
#define ASSERT(cond, msg) if (!(cond)) my_infinite_loop()

void top() {
    char text[TEXT_SIZE] = { };
    int i = 4;

    ASSERT(i<4, "Index out of range");
    char dummy = text[i]; // No false positive
}

For example, you could include the inline definition of this function in the same header where you define ASSERT

// a.cpp
[[noreturn]] void my_infinite_loop() {
  while (true) {
     // infinite loop
  }
}

// b.cpp
[[noreturn]] void my_infinite_loop();

#define TEXT_SIZE 4
#define ASSERT(cond, msg) if (!(cond)) my_infinite_loop()

void top() {
    char text[TEXT_SIZE] = { };
    int i = 4;

    ASSERT(i<4, "Index out of range");
    char dummy = text[i]; // False positive
}

The [[noreturn]] annotation will tell the analyzer the my_infinite_loop will never return, so if cond is false, it will not go beyond my_infinite_loop call.

See also a similar issue in another thread:

2 Likes
7

Adding [[noreturn]] solved the issue. Thank you a lot!

11

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.