On Mon, Jan 30, 2012 at 4:02 PM, Jean-Daniel Dupas <[email protected]> wrote: > > Le 30 janv. 2012 à 22:10, Eli Friedman a écrit : > > > On Mon, Jan 30, 2012 at 11:46 AM, Jean-Daniel Dupas > <[email protected]> wrote: > > Author: jddupas > > Date: Mon Jan 30 13:46:17 2012 > > New Revision: 149268 > > > URL: http://llvm.org/viewvc/llvm-project?rev=149268&view=rev > > Log: > > Disable "non literal format string" for NSString that result from a macro > expansion. > > This is to prevent diagnostic when using NSLocalizedString or > CFCopyLocalizedString > > macros which are usually used in place of NS and CF strings literals. > > > > Modified: > > cfe/trunk/lib/Sema/SemaChecking.cpp > > cfe/trunk/test/SemaObjC/format-strings-objc.m > > > Modified: cfe/trunk/lib/Sema/SemaChecking.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=149268&r1=149267&r2=149268&view=diff > > ============================================================================== > > --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) > > +++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jan 30 13:46:17 2012 > > @@ -1586,6 +1586,13 @@ > > format_idx, firstDataArg, Type)) > > return; // Literal format string found, check done! > > > + // Do not emit diag when the string param is a macro expansion and the > > + // format is either NSString or CFString. This is a hack to prevent > > + // diag when using the NSLocalizedString and CFCopyLocalizedString macros > > + // which are usually used in place of NS and CF string literals. > > + if (Type == FST_NSString && Args[format_idx]->getLocStart().isMacroID()) > > + return; > > + > > // If there are no arguments specified, warn with -Wformat-security, > otherwise > > // warn only with -Wformat-nonliteral. > > if (NumArgs == format_idx+1) > > > Modified: cfe/trunk/test/SemaObjC/format-strings-objc.m > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-strings-objc.m?rev=149268&r1=149267&r2=149268&view=diff > > ============================================================================== > > --- cfe/trunk/test/SemaObjC/format-strings-objc.m (original) > > +++ cfe/trunk/test/SemaObjC/format-strings-objc.m Mon Jan 30 13:46:17 2012 > > @@ -1,4 +1,4 @@ > > -// RUN: %clang_cc1 -fsyntax-only -verify %s > > +// RUN: %clang_cc1 -Wformat-nonliteral -fsyntax-only -verify %s > > > //===----------------------------------------------------------------------===// > > // The following code is reduced using delta-debugging from > > @@ -107,3 +107,11 @@ > > NSString * ns3 = ns1; > > NSLog(ns3); // expected-warning {{format string is not a string literal}}} > > } > > + > > +// Do not emit warnings when using NSLocalizedString > > +extern NSString *GetLocalizedString(NSString *str); > > +#define NSLocalizedString(key) GetLocalizedString(key) > > + > > +void check_NSLocalizedString() { > > + [Foo fooWithFormat:NSLocalizedString(@"format"), @"arg"]; // no-warning > > +} > > > I'm not sure this is the right approach... see also > http://llvm.org/bugs/show_bug.cgi?id=3814 , which is essentially the > same issue. > > > gettext convention are not the same as Cocoa ones. > > From gettext man page: "By convention, [msgid] is the English version of > the message, with non-ASCII characters replaced by ASCII approximations." > which means that the gettext macro argument should be a valid format string > when used as printf argument. > If so, the gettext functions declaration should use the format_arg() > attribute which is already handled in clang. > > Indeed, using a recent version of gettext (which use format_arg), I get the > correct behavior: > > ---- fmt.c --- > #include <stdio.h> > #include <libintl.h> > #include <gettext-po.h> > > int test(int arg) { > return printf(gettext("this is a format string: %s"), arg); > } > --------------- > > fmt.c:7:51: warning: format specifies type 'char *' but the argument has > type 'int' > return printf(gettext("this is a format string: %s"), arg); > ~^ ~~~
Oh, cool; didn't realize that was there. > The problem with Cocoa localization macro is that there is not such > convention, and it is very common to use a key that is not a version of the > localized string at all. So even if they were declared using format_arg > (which is not the case), it would not help. The issue I'm worried about is that -Wformat-security is supposed to warn about any format argument we can't reason about. The behavior with your patch subverts the warning; we don't know that whatever random macro is used in the first argument is doing something safe. -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
