On Tue, 21 Jan 2020 at 04:35, Joseph Myers <jos...@codesourcery.com> wrote:
>
> On Mon, 20 Jan 2020, Prathamesh Kulkarni wrote:
>
> > Hi,
> > This patch attempts to add returns_arg attribute for c-family
> > languages. For C++ methods, first arg is assumed to be this pointer,
>
> This is missing .texi documentation explaining the attribute and the cases
> for which it would be useful.
>
> A restriction to the first 4 arguments is not a good design of a language
> feature, whatever implementation issues there may be.
>
> Do you intend to update builtins.def in a followup patch for the various
> built-in functions (e.g. memcpy) for which such an attribute would be
> applicable?
>
> When extracting an integer value from an INTEGER_CST provided in user
> source code, you should always use tree_to_uhwi / tree_to_shwi as
> appropriate, after checking the relevant tree_fits_*, rather than using
> TREE_INT_CST_LOW directly, to avoid mishandling arguments that have a
> small number in the low part of the INTEGER_CST but have bits set in the
> high part as well. Any direct use of TREE_INT_CST_LOW should have a
> specific justification for why it is correct to discard the high part of
> the integer. See c-attribs.c:positional_argument, and try to use that
> function if possible rather than reimplementing bits of it, so that
> handling of attribute arguments giving the position of a function argument
> can be as consistent as possible between different attributes.
>
> There are coding style issues, e.g. diagnostics should not end with '.'
> and lines should be broken before not after an operator.
Hi Joseph,
Thanks for the suggestions. Using positional_argument helped to
simplify the patch,
and also catches the case when return-type and arg-type differ.
Does it look OK ?
I will update builtins.def in follow-up patch.
The middle-end representation issue of ERF_RETURNS_ARG still remains,
which restricts the attribute till first four args. The patch simply
emits sorry(), for arguments beyond first four..
I will try to address this in follow up patch.
Thanks,
Prathamesh
>
> --
> Joseph S. Myers
> jos...@codesourcery.com
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index dc9579c5c60..baed1b811ba 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -150,6 +150,7 @@ static tree handle_designated_init_attribute (tree *, tree, tree, int, bool *);
static tree handle_patchable_function_entry_attribute (tree *, tree, tree,
int, bool *);
static tree handle_copy_attribute (tree *, tree, tree, int, bool *);
+static tree handle_returns_arg_attribute (tree *, tree, tree, int, bool *);
/* Helper to define attribute exclusions. */
#define ATTR_EXCL(name, function, type, variable) \
@@ -484,6 +485,8 @@ const struct attribute_spec c_common_attribute_table[] =
handle_noinit_attribute, attr_noinit_exclusions },
{ "access", 1, 3, false, true, true, false,
handle_access_attribute, NULL },
+ { "returns_arg", 1, 1, true, false, false, false,
+ handle_returns_arg_attribute, NULL },
{ NULL, 0, 0, false, false, false, false, NULL, NULL }
};
@@ -4603,6 +4606,55 @@ handle_patchable_function_entry_attribute (tree *, tree name, tree args,
return NULL_TREE;
}
+/* Handle a "returns_arg" attribute; arguments as in
+ struct attribute_spec.handler. */
+
+static tree
+handle_returns_arg_attribute (tree *node, tree name, tree args,
+ int flags, bool *no_add_attrs)
+{
+ tree decl = *node;
+ tree rettype = TREE_TYPE (decl);
+
+ if (TREE_CODE (rettype) == METHOD_TYPE
+ || TREE_CODE (rettype) == FUNCTION_TYPE)
+ rettype = TREE_TYPE (rettype);
+
+ if (VOID_TYPE_P (rettype))
+ {
+ warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+ "%qE attribute ignored on a function returning %qT.",
+ name, rettype);
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+
+ if (!positional_argument (TREE_TYPE (decl), name, TREE_VALUE (args),
+ TREE_CODE (rettype)))
+ {
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+
+ *no_add_attrs = false;
+ gcc_assert (args);
+ tree val = TREE_VALUE (args);
+ unsigned HOST_WIDE_INT argnum = tree_to_uhwi (val);
+
+ if (argnum >= 4)
+ sorry ("returns_arg attr can only be applied to first four args.\n");
+
+ char s[2];
+ s[0] = argnum + '0';
+ s[1] = '\0';
+
+ tree attr = tree_cons (get_identifier ("fn spec"),
+ build_tree_list (NULL_TREE, build_string (1, s)),
+ NULL_TREE);
+ decl_attributes (node, attr, flags);
+ return NULL_TREE;
+}
+
/* Attempt to partially validate a single attribute ATTR as if
it were to be applied to an entity OPER. */
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index ec99c38a607..3531e0c8292 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3566,6 +3566,19 @@ diagnosed. Because a pure function cannot have any observable side
effects it does not make sense for such a function to return @code{void}.
Declaring such a function is diagnosed.
+@item returns_arg (@var{position})
+@cindex @code{returns_arg} function attribute
+The @code{returns_arg}, when applied to an argument states that
+the function returns that argument umodified.
+For instance, the declaration:
+
+@smallexample
+int foo(int x) __attribute__((returns_arg(1)));
+@end smallexample
+
+@noindent
+lets the compiler know that foo returns x.
+
@item returns_nonnull
@cindex @code{returns_nonnull} function attribute
The @code{returns_nonnull} attribute specifies that the function
diff --git a/gcc/testsuite/g++.dg/Wattributes-6.C b/gcc/testsuite/g++.dg/Wattributes-6.C
new file mode 100644
index 00000000000..fcf660a4684
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wattributes-6.C
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* Check that 'this' is counted as first arg to the attribute. */
+
+struct X
+{
+ X *f() __attribute__((returns_arg(1)));
+};
+
+int main()
+{
+ X x;
+ x.f();
+}
diff --git a/gcc/testsuite/gcc.dg/Wattributes-11.c b/gcc/testsuite/gcc.dg/Wattributes-11.c
new file mode 100644
index 00000000000..81245b44cfa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wattributes-11.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Wattributes" } */
+
+int f1 (int) __attribute__((returns_arg)); /* { dg-error "wrong number of arguments specified for 'returns_arg' attribute" } */
+
+void f2 (int) __attribute__((returns_arg (1))); /* { dg-warning "'returns_arg' attribute ignored on a function returning 'void'" } */
+
+int f3 (int) __attribute__((returns_arg ("foo"))); /* { dg-warning "'returns_arg' attribute argument has type 'char\\\[4\\\]'" } */
+
+int f4 (char *) __attribute__((returns_arg (1))); /* { dg-warning "'returns_arg' attribute argument value '1' refers to parameter type 'char \\*'" } */
