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 <mse...@gmail.com> wrote:
On 1/5/19 3:31 AM, Iain Sandoe wrote:
Hi Martin,
On 4 Jan 2019, at 22:30, Mike Stump <mikest...@comcast.net> wrote:
On Jan 4, 2019, at 2:03 PM, Martin Sebor <mse...@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.
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 */
