sprintf precision typecasting leading to compilation warning

user_14174771 0
Viewed 158

I have the below C++ code which issues a compilation warning as shown below.

Case 1:

char temp_buffer[80];
double **data;
....
sprintf(temp_buffer, "%.*g", sizeof(temp_buffer), **data);

Compilation warning:

extension.C:1031:41: warning: field precision specifier '.*' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Wformat=]
 1031 |                 sprintf(temp_buffer, "%.*g", sizeof(temp_buffer), **data);
      |                                       ~~^~   ~~~~~~~~~~~~~~~~~~~
      |                                         |    |
      |                                         int  long unsigned int

I am fixing the warning by type-casting the sizeof to int like below.

Case 2:

char temp_buffer[80];
double **data;
...
sprintf(temp_buffer, "%.*g", (int)sizeof(temp_buffer), **data);

But now I get the below warning:

extension/extension.C:1031:39: warning: \u2018%.*g\u2019 directive writing between 1 and 87 bytes into a region of size 80 [-Wformat-overflow=]
 1031 |                 sprintf(temp_buffer, "%.*g", (int)sizeof(temp_buffer), **data);
      |                                       ^~~~
extension/extension.C:1031:38: note: assuming directive output of 86 bytes
 1031 |                 sprintf(temp_buffer, "%.*g", (int)sizeof(temp_buffer), **data);
      |                                      ^~~~~~
In file included from /usr/include/stdio.h:873,
                 from /opt/python/python-3.9/include/python3.9/Python.h:25,
                 from /codemill/dhamodha/projects/base/src/lib/despython/desnumpyinit.h:19,
                 from extension/extension.C:8:
/usr/include/bits/stdio2.h:36:34: note: \u2018__builtin___sprintf_chk\u2019 output between 2 and 88 bytes into a destination of size 80
   36 |   return __builtin___sprintf_chk (__s, __USE_FORTIFY_LEVEL - 1,
      |          ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   37 |       __bos (__s), __fmt, __va_arg_pack ());

I was under the assumption that '*' in the precision internally coerces the long unsigned int to int. But had that been the case the new warning wouldn't happen if I introduce sizeof typecasting.

Can someone help me in understand what's going on and solve the warnings?

Case 3:

Say I modify the temp_buffer to size 8, then I see both warnings:

extension/extension.C:1033:20: warning: \u2018%.*g\u2019 directive writing between 1 and 310 bytes into a region of size 8 [-Wformat-overflow=]
 1033 |   sprintf(temp_b, "%.*g", sizeof(temp_buffer), **data);
      |                    ^~~~
extension/extension.C:1033:19: note: assuming directive output of 12 bytes
 1033 |   sprintf(temp_b, "%.*g", sizeof(temp_buffer), **data);
      |                   ^~~~~~
In file included from /usr/include/stdio.h:873,
                 from /opt/python/python-3.9/include/python3.9/Python.h:25,
                 from /codemill/dhamodha/projects/base/src/lib/despython/desnumpyinit.h:19,
                 from extension/extension.C:8:
/usr/include/bits/stdio2.h:36:34: note: \u2018__builtin___sprintf_chk\u2019 output between 2 and 311 bytes into a destination of size 8
   36 |   return __builtin___sprintf_chk (__s, __USE_FORTIFY_LEVEL - 1,
      |          ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   37 |       __bos (__s), __fmt, __va_arg_pack ());
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
precision printf integer c++
2 Answers

You could assign the width before use:

int width = sizeof(temp_buffer);
sprintf(temp_buffer, "%.*g", width, **data);

The warning would go away, that however would not solve the underlying problem. If you want to avoid potential buffer overflow, a reasonable option would be to use the correct size:

sprintf(temp_buffer, "%.*g", (int)(sizeof(temp_buffer)) - 8, **data);

In your original code what you're saying is that you want the number of decimal places to be the size of the buffer, that leaves no space for the signs, scientific notation and the nul byte. You need to pass a decimal part size small enough so that all of those can fit the the destination buffer.

Even though the compiler mentions 7 bytes I believe 8 bytes would be prefereable, 2 for the signs, 1 for the dot, 1 for the e, 3 for the exponent and 1 for the nul byte.

Though I think having a buffer large enough to have the longest possible representation could be safer, given that the longest possible representation of a double can be as much as 758 bytes (though this may not be the case for all compilers), a buffer that can take 800 bytes would be able to deal with this, assuming the above byte count, consistent with the compilation with gcc 11.3.

That all being said the correct way to go about this is to use the much safer snprintf which allows you to pass the maximum buffer size as argument:

snprintf(buffer, sizeof buffer, "%.*g", (int)sizeof buffer - 8, **data);

As for your different use cases, the behavior is undefined without the cast because you do not pass the correct type of arguments to a ... parameter as is sprintf third parameter (in most cases the size of an unsigned long is larger than int), as such the compiler is not mandated to issue any warnings. The fact that it does can be considered a courtesy, clang for example issues no warnings, which is perfectly fine as per the standard definition of undefined behavior linked above.

To know the specific reason why in some cases there are the two different warnings and in others only one is a matter that is specific to the compiler, I consider it to be speculative to try to figure out why this is so.

Regarding the overflow values issued in the warnings, I found it curious and asked a question regarding it, the answers are elucidative:

user_68659320

Can someone help me in understand what's going on and solve the warnings?

With sprintf(temp_buffer, "%.*g", precision, **data);, precision controls the number of digits in the significand of exponential display.

Consider sprintf(temp_buffer, "%.*g", 5, 1234567e10); would form "1.2346e+16".

Rather than give a precession that is the size of the buffer, account for other characters like the sign, decimal point, 'e', and exponent (for common double, might be 4 characters like "-300") and the null character. In this case 8.

#define G_OVERHEAD 8
sprintf(temp_buffer, "%#.*g", (int) sizeof temp_buffer - G_OVERHEAD, **data);

This code is risky as G_OVERHEAD may be larger. Consider startup code that determines worst case, such as the following.

int g_overhead = snprintf(0, 0, "%#.*g", 2, -DBL_TRUE_MIN) + 1 /*\0*/ - 2 /*Precision*/;

// Common result: 8 for -4.9e-324
user_24103590
Related