Extending printf format string checking to cover ObjC format strings in NSLog

Hello,

Current printf format string checking is very useful but it's limited to printf -style functions which use C format strings. I would like to propose extending this functionality to cover Objective-C format strings in NSLog function. A simple implementation of this extension is attached to this message.

Speaking of format strings in ObjC, are there any plans to extend format string checking to cover ObjC methods which accept format strings as one of their arguments, such as various -[NSString initWithFormat:...] methods?

Best regards,
Nikita Zhuk

nslog-format-string-check.txt (3.47 KB)

Note that '%@' is also supported in CoreFoundation fonctions: CFStringCreateWithFormat() for example. If the format string checking is extended, you may considere thoses cases and not limite it to objc sources.

Thanks Nikita! Applied:

   http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20080616/006125.html

This is a good introductory patch for ObjC format strings. A couple points worth thinking about:

1) NSLog uses the "NSString" attribute to document it has a "format-string" interface. From Foundation.h:

   extern void NSLog(NSString *format, ...) __attribute__((format(__NSString__, 1, 2)));

Instead of checking for "NSLog", we should probably generalize the checking to just use the NSString attribute. Right now we parse the NSString attribute, and create a FormatAttr object to represent that attribute. We should probably generalize FormatAttr (or add a flag) to indicate that it came from __NSString__.

2) Format string checking for __NSString__ is not exactly the same as printf checking. According to Apple's documentation, the set of format specifiers is not the same as for printf:

   http://developer.apple.com/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html#//apple_ref/doc/uid/TP40004265

For example, Objective-C format strings support things like %D, %qX, etc., but these are not supported by printf. printf also supports %n (which is a security hole), but Objective-C format strings do not support %n. We'll need to extend the format-string checking to distinguish between these two modes.

FYI: The C99 documentation for printf format-string arguments is in section 7.19.6.1 of the C99 Standard.

Overall, our format string checking needs to be greatly improved, and include things like more comprehensive type checking between specifiers and arguments, etc. I plan on working on this over the next couple days.

Thanks for the patch!

Ted

This functionality should eventually go in as well. The set of such CF functions should be small. Other functions that use Objective-C style format strings should generally be labeled with the __NSString__ attribute. Unfortunately, it looks like the "initWithFormat:" methods do not have this attribute:

(kremenek@grue:Sema)$ pwd
/Users/kremenek/llvm/tools/clang/test/Sema
(kremenek@grue:Sema)$ clang -E cocoa.m | grep initWithFormat | grep String
- (id)initWithFormat:(NSString *)format, ...;
- (id)initWithFormat:(NSString *)format arguments:(va_list)argList;
- (id)initWithFormat:(NSString *)format locale:(id)locale, ...;
- (id)initWithFormat:(NSString *)format locale:(id)locale arguments:(va_list)argList;

Fortunately, it looks like the set of methods to hardwire support for is small.

I wasn't aware of format(__NSString__) attribute, but it would clearly be beneficial to recognize format NSStrings based on it.

Actually the NSLog() function seems to be the only function or method in AppKit & Foundation headers which uses this attribute. I've found several functions and methods which use format NSStrings and don't use the format(__NSString__) attribute: 12 functions in AppKit/NSPanel.h, 4 functions in CoreFoundation/CFString.h, 4 methods in Foundation/NSException.h and 8 methods in Foundation/NSString.h.
So hardcoding both function and method names seems inevitable.

- Nikita

Agreed. Unfortunately, this kind of redundancy is necessary. Within Apple I can file bug reports to get the headers fixed, but clearly that only works for code going forward.