Allow printf-like calls in checked scopes if they pass -Wformat validation

[secure-sw-dev-bot]

This issue was copied from checkedc/checkedc-clang#1160

---

Checked C currently doesn't allow code in a checked scope to call variadic functions. The most commonly used variadic functions are ones such as printf that take a format string; in principle, there could be other variadic functions, but I don't know if there are any others that are commonly used in real code.

In both Microsoft's and CCI's ports of example programs, the practice so far has been to wrap every call to a printf-like function in an _Unchecked { ... } block. printf calls are frequent enough that this becomes a significant limitation in the ability to verify the spatial memory safety of a program with Checked C, especially because we loosen the checking of the entire call expression, including operations within the argument expressions that have nothing to do with passing the resulting argument values to printf.

I think Clang's existing format validation code (used by -Wformat) already checks most of the conditions that would be needed to ensure that a call to a printf-like function is safe. It may need minor extensions for Checked C, such as checking that the argument corresponding to a %s in the format string is an _Nt_array_ptr<const char> rather than (for example) a _Ptr<const char>. But then it should be safe to allow a printf call in a checked scope if it completely passes format validation; any format validation failure would be reported as an error regardless of whether -Wformat is turned on in the compiler options. The same would be true for a call to any variadic function with __attribute__((format(printf))). There would be no change to the treatment of printf calls in unchecked scopes (they would continue to produce -Wformat warnings only if -Wformat is turned on), except that the Checked C extensions might produce additional -Wformat warnings (e.g., if a _Ptr<const char> is passed to a %s in an unchecked scope), which would be a useful enhancement in its own right. A call to a variadic function without __attribute__((format(printf))) from a checked scope would continue to produce a "cannot use a variable arguments function in a checked scope or function" error. And defining a variadic function in a checked scope would continue to be an error regardless of whether it has __attribute__((format(printf))), because I assume it would be infeasible for the compiler to check that the implementation of a printf-like function (using va_list, etc.) is safe with respect to the format string.

The analogous enhancement for __attribute__((format(scanf))) could be made if there is sufficient demand.

Mike says you're already planning to work on some kind of support for printf calls in checked scopes, but I didn't see an existing issue, so I thought I would go ahead and file one. And maybe you'll find some of my design proposal above helpful if you haven't thought through all of it already.

[secure-sw-dev-bot]

Comment from @sulekhark:

Yes, we are planning to extend the format validation that Clang does for -Wformat to perform Checked-C-relevant validations in the same way as you are suggesting.

[secure-sw-dev-bot]

Comment from @mgrang:

checkedc/checkedc-clang#1174 added support for calling variadic functions like printf/scanf, etc in checked scopes. Bounds checking of arguments to variadic functions and handling of several format specifiers is yet to be implemented. checkedc/checkedc-clang#1178 tracks these.

[secure-sw-dev-bot]

Comment from @mattmccutchen-cci:

Thanks! It will be great to start getting experience with the printf checking in our ports, despite the current holes in it.

A side issue: as I briefly suggested in my original post, IMO the Checked-C-specific format string checks (currently in CheckVarargsInCheckedScope) should generate at least -Wformat warnings (or maybe even errors) in unchecked scopes too, if checked pointers or arrays are being used. (No new diagnostics would be generated in plain-C code that does not use checked pointers or arrays.) This would be consistent with the passing of arguments to non-variadic functions and would help catch problems sooner. In essence, we want to check a printf argument corresponding to a %s as if the declared parameter type were const char * : itype(_Nt_array_ptr<const char>), and so forth. I realize this is outside the scope of the original title of this issue. Do you prefer that I broaden the title or file a separate issue?

[dtarditi]

I opened a new issue for the last comment. This feature is implemented, but has some holes mentioned in #1174.