| Message ID | 164d8b08-ced6-f2b2-ae6e-ee96afebb52e@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 08/06/2017 02:07 PM, Martin Sebor wrote: > Part 2 of the series adds attribute nostring to annotate arrays > of and pointers to char with that are intended to store sequences > of characters that aren't necessarily valid (nul-terminated) > strings. In the subsequent patch the attribute is relied on to > avoid diagnosing strcncpy calls that truncate strings and create > such copies. In the future I'd like to also use the attribute > to diagnose when arrays or pointers with the attribute are passed > to functions that expect nul-terminated strings (such as strlen > or strcpy). > > Martin > > > gcc-81117-2.diff > > > PR c/81117 - Improve buffer overflow checking in strncpy > > gcc/ChangeLog: > > PR c/81117 > * builtin-attrs.def (attribute nonstring): New. > * doc/extend.texi (attribute nonstring): Document new attribute. > > gcc/c-family/ChangeLog: > > PR c/81117 > * c-attribs.c (c_common_attribute_table): Add nonstring entry. > (handle_nonstring_attribute): New function. > > gcc/testsuite/ChangeLog: > > PR c/81117 > * c-c++-common/attr-nonstring-1.c: New test. > > --- a/gcc/builtin-attrs.def > +++ b/gcc/builtin-attrs.def > @@ -93,6 +93,7 @@ DEF_ATTR_IDENT (ATTR_FORMAT, "format") > DEF_ATTR_IDENT (ATTR_FORMAT_ARG, "format_arg") > DEF_ATTR_IDENT (ATTR_MALLOC, "malloc") > DEF_ATTR_IDENT (ATTR_NONNULL, "nonnull") > +DEF_ATTR_IDENT (ATTR_NONSTRING, "nonstring") > DEF_ATTR_IDENT (ATTR_NORETURN, "noreturn") > DEF_ATTR_IDENT (ATTR_NOTHROW, "nothrow") > DEF_ATTR_IDENT (ATTR_LEAF, "leaf") So all the attributes here are associated with functions I believe. You're defining a variable attribute. In fact, I'm not even sure that variable attributes get a DEF_ATTR_<whatever> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > index b253ccc..1954ca5 100644 > --- a/gcc/doc/extend.texi > +++ b/gcc/doc/extend.texi > @@ -5835,6 +5835,30 @@ The @code{deprecated} attribute can also be used for functions and > types (@pxref{Common Function Attributes}, > @pxref{Common Type Attributes}). > > +@item nonstring (@var{nonstring}) > +@cindex @code{nonstring} variable attribute > +The @code{nonstring} variable attribute specifies that an object or member > +declaration with type array of @code{char} or pointer to @code{char} is > +intended to store character arrays that do not necessarily contain > +a terminating @code{NUL} character. This is useful to avoid warnings > +when such an array or pointer is used as an argument to a bounded string > +manipulation function such as @code{strncpy}. For example, without the > +attribute, GCC will issue a warning for the call below because it may > +truncate the copy without appending the terminating NUL character. Using > +the attribute makes it possible to suppress the warning. [ ... ] I think this is in the wrong section, I believe it belongs in the "Variable Attributes" section. Assuming you don't actually need the ATTR_NONSTRING, this patch is fine with that hunk removed and the documentation moved into the right section. jeff
On 08/09/2017 11:00 PM, Jeff Law wrote: > On 08/06/2017 02:07 PM, Martin Sebor wrote: >> Part 2 of the series adds attribute nostring to annotate arrays >> of and pointers to char with that are intended to store sequences >> of characters that aren't necessarily valid (nul-terminated) >> strings. In the subsequent patch the attribute is relied on to >> avoid diagnosing strcncpy calls that truncate strings and create >> such copies. In the future I'd like to also use the attribute >> to diagnose when arrays or pointers with the attribute are passed >> to functions that expect nul-terminated strings (such as strlen >> or strcpy). >> >> Martin >> >> >> gcc-81117-2.diff >> >> >> PR c/81117 - Improve buffer overflow checking in strncpy >> >> gcc/ChangeLog: >> >> PR c/81117 >> * builtin-attrs.def (attribute nonstring): New. >> * doc/extend.texi (attribute nonstring): Document new attribute. >> >> gcc/c-family/ChangeLog: >> >> PR c/81117 >> * c-attribs.c (c_common_attribute_table): Add nonstring entry. >> (handle_nonstring_attribute): New function. >> >> gcc/testsuite/ChangeLog: >> >> PR c/81117 >> * c-c++-common/attr-nonstring-1.c: New test. >> >> --- a/gcc/builtin-attrs.def >> +++ b/gcc/builtin-attrs.def >> @@ -93,6 +93,7 @@ DEF_ATTR_IDENT (ATTR_FORMAT, "format") >> DEF_ATTR_IDENT (ATTR_FORMAT_ARG, "format_arg") >> DEF_ATTR_IDENT (ATTR_MALLOC, "malloc") >> DEF_ATTR_IDENT (ATTR_NONNULL, "nonnull") >> +DEF_ATTR_IDENT (ATTR_NONSTRING, "nonstring") >> DEF_ATTR_IDENT (ATTR_NORETURN, "noreturn") >> DEF_ATTR_IDENT (ATTR_NOTHROW, "nothrow") >> DEF_ATTR_IDENT (ATTR_LEAF, "leaf") > So all the attributes here are associated with functions I believe. > You're defining a variable attribute. In fact, I'm not even sure that > variable attributes get a DEF_ATTR_<whatever> I assumed every attribute needed to define an identifier but nothing broke after I removed it so it looks like you're right variable attributes don't need one. Go figure. It would be nice if there was a comment above the block that mentioned that. I'll try to remember to add one separately. >> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi >> index b253ccc..1954ca5 100644 >> --- a/gcc/doc/extend.texi >> +++ b/gcc/doc/extend.texi >> @@ -5835,6 +5835,30 @@ The @code{deprecated} attribute can also be used for functions and >> types (@pxref{Common Function Attributes}, >> @pxref{Common Type Attributes}). >> >> +@item nonstring (@var{nonstring}) >> +@cindex @code{nonstring} variable attribute >> +The @code{nonstring} variable attribute specifies that an object or member >> +declaration with type array of @code{char} or pointer to @code{char} is >> +intended to store character arrays that do not necessarily contain >> +a terminating @code{NUL} character. This is useful to avoid warnings >> +when such an array or pointer is used as an argument to a bounded string >> +manipulation function such as @code{strncpy}. For example, without the >> +attribute, GCC will issue a warning for the call below because it may >> +truncate the copy without appending the terminating NUL character. Using >> +the attribute makes it possible to suppress the warning. > [ ... ] > I think this is in the wrong section, I believe it belongs in the > "Variable Attributes" section. It is in the Variable Attributes section. The "pxref{Common Type Attributes})." reference above is just a cross-reference to the Type Attributes section. > Assuming you don't actually need the ATTR_NONSTRING, this patch is fine > with that hunk removed and the documentation moved into the right section. Okay, thanks. Martin
On Mon, 14 Aug 2017, Martin Sebor wrote: > I assumed every attribute needed to define an identifier but > nothing broke after I removed it so it looks like you're right > variable attributes don't need one. Go figure. It would be > nice if there was a comment above the block that mentioned that. > I'll try to remember to add one separately. builtin-attrs.def is only for attributes used in defining built-in functions. If no built-in function uses an attribute, it should not be defined there.
On Sun, 6 Aug 2017, Martin Sebor wrote:
> +@item nonstring (@var{nonstring})
Hello,
what is the "(@var{nonstring})" for? This attribute does not seem to take
any argument...
On 11/10/2017 03:52 PM, Marc Glisse wrote: > On Sun, 6 Aug 2017, Martin Sebor wrote: > >> +@item nonstring (@var{nonstring}) > > Hello, > > what is the "(@var{nonstring})" for? This attribute does not seem to > take any argument... It's a copy and paste typo. I removed it in r254659. Thanks for pointing it out. Martin
PR c/81117 - Improve buffer overflow checking in strncpy gcc/ChangeLog: PR c/81117 * builtin-attrs.def (attribute nonstring): New. * doc/extend.texi (attribute nonstring): Document new attribute. gcc/c-family/ChangeLog: PR c/81117 * c-attribs.c (c_common_attribute_table): Add nonstring entry. (handle_nonstring_attribute): New function. gcc/testsuite/ChangeLog: PR c/81117 * c-c++-common/attr-nonstring-1.c: New test. --- a/gcc/builtin-attrs.def +++ b/gcc/builtin-attrs.def @@ -93,6 +93,7 @@ DEF_ATTR_IDENT (ATTR_FORMAT, "format") DEF_ATTR_IDENT (ATTR_FORMAT_ARG, "format_arg") DEF_ATTR_IDENT (ATTR_MALLOC, "malloc") DEF_ATTR_IDENT (ATTR_NONNULL, "nonnull") +DEF_ATTR_IDENT (ATTR_NONSTRING, "nonstring") DEF_ATTR_IDENT (ATTR_NORETURN, "noreturn") DEF_ATTR_IDENT (ATTR_NOTHROW, "nothrow") DEF_ATTR_IDENT (ATTR_LEAF, "leaf") diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 0d9ab2d..69d020c 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -116,6 +116,7 @@ static tree handle_deprecated_attribute (tree *, tree, tree, int, static tree handle_vector_size_attribute (tree *, tree, tree, int, bool *); static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *); +static tree handle_nonstring_attribute (tree *, tree, tree, int, bool *); 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, @@ -270,6 +271,8 @@ const struct attribute_spec c_common_attribute_table[] = handle_tls_model_attribute, false }, { "nonnull", 0, -1, false, true, true, handle_nonnull_attribute, false }, + { "nonstring", 0, 0, true, false, false, + handle_nonstring_attribute, false }, { "nothrow", 0, 0, true, false, false, handle_nothrow_attribute, false }, { "may_alias", 0, 0, false, true, false, NULL, false }, @@ -2970,6 +2973,48 @@ handle_nonnull_attribute (tree *node, tree ARG_UNUSED (name), return NULL_TREE; } +/* Handle the "nonstring" variable attribute. */ + +static tree +handle_nonstring_attribute (tree *node, tree name, tree ARG_UNUSED (args), + int ARG_UNUSED (flags), bool *no_add_attrs) +{ + gcc_assert (!args); + tree_code code = TREE_CODE (*node); + + if (VAR_P (*node) + || code == FIELD_DECL + || code == PARM_DECL) + { + tree type = TREE_TYPE (*node); + + if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE) + { + tree eltype = TREE_TYPE (type); + if (eltype == char_type_node) + return NULL_TREE; + } + + warning (OPT_Wattributes, + "%qE attribute ignored on objects of type %qT", + name, type); + *no_add_attrs = true; + return NULL_TREE; + } + + if (code == FUNCTION_DECL) + warning (OPT_Wattributes, + "%qE attribute does not apply to functions", name); + else if (code == TYPE_DECL) + warning (OPT_Wattributes, + "%qE attribute does not apply to types", name); + else + warning (OPT_Wattributes, "%qE attribute ignored", name); + + *no_add_attrs = true; + return NULL_TREE; +} + /* Handle a "nothrow" attribute; arguments as in struct attribute_spec.handler. */ diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index b253ccc..1954ca5 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -5835,6 +5835,30 @@ The @code{deprecated} attribute can also be used for functions and types (@pxref{Common Function Attributes}, @pxref{Common Type Attributes}). +@item nonstring (@var{nonstring}) +@cindex @code{nonstring} variable attribute +The @code{nonstring} variable attribute specifies that an object or member +declaration with type array of @code{char} or pointer to @code{char} is +intended to store character arrays that do not necessarily contain +a terminating @code{NUL} character. This is useful to avoid warnings +when such an array or pointer is used as an argument to a bounded string +manipulation function such as @code{strncpy}. For example, without the +attribute, GCC will issue a warning for the call below because it may +truncate the copy without appending the terminating NUL character. Using +the attribute makes it possible to suppress the warning. + +@smallexample +struct Data +@{ + char name [32] __attribute__ ((nonstring)); +@}; +void f (struct Data *pd, const char *s) +@{ + strncpy (pd->name, s, sizeof pd->name); + @dots{} +@} +@end smallexample + @item mode (@var{mode}) @cindex @code{mode} variable attribute This attribute specifies the data type for the declaration---whichever diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-1.c b/gcc/testsuite/c-c++-common/attr-nonstring-1.c new file mode 100644 index 0000000..10a6688 --- /dev/null +++ b/gcc/testsuite/c-c++-common/attr-nonstring-1.c @@ -0,0 +1,60 @@ +/* Test to exercise attribute "nonstring" syntax. + { dg-do compile } + { dg-options "-Wattributes" } */ + +#define ATTR(list) __attribute__ (list) +#define NONSTR ATTR ((nonstring)) + +/* Verify it's accepted on char arrays. */ +extern NONSTR char nsx_1[]; +extern char NONSTR nsx_2[]; +extern char nsx_3[] NONSTR; + +extern NONSTR char ns1[1]; +extern char NONSTR ns3[3]; +extern char ns5[5] NONSTR; + +/* Verify it's accepted on char pointers. */ +extern NONSTR char* pns_1; +extern char NONSTR* pns_2; +extern char* NONSTR pns_3; + +struct S +{ +/* Verify it's accepted on char member pointers. */ + NONSTR char* mpns_1; + char NONSTR* mpns_2; + char* NONSTR mpns_3; + +/* Verify it's accepted on char member arrays. */ + NONSTR char mns1[1]; + char NONSTR mns3[3]; + char mns5[5] NONSTR; + +/* Verify it's accepted on char flexible array members. */ + char mnsx[] NONSTR; +}; + +/* Verify it's rejected on non-array and non-pointer objects. */ +extern NONSTR char c1; /* { dg-warning ".nonstring. attribute ignored on objects of type .char." } */ + +extern NONSTR int i1; /* { dg-warning ".nonstring. attribute ignored on objects of type .int." } */ + +extern NONSTR int ia1[]; /* { dg-warning ".nonstring. attribute ignored on objects of type .int *\\\[\\\]." } */ + +extern NONSTR int* pi1; /* { dg-warning ".nonstring. attribute ignored on objects of type .int *\\*." } */ + +extern NONSTR +void f (void); /* { dg-warning ".nonstring. attribute does not apply to functions" } */ + +struct NONSTR +NonStrType { int i; }; /* { dg-warning ".nonstring. attribute does not apply to types" } */ + +typedef char NONSTR nschar_t; /* { dg-warning ".nonstring. attribute does not apply to types" } */ + +void func (NONSTR char *pns1, char NONSTR *pns2, char* NONSTR pns3) +{ + (void)pns1; + (void)pns2; + (void)pns3; +}