Merge pull request #11034 from WalterBright/printfParams2

switch printf detection to pragma(printf)

changelog/printfParams2.dd

@@ -1,4 +1,4 @@
-# Validate printf and scanf (variants too) arguments against format specifiers
+Validate printf and scanf (variants too) arguments against format specifiers
 
 Follows the C99 specification 7.19.6.1 for printf and 7.19.6.2 for scanf.
 
@@ -24,28 +24,46 @@ No attempt is made to fix the arguments or the format string.
 
 In order to use non-Standard printf/scanf formats, an easy workaround is:
 
-```
+---
 printf("%k\n", value);  // error: non-Standard format k
-```
-```
+---
+---
 const format = "%k\n";
 printf(format.ptr, value);  // no error
-```
+---
 
 Most of the errors detected are portability issues. For instance,
 
-```
+---
 string s;
 printf("%.*s\n", s.length, s.ptr);
 printf("%d\n", s.sizeof);
 ulong u;
 scanf("%lld%*c\n", &u);
-```
+---
 should be replaced with:
-```
+---
 string s;
 printf("%.*s\n", cast(int) s.length, s.ptr);
 printf("%zd\n", s.sizeof);
 ulong u;
 scanf("%llu%*c\n", &u);
-```
+---
+
+Printf-like and scanf-like functions are detected by prefixing them
+with `pragma(printf)` for printf-like functions or `pragma(scanf)` for scanf-like functions.
+
+In addition to
+the pragma, the functions must conform to the following characteristics:
+
+$(OL
+$(LI be `extern (C)` or `extern (C++)`)
+$(LI have the format parameter declared as `const(char)*`)
+$(LI have the format parameter immediately precede the `...` for non-v functions,
+or immediately precede the `va_list` parameter (which is the last parameter for "v"
+variants of `printf` and `scanf`))
+)
+
+which enables automatic detection of the format string argument and the argument list.
+
+Checking of "v" format strings is not implemented yet.

src/dmd/attrib.d

@@ -846,6 +846,18 @@ extern (C++) final class PragmaDeclaration : AttribDeclaration
             }
             return createNewScope(sc, sc.stc, sc.linkage, sc.cppmangle, sc.protection, sc.explicitProtection, sc.aligndecl, inlining);
         }
+        if (ident == Id.printf || ident == Id.scanf)
+        {
+            auto sc2 = sc.push();
+
+            if (ident == Id.printf)
+                // Override previous setting, never let both be set
+                sc2.flags = (sc2.flags & ~SCOPE.scanf) | SCOPE.printf;
+            else
+                sc2.flags = (sc2.flags & ~SCOPE.printf) | SCOPE.scanf;
+
+            return sc2;
+        }
         return sc;
     }
 

src/dmd/dscope.d

@@ -62,11 +62,16 @@ enum SCOPE
 
     fullinst      = 0x10000,  /// fully instantiate templates
     alias_        = 0x20000,  /// inside alias declaration.
+
+    // The following are mutually exclusive
+    printf        = 0x4_0000, /// printf-style function
+    scanf         = 0x8_0000, /// scanf-style function
 }
 
 // Flags that are carried along with a scope push()
 enum SCOPEpush = SCOPE.contract | SCOPE.debug_ | SCOPE.ctfe | SCOPE.compile | SCOPE.constraint |
-                 SCOPE.noaccesscheck | SCOPE.onlysafeaccess | SCOPE.ignoresymbolvisibility;
+                 SCOPE.noaccesscheck | SCOPE.onlysafeaccess | SCOPE.ignoresymbolvisibility |
+                 SCOPE.printf | SCOPE.scanf;
 
 struct Scope
 {

src/dmd/dsymbolsem.d

@@ -2009,6 +2009,8 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
         }
         else if (pd.ident == Id.printf || pd.ident == Id.scanf)
         {
+            if (pd.args && pd.args.dim != 0)
+                pd.error("takes no argument");
             goto Ldecl;
         }
         else if (global.params.ignoreUnsupportedPragmas)
@@ -3481,6 +3483,62 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
             }
         }
 
+        if (const pors = sc.flags & (SCOPE.printf | SCOPE.scanf))
+        {
+            /* printf/scanf-like functions must be of the form:
+             *    extern (C/C++) T printf([parameters...], const(char)* format, ...);
+             * or:
+             *    extern (C/C++) T vprintf([parameters...], const(char)* format, va_list);
+             */
+
+            static bool isPointerToChar(Parameter p)
+            {
+                if (auto tptr = p.type.isTypePointer())
+                {
+                    return tptr.next.ty == Tchar;
+                }
+                return false;
+            }
+
+            static bool isVa_list(Parameter p)
+            {
+                // What it's actually pointing to depends on the target
+                return p.type.isTypePointer() !is null;
+            }
+
+            const nparams = f.parameterList.length;
+            if ((f.linkage == LINK.c || f.linkage == LINK.cpp) &&
+
+                (f.parameterList.varargs == VarArg.variadic &&
+                 nparams >= 1 &&
+                 isPointerToChar(f.parameterList[nparams - 1]) ||
+
+                 f.parameterList.varargs == VarArg.none &&
+                 nparams >= 2 &&
+                 isPointerToChar(f.parameterList[nparams - 2]) &&
+                 isVa_list(f.parameterList[nparams - 1])
+                )
+               )
+            {
+                funcdecl.flags |= (pors == SCOPE.printf) ? FUNCFLAG.printf : FUNCFLAG.scanf;
+            }
+            else
+            {
+                const p = (pors == SCOPE.printf ? Id.printf : Id.scanf).toChars();
+                if (f.parameterList.varargs == VarArg.variadic)
+                {
+                    funcdecl.error("`pragma(%s)` functions must be `extern(C) %s %s([parameters...], const(char)*, ...)"
+                                   ~ " not `%s`",
+                        p, f.next.toChars(), funcdecl.toChars(), funcdecl.type.toChars());
+                }
+                else
+                {
+                    funcdecl.error("`pragma(%s)` functions must be `extern(C) %s %s([parameters...], const(char)*, va_list)",
+                        p, f.next.toChars(), funcdecl.toChars());
+                }
+            }
+        }
+
         id = parent.isInterfaceDeclaration();
         if (id)
         {

src/dmd/expressionsem.d

@@ -2167,35 +2167,24 @@ private bool functionParameters(const ref Loc loc, Scope* sc,
 
     /* If calling C scanf(), printf(), or any variants, check the format string against the arguments
      */
-    if (tf.linkage == LINK.c && fd)
+    if (fd && fd.flags & FUNCFLAG.printf && tf.parameterList.varargs == VarArg.variadic)
     {
-        int paramOffset = 0;
-        bool function(ref const(Loc) loc, scope const(char[]) format, scope Expression[] args) chkFn;
-
-        if (fd.ident == Id.printf)
+        if (auto se = (*arguments)[nparams - 1].isStringExp())
         {
-            paramOffset = 1;
-            chkFn = &checkPrintfFormat;
+            checkPrintfFormat(se.loc, se.peekString(), (*arguments)[nparams .. nargs]);
         }
-        else if (fd.ident == Id.scanf)
+    }
+    else if (fd && fd.flags & FUNCFLAG.scanf && tf.parameterList.varargs == VarArg.variadic)
+    {
+        if (auto se = (*arguments)[nparams - 1].isStringExp())
         {
-            paramOffset = 1;
-            chkFn = &checkScanfFormat;
-        }
-        else if (fd.ident == Id.sscanf || fd.ident == Id.fscanf)
-        {
-            paramOffset = 2;
-            chkFn = &checkScanfFormat;
-        }
-
-        if (paramOffset && nparams >= paramOffset)
-        {
-            if (auto se = (*arguments)[paramOffset - 1].isStringExp())
-            {
-                chkFn(se.loc, se.peekString(), (*arguments)[paramOffset .. nargs]);
-            }
+            checkScanfFormat(se.loc, se.peekString(), (*arguments)[nparams .. nargs]);
         }
     }
+    else
+    {
+        // TODO: not checking the "v" functions yet (for those, check format string only, not args)
+    }
 
     /* Remaining problems:
      * 1. order of evaluation - some function push L-to-R, others R-to-L. Until we resolve what array assignment does (which is

src/dmd/func.d

@@ -166,6 +166,8 @@ enum FUNCFLAG : uint
     inferScope       = 0x40,   /// infer 'scope' for parameters
     hasCatches       = 0x80,   /// function has try-catch statements
     compileTimeOnly  = 0x100,  /// is a compile time only function; no code will be generated for it
+    printf           = 0x200,  /// is a printf-like function
+    scanf            = 0x400,  /// is a scanf-like function
 }
 
 /***********************************************************

src/dmd/id.d

@@ -129,12 +129,8 @@ immutable Msgtable[] msgtable =
     { "_assert", "assert" },
     { "_unittest", "unittest" },
     { "_body", "body" },
-    { "fprintf" },
     { "printf" },
-    { "sprintf" },
-    { "fscanf" },
     { "scanf" },
-    { "sscanf" },
 
     { "TypeInfo" },
     { "TypeInfo_Class" },

src/dmd/scope.h

@@ -57,6 +57,10 @@ class CPPNamespaceDeclaration;
 #define SCOPEfullinst       0x10000 // fully instantiate templates
 #define SCOPEalias          0x20000 // inside alias declaration
 
+// The following are mutually exclusive
+#define SCOPEprintf         0x40000 // printf-style function
+#define SCOPEscanf          0x80000 // scanf-style function
+
 struct Scope
 {
     Scope *enclosing;           // enclosing Scope

test/fail_compilation/format.d

@@ -0,0 +1,44 @@
+/*
+TEST_OUTPUT:
+---
+fail_compilation/format.d(101): Error: function `format.printf1` `pragma(printf)` functions must be `extern(C) void printf1([parameters...], const(char)*, ...) not `void(const(char)*, ...)`
+fail_compilation/format.d(102): Error: function `format.printf2` `pragma(printf)` functions must be `extern(C) int printf2([parameters...], const(char)*, ...) not `extern (C) int(const(int)*, ...)`
+fail_compilation/format.d(103): Error: function `format.printf3` `pragma(printf)` functions must be `extern(C) int printf3([parameters...], const(char)*, va_list)
+fail_compilation/format.d(104): Error: function `format.printf4` `pragma(printf)` functions must be `extern(C) int printf4([parameters...], const(char)*, ...) not `extern (C) int(const(char)*, int, ...)`
+---
+ */
+
+#line 100
+
+pragma(printf)           void printf1(const(char)*, ...);
+pragma(printf) extern (C) int printf2(const(int )*, ...);
+pragma(printf) extern (C) int printf3(const(char)*);
+pragma(printf) extern (C) int printf4(const(char)*, int, ...);
+
+pragma(printf) extern (C) int printf5(const(char)*, ...);
+pragma(printf) extern (C) int printf6(immutable(char)*, ...);
+pragma(printf) extern (C) int printf7(char*, ...);
+
+/*
+TEST_OUTPUT:
+---
+fail_compilation/format.d(203): Error: function `format.vprintf1` `pragma(printf)` functions must be `extern(C) void vprintf1([parameters...], const(char)*, va_list)
+fail_compilation/format.d(204): Error: function `format.vprintf2` `pragma(printf)` functions must be `extern(C) int vprintf2([parameters...], const(char)*, va_list)
+fail_compilation/format.d(205): Error: function `format.vprintf3` `pragma(printf)` functions must be `extern(C) int vprintf3([parameters...], const(char)*, va_list)
+fail_compilation/format.d(206): Error: function `format.vprintf4` `pragma(printf)` functions must be `extern(C) int vprintf4([parameters...], const(char)*, va_list)
+---
+ */
+
+#line 200
+
+import core.stdc.stdarg;
+
+pragma(printf)           void vprintf1(const(char)*, va_list);
+pragma(printf) extern (C) int vprintf2(const(int )*, va_list);
+pragma(printf) extern (C) int vprintf3(const(char)*);
+pragma(printf) extern (C) int vprintf4(const(char)*, int, va_list);
+
+pragma(printf) extern (C) int vprintf5(const(char)*, va_list);
+pragma(printf) extern (C) int vprintf6(immutable(char)*, va_list);
+pragma(printf) extern (C) int vprintf7(char*, va_list);
+