This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PR80806
- From: Martin Sebor <msebor at gmail dot com>
- To: Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>, gcc Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <rguenther at suse dot de>
- Date: Tue, 23 May 2017 09:58:08 -0600
- Subject: Re: PR80806
- Authentication-results: sourceware.org; auth=none
- References: <CAAgBjMkiA6FwzYYVDBA7vymvRDhd_a08vamPf_42JQ5DV5SQ8g@mail.gmail.com>
On 05/18/2017 12:55 PM, Prathamesh Kulkarni wrote:
Hi,
The attached patch tries to fix PR80806 by warning when a variable is
set using memset (and friends) but not used. I chose to warn in dse
pass since dse would detect if the variable passed as 1st argument is
a dead store. Does this approach look OK ?
Detecting -Wunused-but-set-variable in the optimizer means that
the warning will not be issued without optimization. It also
means that the warning will trigger in cases where the variable
is used conditionally and the condition is subject to constant
propagation. For instance:
void sink (void*);
void test (int i)
{
char buf[10]; // -Wunused-but-set-variable
memset (buf, 0, sizeof(buf));
if (i)
sink (buf);
}
void f (void)
{
test (0);
}
I suspect this would be considered a false positive by most users.
In my view, it would be more in line with the design of the warning
to enhance the front end to detect this case, and it would avoid
these issues.
I have a patch that does that. Rather than checking the finite
set of known built-in functions like memset that are known not
to read the referenced object, I took the approach of adding
a new function attribute (I call it write-only) and avoiding
setting the DECL_READ_P flag for DECLs that are passed to
function arguments decorated with the attribute. That makes
it possible to issue the warning even if the variable is passed
to ordinary (non-built-in) functions like getline(), and should
open up optimization opportunities beyond built-ins. The only
wrinkle is that the front end sets DECL_READ_P even for uses that
aren't reads such as a sizeof expression, so while an otherwise
unused buf is diagnosed given a call to memset(buf, 0, 10), it
isn't diagnosed if a call is made to memset(buf, 0, sizeof buf).
I am yet to see what impact not setting DECL_READ_P would have
when the decl is used without being evaluated. (In any event,
setting DECL_READ_P on a use that doesn't involve reading the
DECL doesn't seem right.)
I attach what I have so far in case you would like to check it
out. I think you have more experience with DSE than me so I'd
be interested in your thoughts on making use of the attribute
for optimization. (Another couple attributes I'm considering
to complement write-only is read-only and read-write, also
with the hope of improving both warnings and code generation.
Ideas on those would be welcome as well.)
There were following fallouts observed during bootstrap build:
* double-int.c (div_and_round_double):
Warning emitted 'den' set but not used for following call to memset:
memset (den, 0, sizeof den);
I assume the warning is correct since there's immediately call to:
encode (den, lden, hden);
and encode overwrites all the contents of den.
Should the above call to memset be removed from the source ?
IIUC, this seems to be a more involved example of the simple one
above. AFAICS, the buffer is subsequently read in the function,
but only conditionally.
That said, since encode() overwrites the whole buffer right after
it has been cleared by memset, I would think that a warning pointing
that out could be helpful (although I'm not sure -Wunused is the
right warning to issue).
* tree-streamer.c (streamer_check_handled_ts_structures)
The function defines a local array bool handled_p[LAST_TS_ENUM];
and the warning is emitted for:
memset (&handled_p, 0, sizeof (handled_p));
That's because the function then initializes each element of the array
handled_p to true
making the memset call redundant.
I am not sure if warning for the above case is a good idea ? The call
to memset() seems deliberate, to initialize all elements to 0, and
later assert checks if all the elements were explicitly set to true.
Right. Warning on this function doesn't seem right since
the variable is subsequently used in the test loop.
* value-prof.c (free_hist):
Warns for the call to memset:
static int
free_hist (void **slot, void *data ATTRIBUTE_UNUSED)
{
histogram_value hist = *(histogram_value *) slot;
free (hist->hvalue.counters);
if (flag_checking)
memset (hist, 0xab, sizeof (*hist));
free (hist);
return 1;
}
Assuming flag_checking was true, the call to memset would be dead
anyway since it would be immediately freed ? Um, I don't understand
the purpose of memset in the above function.
I'm guessing it's an effort to invalidate the memory to detect
subsequent accesses to its contents. Warning on such code would
be valuable because it's a common misconception that a memset
can be used to wipe sensitive data (e.g., passwords) from memory
before freeing it, to prevent them from leaking into compromised
stack frames (or core files). But it should be a warning that's
distinct from -Wunused.
Martin
diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def
index 38fb1bb8..24e6f6a 100644
--- a/gcc/builtin-attrs.def
+++ b/gcc/builtin-attrs.def
@@ -113,6 +113,7 @@ DEF_ATTR_IDENT (ATTR_TM_REGPARM, "*tm regparm")
DEF_ATTR_IDENT (ATTR_TM_TMPURE, "transaction_pure")
DEF_ATTR_IDENT (ATTR_RETURNS_TWICE, "returns_twice")
DEF_ATTR_IDENT (ATTR_RETURNS_NONNULL, "returns_nonnull")
+DEF_ATTR_IDENT (ATTR_WRITE_ONLY, "write_only")
DEF_ATTR_TREE_LIST (ATTR_NOVOPS_LIST, ATTR_NOVOPS, ATTR_NULL, ATTR_NULL)
@@ -196,6 +197,7 @@ DEF_ATTR_TREE_LIST (ATTR_NOTHROW_NONNULL, ATTR_NONNULL, ATTR_NULL, \
DEF_ATTR_TREE_LIST (ATTR_NOTHROW_NONNULL_LEAF, ATTR_NONNULL, ATTR_NULL, \
ATTR_NOTHROW_LEAF_LIST)
DEF_ATTR_TREE_LIST (ATTR_NOTHROW_NONNULL_LEAF_LIST, ATTR_LEAF, ATTR_NULL, ATTR_NOTHROW_NONNULL_LEAF)
+
/* Nothrow functions whose first parameter is a nonnull pointer. */
DEF_ATTR_TREE_LIST (ATTR_NOTHROW_NONNULL_1, ATTR_NONNULL, ATTR_LIST_1, \
ATTR_NOTHROW_LIST)
@@ -255,10 +257,21 @@ DEF_ATTR_TREE_LIST (ATTR_NOTHROW_NONNULL_TYPEGENERIC_LEAF,
/* Nothrow const functions whose pointer parameter(s) are all nonnull. */
DEF_ATTR_TREE_LIST (ATTR_CONST_NOTHROW_NONNULL, ATTR_CONST, ATTR_NULL, \
ATTR_NOTHROW_NONNULL)
+
/* Nothrow leaf functions whose pointer parameter(s) are all nonnull,
and which return their first argument. */
DEF_ATTR_TREE_LIST (ATTR_RET1_NOTHROW_NONNULL_LEAF, ATTR_FNSPEC, ATTR_LIST_STR1, \
ATTR_NOTHROW_NONNULL_LEAF)
+
+DEF_ATTR_TREE_LIST (ATTR_NOTHROW_WRONLY1_LEAF, ATTR_WRITE_ONLY, \
+ ATTR_LIST_1, ATTR_NOTHROW_LEAF_LIST)
+
+DEF_ATTR_TREE_LIST (ATTR_NOTHROW_WRONLY2_LEAF, ATTR_WRITE_ONLY, \
+ ATTR_LIST_2, ATTR_NOTHROW_LEAF_LIST)
+
+DEF_ATTR_TREE_LIST (ATTR_RET1_NOTHROW_WRONLY1_LEAF, ATTR_WRITE_ONLY, \
+ ATTR_LIST_1, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+
/* Nothrow leaf functions whose pointer parameter(s) are all nonnull,
and return value is also nonnull. */
DEF_ATTR_TREE_LIST (ATTR_RETNONNULL_NOTHROW_LEAF, ATTR_RETURNS_NONNULL, ATTR_NULL, \
diff --git a/gcc/builtins.def b/gcc/builtins.def
index 58d78db..59d1453 100644
--- a/gcc/builtins.def
+++ b/gcc/builtins.def
@@ -652,15 +652,15 @@ DEF_C99_COMPL_BUILTIN (BUILT_IN_CTANL, "ctanl", BT_FN_COMPLEX_LONGDOUBLE_
/* bcmp, bcopy and bzero have traditionally accepted NULL pointers
when the length parameter is zero, so don't apply attribute "nonnull". */
DEF_EXT_LIB_BUILTIN (BUILT_IN_BCMP, "bcmp", BT_FN_INT_CONST_PTR_CONST_PTR_SIZE, ATTR_PURE_NOTHROW_LEAF_LIST)
-DEF_EXT_LIB_BUILTIN (BUILT_IN_BCOPY, "bcopy", BT_FN_VOID_CONST_PTR_PTR_SIZE, ATTR_NOTHROW_LEAF_LIST)
-DEF_EXT_LIB_BUILTIN (BUILT_IN_BZERO, "bzero", BT_FN_VOID_PTR_SIZE, ATTR_NOTHROW_LEAF_LIST)
+DEF_EXT_LIB_BUILTIN (BUILT_IN_BCOPY, "bcopy", BT_FN_VOID_CONST_PTR_PTR_SIZE, ATTR_NOTHROW_WRONLY2_LEAF)
+DEF_EXT_LIB_BUILTIN (BUILT_IN_BZERO, "bzero", BT_FN_VOID_PTR_SIZE, ATTR_NOTHROW_WRONLY1_LEAF)
DEF_EXT_LIB_BUILTIN (BUILT_IN_INDEX, "index", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
DEF_LIB_BUILTIN (BUILT_IN_MEMCHR, "memchr", BT_FN_PTR_CONST_PTR_INT_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
DEF_LIB_BUILTIN (BUILT_IN_MEMCMP, "memcmp", BT_FN_INT_CONST_PTR_CONST_PTR_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN_CHKP (BUILT_IN_MEMCPY, "memcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN_CHKP (BUILT_IN_MEMMOVE, "memmove", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN_CHKP (BUILT_IN_MEMCPY, "memcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_WRONLY1_LEAF)
+DEF_LIB_BUILTIN_CHKP (BUILT_IN_MEMMOVE, "memmove", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RET1_NOTHROW_WRONLY1_LEAF)
DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY, "mempcpy", BT_FN_PTR_PTR_CONST_PTR_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
-DEF_LIB_BUILTIN_CHKP (BUILT_IN_MEMSET, "memset", BT_FN_PTR_PTR_INT_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN_CHKP (BUILT_IN_MEMSET, "memset", BT_FN_PTR_PTR_INT_SIZE, ATTR_RET1_NOTHROW_WRONLY1_LEAF)
DEF_EXT_LIB_BUILTIN (BUILT_IN_RINDEX, "rindex", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY, "stpcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_RETNONNULL_NOTHROW_LEAF)
DEF_EXT_LIB_BUILTIN (BUILT_IN_STPNCPY, "stpncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
@@ -668,7 +668,7 @@ DEF_EXT_LIB_BUILTIN (BUILT_IN_STRCASECMP, "strcasecmp", BT_FN_INT_CONST_STRIN
DEF_LIB_BUILTIN_CHKP (BUILT_IN_STRCAT, "strcat", BT_FN_STRING_STRING_CONST_STRING, ATTR_RET1_NOTHROW_NONNULL_LEAF)
DEF_LIB_BUILTIN_CHKP (BUILT_IN_STRCHR, "strchr", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
DEF_LIB_BUILTIN (BUILT_IN_STRCMP, "strcmp", BT_FN_INT_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN_CHKP (BUILT_IN_STRCPY, "strcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN_CHKP (BUILT_IN_STRCPY, "strcpy", BT_FN_STRING_STRING_CONST_STRING, ATTR_RET1_NOTHROW_WRONLY1_LEAF)
DEF_LIB_BUILTIN (BUILT_IN_STRCSPN, "strcspn", BT_FN_SIZE_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
DEF_EXT_LIB_BUILTIN (BUILT_IN_STRDUP, "strdup", BT_FN_STRING_CONST_STRING, ATTR_MALLOC_NOTHROW_NONNULL_LEAF)
DEF_EXT_LIB_BUILTIN (BUILT_IN_STRNDUP, "strndup", BT_FN_STRING_CONST_STRING_SIZE, ATTR_MALLOC_NOTHROW_NONNULL_LEAF)
@@ -676,7 +676,7 @@ DEF_LIB_BUILTIN_CHKP (BUILT_IN_STRLEN, "strlen", BT_FN_SIZE_CONST_STRING, ATTR
DEF_EXT_LIB_BUILTIN (BUILT_IN_STRNCASECMP, "strncasecmp", BT_FN_INT_CONST_STRING_CONST_STRING_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
DEF_LIB_BUILTIN (BUILT_IN_STRNCAT, "strncat", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
DEF_LIB_BUILTIN (BUILT_IN_STRNCMP, "strncmp", BT_FN_INT_CONST_STRING_CONST_STRING_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
-DEF_LIB_BUILTIN (BUILT_IN_STRNCPY, "strncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_LIB_BUILTIN (BUILT_IN_STRNCPY, "strncpy", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_WRONLY1_LEAF)
DEF_LIB_BUILTIN (BUILT_IN_STRPBRK, "strpbrk", BT_FN_STRING_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
DEF_LIB_BUILTIN (BUILT_IN_STRRCHR, "strrchr", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
DEF_LIB_BUILTIN (BUILT_IN_STRSPN, "strspn", BT_FN_SIZE_CONST_STRING_CONST_STRING, ATTR_PURE_NOTHROW_NONNULL_LEAF)
@@ -825,7 +825,7 @@ DEF_EXT_LIB_BUILTIN (BUILT_IN_FFSLL, "ffsll", BT_FN_INT_LONGLONG, ATTR_CONST_
DEF_EXT_LIB_BUILTIN (BUILT_IN_FORK, "fork", BT_FN_PID, ATTR_NOTHROW_LIST)
DEF_GCC_BUILTIN (BUILT_IN_FRAME_ADDRESS, "frame_address", BT_FN_PTR_UINT, ATTR_NULL)
/* [trans-mem]: Adjust BUILT_IN_TM_FREE if BUILT_IN_FREE is changed. */
-DEF_LIB_BUILTIN (BUILT_IN_FREE, "free", BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST)
+DEF_LIB_BUILTIN (BUILT_IN_FREE, "free", BT_FN_VOID_PTR, ATTR_NOTHROW_WRONLY1_LEAF)
DEF_GCC_BUILTIN (BUILT_IN_FROB_RETURN_ADDR, "frob_return_addr", BT_FN_PTR_PTR, ATTR_NULL)
DEF_EXT_LIB_BUILTIN (BUILT_IN_GETTEXT, "gettext", BT_FN_STRING_CONST_STRING, ATTR_FORMAT_ARG_1)
DEF_C99_BUILTIN (BUILT_IN_IMAXABS, "imaxabs", BT_FN_INTMAX_INTMAX, ATTR_CONST_NOTHROW_LEAF_LIST)
@@ -916,16 +916,16 @@ DEF_BUILTIN_STUB (BUILT_IN_MEMCMP_EQ, "__builtin_memcmp_eq")
/* Object size checking builtins. */
DEF_GCC_BUILTIN (BUILT_IN_OBJECT_SIZE, "object_size", BT_FN_SIZE_CONST_PTR_INT, ATTR_PURE_NOTHROW_LEAF_LIST)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMCPY_CHK, "__memcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMMOVE_CHK, "__memmove_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMCPY_CHK, "__memcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_WRONLY1_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMMOVE_CHK, "__memmove_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RET1_NOTHROW_WRONLY1_LEAF)
DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMPCPY_CHK, "__mempcpy_chk", BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMSET_CHK, "__memset_chk", BT_FN_PTR_PTR_INT_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_MEMSET_CHK, "__memset_chk", BT_FN_PTR_PTR_INT_SIZE_SIZE, ATTR_RET1_NOTHROW_WRONLY1_LEAF)
DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STPCPY_CHK, "__stpcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
DEF_EXT_LIB_BUILTIN (BUILT_IN_STPNCPY_CHK, "__stpncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RETNONNULL_NOTHROW_LEAF)
DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCAT_CHK, "__strcat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCPY_CHK, "__strcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN_CHKP (BUILT_IN_STRCPY_CHK, "__strcpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE, ATTR_RET1_NOTHROW_WRONLY1_LEAF)
DEF_EXT_LIB_BUILTIN (BUILT_IN_STRNCAT_CHK, "__strncat_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
-DEF_EXT_LIB_BUILTIN (BUILT_IN_STRNCPY_CHK, "__strncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RET1_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN (BUILT_IN_STRNCPY_CHK, "__strncpy_chk", BT_FN_STRING_STRING_CONST_STRING_SIZE_SIZE, ATTR_RET1_NOTHROW_WRONLY1_LEAF)
DEF_EXT_LIB_BUILTIN (BUILT_IN_SNPRINTF_CHK, "__snprintf_chk", BT_FN_INT_STRING_SIZE_INT_SIZE_CONST_STRING_VAR, ATTR_FORMAT_PRINTF_NOTHROW_5_6)
DEF_EXT_LIB_BUILTIN (BUILT_IN_SPRINTF_CHK, "__sprintf_chk", BT_FN_INT_STRING_INT_SIZE_CONST_STRING_VAR, ATTR_NOTHROW_NONNULL_1_FORMAT_PRINTF_4_5)
DEF_EXT_LIB_BUILTIN (BUILT_IN_VSNPRINTF_CHK, "__vsnprintf_chk", BT_FN_INT_STRING_SIZE_INT_SIZE_CONST_STRING_VALIST_ARG, ATTR_FORMAT_PRINTF_NOTHROW_5_0)
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 695c58c..46ab8f0 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-iterator.h"
#include "opts.h"
#include "gimplify.h"
+#include "bitmap.h"
static tree handle_packed_attribute (tree *, tree, tree, int, bool *);
static tree handle_nocommon_attribute (tree *, tree, tree, int, bool *);
@@ -116,6 +117,7 @@ static tree handle_nothrow_attribute (tree *, tree, tree, int, bool *);
static tree handle_cleanup_attribute (tree *, tree, tree, int, bool *);
static tree handle_warn_unused_result_attribute (tree *, tree, tree, int,
bool *);
+static tree handle_write_only_attribute (tree *, tree, tree, int, bool *);
static tree handle_sentinel_attribute (tree *, tree, tree, int, bool *);
static tree handle_type_generic_attribute (tree *, tree, tree, int, bool *);
static tree handle_alloc_size_attribute (tree *, tree, tree, int, bool *);
@@ -345,6 +347,8 @@ const struct attribute_spec c_common_attribute_table[] =
handle_bnd_instrument, false },
{ "fallthrough", 0, 0, false, false, false,
handle_fallthrough_attribute, false },
+ { "write_only", 0, -1, false, true, true,
+ handle_write_only_attribute, false },
{ NULL, 0, 0, false, false, false, NULL, false }
};
@@ -2815,7 +2819,7 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED (name),
&& TREE_CODE (arg) != FUNCTION_DECL)
TREE_VALUE (args) = arg = default_conversion (arg);
- if (!get_nonnull_operand (arg, &arg_num))
+ if (!get_attribute_operand (arg, &arg_num))
{
error ("nonnull argument has invalid operand number (argument %lu)",
(unsigned long) attr_arg_num);
@@ -2860,6 +2864,121 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED (name),
return NULL_TREE;
}
+
+/* Handle the "write_only" attribute. */
+
+static tree
+handle_write_only_attribute (tree *node, tree name,
+ tree operands, int ARG_UNUSED (flags),
+ bool *no_add_attrs)
+{
+ tree type = *node;
+
+ /* If no operands are specified, all pointer arguments should be
+ write-only. Verify a full prototype is given so that the arguments
+ will have the correct types when we actually check them later.
+ Avoid diagnosing type-generic built-ins since those have no
+ prototype. */
+ if (!operands
+ && !prototype_p (type)
+ && (!TYPE_ATTRIBUTES (type)
+ || !lookup_attribute ("type generic", TYPE_ATTRIBUTES (type))))
+ {
+ error ("attribute %qE without arguments on a non-prototype", name);
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+
+ /* True if operands were specified. */
+ bool has_ops = operands;
+
+ /* A bitmap of operand values already handled. Used to warn about
+ duplicates. */
+ bitmap_head operset;
+ bitmap_initialize (&operset, 0);
+
+ /* Attribute operands have been specified. Verify that each operand
+ value, OPERVAL, references a non-const pointer argument to the
+ function. */
+ for (unsigned operno = 1; ; operno++,
+ operands = operands ? TREE_CHAIN (operands) : NULL_TREE)
+ {
+ /* The argument number the attribute operand corresponds to. */
+ unsigned HOST_WIDE_INT operval;
+
+ if (operands)
+ {
+ tree oper = TREE_VALUE (operands);
+ if (oper && TREE_CODE (oper) != IDENTIFIER_NODE
+ && TREE_CODE (oper) != FUNCTION_DECL)
+ TREE_VALUE (operands) = oper = default_conversion (oper);
+
+ if (!get_attribute_operand (oper, &operval))
+ {
+ error ("attribute %<%E(%E)%> invalid operand", name, oper);
+ *no_add_attrs = true;
+ break;
+ }
+ }
+ else if (!has_ops)
+ operval = operno;
+ else
+ break;
+
+ if (bitmap_bit_p (&operset, operval))
+ warning (OPT_Wattributes, "duplicate attribute %<%E(%u)%>",
+ name, operval);
+
+ bitmap_set_bit (&operset, operval);
+
+ if (!prototype_p (type))
+ continue;
+
+ tree argtype;
+
+ function_args_iterator iter;
+ function_args_iter_init (&iter, type);
+
+ for (unsigned idx = 1; ; ++idx, function_args_iter_next (&iter))
+ {
+ argtype = function_args_iter_cond (&iter);
+ if (!argtype || VOID_TYPE_P (argtype))
+ {
+ if (has_ops)
+ {
+ error ("attribute %<%E(%u)%> exceeds number of arguments %u",
+ name, operval, idx - 1);
+ *no_add_attrs = true;
+ }
+ bitmap_clear (&operset);
+ return NULL_TREE;
+ }
+
+ if (idx == operval)
+ break;
+ }
+
+ if (TREE_CODE (argtype) != POINTER_TYPE)
+ {
+ error ("attribute %<%E(%u)%> references non-pointer argument "
+ "type %qT", name, operval, argtype);
+ *no_add_attrs = true;
+ break;
+ }
+
+ if (TYPE_READONLY (TREE_TYPE (argtype)))
+ {
+ error ("attribute %<%E(%u)%> references %qs-qualified argument "
+ "type %qT", name, operval, "const", argtype);
+ *no_add_attrs = true;
+ break;
+ }
+ }
+
+ bitmap_clear (&operset);
+ return NULL_TREE;
+}
+
/* Handle a "nothrow" attribute; arguments as in
struct attribute_spec.handler. */
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 5f4488a..8f95bae 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5277,7 +5277,7 @@ nonnull_check_p (tree args, unsigned HOST_WIDE_INT param_num)
for (; args; args = TREE_CHAIN (args))
{
- bool found = get_nonnull_operand (TREE_VALUE (args), &arg_num);
+ bool found = get_attribute_operand (TREE_VALUE (args), &arg_num);
gcc_assert (found);
@@ -5314,11 +5314,11 @@ check_nonnull_arg (void *ctx, tree param, unsigned HOST_WIDE_INT param_num)
}
}
-/* Helper for nonnull attribute handling; fetch the operand number
- from the attribute argument list. */
+/* Helper for attribute handling; fetch the operand number from
+ the attribute argument list. */
bool
-get_nonnull_operand (tree arg_num_expr, unsigned HOST_WIDE_INT *valp)
+get_attribute_operand (tree arg_num_expr, unsigned HOST_WIDE_INT *valp)
{
/* Verify the arg number is a small constant. */
if (tree_fits_uhwi_p (arg_num_expr))
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 502dc2f..266313c 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -864,7 +864,7 @@ extern bool pointer_to_zero_sized_aggr_p (tree);
extern bool bool_promoted_to_int_p (tree);
extern tree fold_for_warn (tree);
extern tree c_common_get_narrower (tree, int *);
-extern bool get_nonnull_operand (tree, unsigned HOST_WIDE_INT *);
+extern bool get_attribute_operand (tree, unsigned HOST_WIDE_INT *);
#define c_sizeof(LOC, T) c_sizeof_or_alignof_type (LOC, T, true, false, 1)
#define c_alignof(LOC, T) c_sizeof_or_alignof_type (LOC, T, false, false, 1)
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 2e01316..95baad6 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -178,6 +178,9 @@ struct GTY(()) c_parser {
BOOL_BITFIELD in_if_block : 1;
/* True if we want to lex an untranslated string. */
BOOL_BITFIELD lex_untranslated_string : 1;
+ /* False if taking the address of a DECL should not set its DECL_READ_P
+ flag. */
+ BOOL_BITFIELD no_set_read : 1;
/* Objective-C specific parser/lexer information. */
@@ -1253,7 +1256,8 @@ static tree c_parser_transaction_cancel (c_parser *);
static struct c_expr c_parser_expression (c_parser *);
static struct c_expr c_parser_expression_conv (c_parser *);
static vec<tree, va_gc> *c_parser_expr_list (c_parser *, bool, bool,
- vec<tree, va_gc> **, location_t *,
+ vec<tree, va_gc> **, tree,
+ location_t *,
tree *, vec<location_t> *,
unsigned int * = NULL);
static void c_parser_oacc_declare (c_parser *);
@@ -4215,8 +4219,8 @@ c_parser_attributes (c_parser *parser)
{
tree tree_list;
c_parser_consume_token (parser);
- expr_list = c_parser_expr_list (parser, false, true,
- NULL, NULL, NULL, NULL);
+ expr_list = c_parser_expr_list (parser, false, true, NULL,
+ NULL_TREE, NULL, NULL, NULL);
tree_list = build_tree_list_vec (expr_list);
attr_args = tree_cons (NULL_TREE, arg1, tree_list);
release_tree_vector (expr_list);
@@ -4228,8 +4232,8 @@ c_parser_attributes (c_parser *parser)
attr_args = NULL_TREE;
else
{
- expr_list = c_parser_expr_list (parser, false, true,
- NULL, NULL, NULL, NULL);
+ expr_list = c_parser_expr_list (parser, false, true, NULL,
+ NULL_TREE, NULL, NULL, NULL);
attr_args = build_tree_list_vec (expr_list);
release_tree_vector (expr_list);
}
@@ -6973,7 +6977,8 @@ c_parser_unary_expression (c_parser *parser)
case CPP_AND:
c_parser_consume_token (parser);
op = c_parser_cast_expression (parser, NULL);
- mark_exp_read (op.value);
+ if (!parser->no_set_read)
+ mark_exp_read (op.value);
return parser_build_unary_op (op_loc, ADDR_EXPR, op);
case CPP_MULT:
{
@@ -8415,8 +8420,10 @@ c_parser_postfix_expression_after_primary (c_parser *parser,
exprlist = NULL;
else
exprlist = c_parser_expr_list (parser, true, false, &origtypes,
+ expr.value,
sizeof_arg_loc, sizeof_arg,
&arg_loc, &literal_zero_mask);
+
c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
"expected %<)%>");
orig_expr = expr;
@@ -8658,6 +8665,32 @@ c_parser_check_literal_zero (c_parser *parser, unsigned *literal_zero_mask,
}
}
+/* Return true if expression number IDX and type ARGTYPE eclared with
+ the write-only attribute in ATTRS is itself declared write-only.
+ ATTRS may be null. */
+
+static bool
+is_write_only_p (tree attrs, tree argtype, unsigned idx)
+{
+ if (!attrs)
+ return false;
+
+ if (!POINTER_TYPE_P (argtype))
+ return false;
+
+ if (TREE_READONLY (TREE_TYPE (argtype)))
+ return false;
+
+ for (tree a = attrs; a; a = TREE_CHAIN (a))
+ {
+ unsigned HOST_WIDE_INT wronlyidx;
+ if (!get_attribute_operand (a, &wronlyidx) || wronlyidx == idx)
+ return true;
+ }
+ return false;
+}
+
+
/* Parse a non-empty list of expressions. If CONVERT_P, convert
functions and arrays to pointers and lvalues to rvalues. If
FOLD_P, fold the expressions. If LOCATIONS is non-NULL, save the
@@ -8670,7 +8703,7 @@ c_parser_check_literal_zero (c_parser *parser, unsigned *literal_zero_mask,
static vec<tree, va_gc> *
c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
- vec<tree, va_gc> **p_orig_types,
+ vec<tree, va_gc> **p_orig_types, tree func,
location_t *sizeof_arg_loc, tree *sizeof_arg,
vec<location_t> *locations,
unsigned int *literal_zero_mask)
@@ -8680,6 +8713,8 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
struct c_expr expr;
location_t loc = c_parser_peek_token (parser)->location;
location_t cur_sizeof_arg_loc = UNKNOWN_LOCATION;
+
+ /* Zero-based expression index within the list. */
unsigned int idx = 0;
ret = make_tree_vector ();
@@ -8693,9 +8728,30 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
cur_sizeof_arg_loc = c_parser_peek_2nd_token (parser)->location;
if (literal_zero_mask)
c_parser_check_literal_zero (parser, literal_zero_mask, 0);
- expr = c_parser_expr_no_commas (parser, NULL);
+
+ tree functype = func ? TREE_TYPE (func) : NULL_TREE;
+ tree attrs = (functype
+ ? lookup_attribute ("write_only", TYPE_ATTRIBUTES (functype))
+ : NULL_TREE);
+
+ tree argtypes = func ? TYPE_ARG_TYPES (TREE_TYPE (func)) : NULL_TREE;
+
+ /* Determine if the expression in the list is declared write-only
+ and only mark it DECL_READ_P() if it isn't. */
+ bool wronly
+ = argtypes && is_write_only_p (attrs, TREE_VALUE (argtypes), idx + 1);
+
+ {
+ /* Avoid setting the read bit for write-only decls. */
+ bool save_set_read = parser->no_set_read;
+ parser->no_set_read = wronly;
+ expr = c_parser_expr_no_commas (parser, NULL);
+ parser->no_set_read = save_set_read;
+ }
+
if (convert_p)
- expr = convert_lvalue_to_rvalue (loc, expr, true, true);
+ expr = convert_lvalue_to_rvalue (loc, expr, true, !wronly);
+
if (fold_p)
expr.value = c_fully_fold (expr.value, false, NULL);
ret->quick_push (expr.value);
@@ -8721,9 +8777,28 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
cur_sizeof_arg_loc = UNKNOWN_LOCATION;
if (literal_zero_mask)
c_parser_check_literal_zero (parser, literal_zero_mask, idx + 1);
- expr = c_parser_expr_no_commas (parser, NULL);
+
+ /* Determine if the next expression in the list is declared
+ write-only and only mark it DECL_READ_P() if it isn't. */
+ if (argtypes && attrs)
+ {
+ argtypes = TREE_CHAIN (argtypes);
+ wronly = is_write_only_p (attrs, TREE_VALUE (argtypes), idx + 2);
+ }
+ else
+ wronly = false;
+
+ {
+ /* Avoid setting the read bit for write-only decls. */
+ bool save_set_read = parser->no_set_read;
+ parser->no_set_read = wronly;
+ expr = c_parser_expr_no_commas (parser, NULL);
+ parser->no_set_read = save_set_read;
+ }
+
if (convert_p)
- expr = convert_lvalue_to_rvalue (loc, expr, true, true);
+ expr = convert_lvalue_to_rvalue (loc, expr, true, !wronly);
+
if (fold_p)
expr.value = c_fully_fold (expr.value, false, NULL);
vec_safe_push (ret, expr.value);
@@ -8742,6 +8817,7 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
}
if (orig_types)
*p_orig_types = orig_types;
+
return ret;
}
@@ -9831,8 +9907,8 @@ static tree
c_parser_objc_keywordexpr (c_parser *parser)
{
tree ret;
- vec<tree, va_gc> *expr_list = c_parser_expr_list (parser, true, true,
- NULL, NULL, NULL, NULL);
+ vec<tree, va_gc> *expr_list = c_parser_expr_list (parser, true, true, NULL,
+ NULL_TREE, NULL, NULL, NULL);
if (vec_safe_length (expr_list) == 1)
{
/* Just return the expression, remove a level of
@@ -10682,7 +10758,8 @@ c_parser_oacc_wait_list (c_parser *parser, location_t clause_loc, tree list)
if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
return list;
- args = c_parser_expr_list (parser, false, true, NULL, NULL, NULL, NULL);
+ args = c_parser_expr_list (parser, false, true, NULL, NULL_TREE, NULL,
+ NULL, NULL);
if (args->length () == 0)
{
diff --git a/gcc/testsuite/gcc.dg/attr-write-only-2.c b/gcc/testsuite/gcc.dg/attr-write-only-2.c
new file mode 100644
index 0000000..a15bd55
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/attr-write-only-2.c
@@ -0,0 +1,109 @@
+/* Test to verify that attribute write_only is recognized by
+ -Wunused-but-set-variable.
+ { dg-do compile }
+ { dg-options "-Wunused-but-set-variable -Wunused-but-set-parameter" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+void bcopy (const void*, void*, size_t);
+void bzero (void*, size_t);
+void* memcpy (void*, const void*, size_t);
+void* memset (void*, int, size_t);
+
+char* strcat (char*, const char*);
+char* strcpy (char*, const char*);
+char* strncat (char*, const char*, size_t);
+char* strncpy (char*, const char*, size_t);
+
+#define WRONLY(...) __attribute__ ((write_only (__VA_ARGS__)))
+
+void wronly_1 (void*) WRONLY (1);
+
+enum { N = sizeof (int) };
+
+void test_bcopy (int a, /* { dg-warning "-Wunused-but-set-parameter" } */
+ const void *p)
+{
+ int i; /* { dg-warning "-Wunused-but-set-variable" } */
+ char ar[8]; /* { dg-warning "-Wunused-but-set-variable" } */
+
+ bcopy (p, &a, N);
+ bcopy (p, &i, N);
+ bcopy (p, &ar, N);
+}
+
+void test_bzero (int a) /* { dg-warning "-Wunused-but-set-parameter" } */
+{
+ int i; /* { dg-warning "-Wunused-but-set-variable" } */
+ char ar[8]; /* { dg-warning "-Wunused-but-set-variable" } */
+
+ bzero (&a, N);
+ bzero (&i, N);
+ bzero (&ar, N);
+}
+
+void test_memset (int a) /* { dg-warning "-Wunused-but-set-parameter" } */
+{
+ int i; /* { dg-warning "-Wunused-but-set-variable" } */
+ char ar[8]; /* { dg-warning "-Wunused-but-set-variable" } */
+
+ memset (&a, 0, N);
+ memset (&i, 0, N);
+ memset (&ar, 0, N);
+}
+
+void test_memcpy (int a, /* { dg-warning "-Wunused-but-set-parameter" } */
+ const void *p)
+{
+ int i; /* { dg-warning "-Wunused-but-set-variable" } */
+ char ar[8]; /* { dg-warning "-Wunused-but-set-variable" } */
+
+ memcpy (&a, p, N);
+ memcpy (&i, p, N);
+ memcpy (&ar, p, N);
+}
+
+void test_strcat (const char *s)
+{
+ char c = '\0';
+ char ar[8] = "";
+
+ strcat (&c, "");
+ strcat (ar, s);
+}
+
+void test_strcpy (const char *s)
+{
+ char c; /* { dg-warning "-Wunused-but-set-variable" } */
+ char d = '\0'; /* { dg-warning "-Wunused-but-set-variable" } */
+ char ar[8]; /* { dg-warning "-Wunused-but-set-variable" } */
+ char ar2[8] = ""; /* { dg-warning "-Wunused-but-set-variable" } */
+
+ strcpy (&c, "");
+ strcpy (&d, "");
+ strcpy (ar, s);
+ strcpy (ar2, s);
+}
+
+void test_strncpy (const char *s, unsigned n)
+{
+ char c; /* { dg-warning "-Wunused-but-set-variable" } */
+ char d = '\0'; /* { dg-warning "-Wunused-but-set-variable" } */
+ char ar[8]; /* { dg-warning "-Wunused-but-set-variable" } */
+ char ar2[8] = ""; /* { dg-warning "-Wunused-but-set-variable" } */
+
+ strncpy (&c, "", 1);
+ strncpy (&d, "", n);
+ strncpy (ar, s, n);
+ strncpy (ar2, s, n);
+}
+
+void test_usrdef (int a) /* { dg-warning "-Wunused-but-set-parameter" } */
+{
+ int i; /* { dg-warning "-Wunused-but-set-variable" } */
+ char ar[8]; /* { dg-warning "-Wunused-but-set-variable" } */
+
+ wronly_1 (&a);
+ wronly_1 (&i);
+ wronly_1 (&ar);
+}
diff --git a/gcc/testsuite/gcc.dg/attr-write-only.c b/gcc/testsuite/gcc.dg/attr-write-only.c
new file mode 100644
index 0000000..e238c81
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/attr-write-only.c
@@ -0,0 +1,34 @@
+/* Test to verify the handling of attribute write_only syntax.
+ { dg-do compile }
+ { dg-options "-Wall -ftrack-macro-expansion=0" } */
+
+#define WRONLY(...) __attribute__ ((write_only (__VA_ARGS__)))
+
+void wronly_v_all (void) __attribute__ ((write_only)); /* { dg-error "attribute .write_only\\(1\\). exceeds number of arguments 0" } */
+
+void wronly_i_all (int) __attribute__ ((write_only)); /* { dg-error "attribute .write_only\\(1\\). references non-pointer argument type .int." } */
+
+void wronly_v_1 (void*) WRONLY (1);
+void wronly_iv_2 (int, void*) WRONLY (2);
+void wronly_iiv_3 (int, int, void*) WRONLY (3);
+
+void wronly_v_0p1 (void*) WRONLY (0 + 1);
+void wronly_v_2m1 (void*) WRONLY (2 - 1);
+
+void wronly_v_1_1 (void*) WRONLY (1, 1); /* { dg-warning "duplicate attribute .write_only\\(1\\)." } */
+void wronly_vv_1_2 (void*, void*) WRONLY (1, 2);
+void wronly_vv_2_1 (void*, void*) WRONLY (2, 1);
+void wronly_vv_all (void*, void*) __attribute__ ((write_only));
+
+void wronly_vv_1_1 (void*, void*) WRONLY (1, 1); /* { dg-warning "duplicate attribute .write_only\\(1\\)." } */
+void wronly_vv_1_1 (void*, void*) WRONLY (2, 2); /* { dg-warning "duplicate attribute .write_only\\(2\\)." } */
+
+void wronly_4_exceed (int, int, int) WRONLY (4); /* { dg-error "attribute .write_only\\(4\\). exceeds number of arguments 3" } */
+
+void wronly_1_i (int) WRONLY (1); /* { dg-error "attribute .write_only\\(1\\). references non-pointer argument type .int." } */
+
+void wronly_2_cst (int, const char*) WRONLY (2); /* { dg-error "attribute .write_only\\(2\\). references .const.-qualified argument type .const char *." } */
+
+void wronly_m1_i (void*) WRONLY (-1); /* { dg-error "attribute .write_only\\(-1\\). invalid operand" } */
+
+void wronly_str_i (void*) WRONLY ("blah"); /* { dg-error "attribute .write_only\\(\"blah\"\\). invalid operand" } */