This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH PING²] Extend the `sentinel' attribute
Hi,
"Joseph S. Myers" <joseph@codesourcery.com> writes:
> If an integer constant is used as the sentinel value in the attribute, I
> think you need to check that the value in the call has compatible type
> (after removing any qualifiers), or at least the same width (so that
> passing 1 in one place and 1LL in the other is diagnosed, for example).
Good point. Here's an updated patch that takes this into account.
There's a problem, though. The "foo12 ("a", 0);" at sentinel-1.c:79
doesn't pass. Strangely enough, "POINTER_TYPE_P (value)" wrongfully
returns true for the "0" in there, which triggers the "missing sentinel"
warning; even more strangely, commenting the `foo11 ()' declaration or
moving it a couple of lines down fixes the bug. Any idea?
> A -Wstrict-null-sentinel test of the case
>
> + foo12 ("a", (void *) 0); /* Unless `-Wstrict-null-sentinel' is passed,
> + using a null pointer instead of zero is OK. */
>
> would also be a good idea (and if the value 0 in the attribute was not the
> same width as pointers, I don't think -Wstrict-null-sentinel should be
> required for the warning).
Right. However, it would be a C++ test, and I'm not sure whether/how it
can be run under `gcc.dg/format' (I tried adding "{ dg-option "-x c++" }"
but that doesn't work since "-x c++" appears too late on the command
line). Any idea?
> What's the copyright assignment status?
It falls under the INRIA copyright assignment (I'm an INRIA employee).
Thanks,
Ludovic.
gcc/ChangeLog
2008-04-17 Ludovic Courtès <ludo@gnu.org>
PR28319
* c-common.c (c_common_attribute_table)[sentinel]: Set max
argument count to 2.
(check_function_sentinel): Check SENTINEL against the
user-provided value rather than against zero. Check
SENTINEL's "pointerness" against that of the user-specified
value instead of always expecting a pointer type.
(handle_sentinel_attribute): Validate second optional argument.
* doc/extend.texi (Function Attributes): Document `sentinel'
change.
gcc/testsuite/ChangeLog
2008-04-17 Ludovic Courtès <ludo@gnu.org>
PR28319
* gcc.dg/format/sentinel-1.c (foo10): Add third `sentinel'
argument.
(foo11, foo12, foo13, foo14, foo15): New declarations.
(bar): Use them.
diff --git a/gcc/c-common.c b/gcc/c-common.c
index 2bc7434..4c3cfc5 100644
--- a/gcc/c-common.c
+++ b/gcc/c-common.c
@@ -647,7 +647,7 @@ const struct attribute_spec c_common_attribute_table[] =
handle_cleanup_attribute },
{ "warn_unused_result", 0, 0, false, true, true,
handle_warn_unused_result_attribute },
- { "sentinel", 0, 1, false, true, true,
+ { "sentinel", 0, 2, false, true, true,
handle_sentinel_attribute },
/* For internal use (marking of builtins) only. The name contains space
to prevent its usage in source code. */
@@ -6214,7 +6214,7 @@ check_function_sentinel (tree attrs, int nargs, tree *argarray, tree typelist)
{
int len = 0;
int pos = 0;
- tree sentinel;
+ tree sentinel, value = null_pointer_node;
/* Skip over the named arguments. */
while (typelist && len < nargs)
@@ -6227,6 +6227,14 @@ check_function_sentinel (tree attrs, int nargs, tree *argarray, tree typelist)
{
tree p = TREE_VALUE (TREE_VALUE (attr));
pos = TREE_INT_CST_LOW (p);
+
+ value = TREE_CHAIN (TREE_VALUE (attr));
+ if (value)
+ /* Use the user-provided sentinel value. */
+ value = TREE_VALUE (value);
+ else
+ /* Default to `NULL'. */
+ value = null_pointer_node;
}
/* The sentinel must be one of the varargs, i.e.
@@ -6240,14 +6248,28 @@ check_function_sentinel (tree attrs, int nargs, tree *argarray, tree typelist)
/* Validate the sentinel. */
sentinel = argarray[nargs - 1 - pos];
- if ((!POINTER_TYPE_P (TREE_TYPE (sentinel))
- || !integer_zerop (sentinel))
- /* Although __null (in C++) is only an integer we allow it
- nevertheless, as we are guaranteed that it's exactly
- as wide as a pointer, and we don't want to force
- users to cast the NULL they have written there.
- We warn with -Wstrict-null-sentinel, though. */
- && (warn_strict_null_sentinel || null_node != sentinel))
+ if (((POINTER_TYPE_P (TREE_TYPE (sentinel))
+ != POINTER_TYPE_P (TREE_TYPE (value)))
+ /* Although __null (in C++) is only an integer we allow it
+ nevertheless, as we are guaranteed that it's exactly
+ as wide as a pointer, and we don't want to force
+ users to cast the NULL they have written there.
+ We warn with -Wstrict-null-sentinel, though. */
+ && ((value == null_pointer_node)
+ ? (warn_strict_null_sentinel || null_node != sentinel)
+ : 1))
+
+ /* SENTINEL and VALUE have an integral type but these are different
+ types or types that don't have the same width. */
+ || (!POINTER_TYPE_P (TREE_TYPE (sentinel))
+ && !POINTER_TYPE_P (TREE_TYPE (value))
+ ? (TREE_CODE (value) != TREE_CODE (sentinel)
+ || (!tree_int_cst_equal (TYPE_SIZE (TREE_TYPE (value)),
+ TYPE_SIZE (TREE_TYPE (sentinel)))))
+ : 0)
+
+ || !tree_int_cst_equal (sentinel, value))
+
warning (OPT_Wformat, "missing sentinel in function call");
}
}
@@ -6434,6 +6456,21 @@ handle_sentinel_attribute (tree *node, tree name, tree args,
"requested position is less than zero");
*no_add_attrs = true;
}
+
+ args = TREE_CHAIN (args);
+ if (args)
+ {
+ /* Check the user-provided sentinel value. */
+ tree value = TREE_VALUE (args);
+
+ if (TREE_CODE (value) != INTEGER_CST)
+ {
+ warning (OPT_Wattributes,
+ "requested sentinel value is not "
+ "an integer constant");
+ *no_add_attrs = true;
+ }
+ }
}
}
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 5dfbbcf..0a71952 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2752,26 +2752,37 @@ section, consider using the facilities of the linker instead.
@item sentinel
@cindex @code{sentinel} function attribute
This function attribute ensures that a parameter in a function call is
-an explicit @code{NULL}. The attribute is only valid on variadic
-functions. By default, the sentinel is located at position zero, the
-last parameter of the function call. If an optional integer position
-argument P is supplied to the attribute, the sentinel must be located at
-position P counting backwards from the end of the argument list.
+an explicit @code{NULL} or some other user-specified integer constant.
+The attribute is only valid on variadic functions. By default, the
+sentinel is located at position zero, the last parameter of the function
+call. If an optional integer position argument P is supplied to the
+attribute, the sentinel must be located at position P counting backwards
+from the end of the argument list.
+
+Optionally, a second integer constant can be passed that determines the
+expected sentinel value. When this integer constant has a pointer type,
+the sentinel is expected to have pointer type as well.
@smallexample
__attribute__ ((sentinel))
is equivalent to
__attribute__ ((sentinel(0)))
+and is equivalent to
+__attribute__ ((sentinel(0, (void *) 0)))
@end smallexample
+A valid @code{NULL} in this context is defined as zero with any pointer
+type. If your system defines the @code{NULL} macro with an integer type
+then you need to add an explicit cast. GCC replaces @code{stddef.h}
+with a copy that redefines NULL appropriately. An exception is the C++
+@code{__null} value, which, although it is an integer constant, is
+considered a valid sentinel when @code{(void *) 0} is expected
+(@pxref{C++ Dialect Options, @code{-Wstrict-null-sentinel} option}).
+
The attribute is automatically set with a position of 0 for the built-in
functions @code{execl} and @code{execlp}. The built-in function
@code{execle} has the attribute set with a position of 1.
-A valid @code{NULL} in this context is defined as zero with any pointer
-type. If your system defines the @code{NULL} macro with an integer type
-then you need to add an explicit cast. GCC replaces @code{stddef.h}
-with a copy that redefines NULL appropriately.
The warnings for missing or incorrect sentinels are enabled with
@option{-Wformat}.
diff --git a/gcc/testsuite/gcc.dg/format/sentinel-1.c b/gcc/testsuite/gcc.dg/format/sentinel-1.c
index 1dc0908..be22ecd 100644
--- a/gcc/testsuite/gcc.dg/format/sentinel-1.c
+++ b/gcc/testsuite/gcc.dg/format/sentinel-1.c
@@ -10,6 +10,11 @@ extern int execlp (const char *, const char *, ...);
extern int execle (const char *, const char *, ...);
extern char *envp[];
+enum foo
+ {
+ the_enum = 0
+ };
+
#define ATTR __attribute__ ((__sentinel__))
extern int a ATTR; /* { dg-warning "applies to function types" "sentinel" } */
@@ -23,7 +28,15 @@ extern void foo6 (const char *, ...) __attribute__ ((__sentinel__(5)));
extern void foo7 (const char *, ...) __attribute__ ((__sentinel__(0)));
extern void foo8 (const char *, ...) __attribute__ ((__sentinel__("a"))); /* { dg-warning "not an integer constant" "sentinel" } */
extern void foo9 (const char *, ...) __attribute__ ((__sentinel__(-1))); /* { dg-warning "less than zero" "sentinel" } */
-extern void foo10 (const char *, ...) __attribute__ ((__sentinel__(1,3))); /* { dg-error "wrong number of arguments" "sentinel" } */
+extern void foo10 (const char *, ...) __attribute__ ((__sentinel__(1,2,3))); /* { dg-error "wrong number of arguments" "sentinel" } */
+
+extern void foo11 (const char *, ...) __attribute__ ((__sentinel__(0,NULL)));
+extern void foo12 (const char *, ...) __attribute__ ((__sentinel__(0,0)));
+extern void foo13 (const char *, ...) __attribute__ ((__sentinel__(0,-77)));
+extern void foo14 (const char *, ...) __attribute__ ((__sentinel__(0,(void *) -1)));
+extern void foo15 (const char *, ...) __attribute__ ((__sentinel__(2,777LL)));
+extern void foo16 (const char *, ...) __attribute__ ((__sentinel__(1,the_enum)));
+extern void foo17 (const char *, ...) __attribute__ ((__sentinel__(0,"a"))); /* { dg-warning "not an integer constant" "sentinel" } */
extern void bar(void)
{
@@ -59,6 +72,25 @@ extern void bar(void)
foo7 ("a", 1, 2, 3, NULL);
+ foo11 ("a", 1, 2, 3, NULL);
+ foo11 ("a", 1); /* { dg-warning "missing sentinel" "sentinel" } */
+ foo11 ("a", 0); /* { dg-warning "missing sentinel" "sentinel" } */
+
+ foo12 ("a", 0);
+ foo12 ("a", 1); /* { dg-warning "missing sentinel" "sentinel" } */
+ foo12 ("a", (void *) 0); /* Unless `-Wstrict-null-sentinel' is passed (C++),
+ using a null pointer instead of zero is OK. */
+ foo13 ("a", 1, 2, 3, 4, -77);
+ foo13 ("a", 1, 2); /* { dg-warning "missing sentinel" "sentinel" } */
+ foo14 ("a", 1, -1); /* { dg-warning "missing sentinel" "sentinel" } */
+ foo14 ("a", 1, (void *) -1);
+
+ foo15 ("a", 2, 3, 777); /* { dg-warning "missing sentinel" "sentinel" } */
+ foo15 ("a", 2, 3, 777, 4, 5); /* { dg-warning "missing sentinel" "sentinel" } */
+ foo15 ("a", 2, 3, 777LL, 4, 5);
+ foo16 ("a", 0ULL, -1); /* { dg-warning "missing sentinel" "sentinel" } */
+ foo16 ("a", the_enum, -1);
+
execl ("/bin/ls", "/bin/ls", "-aFC"); /* { dg-warning "missing sentinel" "sentinel" } */
execl ("/bin/ls", "/bin/ls", "-aFC", 0); /* { dg-warning "missing sentinel" "sentinel" } */
execl ("/bin/ls", "/bin/ls", "-aFC", NULL);