[missing diag] Detection of format string errors should be part of the regular C++ compile instead of /analyze - by Bruce Dawson

Status : 

  Fixed<br /><br />
		This item has been fixed in the current or upcoming version of this product.<br /><br />
		A more detailed explanation for the resolution of this particular item may have been provided in the comments section.


96
1
Sign in
to vote
ID 799869 Comments
Status Closed Workarounds
Type Bug Repros 13
Opened 9/6/2013 9:18:36 AM
Access Restriction Public

Description

We have been running VS 2012's C++ /analyze on most of our code for two years now. Because /analyze takes so long to run (up to five hours) we run it as a daily job on build machines. This means that if a developer checks in code that /analyze doesn't like it can take more than 24 hours before the developer is notified of the problem.

During this two year period I have found that one of the most common types of errors that /analyze finds in our code base is printf format string mismatches, such as this:

void TestPrint()
{
    // warning C6273: Non-integer passed as _Param_(2) when an integer is required in call to 'printf' Actual type: 'double'
    printf("The int value is %d\n", 2.0);
}

Detecting this type of problem can be done very quickly, without noticeably slowing down compilation, so these warnings should be detected during normal compiles. This would avoid roughly 45% of our /analyze failures. It would change the turnaround on these bugs from 24+ hours down to just seconds.

Clang and gcc detect format-string errors as part of their regular compilation -- VC++ needs to catch up.

/analyze is a great feature that finds many important bugs. However it's value is severely diminished by being so slow and noisy. Moving the warnings that are fast and 100% reliable to the regular compile would enormously increase the value of the static analysis. It would allow teams that never use /analyze to benefit from the format string checks, and it would let teams that do use /analyze reduce the number of errors that get checked in.
Sign in to post a comment.
Posted by Microsoft on 5/4/2015 at 4:03 PM
Thank you for providing this feedback to us. Given the time constraints, in the coming release we've only implemented the checking of format specifiers of standard printf/scanf functions. There will be a blog post about performed built-in checks. For the checking of user-defined functions that accept format strings, currently the user would still have to rely on the same mechanism as /analyze does.

Yuriy
Posted by UnitUniverse on 9/12/2013 at 8:05 AM
Yes, providen switchers to enable the detecting is an considerable idea. But, as i written previously, the printf extracting tail arguments with binary cutters, I'm doubt if they could implement such kind of checking mechanism.
Posted by UnitUniverse on 9/12/2013 at 7:53 AM
"The argument "the compiler should only care about syntax" doesn't make sense to me. Most warnings are about semantics, not syntax: unused variables, sign mismatches, precision loss due to implicit conversions etc. Format string misuse is an important class of errors that can be detected at compile time, so it should be."

-- It's different. If the compiler decides to warning and detecting unused variables, It will analize everywhere which can such routine be applied; If the compiler decides to detecting mismatched signs, then anywhere that has the integer comparison expressions will be on the proceeding lists; If ...
However, format strings are just regular strings when they are not as the argument of the printf. So if the compiler was modified to enable these warnings, activating special green lights to printf only was not fair, when someone personally made a function with the content of the printf but being given another name rather than 'printf' still get no warning. If such verification will be implemented then every function in the formal of printf must also be proceed rather than printf or such kind of functions on the standard library only.
Posted by Mihnea Balta on 9/10/2013 at 5:23 AM
This should be obvious, but it seems to elude some people: it's not about modifying the compiler to do "special processing of printf". It's about being able to tell the compiler that a certain function argument is a format string, and getting warnings when it's being used improperly. Lots of people have their own printf-like logging functions, and you want to get warnings for those too. GCC allows this through __attribute__((format)): http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html. VC has
_Printf_format_string_ (which is slightly less flexible, but whatever), but it's only taken into account during static analysis. Bruce is saying the checking is cheap enough that it can be performed during normal compilation.

The argument "the compiler should only care about syntax" doesn't make sense to me. Most warnings are about semantics, not syntax: unused variables, sign mismatches, precision loss due to implicit conversions etc. Format string misuse is an important class of errors that can be detected at compile time, so it should be.
Posted by Bruce Dawson on 9/9/2013 at 8:02 AM
Replying to UnitUniverse: printf is inherently problematic (the VC++ implementation has nothing to do with this). /analyze is invaluable at finding bugs in the usage of printf, and this would make /analyze (perhaps /analyze:fast) much more valuable. Switching all usage of printf style functions to a different design is not practical for many developers. Many parts of C++ are error prone, and the purpose of /analyze is to manage this risk. See this article for details:

http://randomascii.wordpress.com/2013/06/24/two-years-and-thousands-of-bugs-of-static-analysis/
Posted by Microsoft on 9/9/2013 at 2:49 AM
Thank you for submitting feedback on Visual Studio and .NET Framework. Your issue has been routed to the appropriate VS development team for investigation. We will contact you if we require any additional information.
Posted by UnitUniverse on 9/6/2013 at 11:16 PM
printf is a function, just like any other functions, rather than part of the C++ syntax system, so i think it's unecessary(actually it shouldn't either) to modify compiler to do special process to it.
The real problem is that the printf in vc internally uses a kind of binary extraction(binary masks) method to get each extended arguments. What i suggest is to use the variadic-template version rather than C-styled functions in such cases to avoid the category verification mechanism being by-passed.
Posted by Microsoft on 9/6/2013 at 9:51 AM
Thank you for your feedback, we are currently reviewing the issue you have submitted. If this issue is urgent, please contact support directly(http://support.microsoft.com)