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 :-)
> 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?
Otherwise it looks OK to me.
jeff
