cpp:S5270 seems to be reported incorrectly

[Tom_Isaacson2 (Tom Isaacson)]

• SonarQube Server v9.9.7 (build 96285)

I have a custom logging function which has been working fine for years. SonarQube is now telling me this code has a major bug.

    template <class... Args>
    void log(int priority, const char* format, const char* file, int line, Args... args) {
        // Add time, file and line to start of format string
        const auto system_time = std::chrono::system_clock::now();
        const std::chrono::milliseconds ms =
            std::chrono::duration_cast<std::chrono::milliseconds>(system_time.time_since_epoch()) % 10000;
        // Try a buffer, see if it fits
        char time_buffer[80];
        char buffer[1024];
        tm now_tm;
        {
            const std::time_t time = std::chrono::system_clock::to_time_t(system_time);
            localtime_r(&time, &now_tm);
        }
        std::strftime(time_buffer, 80, "%Y-%m-%d %H:%M:%S", &now_tm);
        int size = snprintf(buffer, 1024, "[%s.%04ld] [%s:%d] %s\n", time_buffer, ms.count(), file, line, format);
        std::string full_format;
        if (size < 1024) {
            full_format = buffer;
        }
        else{
            full_format.resize(size);
            snprintf(&full_format[0], size + 1, "[%s.%04ld] [%s:%d] %s\n", time_buffer, ms.count(), file, line, format);
        }

        // Try a buffer, see if it fits
        // Suppress "format string argument" error for the next line
        // codeql: [query]@off
        // lgtm [cpp/printf-nonliteral-format-string]
        size = snprintf(buffer, 1024, full_format.c_str(), args...);

On this line I get the error: cannot pass object of non-trivial type ‘std::basic_string’ through variadic function; call will abort at runtime and the args... parameter is underlined. I’ve read the “Why is this an issue?” section and the link to cpp:S5270 User-defined types should not be passed as variadic arguments but neither make sense in this context - I’m not passing a string directly.

[Tomasz_Kaminski (Tomasz Kamiński)]

Hi @Tom_Isaacson2,

This rule is raises issue on template instantiations, and I suspect that one of the args... that are passed to snprintf is of std::string type, as in following example:

template<typename... Args>
void foo(Args&... args) {
  std::printf("%s", args...);
}

int main() {
    std::string s;
    foo(s);  // `std::string` is passed
}

I would expect this to be caused by missing c_str() for the std::string argument on one of the log invocations.

To detect the issue, I would suggest adding following inside the function:

static_assert(!(std::is_same_v<std::string, Args> || ...), 
              "Call `c_str` on std::string before passing to this function.");

Such static assert in this function, will detect any current and future violations during the compilation time. As shown here.

[Tom_Isaacson2 (Tom Isaacson)]

Fantastic, that worked. Turns out there was one logging call that was using a string instead of calling c_str(). I’m surprised that SonarQube didn’t identify that line rather than the logging but adding the assert stops that happening again.

[Tomasz_Kaminski (Tomasz Kamiński)]

I’m surprised that SonarQube didn’t identify that line rather than the logging but adding the assert stops that happening again.

In this rule we opted for wide coverage, so we detect any non-trivial type being passed.

But generally, that particular issue is caused by challenges with reporting from template instantiation. The template instantiations may be used in many places of program, so either:

• we raise a large number of issues
• we point to the uses as secondary, but the issue does not disappear after one is fixed

We are not yet aware of better solution for this problem, but we believed that this rule is still valuable, even with sub-par reporting.