Re: [PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Martin Sebor]

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

Re: [PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Marc Glisse]

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...


--
Marc Glisse

Re: [PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Joseph Myers]

On Mon, 14 Aug 2017, Martin Sebor wrote:

> Okay.  I expanded on that point in the updated comments below.
> 
> Martin
> 
> 2017-08-14  Martin Sebor  
> 
> * builtin-attrs.def: Add comments.

This version is OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Martin Sebor]

On 08/14/2017 01:50 PM, Joseph Myers wrote:

On Mon, 14 Aug 2017, Martin Sebor wrote:


 /* This header provides a declarative way of describing the attributes
-   that are applied to some functions by default.
+   that are applied to some built-in functions by default.  Attributes
+   that apply to types or variables but not functions need not and
+   should not be defined here.


It's not just type and variable attributes that shouldn't be here.  Any
function attribute that's not used by at least one built-in function
shouldn't be here either.  Every tree constructed here adds to startup
costs; they should only be present if actually used.


Okay.  I expanded on that point in the updated comments below.

Martin

2017-08-14  Martin Sebor  

* builtin-attrs.def: Add comments.

Index: gcc/builtin-attrs.def
===
--- gcc/builtin-attrs.def   (revision 251097)
+++ gcc/builtin-attrs.def   (working copy)
@@ -18,7 +18,10 @@ along with GCC; see the file COPYING3.  If not see
 .  */

 /* This header provides a declarative way of describing the attributes
-   that are applied to some functions by default.
+   that are applied to some built-in functions by default.  Attributes
+   that are meant to be used by user-defined functions but aren't used
+   by any built-ins, or attributes that apply to types or variables
+   but not to functions need not and should not be defined here.

Before including this header, you must define the following macros.
In each case where there is an ENUM, it is an identifier used to
@@ -85,7 +88,9 @@ DEF_LIST_INT_INT (5,0)
 DEF_LIST_INT_INT (5,6)
 #undef DEF_LIST_INT_INT

-/* Construct trees for identifiers.  */
+/* Construct trees for identifiers used in built-in function attributes.
+   The construction contributes to startup costs so only attributes that
+   are used to define built-ins should be defined here.  */
 DEF_ATTR_IDENT (ATTR_ALLOC_SIZE, "alloc_size")
 DEF_ATTR_IDENT (ATTR_COLD, "cold")
 DEF_ATTR_IDENT (ATTR_CONST, "const")

Re: [PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Joseph Myers]

On Mon, 14 Aug 2017, Martin Sebor wrote:

>  /* This header provides a declarative way of describing the attributes
> -   that are applied to some functions by default.
> +   that are applied to some built-in functions by default.  Attributes
> +   that apply to types or variables but not functions need not and
> +   should not be defined here.

It's not just type and variable attributes that shouldn't be here.  Any 
function attribute that's not used by at least one built-in function 
shouldn't be here either.  Every tree constructed here adds to startup 
costs; they should only be present if actually used.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Martin Sebor]

On 08/14/2017 12:09 PM, Joseph Myers wrote:

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.


Okay, thanks for confirming that.  Here's a patch to make it
clear.  Please let me know if it's okay to commit.

Martin

2017-08-14  Martin Sebor  

* builtin-attrs.def: Add comments.

Index: gcc/builtin-attrs.def
===
--- gcc/builtin-attrs.def   (revision 251097)
+++ gcc/builtin-attrs.def   (working copy)
@@ -18,7 +18,9 @@ along with GCC; see the file COPYING3.  If not see
 .  */

 /* This header provides a declarative way of describing the attributes
-   that are applied to some functions by default.
+   that are applied to some built-in functions by default.  Attributes
+   that apply to types or variables but not functions need not and
+   should not be defined here.

Before including this header, you must define the following macros.
In each case where there is an ENUM, it is an identifier used to
@@ -85,7 +87,7 @@ DEF_LIST_INT_INT (5,0)
 DEF_LIST_INT_INT (5,6)
 #undef DEF_LIST_INT_INT

-/* Construct trees for identifiers.  */
+/* Construct trees for identifiers used in function attributes.  */
 DEF_ATTR_IDENT (ATTR_ALLOC_SIZE, "alloc_size")
 DEF_ATTR_IDENT (ATTR_COLD, "cold")
 DEF_ATTR_IDENT (ATTR_CONST, "const")

Re: [PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Joseph Myers]

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.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Martin Sebor]

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_


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

Re: [PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Jeff Law]

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_




> 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

[PATCH 2/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Martin Sebor]

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

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