On 11/21/19 3:38 PM, Jeff Law wrote:
On 11/21/19 10:11 AM, Martin Sebor wrote:
Attached is another revision of this enhancement, this one
incorporating Richard's request for a more efficient encoding
of the attributes to enable faster parsing.  There is just one
attribute called access, with the rest being arguments.  This
is transformed to access (mode-string) where the mode-string
is a STRING_CST describing the access mode (read/write/both)
and the two positional arguments for the function type.

I have also removed the attributes from the built-in functions
since they're not used for anything yet, and added more argument
validation.

To test this a little more extensively, I annotated a few Glibc
<unistd.h> functions with the new attribute and rebuilt it and
its test suite.  That exposed a problem with function pointers
not being handled correctly so I fixed that by letting
the access attribute apply to function pointers (and function
pointer types in general).

When this is finalized, if there's time I'm still hoping to get
back to the parts of the patch that make use of the attribute
for -Wunused and -Wuninitialized that I removed on Jeff's request
for smaller, independent changes.

In GCC 11 I'd like to look into tying this attribute in with
_FORTIFY_SOURCE.

Martin

gcc-83859.diff

PR middle-end/83859 - attributes to associate pointer arguments and sizes

gcc/ChangeLog:

        PR middle-end/83859
        * attribs.h (struct attr_access): New.
        * attribs.c (decl_attributes): Add an informational note.
        * builtins.c (check_access): Make extern.  Consistently set no-warning
        after issuing a warning.  Handle calls through function pointers.  Set
        no-warning.
        * builtins.h (check_access): Declare.
        * calls.c (rdwr_access_hash): New type.
        (rdwr_map): Same.
        (init_attr_rdwr_indices): New function.
        (maybe_warn_rdwr_sizes): Same.
        (initialize_argument_information): Call init_attr_rdwr_indices.
        Call maybe_warn_rdwr_sizes.
        * doc/extend.texi (attribute access): Document new attribute.

gcc/c-family/ChangeLog:

        PR middle-end/83859
        * c-attribs.c (handle_access_attribute): New function.
        (c_common_attribute_table): Add new attribute.
        (get_argument_type): New function.
        (append_access_attrs): New function.
        (get_nonnull_operand): Rename...
        (get_attribute_operand): ...to this.
        * c-common.c (get_nonnull_operand): Rename...
        (get_attribute_operand): ...to this.

gcc/testsuite/ChangeLog:

        PR middle-end/83859
        * c-c++-common/attr-nonstring-8.c: Adjust text of expected warning.
        * gcc.dg/Wstringop-overflow-22.c: New test.
        * gcc.dg/Wstringop-overflow-23.c: New test.
        * gcc.dg/attr-access-read-only.c: New test.
        * gcc.dg/attr-access-read-write.c: New test.
        * gcc.dg/attr-access-read-write-2.c: New test.
        * gcc.dg/attr-access-write-only.c: New test.


diff --git a/gcc/calls.c b/gcc/calls.c
index 62921351b11..15627abbd0d 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -52,6 +52,8 @@ along with GCC; see the file COPYING3.  If not see
  #include "tree-ssa-strlen.h"
  #include "intl.h"
  #include "stringpool.h"
+#include "hash-map.h"
+#include "hash-traits.h"
  #include "attribs.h"
  #include "builtins.h"
  #include "gimple-fold.h"
@@ -1258,6 +1260,9 @@ alloc_max_size (void)
  bool
  get_size_range (tree exp, tree range[2], bool allow_zero /* = false */)
  {
+  if (!exp)
+    return false;
+
    if (tree_fits_uhwi_p (exp))
      {
        /* EXP is a constant.  */
This change isn't mentioned anywhere in the ChangeLog.  If its
intentional, please mention it in the ChangeLog.  If it's not
intentional, then drop it :-)

It's intentional.  I'll add it to the ChangeLog.

diff --git a/gcc/gimple.h b/gcc/gimple.h
index 5a190b1714d..10223386d04 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -24,6 +24,8 @@ along with GCC; see the file COPYING3.  If not see
#include "tree-ssa-alias.h"
  #include "gimple-expr.h"
+#include "function.h"
+#include "basic-block.h"
I thought I asked before, can these be moved into the .c files where
they're actually needed?

With the latest changes the include directives are no loner needed.

(You did ask and I answered the question a couple of replies ago
when they still were necessary:
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01473.html)


Otherwise it looks OK to me.

Okay, I'll commit it tomorrow if Richard has no further suggestions
for changes.

Martin


jeff


Reply via email to