[PATCH] restore CFString handling in attribute format (PR 88638)

[PATCH] restore CFString handling in attribute format (PR 88638)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

The improved handling of attribute positional arguments added
in r266195 introduced a regression on Darwin where attribute
format with the CFString archetype accepts CFString* parameter
types in positions where only char* would otherwise be allowed.
This is specific to Darwin so it didn't show up in tests.
The attached patch restores the feature.

r266195 also replaced an error issued for an invalid attribute
format parameter with just a warning.  On reflection I think
that was a mistake and an error is more appropriate.  The patch
also restores the error.

Finally, I've adjusted the Darwin Format Checks documentation
to better match the effects of the archetype (i.e., no checking
at call sites but no undefined behavior).

Besides testing natively on x86_64-linux I have also manually
run the affected tests with an x86_64-apple-darwin18 cross to
verify the changes (they are UNSUPPORTED under the test suite).

Martin

[-- Attachment #2: gcc-88638.diff --]
[-- Type: text/x-patch, Size: 16790 bytes --]

PR target/88638 - FAIL: fsf-nsstring-format-1.s on darwin

gcc/c-family/ChangeLog:

	PR target/88638
	* c-attribs.c (positional_argument): Call valid_format_string_type_p
	and issue errors if it fails.
	* c-common.h (valid_format_string_type_p): Declare.
	* c-format.c (valid_stringptr_type_p): Rename...
	(valid_format_string_type_p): ...to this and make extern.
	(handle_format_arg_attribute): Adjust to new name.
	(check_format_string): Same.

gcc/testsuite/ChangeLog:

	PR target/88638
	* gcc.dg/format/attr-8.c: New test.
	* gcc.dg/darwin-cfstring-format-1.c: Adjust diagnostics.
	* gcc.dg/format/attr-3.c: Same.
	* obj-c++.dg/fsf-nsstring-format-1.mm: Same.
	* objc.dg/fsf-nsstring-format-1.m: Same.

gcc/ChangeLog:

	PR target/88638
	* doc/extend.texi (Darwin Format Checks): Clarify.

Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 267580)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -631,21 +631,32 @@ positional_argument (const_tree fntype, const_tree
 	}
 
       /* Where the expected code is STRING_CST accept any pointer
-	 to a narrow character type, qualified or otherwise.  */
+	 expected by attribute format (this includes possibly qualified
+	 char pointers and, for targets like Darwin, also pointers to
+	 struct CFString).  */
       bool type_match;
-      if (code == STRING_CST && POINTER_TYPE_P (argtype))
-	{
-	  tree type = TREE_TYPE (argtype);
-	  type = TYPE_MAIN_VARIANT (type);
-	  type_match = (type == char_type_node
-			|| type == signed_char_type_node
-			|| type == unsigned_char_type_node);
-	}
+      if (code == STRING_CST)
+	type_match = valid_format_string_type_p (argtype);
       else
 	type_match = TREE_CODE (argtype) == code;
 
       if (!type_match)
 	{
+	  if (code == STRING_CST)
+	    {
+	      /* Reject invalid format strings with an error.  */
+	      if (argno < 1)
+		error ("%qE attribute argument value %qE refers to "
+		       "parameter type %qT",
+		       atname, pos, argtype);
+	      else
+		error ("%qE attribute argument %i value %qE refers to "
+		       "parameter type %qT",
+		       atname, argno, pos, argtype);
+
+	      return NULL_TREE;
+	    }
+
 	  if (argno < 1)
 	    warning (OPT_Wattributes,
 		     "%qE attribute argument value %qE refers to "
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h	(revision 267580)
+++ gcc/c-family/c-common.h	(working copy)
@@ -1335,6 +1335,9 @@ extern tree tm_mask_to_attr (int);
 extern tree find_tm_attribute (tree);
 extern const struct attribute_spec::exclusions attr_cold_hot_exclusions[];
 
+/* In c-format.c.  */
+extern bool valid_format_string_type_p (tree);
+
 /* A bitmap of flags to positional_argument.  */
 enum posargflags {
   /* Consider positional attribute argument value zero valid.  */
Index: gcc/c-family/c-format.c
===================================================================
--- gcc/c-family/c-format.c	(revision 267580)
+++ gcc/c-family/c-format.c	(working copy)
@@ -122,8 +122,8 @@ format_warning_at_char (location_t fmt_string_loc,
    The function returns true if strref points to any string type valid for the 
    language dialect and target.  */
 
-static bool
-valid_stringptr_type_p (tree strref)
+bool
+valid_format_string_type_p (tree strref)
 {
   return (strref != NULL
 	  && TREE_CODE (strref) == POINTER_TYPE
@@ -160,7 +160,7 @@ handle_format_arg_attribute (tree *node, tree atna
 	return NULL_TREE;
     }
 
-  if (!valid_stringptr_type_p (TREE_TYPE (type)))
+  if (!valid_format_string_type_p (TREE_TYPE (type)))
     {
       if (!(flags & (int) ATTR_FLAG_BUILT_IN))
 	error ("function does not return string type");
@@ -194,7 +194,7 @@ check_format_string (const_tree fntype, unsigned H
     }
 
   if (!ref
-      || !valid_stringptr_type_p (ref))
+      || !valid_format_string_type_p (ref))
     {
       if (!(flags & (int) ATTR_FLAG_BUILT_IN))
 	error ("format string argument is not a string type");
@@ -267,13 +267,21 @@ check_format_string (const_tree fntype, unsigned H
   gcc_unreachable ();
 }
 
-/* Verify EXPR is a constant, and store its value.
-   If validated_p is true there should be no errors.
+/* Under the control of FLAGS, verify EXPR is a valid constant that
+   refers to a positional argument ARGNO having a string type (char*
+   or, for targets like Darwin, a pointer to struct CFString) to
+   a function type FNTYPE declared with attribute ATNAME.
+   If valid, store the constant's integer value in *VALUE and return
+   the value.
+   If VALIDATED_P is true assert the validation is successful.
    Returns the converted constant value on success, null otherwise.  */
+
 static tree
 get_constant (const_tree fntype, const_tree atname, tree expr, int argno,
 	      unsigned HOST_WIDE_INT *value, int flags, bool validated_p)
 {
+  /* Require the referenced argument to have a string type.  For targets
+     like Darwin, also accept pointers to struct CFString.  */
   if (tree val = positional_argument (fntype, atname, expr, STRING_CST,
 				      argno, flags))
     {
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 267580)
+++ gcc/doc/extend.texi	(working copy)
@@ -22368,10 +22368,12 @@ bit-fields.  See the Solaris man page for @code{cm
 @node Darwin Format Checks
 @subsection Darwin Format Checks
 
-Darwin targets support the @code{CFString} (or @code{__CFString__}) in the format
-attribute context.  Declarations made with such attribution are parsed for correct syntax
-and format argument types.  However, parsing of the format string itself is currently undefined
-and is not carried out by this version of the compiler.
+In addition to the full set of archetypes, Darwin targets also support
+the @code{CFString} (or @code{__CFString__}) archetype in the @code{format}
+attribute.  Declarations with this archetype are parsed for correct syntax
+and argument types.  However, parsing of the format string itself and
+validating arguments against it in calls to such functions is currently
+not performed.
 
 Additionally, @code{CFStringRefs} (defined by the @code{CoreFoundation} headers) may
 also be used as format arguments.  Note that the relevant headers are only likely to be
Index: gcc/testsuite/gcc.dg/darwin-cfstring-format-1.c
===================================================================
--- gcc/testsuite/gcc.dg/darwin-cfstring-format-1.c	(revision 267580)
+++ gcc/testsuite/gcc.dg/darwin-cfstring-format-1.c	(working copy)
@@ -15,7 +15,7 @@ typedef const struct __CFString * CFStringRef;
 int s1 (CFStringRef fmt, ...) __attribute__((format(CFString, 1, 2))) ; /* OK */
 int s2 (int a, CFStringRef fmt, ... ) __attribute__((format(__CFString__, 2, 3))) ; /* OK */
 
-int s2a (int a, CFStringRef fmt, ... ) __attribute__((format(CFString, 2, 2))) ; /* { dg-error "format string argument follows the args to be formatted" } */
+int s2a (int a, CFStringRef fmt, ... ) __attribute__((format(CFString, 2, 2))) ; /* { dg-error ".format. attribute argument 3 value .2. does not refer to a variable argument list" } */
 
 int s3 (const char *fmt, ... ) __attribute__((format(__CFString__, 1, 2))) ; /* { dg-error "format argument should be a .CFString. reference but a string was found" } */
 int s4 (CFStringRef fmt, ... ) __attribute__((format(printf, 1, 2))) ; /* { dg-error "found a .CFStringRef.* but the format argument should be a string" } */
@@ -23,7 +23,7 @@ int s4 (CFStringRef fmt, ... ) __attribute__((form
 char *s5 (char dum, char *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 CFStringRef s6 (CFStringRef dum, CFStringRef fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 
-char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "format string argument is not a string type" } */
+char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error ".format_arg. attribute argument value .2. refers to parameter type .void \\\*." } */
 int s8 (CFStringRef dum, CFStringRef fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "function does not return string type" } */
 
 void foo (void)
Index: gcc/testsuite/gcc.dg/format/attr-3.c
===================================================================
--- gcc/testsuite/gcc.dg/format/attr-3.c	(revision 267580)
+++ gcc/testsuite/gcc.dg/format/attr-3.c	(working copy)
@@ -56,16 +56,16 @@ extern void fg3 () __attribute__((format(gnu_attr_
 
 /* The format string argument must be a string type, and the arguments to
    be formatted must be the "...".  */
-extern void fh0 (int, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-warning ".format. attribute argument 2 value .1. refers to parameter type .int." "format int string" } */
-extern void fh1 (signed char *, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error "not a string" "signed char string" } */
-extern void fh2 (unsigned char *, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error "not a string" "unsigned char string" } */
+extern void fh0 (int, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .int." "format int string" } */
+extern void fh1 (signed char *, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .signed char \\\*." "signed char string" } */
+extern void fh2 (unsigned char *, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .unsigned char \\\*." "unsigned char string" } */
 extern void fh3 (const char *, ...) __attribute__((format(gnu_attr_printf, 1, 3))); /* { dg-error "is not" "not ..." } */
 extern void fh4 (const char *, int, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format. attribute argument 3 value .2. does not refer to a variable argument list" "not ..." } */
 
 /* format_arg formats must take and return a string.  */
-extern char *fi0 (int) __attribute__((format_arg(1))); /* { dg-warning ".format_arg. attribute argument value .1. refers to parameter type .int." } */
-extern char *fi1 (signed char *) __attribute__((format_arg(1))); /* { dg-error "not a string" "format_arg signed char string" } */
-extern char *fi2 (unsigned char *) __attribute__((format_arg(1))); /* { dg-error "not a string" "format_arg unsigned char string" } */
+extern char *fi0 (int) __attribute__((format_arg(1))); /* { dg-error ".format_arg. attribute argument value .1. refers to parameter type .int." } */
+extern char *fi1 (signed char *) __attribute__((format_arg(1))); /* { dg-error ".format_arg. attribute argument value .1. refers to parameter type .signed char \\\*." "format_arg signed char string" } */
+extern char *fi2 (unsigned char *) __attribute__((format_arg(1))); /* { dg-error ".format_arg. attribute argument value .1. refers to parameter type .unsigned char \\\*." "format_arg unsigned char string" } */
 extern int fi3 (const char *) __attribute__((format_arg(1))); /* { dg-error "not return string" "format_arg ret int string" } */
 extern signed char *fi4 (const char *) __attribute__((format_arg(1))); /* { dg-error "not return string" "format_arg ret signed char string" } */
 extern unsigned char *fi5 (const char *) __attribute__((format_arg(1))); /* { dg-error "not return string" "format_arg ret unsigned char string" } */
Index: gcc/testsuite/gcc.dg/format/attr-8.c
===================================================================
--- gcc/testsuite/gcc.dg/format/attr-8.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/format/attr-8.c	(working copy)
@@ -0,0 +1,34 @@
+/* Test to verify that parameters of qualified narrow char pointer type
+   are accepted for attribute format printf, but others are rejected.
+   { dg-do compile }
+   { dg-options "-std=gnu99 -Wformat" } */
+
+#define DONT_GNU_PROTOTYPE
+#include "format.h"
+
+#define FORMAT(archetype, arg1, arg2)   \
+  __attribute__  ((format (archetype, arg1, arg2)))
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpc_1_2 (char *, ...);
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpcc_1_2 (const char *, ...);
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void frpc_1_2 (char * restrict, ...);
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpvc_1_2 (volatile char *, ...);
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpcvc_1_2 (const volatile char *, ...);
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpv_1_2 (void *, ...);       /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .void \\\*." } */
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fppc_1_2 (char **, ...);     /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .char \\\*\\\*." } */
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpwc_1_2 (wchar_t *, ...);   /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .wchar_t \\\*." } */
Index: gcc/testsuite/obj-c++.dg/fsf-nsstring-format-1.mm
===================================================================
--- gcc/testsuite/obj-c++.dg/fsf-nsstring-format-1.mm	(revision 267580)
+++ gcc/testsuite/obj-c++.dg/fsf-nsstring-format-1.mm	(working copy)
@@ -28,7 +28,7 @@ int s1b (NSString *fmt, ...) __attribute__((format
 
 int s2 (int a, NSString *fmt, ... ) __attribute__((format(__NSString__, 2, 3))) ; /* OK */
 
-int s2a (int a, NSString *fmt, ... ) __attribute__((format(NSString, 2, 2))) ; /* { dg-error "format string argument follows the args to be formatted" } */
+int s2a (int a, NSString *fmt, ... ) __attribute__((format(NSString, 2, 2))) ; /* { dg-error ".format. attribute argument 3 value .2. does not refer to a variable argument list" } */
 
 int s3 (const char *fmt, ... ) __attribute__((format(__NSString__, 1, 2))) ; /* { dg-error "format argument should be a .NSString. reference but a string was found" } */
 int s4 (NSString *fmt, ... ) __attribute__((format(printf, 1, 2))) ; /* { dg-error "found a .NSString. reference but the format argument should be a string" } */
@@ -36,7 +36,7 @@ int s4 (NSString *fmt, ... ) __attribute__((format
 char *s5 (char dum, char *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 NSString *s6 (NSString *dum, NSString *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 
-char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "format string argument is not a string type" } */
+char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error ".format_arg. attribute argument value .2. refers to parameter type .void\\\*." } */
 int s8 (NSString *dum, NSString *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "function does not return string type" } */
 
 char *s9 (int dum, char *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
Index: gcc/testsuite/objc.dg/fsf-nsstring-format-1.m
===================================================================
--- gcc/testsuite/objc.dg/fsf-nsstring-format-1.m	(revision 267580)
+++ gcc/testsuite/objc.dg/fsf-nsstring-format-1.m	(working copy)
@@ -21,7 +21,7 @@ int s1b (NSString *fmt, ...) __attribute__((format
 
 int s2 (int a, NSString *fmt, ... ) __attribute__((format(__NSString__, 2, 3))) ; /* OK */
 
-int s2a (int a, NSString *fmt, ... ) __attribute__((format(NSString, 2, 2))) ; /* { dg-error "format string argument follows the args to be formatted" } */
+int s2a (int a, NSString *fmt, ... ) __attribute__((format(NSString, 2, 2))) ; /* { dg-error ".format. attribute argument 3 value .2. does not refer to a variable argument list" } */
 
 int s3 (const char *fmt, ... ) __attribute__((format(__NSString__, 1, 2))) ; /* { dg-error "format argument should be a .NSString. reference but a string was found" } */
 int s4 (NSString *fmt, ... ) __attribute__((format(printf, 1, 2))) ; /* { dg-error "found a .NSString. reference but the format argument should be a string" } */
@@ -29,7 +29,7 @@ int s4 (NSString *fmt, ... ) __attribute__((format
 char *s5 (char dum, char *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 NSString *s6 (NSString *dum, NSString *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 
-char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "format string argument is not a string type" } */
+char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error ".format_arg. attribute argument value .2. refers to parameter type .void \\\*." } */
 int s8 (NSString *dum, NSString *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "function does not return string type" } */
 
 char *s9 (int dum, char *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */

Re: [PATCH] restore CFString handling in attribute format (PR 88638)

[Mike Stump]

On Jan 4, 2019, at 2:03 PM, Martin Sebor <msebor@gmail.com> wrote:
> 
> The improved handling of attribute positional arguments added
> in r266195 introduced a regression on Darwin where attribute
> format with the CFString archetype accepts CFString* parameter
> types in positions where only char* would otherwise be allowed.

You didn't ask Ok?  I'll assume you want a review...  The darwin bits and the testsuite bits look fine.

That just leaves the C bits for someone...

Re: [PATCH] restore CFString handling in attribute format (PR 88638)

[Iain Sandoe]

Hi Martin,

> On 4 Jan 2019, at 22:30, Mike Stump <mikestump@comcast.net> wrote:
> 
> On Jan 4, 2019, at 2:03 PM, Martin Sebor <msebor@gmail.com> wrote:
>> 
>> The improved handling of attribute positional arguments added
>> in r266195 introduced a regression on Darwin where attribute
>> format with the CFString archetype accepts CFString* parameter
>> types in positions where only char* would otherwise be allowed.
> 
> You didn't ask Ok?  I'll assume you want a review...  The darwin bits and the testsuite bits look fine.

>> 
>> Index: gcc/doc/extend.texi
>> ===================================================================
>> --- gcc/doc/extend.texi	(revision 267580)
>> +++ gcc/doc/extend.texi	(working copy)
>> @@ -22368,10 +22368,12 @@ bit-fields.  See the Solaris man page for @code{cm
>>  @node Darwin Format Checks
>>  @subsection Darwin Format Checks
>>  
>> -Darwin targets support the @code{CFString} (or @code{__CFString__}) in the format
>> -attribute context.  Declarations made with such attribution are parsed for correct syntax
>> -and format argument types.  However, parsing of the format string itself is currently undefined
>> -and is not carried out by this version of the compiler.
>> +In addition to the full set of archetypes, Darwin targets also support
>> +the @code{CFString} (or @code{__CFString__}) archetype in the @code{format}
>> +attribute.  Declarations with this archetype are parsed for correct syntax
>> +and argument types.  However, parsing of the format string itself and
>> +validating arguments against it in calls to such functions is currently
>> +not performed.
>>  
>>  Additionally, @code{CFStringRefs} (defined by the @code{CoreFoundation} headers) may
>>  also be used as format arguments.  Note that the relevant headers are only likely to be
>> 

I find “archetype(s)” to be an usual (and possibly unfamiliar to many) word for this context.

how about:

s/archetype in/variant for the/ 

and then
 s/with this archetype/with this variant/

in the following sentence.

However, just 0.02GBP .. (fixing the fails is more important than bikeshedding the wording).

Iain

Re: [PATCH] restore CFString handling in attribute format (PR 88638)

[Martin Sebor]

On 1/5/19 3:31 AM, Iain Sandoe wrote:
> Hi Martin,
> 
>> On 4 Jan 2019, at 22:30, Mike Stump <mikestump@comcast.net> wrote:
>>
>> On Jan 4, 2019, at 2:03 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>
>>> The improved handling of attribute positional arguments added
>>> in r266195 introduced a regression on Darwin where attribute
>>> format with the CFString archetype accepts CFString* parameter
>>> types in positions where only char* would otherwise be allowed.
>>
>> You didn't ask Ok?  I'll assume you want a review...  The darwin bits and the testsuite bits look fine.
> 
>>>
>>> Index: gcc/doc/extend.texi
>>> ===================================================================
>>> --- gcc/doc/extend.texi	(revision 267580)
>>> +++ gcc/doc/extend.texi	(working copy)
>>> @@ -22368,10 +22368,12 @@ bit-fields.  See the Solaris man page for @code{cm
>>>   @node Darwin Format Checks
>>>   @subsection Darwin Format Checks
>>>   
>>> -Darwin targets support the @code{CFString} (or @code{__CFString__}) in the format
>>> -attribute context.  Declarations made with such attribution are parsed for correct syntax
>>> -and format argument types.  However, parsing of the format string itself is currently undefined
>>> -and is not carried out by this version of the compiler.
>>> +In addition to the full set of archetypes, Darwin targets also support
>>> +the @code{CFString} (or @code{__CFString__}) archetype in the @code{format}
>>> +attribute.  Declarations with this archetype are parsed for correct syntax
>>> +and argument types.  However, parsing of the format string itself and
>>> +validating arguments against it in calls to such functions is currently
>>> +not performed.
>>>   
>>>   Additionally, @code{CFStringRefs} (defined by the @code{CoreFoundation} headers) may
>>>   also be used as format arguments.  Note that the relevant headers are only likely to be
>>>
> 
> I find “archetype(s)” to be an usual (and possibly unfamiliar to many) word for this context.
> 
> how about:
> 
> s/archetype in/variant for the/
> 
> and then
>   s/with this archetype/with this variant/
> 
> in the following sentence.
> 
> However, just 0.02GBP .. (fixing the fails is more important than bikeshedding the wording).

Thanks for chiming in!  I used archetype because that's the term
used in the attribute format specification to describe the first
argument.  I do tend to agree that archetype alone may not be
sufficiently familiar to all users.  I'm happy to add text to
make that clear.  Would you find the following better?

   In addition to the full set of format archetypes (attribute
   format style arguments such as @code{printf}, @code{scanf},
   @code{strftime}, and @code{strfmon}), Darwin targets also
   support the @code{CFString} (or @code{__CFString__}) archetype...

FWIW, I wanted to figure out how the CFString attribute made it
possible to differentiate between printf and scanf (and the other)
kinds of functions, for example, so I could add new tests for it,
but I couldn't tell that from the manual.  So I'm trying to update
the text to make it clear that although CFString is just like
the sprintf and scanf format arguments/archetypes, beyond
validating declarations that use it, the attribute serves no
functional purpose, so the printf/scanf distinction is moot.

Out of curiosity, is the attribute used for function call
validation by compilers other than GCC?  I couldn't find
anything online.

Thanks again!
Martin

Re: [PATCH] restore CFString handling in attribute format (PR 88638)

[Iain Sandoe]

> On 5 Jan 2019, at 17:39, Martin Sebor <msebor@gmail.com> wrote:
> 
> On 1/5/19 3:31 AM, Iain Sandoe wrote:
>> Hi Martin,
>>> On 4 Jan 2019, at 22:30, Mike Stump <mikestump@comcast.net> wrote:
>>> 
>>> On Jan 4, 2019, at 2:03 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>> 
>>>> The improved handling of attribute positional arguments added
>>>> in r266195 introduced a regression on Darwin where attribute
>>>> format with the CFString archetype accepts CFString* parameter
>>>> types in positions where only char* would otherwise be allowed.
>>> 
>>> You didn't ask Ok?  I'll assume you want a review...  The darwin bits and the testsuite bits look fine.
>>>> 
>>>> Index: gcc/doc/extend.texi
>>>> ===================================================================
>>>> --- gcc/doc/extend.texi	(revision 267580)
>>>> +++ gcc/doc/extend.texi	(working copy)
>>>> @@ -22368,10 +22368,12 @@ bit-fields.  See the Solaris man page for @code{cm
>>>>  @node Darwin Format Checks
>>>>  @subsection Darwin Format Checks
>>>>  -Darwin targets support the @code{CFString} (or @code{__CFString__}) in the format
>>>> -attribute context.  Declarations made with such attribution are parsed for correct syntax
>>>> -and format argument types.  However, parsing of the format string itself is currently undefined
>>>> -and is not carried out by this version of the compiler.
>>>> +In addition to the full set of archetypes, Darwin targets also support
>>>> +the @code{CFString} (or @code{__CFString__}) archetype in the @code{format}
>>>> +attribute.  Declarations with this archetype are parsed for correct syntax
>>>> +and argument types.  However, parsing of the format string itself and
>>>> +validating arguments against it in calls to such functions is currently
>>>> +not performed.
>>>>    Additionally, @code{CFStringRefs} (defined by the @code{CoreFoundation} headers) may
>>>>  also be used as format arguments.  Note that the relevant headers are only likely to be
>>>> 
>> I find “archetype(s)” to be an usual (and possibly unfamiliar to many) word for this context.
>> how about:
>> s/archetype in/variant for the/
>> and then
>>  s/with this archetype/with this variant/
>> in the following sentence.
>> However, just 0.02GBP .. (fixing the fails is more important than bikeshedding the wording).
> 
> Thanks for chiming in!  I used archetype because that's the term
> used in the attribute format specification to describe the first
> argument.  I do tend to agree that archetype alone may not be
> sufficiently familiar to all users.  I'm happy to add text to
> make that clear.  Would you find the following better?
> 
>  In addition to the full set of format archetypes (attribute
>  format style arguments such as @code{printf}, @code{scanf},
>  @code{strftime}, and @code{strfmon}), Darwin targets also
>  support the @code{CFString} (or @code{__CFString__}) archetype…

Yes, that makes is clearer

(as an aside, I think that to many people the meaning of archetype - as ‘generic’  or ‘root example’
  etc  tends to come to mind before the ‘template/mold’ meaning … however couldn’t think of 
 a better term that’s not already overloaded).

> FWIW, I wanted to figure out how the CFString attribute made it
> possible to differentiate between printf and scanf (and the other)
> kinds of functions, for example, so I could add new tests for it,
> but I couldn't tell that from the manual.  So I'm trying to update
> the text to make it clear that although CFString is just like
> the sprintf and scanf format arguments/archetypes, beyond
> validating declarations that use it, the attribute serves no
> functional purpose, so the printf/scanf distinction is moot.

The CFString container** is more general than our implementation, e.g. it should be able
to contain a variety of string formats (e.g. UTF etc.).   AFAIR at the time I implemented
it for FSF GCC (it was originally in the Apple Local branch), we didn’t have sufficient parsing
support for such things (and the support in the Apple-local branch didn’t look applicable).

If we do more sophisitcated checks, we probably need to take cognisance of the fact that a
fully-implemented CFString impl can have non-ascii payload.   I suspect (although honestly
it’s been a while, I maybe mis-remember) that was the reason I didn’t try to implement the
inspection at the time (if so, there’s probably a code comment to that effect).

> Out of curiosity, is the attribute used for function call
> validation by compilers other than GCC?  I couldn't find
> anything online.

It used to be used for the platform GCC, when that was the “system compiler" (last edition
 apple 4.2.1) and I therefore assume it’s used by clang - but actually haven’t double-checked
at the time we added it - it was relevant.

Let’s revisit this in 10, and see if we can’t sync up with the platform expectations from the clang impl.

thanks for taking care of this,

Iain

** it’s actually a simple ObjectiveC object that happens to be supported by the CoreFoundation (C) 
classes as well as ObjectiveC .. making it possible to pass around general string containers between
the languages.

Re: [PATCH] restore CFString handling in attribute format (PR 88638)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 5593 bytes --]

Attached is an updated patch with the wording change to the manual
discussed below and rebased on the top of today's trunk.

Martin

PS Thanks for the additional info, Iain.

On 1/5/19 10:53 AM, Iain Sandoe wrote:
> 
>> On 5 Jan 2019, at 17:39, Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 1/5/19 3:31 AM, Iain Sandoe wrote:
>>> Hi Martin,
>>>> On 4 Jan 2019, at 22:30, Mike Stump <mikestump@comcast.net> wrote:
>>>>
>>>> On Jan 4, 2019, at 2:03 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>>>
>>>>> The improved handling of attribute positional arguments added
>>>>> in r266195 introduced a regression on Darwin where attribute
>>>>> format with the CFString archetype accepts CFString* parameter
>>>>> types in positions where only char* would otherwise be allowed.
>>>>
>>>> You didn't ask Ok?  I'll assume you want a review...  The darwin bits and the testsuite bits look fine.
>>>>>
>>>>> Index: gcc/doc/extend.texi
>>>>> ===================================================================
>>>>> --- gcc/doc/extend.texi	(revision 267580)
>>>>> +++ gcc/doc/extend.texi	(working copy)
>>>>> @@ -22368,10 +22368,12 @@ bit-fields.  See the Solaris man page for @code{cm
>>>>>   @node Darwin Format Checks
>>>>>   @subsection Darwin Format Checks
>>>>>   -Darwin targets support the @code{CFString} (or @code{__CFString__}) in the format
>>>>> -attribute context.  Declarations made with such attribution are parsed for correct syntax
>>>>> -and format argument types.  However, parsing of the format string itself is currently undefined
>>>>> -and is not carried out by this version of the compiler.
>>>>> +In addition to the full set of archetypes, Darwin targets also support
>>>>> +the @code{CFString} (or @code{__CFString__}) archetype in the @code{format}
>>>>> +attribute.  Declarations with this archetype are parsed for correct syntax
>>>>> +and argument types.  However, parsing of the format string itself and
>>>>> +validating arguments against it in calls to such functions is currently
>>>>> +not performed.
>>>>>     Additionally, @code{CFStringRefs} (defined by the @code{CoreFoundation} headers) may
>>>>>   also be used as format arguments.  Note that the relevant headers are only likely to be
>>>>>
>>> I find “archetype(s)” to be an usual (and possibly unfamiliar to many) word for this context.
>>> how about:
>>> s/archetype in/variant for the/
>>> and then
>>>   s/with this archetype/with this variant/
>>> in the following sentence.
>>> However, just 0.02GBP .. (fixing the fails is more important than bikeshedding the wording).
>>
>> Thanks for chiming in!  I used archetype because that's the term
>> used in the attribute format specification to describe the first
>> argument.  I do tend to agree that archetype alone may not be
>> sufficiently familiar to all users.  I'm happy to add text to
>> make that clear.  Would you find the following better?
>>
>>   In addition to the full set of format archetypes (attribute
>>   format style arguments such as @code{printf}, @code{scanf},
>>   @code{strftime}, and @code{strfmon}), Darwin targets also
>>   support the @code{CFString} (or @code{__CFString__}) archetype…
> 
> Yes, that makes is clearer
> 
> (as an aside, I think that to many people the meaning of archetype - as ‘generic’  or ‘root example’
>    etc  tends to come to mind before the ‘template/mold’ meaning … however couldn’t think of
>   a better term that’s not already overloaded).
> 
>> FWIW, I wanted to figure out how the CFString attribute made it
>> possible to differentiate between printf and scanf (and the other)
>> kinds of functions, for example, so I could add new tests for it,
>> but I couldn't tell that from the manual.  So I'm trying to update
>> the text to make it clear that although CFString is just like
>> the sprintf and scanf format arguments/archetypes, beyond
>> validating declarations that use it, the attribute serves no
>> functional purpose, so the printf/scanf distinction is moot.
> 
> The CFString container** is more general than our implementation, e.g. it should be able
> to contain a variety of string formats (e.g. UTF etc.).   AFAIR at the time I implemented
> it for FSF GCC (it was originally in the Apple Local branch), we didn’t have sufficient parsing
> support for such things (and the support in the Apple-local branch didn’t look applicable).
> 
> If we do more sophisitcated checks, we probably need to take cognisance of the fact that a
> fully-implemented CFString impl can have non-ascii payload.   I suspect (although honestly
> it’s been a while, I maybe mis-remember) that was the reason I didn’t try to implement the
> inspection at the time (if so, there’s probably a code comment to that effect).
> 
>> Out of curiosity, is the attribute used for function call
>> validation by compilers other than GCC?  I couldn't find
>> anything online.
> 
> It used to be used for the platform GCC, when that was the “system compiler" (last edition
>   apple 4.2.1) and I therefore assume it’s used by clang - but actually haven’t double-checked
> at the time we added it - it was relevant.
> 
> Let’s revisit this in 10, and see if we can’t sync up with the platform expectations from the clang impl.
> 
> thanks for taking care of this,
> 
> Iain
> 
> ** it’s actually a simple ObjectiveC object that happens to be supported by the CoreFoundation (C)
> classes as well as ObjectiveC .. making it possible to pass around general string containers between
> the languages.
> 


[-- Attachment #2: gcc-88638.diff --]
[-- Type: text/x-patch, Size: 17267 bytes --]

PR target/88638 - FAIL: fsf-nsstring-format-1.s on darwin

gcc/c-family/ChangeLog:

	PR target/88638
	* c-attribs.c (positional_argument): Call valid_format_string_type_p
	and issue errors if it fails.
	* c-common.h (valid_format_string_type_p): Declare.
	* c-format.c (valid_stringptr_type_p): Rename...
	(valid_format_string_type_p): ...to this and make extern.
	(handle_format_arg_attribute): Adjust to new name.
	(check_format_string): Same.

gcc/testsuite/ChangeLog:

	PR target/88638
	* gcc.dg/format/attr-8.c: New test.
	* gcc.dg/darwin-cfstring-format-1.c: Adjust diagnostics.
	* gcc.dg/format/attr-3.c: Same.
	* obj-c++.dg/fsf-nsstring-format-1.mm: Same.
	* objc.dg/fsf-nsstring-format-1.m: Same.

gcc/ChangeLog:

	PR target/88638
	* doc/extend.texi (Darwin Format Checks): Clarify.

Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 267616)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -631,17 +631,13 @@ positional_argument (const_tree fntype, const_tree
 	  return NULL_TREE;
 	}
 
+      /* Where the expected code is STRING_CST accept any pointer
+	 expected by attribute format (this includes possibly qualified
+	 char pointers and, for targets like Darwin, also pointers to
+	 struct CFString).  */
       bool type_match;
-      if (code == STRING_CST && POINTER_TYPE_P (argtype))
-	{
-	  /* Where the expected code is STRING_CST accept any pointer
-	     to a narrow character type, qualified or otherwise.  */
-	  tree type = TREE_TYPE (argtype);
-	  type = TYPE_MAIN_VARIANT (type);
-	  type_match = (type == char_type_node
-			|| type == signed_char_type_node
-			|| type == unsigned_char_type_node);
-	}
+      if (code == STRING_CST)
+	type_match = valid_format_string_type_p (argtype);
       else if (code == INTEGER_TYPE)
 	/* For integers, accept enums, wide characters and other types
 	   that match INTEGRAL_TYPE_P except for bool.  */
@@ -652,6 +648,21 @@ positional_argument (const_tree fntype, const_tree
 
       if (!type_match)
 	{
+	  if (code == STRING_CST)
+	    {
+	      /* Reject invalid format strings with an error.  */
+	      if (argno < 1)
+		error ("%qE attribute argument value %qE refers to "
+		       "parameter type %qT",
+		       atname, pos, argtype);
+	      else
+		error ("%qE attribute argument %i value %qE refers to "
+		       "parameter type %qT",
+		       atname, argno, pos, argtype);
+
+	      return NULL_TREE;
+	    }
+
 	  if (argno < 1)
 	    warning (OPT_Wattributes,
 		     "%qE attribute argument value %qE refers to "
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h	(revision 267616)
+++ gcc/c-family/c-common.h	(working copy)
@@ -1335,6 +1335,9 @@ extern tree tm_mask_to_attr (int);
 extern tree find_tm_attribute (tree);
 extern const struct attribute_spec::exclusions attr_cold_hot_exclusions[];
 
+/* In c-format.c.  */
+extern bool valid_format_string_type_p (tree);
+
 /* A bitmap of flags to positional_argument.  */
 enum posargflags {
   /* Consider positional attribute argument value zero valid.  */
Index: gcc/c-family/c-format.c
===================================================================
--- gcc/c-family/c-format.c	(revision 267616)
+++ gcc/c-family/c-format.c	(working copy)
@@ -122,8 +122,8 @@ format_warning_at_char (location_t fmt_string_loc,
    The function returns true if strref points to any string type valid for the 
    language dialect and target.  */
 
-static bool
-valid_stringptr_type_p (tree strref)
+bool
+valid_format_string_type_p (tree strref)
 {
   return (strref != NULL
 	  && TREE_CODE (strref) == POINTER_TYPE
@@ -160,7 +160,7 @@ handle_format_arg_attribute (tree *node, tree atna
 	return NULL_TREE;
     }
 
-  if (!valid_stringptr_type_p (TREE_TYPE (type)))
+  if (!valid_format_string_type_p (TREE_TYPE (type)))
     {
       if (!(flags & (int) ATTR_FLAG_BUILT_IN))
 	error ("function does not return string type");
@@ -194,7 +194,7 @@ check_format_string (const_tree fntype, unsigned H
     }
 
   if (!ref
-      || !valid_stringptr_type_p (ref))
+      || !valid_format_string_type_p (ref))
     {
       if (!(flags & (int) ATTR_FLAG_BUILT_IN))
 	error ("format string argument is not a string type");
@@ -267,13 +267,21 @@ check_format_string (const_tree fntype, unsigned H
   gcc_unreachable ();
 }
 
-/* Verify EXPR is a constant, and store its value.
-   If validated_p is true there should be no errors.
+/* Under the control of FLAGS, verify EXPR is a valid constant that
+   refers to a positional argument ARGNO having a string type (char*
+   or, for targets like Darwin, a pointer to struct CFString) to
+   a function type FNTYPE declared with attribute ATNAME.
+   If valid, store the constant's integer value in *VALUE and return
+   the value.
+   If VALIDATED_P is true assert the validation is successful.
    Returns the converted constant value on success, null otherwise.  */
+
 static tree
 get_constant (const_tree fntype, const_tree atname, tree expr, int argno,
 	      unsigned HOST_WIDE_INT *value, int flags, bool validated_p)
 {
+  /* Require the referenced argument to have a string type.  For targets
+     like Darwin, also accept pointers to struct CFString.  */
   if (tree val = positional_argument (fntype, atname, expr, STRING_CST,
 				      argno, flags))
     {
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 267616)
+++ gcc/doc/extend.texi	(working copy)
@@ -22370,10 +22370,14 @@ bit-fields.  See the Solaris man page for @code{cm
 @node Darwin Format Checks
 @subsection Darwin Format Checks
 
-Darwin targets support the @code{CFString} (or @code{__CFString__}) in the format
-attribute context.  Declarations made with such attribution are parsed for correct syntax
-and format argument types.  However, parsing of the format string itself is currently undefined
-and is not carried out by this version of the compiler.
+In addition to the full set of format archetypes (attribute format style
+arguments such as @code{printf}, @code{scanf}, @code{strftime}, and
+@code{strfmon}), Darwin targets also support the @code{CFString} (or
+@code{__CFString__}) archetype in the @code{format} attribute.
+Declarations with this archetype are parsed for correct syntax
+and argument types.  However, parsing of the format string itself and
+validating arguments against it in calls to such functions is currently
+not performed.
 
 Additionally, @code{CFStringRefs} (defined by the @code{CoreFoundation} headers) may
 also be used as format arguments.  Note that the relevant headers are only likely to be
Index: gcc/testsuite/ChangeLog
===================================================================
Index: gcc/testsuite/gcc.dg/darwin-cfstring-format-1.c
===================================================================
--- gcc/testsuite/gcc.dg/darwin-cfstring-format-1.c	(revision 267616)
+++ gcc/testsuite/gcc.dg/darwin-cfstring-format-1.c	(working copy)
@@ -15,7 +15,7 @@ typedef const struct __CFString * CFStringRef;
 int s1 (CFStringRef fmt, ...) __attribute__((format(CFString, 1, 2))) ; /* OK */
 int s2 (int a, CFStringRef fmt, ... ) __attribute__((format(__CFString__, 2, 3))) ; /* OK */
 
-int s2a (int a, CFStringRef fmt, ... ) __attribute__((format(CFString, 2, 2))) ; /* { dg-error "format string argument follows the args to be formatted" } */
+int s2a (int a, CFStringRef fmt, ... ) __attribute__((format(CFString, 2, 2))) ; /* { dg-error ".format. attribute argument 3 value .2. does not refer to a variable argument list" } */
 
 int s3 (const char *fmt, ... ) __attribute__((format(__CFString__, 1, 2))) ; /* { dg-error "format argument should be a .CFString. reference but a string was found" } */
 int s4 (CFStringRef fmt, ... ) __attribute__((format(printf, 1, 2))) ; /* { dg-error "found a .CFStringRef.* but the format argument should be a string" } */
@@ -23,7 +23,7 @@ int s4 (CFStringRef fmt, ... ) __attribute__((form
 char *s5 (char dum, char *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 CFStringRef s6 (CFStringRef dum, CFStringRef fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 
-char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "format string argument is not a string type" } */
+char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error ".format_arg. attribute argument value .2. refers to parameter type .void \\\*." } */
 int s8 (CFStringRef dum, CFStringRef fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "function does not return string type" } */
 
 void foo (void)
Index: gcc/testsuite/gcc.dg/format/attr-3.c
===================================================================
--- gcc/testsuite/gcc.dg/format/attr-3.c	(revision 267616)
+++ gcc/testsuite/gcc.dg/format/attr-3.c	(working copy)
@@ -56,16 +56,16 @@ extern void fg3 () __attribute__((format(gnu_attr_
 
 /* The format string argument must be a string type, and the arguments to
    be formatted must be the "...".  */
-extern void fh0 (int, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-warning ".format. attribute argument 2 value .1. refers to parameter type .int." "format int string" } */
-extern void fh1 (signed char *, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error "not a string" "signed char string" } */
-extern void fh2 (unsigned char *, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error "not a string" "unsigned char string" } */
+extern void fh0 (int, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .int." "format int string" } */
+extern void fh1 (signed char *, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .signed char \\\*." "signed char string" } */
+extern void fh2 (unsigned char *, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .unsigned char \\\*." "unsigned char string" } */
 extern void fh3 (const char *, ...) __attribute__((format(gnu_attr_printf, 1, 3))); /* { dg-error "is not" "not ..." } */
 extern void fh4 (const char *, int, ...) __attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format. attribute argument 3 value .2. does not refer to a variable argument list" "not ..." } */
 
 /* format_arg formats must take and return a string.  */
-extern char *fi0 (int) __attribute__((format_arg(1))); /* { dg-warning ".format_arg. attribute argument value .1. refers to parameter type .int." } */
-extern char *fi1 (signed char *) __attribute__((format_arg(1))); /* { dg-error "not a string" "format_arg signed char string" } */
-extern char *fi2 (unsigned char *) __attribute__((format_arg(1))); /* { dg-error "not a string" "format_arg unsigned char string" } */
+extern char *fi0 (int) __attribute__((format_arg(1))); /* { dg-error ".format_arg. attribute argument value .1. refers to parameter type .int." } */
+extern char *fi1 (signed char *) __attribute__((format_arg(1))); /* { dg-error ".format_arg. attribute argument value .1. refers to parameter type .signed char \\\*." "format_arg signed char string" } */
+extern char *fi2 (unsigned char *) __attribute__((format_arg(1))); /* { dg-error ".format_arg. attribute argument value .1. refers to parameter type .unsigned char \\\*." "format_arg unsigned char string" } */
 extern int fi3 (const char *) __attribute__((format_arg(1))); /* { dg-error "not return string" "format_arg ret int string" } */
 extern signed char *fi4 (const char *) __attribute__((format_arg(1))); /* { dg-error "not return string" "format_arg ret signed char string" } */
 extern unsigned char *fi5 (const char *) __attribute__((format_arg(1))); /* { dg-error "not return string" "format_arg ret unsigned char string" } */
Index: gcc/testsuite/gcc.dg/format/attr-8.c
===================================================================
--- gcc/testsuite/gcc.dg/format/attr-8.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/format/attr-8.c	(working copy)
@@ -0,0 +1,34 @@
+/* Test to verify that parameters of qualified narrow char pointer type
+   are accepted for attribute format printf, but others are rejected.
+   { dg-do compile }
+   { dg-options "-std=gnu99 -Wformat" } */
+
+#define DONT_GNU_PROTOTYPE
+#include "format.h"
+
+#define FORMAT(archetype, arg1, arg2)   \
+  __attribute__  ((format (archetype, arg1, arg2)))
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpc_1_2 (char *, ...);
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpcc_1_2 (const char *, ...);
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void frpc_1_2 (char * restrict, ...);
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpvc_1_2 (volatile char *, ...);
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpcvc_1_2 (const volatile char *, ...);
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpv_1_2 (void *, ...);       /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .void \\\*." } */
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fppc_1_2 (char **, ...);     /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .char \\\*\\\*." } */
+
+FORMAT (gnu_attr_printf, 1, 2)
+  void fpwc_1_2 (wchar_t *, ...);   /* { dg-error ".format. attribute argument 2 value .1. refers to parameter type .wchar_t \\\*." } */
Index: gcc/testsuite/obj-c++.dg/fsf-nsstring-format-1.mm
===================================================================
--- gcc/testsuite/obj-c++.dg/fsf-nsstring-format-1.mm	(revision 267616)
+++ gcc/testsuite/obj-c++.dg/fsf-nsstring-format-1.mm	(working copy)
@@ -28,7 +28,7 @@ int s1b (NSString *fmt, ...) __attribute__((format
 
 int s2 (int a, NSString *fmt, ... ) __attribute__((format(__NSString__, 2, 3))) ; /* OK */
 
-int s2a (int a, NSString *fmt, ... ) __attribute__((format(NSString, 2, 2))) ; /* { dg-error "format string argument follows the args to be formatted" } */
+int s2a (int a, NSString *fmt, ... ) __attribute__((format(NSString, 2, 2))) ; /* { dg-error ".format. attribute argument 3 value .2. does not refer to a variable argument list" } */
 
 int s3 (const char *fmt, ... ) __attribute__((format(__NSString__, 1, 2))) ; /* { dg-error "format argument should be a .NSString. reference but a string was found" } */
 int s4 (NSString *fmt, ... ) __attribute__((format(printf, 1, 2))) ; /* { dg-error "found a .NSString. reference but the format argument should be a string" } */
@@ -36,7 +36,7 @@ int s4 (NSString *fmt, ... ) __attribute__((format
 char *s5 (char dum, char *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 NSString *s6 (NSString *dum, NSString *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 
-char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "format string argument is not a string type" } */
+char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error ".format_arg. attribute argument value .2. refers to parameter type .void\\\*." } */
 int s8 (NSString *dum, NSString *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "function does not return string type" } */
 
 char *s9 (int dum, char *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
Index: gcc/testsuite/objc.dg/fsf-nsstring-format-1.m
===================================================================
--- gcc/testsuite/objc.dg/fsf-nsstring-format-1.m	(revision 267616)
+++ gcc/testsuite/objc.dg/fsf-nsstring-format-1.m	(working copy)
@@ -21,7 +21,7 @@ int s1b (NSString *fmt, ...) __attribute__((format
 
 int s2 (int a, NSString *fmt, ... ) __attribute__((format(__NSString__, 2, 3))) ; /* OK */
 
-int s2a (int a, NSString *fmt, ... ) __attribute__((format(NSString, 2, 2))) ; /* { dg-error "format string argument follows the args to be formatted" } */
+int s2a (int a, NSString *fmt, ... ) __attribute__((format(NSString, 2, 2))) ; /* { dg-error ".format. attribute argument 3 value .2. does not refer to a variable argument list" } */
 
 int s3 (const char *fmt, ... ) __attribute__((format(__NSString__, 1, 2))) ; /* { dg-error "format argument should be a .NSString. reference but a string was found" } */
 int s4 (NSString *fmt, ... ) __attribute__((format(printf, 1, 2))) ; /* { dg-error "found a .NSString. reference but the format argument should be a string" } */
@@ -29,7 +29,7 @@ int s4 (NSString *fmt, ... ) __attribute__((format
 char *s5 (char dum, char *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 NSString *s6 (NSString *dum, NSString *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */
 
-char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "format string argument is not a string type" } */
+char *s7 (int dum, void *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error ".format_arg. attribute argument value .2. refers to parameter type .void \\\*." } */
 int s8 (NSString *dum, NSString *fmt1, ... ) __attribute__((format_arg(2))) ; /* { dg-error "function does not return string type" } */
 
 char *s9 (int dum, char *fmt1, ... ) __attribute__((format_arg(2))) ; /* OK */

Re: [PATCH] restore CFString handling in attribute format (PR 88638)

[Iain Sandoe]

> On 6 Jan 2019, at 23:34, Martin Sebor <msebor@gmail.com> wrote:
> 
> Attached is an updated patch with the wording change to the manual
> discussed below and rebased on the top of today's trunk.

Works for me as well.
thanks for the patch.

Iain

> 
> Martin
> 
> PS Thanks for the additional info, Iain.
> 
> On 1/5/19 10:53 AM, Iain Sandoe wrote:
>>> On 5 Jan 2019, at 17:39, Martin Sebor <msebor@gmail.com> wrote:
>>> 
>>> On 1/5/19 3:31 AM, Iain Sandoe wrote:
>>>> Hi Martin,
>>>>> On 4 Jan 2019, at 22:30, Mike Stump <mikestump@comcast.net> wrote:
>>>>> 
>>>>> On Jan 4, 2019, at 2:03 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>>>> 
>>>>>> The improved handling of attribute positional arguments added
>>>>>> in r266195 introduced a regression on Darwin where attribute
>>>>>> format with the CFString archetype accepts CFString* parameter
>>>>>> types in positions where only char* would otherwise be allowed.
>>>>> 
>>>>> You didn't ask Ok?  I'll assume you want a review...  The darwin bits and the testsuite bits look fine.
>>>>>> 
>>>>>> Index: gcc/doc/extend.texi
>>>>>> ===================================================================
>>>>>> --- gcc/doc/extend.texi	(revision 267580)
>>>>>> +++ gcc/doc/extend.texi	(working copy)
>>>>>> @@ -22368,10 +22368,12 @@ bit-fields.  See the Solaris man page for @code{cm
>>>>>>  @node Darwin Format Checks
>>>>>>  @subsection Darwin Format Checks
>>>>>>  -Darwin targets support the @code{CFString} (or @code{__CFString__}) in the format
>>>>>> -attribute context.  Declarations made with such attribution are parsed for correct syntax
>>>>>> -and format argument types.  However, parsing of the format string itself is currently undefined
>>>>>> -and is not carried out by this version of the compiler.
>>>>>> +In addition to the full set of archetypes, Darwin targets also support
>>>>>> +the @code{CFString} (or @code{__CFString__}) archetype in the @code{format}
>>>>>> +attribute.  Declarations with this archetype are parsed for correct syntax
>>>>>> +and argument types.  However, parsing of the format string itself and
>>>>>> +validating arguments against it in calls to such functions is currently
>>>>>> +not performed.
>>>>>>    Additionally, @code{CFStringRefs} (defined by the @code{CoreFoundation} headers) may
>>>>>>  also be used as format arguments.  Note that the relevant headers are only likely to be
>>>>>> 
>>>> I find “archetype(s)” to be an usual (and possibly unfamiliar to many) word for this context.
>>>> how about:
>>>> s/archetype in/variant for the/
>>>> and then
>>>>  s/with this archetype/with this variant/
>>>> in the following sentence.
>>>> However, just 0.02GBP .. (fixing the fails is more important than bikeshedding the wording).
>>> 
>>> Thanks for chiming in!  I used archetype because that's the term
>>> used in the attribute format specification to describe the first
>>> argument.  I do tend to agree that archetype alone may not be
>>> sufficiently familiar to all users.  I'm happy to add text to
>>> make that clear.  Would you find the following better?
>>> 
>>>  In addition to the full set of format archetypes (attribute
>>>  format style arguments such as @code{printf}, @code{scanf},
>>>  @code{strftime}, and @code{strfmon}), Darwin targets also
>>>  support the @code{CFString} (or @code{__CFString__}) archetype…
>> Yes, that makes is clearer
>> (as an aside, I think that to many people the meaning of archetype - as ‘generic’  or ‘root example’
>>   etc  tends to come to mind before the ‘template/mold’ meaning … however couldn’t think of
>>  a better term that’s not already overloaded).
>>> FWIW, I wanted to figure out how the CFString attribute made it
>>> possible to differentiate between printf and scanf (and the other)
>>> kinds of functions, for example, so I could add new tests for it,
>>> but I couldn't tell that from the manual.  So I'm trying to update
>>> the text to make it clear that although CFString is just like
>>> the sprintf and scanf format arguments/archetypes, beyond
>>> validating declarations that use it, the attribute serves no
>>> functional purpose, so the printf/scanf distinction is moot.
>> The CFString container** is more general than our implementation, e.g. it should be able
>> to contain a variety of string formats (e.g. UTF etc.).   AFAIR at the time I implemented
>> it for FSF GCC (it was originally in the Apple Local branch), we didn’t have sufficient parsing
>> support for such things (and the support in the Apple-local branch didn’t look applicable).
>> If we do more sophisitcated checks, we probably need to take cognisance of the fact that a
>> fully-implemented CFString impl can have non-ascii payload.   I suspect (although honestly
>> it’s been a while, I maybe mis-remember) that was the reason I didn’t try to implement the
>> inspection at the time (if so, there’s probably a code comment to that effect).
>>> Out of curiosity, is the attribute used for function call
>>> validation by compilers other than GCC?  I couldn't find
>>> anything online.
>> It used to be used for the platform GCC, when that was the “system compiler" (last edition
>>  apple 4.2.1) and I therefore assume it’s used by clang - but actually haven’t double-checked
>> at the time we added it - it was relevant.
>> Let’s revisit this in 10, and see if we can’t sync up with the platform expectations from the clang impl.
>> thanks for taking care of this,
>> Iain
>> ** it’s actually a simple ObjectiveC object that happens to be supported by the CoreFoundation (C)
>> classes as well as ObjectiveC .. making it possible to pass around general string containers between
>> the languages.
> 
> <gcc-88638.diff>

Re: [PATCH] restore CFString handling in attribute format (PR 88638)

[Jeff Law]

On 1/6/19 4:34 PM, Martin Sebor wrote:
> Attached is an updated patch with the wording change to the manual
> discussed below and rebased on the top of today's trunk.
> 
> Martin
> 
> PS Thanks for the additional info, Iain.
> 
> On 1/5/19 10:53 AM, Iain Sandoe wrote:
>>
>>> On 5 Jan 2019, at 17:39, Martin Sebor <msebor@gmail.com> wrote:
>>>
>>> On 1/5/19 3:31 AM, Iain Sandoe wrote:
>>>> Hi Martin,
>>>>> On 4 Jan 2019, at 22:30, Mike Stump <mikestump@comcast.net> wrote:
>>>>>
>>>>> On Jan 4, 2019, at 2:03 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>>>>
>>>>>> The improved handling of attribute positional arguments added
>>>>>> in r266195 introduced a regression on Darwin where attribute
>>>>>> format with the CFString archetype accepts CFString* parameter
>>>>>> types in positions where only char* would otherwise be allowed.
>>>>>
>>>>> You didn't ask Ok?  I'll assume you want a review...  The darwin bits and the testsuite bits look fine.
>>>>>
>>>>>>
>>>>>> Index: gcc/doc/extend.texi
>>>>>> ===================================================================
>>>>>> --- gcc/doc/extend.texi    (revision 267580)
>>>>>> +++ gcc/doc/extend.texi    (working copy)
>>>>>> @@ -22368,10 +22368,12 @@ bit-fields.  See the Solaris man page for @code{cm
>>>>>>
>>>>>>   @node Darwin Format Checks
>>>>>>   @subsection Darwin Format Checks
>>>>>>   -Darwin targets support the @code{CFString} (or @code{__CFString__}) in the format
>>>>>>
>>>>>> -attribute context.  Declarations made with such attribution are parsed for correct syntax
>>>>>>
>>>>>> -and format argument types.  However, parsing of the format string itself is currently undefined
>>>>>>
>>>>>> -and is not carried out by this version of the compiler.
>>>>>> +In addition to the full set of archetypes, Darwin targets also support
>>>>>>
>>>>>> +the @code{CFString} (or @code{__CFString__}) archetype in the @code{format}
>>>>>>
>>>>>> +attribute.  Declarations with this archetype are parsed for correct syntax
>>>>>>
>>>>>> +and argument types.  However, parsing of the format string itself and
>>>>>>
>>>>>> +validating arguments against it in calls to such functions is currently
>>>>>>
>>>>>> +not performed.
>>>>>>     Additionally, @code{CFStringRefs} (defined by the @code{CoreFoundation} headers) may
>>>>>>
>>>>>>   also be used as format arguments.  Note that the relevant headers are only likely to be
>>>>>>
>>>>>>
>>>> I find “archetype(s)” to be an usual (and possibly unfamiliar to many) word for this context.
>>>>
>>>> how about:
>>>> s/archetype in/variant for the/
>>>> and then
>>>>   s/with this archetype/with this variant/
>>>> in the following sentence.
>>>> However, just 0.02GBP .. (fixing the fails is more important than bikeshedding the wording).
>>>>
>>>
>>> Thanks for chiming in!  I used archetype because that's the term
>>> used in the attribute format specification to describe the first
>>> argument.  I do tend to agree that archetype alone may not be
>>> sufficiently familiar to all users.  I'm happy to add text to
>>> make that clear.  Would you find the following better?
>>>
>>>   In addition to the full set of format archetypes (attribute
>>>   format style arguments such as @code{printf}, @code{scanf},
>>>   @code{strftime}, and @code{strfmon}), Darwin targets also
>>>   support the @code{CFString} (or @code{__CFString__}) archetype…
>>
>> Yes, that makes is clearer
>>
>> (as an aside, I think that to many people the meaning of archetype - as ‘generic’  or ‘root example’
>>
>>    etc  tends to come to mind before the ‘template/mold’ meaning … however couldn’t think of
>>
>>   a better term that’s not already overloaded).
>>
>>> FWIW, I wanted to figure out how the CFString attribute made it
>>> possible to differentiate between printf and scanf (and the other)
>>> kinds of functions, for example, so I could add new tests for it,
>>> but I couldn't tell that from the manual.  So I'm trying to update
>>> the text to make it clear that although CFString is just like
>>> the sprintf and scanf format arguments/archetypes, beyond
>>> validating declarations that use it, the attribute serves no
>>> functional purpose, so the printf/scanf distinction is moot.
>>
>> The CFString container** is more general than our implementation, e.g. it should be able
>>
>> to contain a variety of string formats (e.g. UTF etc.).   AFAIR at the time I implemented
>>
>> it for FSF GCC (it was originally in the Apple Local branch), we didn’t have sufficient parsing
>>
>> support for such things (and the support in the Apple-local branch didn’t look applicable).
>>
>>
>> If we do more sophisitcated checks, we probably need to take cognisance of the fact that a
>>
>> fully-implemented CFString impl can have non-ascii payload.   I suspect (although honestly
>>
>> it’s been a while, I maybe mis-remember) that was the reason I didn’t try to implement the
>>
>> inspection at the time (if so, there’s probably a code comment to that effect).
>>
>>
>>> Out of curiosity, is the attribute used for function call
>>> validation by compilers other than GCC?  I couldn't find
>>> anything online.
>>
>> It used to be used for the platform GCC, when that was the “system compiler" (last edition
>>
>>   apple 4.2.1) and I therefore assume it’s used by clang - but actually haven’t double-checked
>>
>> at the time we added it - it was relevant.
>>
>> Let’s revisit this in 10, and see if we can’t sync up with the platform expectations from the clang impl.
>>
>>
>> thanks for taking care of this,
>>
>> Iain
>>
>> ** it’s actually a simple ObjectiveC object that happens to be supported by the CoreFoundation (C)
>>
>> classes as well as ObjectiveC .. making it possible to pass around general string containers between
>>
>> the languages.
>>
> 
> 
> gcc-88638.diff
> 
> PR target/88638 - FAIL: fsf-nsstring-format-1.s on darwin
> 
> gcc/c-family/ChangeLog:
> 
> 	PR target/88638
> 	* c-attribs.c (positional_argument): Call valid_format_string_type_p
> 	and issue errors if it fails.
> 	* c-common.h (valid_format_string_type_p): Declare.
> 	* c-format.c (valid_stringptr_type_p): Rename...
> 	(valid_format_string_type_p): ...to this and make extern.
> 	(handle_format_arg_attribute): Adjust to new name.
> 	(check_format_string): Same.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR target/88638
> 	* gcc.dg/format/attr-8.c: New test.
> 	* gcc.dg/darwin-cfstring-format-1.c: Adjust diagnostics.
> 	* gcc.dg/format/attr-3.c: Same.
> 	* obj-c++.dg/fsf-nsstring-format-1.mm: Same.
> 	* objc.dg/fsf-nsstring-format-1.m: Same.
> 
> gcc/ChangeLog:
> 
> 	PR target/88638
> 	* doc/extend.texi (Darwin Format Checks): Clarify.
OK
jeff

Re: [PATCH] restore CFString handling in attribute format (PR 88638)

[Dominique d'Humières]

Hi Martin,

The patch on top of r267591 fixes pr88638 without regression.

Note

 FAIL: c-c++-common/attributes-4.c  -std=gnu++14 (test for excess errors)
 FAIL: c-c++-common/attributes-4.c  -std=gnu++17 (test for excess errors)
 FAIL: c-c++-common/attributes-4.c  -std=gnu++98 (test for excess errors)

Thanks for the fix.

Dominique

Re: [PATCH] restore CFString handling in attribute format (PR 88638)

[Martin Sebor]

On 1/5/19 2:41 PM, Dominique d'Humières wrote:
> Hi Martin,
> 
> The patch on top of r267591 fixes pr88638 without regression.
> 
> Note
> 
>   FAIL: c-c++-common/attributes-4.c  -std=gnu++14 (test for excess errors)
>   FAIL: c-c++-common/attributes-4.c  -std=gnu++17 (test for excess errors)
>   FAIL: c-c++-common/attributes-4.c  -std=gnu++98 (test for excess errors)
> 
> Thanks for the fix.

I just committed it.  I don't see this test fail with my Darwin
cross-compiler.  The failures I do see in attr*.c tests are these:

FAIL: gcc.dg/attr-copy-6.c (test for excess errors)
FAIL: gcc.dg/attr-ms_struct-packed1.c (test for excess errors)
FAIL: gcc.dg/attr-weakref-1-darwin.c (test for excess errors)
FAIL: gcc.dg/attr-weakref-1.c (test for excess errors)
FAIL: c-c++-common/attr-aligned-1.c  -Wc++-compat  (test for excess errors)
XPASS: c-c++-common/attr-nonstring-3.c  -Wc++-compat  pr86688 (test for 
warnings, line 409)

The attr-copy-6.c failure is due to
   error: only weak aliases are supported in this configuration

The other FAILs are all because the tests are expected to run but
can't in this configuration.

The XPASS seems to be new and present in native builds as well so
it's something to look into.

Martin