Hi,
this is what I came up with. I couldn't eliminate all the manual
checking, as positional_argument doesn't check for nested types for
example, but I tried to use it wherever I could.
Regtested with the rest of the series without problems. OK for master?
Best regards,
Josef
-- >8 --
This patch replaces some of the manual bounds checking done in the
callback_only attribute handler with calls to positional_argument.
Since positional_argument issues warnings and the attribute handler
issues errors, I decided to downgrade the errors to warnings for
consistency.
gcc/c-family/ChangeLog:
* c-attribs.cc (handle_callback_only_attribute): Use
positional_argument for bounds checking in callback_only
attribute handler, downgrade errors to warnings for
consistency.
gcc/testsuite/ChangeLog:
* gcc.dg/attr-callback.c: Expect warnings instead of errors,
adjust expected messages.
Signed-off-by: Josef Melcr <[email protected]>
---
gcc/c-family/c-attribs.cc | 103 ++++++++++++---------------
gcc/testsuite/gcc.dg/attr-callback.c | 46 ++++++++----
2 files changed, 77 insertions(+), 72 deletions(-)
diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 1ed02850a91..01150136a55 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -4682,66 +4682,52 @@ handle_callback_only_attribute (tree *node, tree name,
tree args,
tree decl = *node;
if (TREE_CODE (decl) != FUNCTION_DECL)
{
- error_at (DECL_SOURCE_LOCATION (decl),
- "%qE attribute can only be used on functions", name);
+ warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+ "%qE attribute can only be used on functions", name);
*no_add_attrs = true;
}
- tree cb_fn_idx_node = TREE_VALUE (args);
- if (TREE_CODE (cb_fn_idx_node) != INTEGER_CST)
+ tree val = positional_argument (decl, name, TREE_VALUE (args), POINTER_TYPE,
+ 1, POSARG_ZERO);
+ if (!val)
{
- error_at (DECL_SOURCE_LOCATION (decl),
- "argument specifying callback function position is not an "
- "integer constant");
*no_add_attrs = true;
return NULL_TREE;
}
+ TREE_VALUE (args) = val;
+
/* We have to use the function type for validation, as
DECL_ARGUMENTS returns NULL at this point. */
- int callback_fn_idx = TREE_INT_CST_LOW (cb_fn_idx_node);
+ int callback_fn_idx = TREE_INT_CST_LOW (val);
tree decl_type_args = TYPE_ARG_TYPES (TREE_TYPE (decl));
tree it;
- int decl_nargs = list_length (decl_type_args);
for (it = decl_type_args; it != NULL_TREE; it = TREE_CHAIN (it))
if (it == void_list_node)
- {
- --decl_nargs;
- break;
- }
+ break;
+
if (callback_fn_idx == CB_UNKNOWN_POS)
{
- error_at (DECL_SOURCE_LOCATION (decl),
- "callback function position cannot be marked as unknown");
+ warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+ "callback function position cannot be marked as unknown");
*no_add_attrs = true;
return NULL_TREE;
}
+
--callback_fn_idx;
- if (callback_fn_idx >= decl_nargs)
- {
- error_at (DECL_SOURCE_LOCATION (decl),
- "callback function position out of range");
- *no_add_attrs = true;
- return NULL_TREE;
- }
- /* Search for the type of the callback function
- in parameters of the original function. */
+ /* Search for the type of the callback function in parameters of the original
+ function. We know it's there because it's been validated by
+ positional_argument. */
tree cfn = chain_index (callback_fn_idx, decl_type_args);
- if (cfn == NULL_TREE)
- {
- error_at (DECL_SOURCE_LOCATION (decl),
- "could not retrieve callback function from arguments");
- *no_add_attrs = true;
- return NULL_TREE;
- }
+ gcc_checking_assert (cfn != NULL_TREE);
cfn = TREE_VALUE (cfn);
tree cfn_pointee_type = TREE_TYPE (cfn);
if (TREE_CODE (cfn) != POINTER_TYPE
|| TREE_CODE (cfn_pointee_type) != FUNCTION_TYPE)
{
- error_at (DECL_SOURCE_LOCATION (decl),
- "argument no. %d is not an address of a function",
- callback_fn_idx + 1);
+ warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+ "argument no. %d is not an address of a function",
+ callback_fn_idx + 1);
*no_add_attrs = true;
return NULL_TREE;
}
@@ -4759,9 +4745,9 @@ handle_callback_only_attribute (tree *node, tree name,
tree args,
}
if (cfn_nargs != type_nargs)
{
- error_at (DECL_SOURCE_LOCATION (decl),
- "argument number mismatch, %d expected, got %d", type_nargs,
- cfn_nargs);
+ warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+ "argument number mismatch, %d expected, got %d", type_nargs,
+ cfn_nargs);
*no_add_attrs = true;
return NULL_TREE;
}
@@ -4777,12 +4763,23 @@ handle_callback_only_attribute (tree *node, tree name,
tree args,
{
if (TREE_CODE (TREE_VALUE (cfn_it)) != INTEGER_CST)
{
- error_at (DECL_SOURCE_LOCATION (decl),
- "argument no. %d is not an integer constant", curr + 1);
+ warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+ "argument no. %d is not an integer constant", curr + 1);
*no_add_attrs = true;
continue;
}
+ tree expected_type = TREE_VALUE (it);
+ tree arg_val = positional_argument (decl, name, TREE_VALUE (cfn_it),
+ TREE_CODE (expected_type), curr + 1,
+ POSARG_ZERO);
+ if (!arg_val)
+ {
+ *no_add_attrs = true;
+ return NULL_TREE;
+ }
+
+ TREE_VALUE (cfn_it) = arg_val;
int arg_idx = TREE_INT_CST_LOW (TREE_VALUE (cfn_it));
/* No need to check for type compatibility,
@@ -4791,29 +4788,19 @@ handle_callback_only_attribute (tree *node, tree name,
tree args,
continue;
arg_idx -= 1;
- /* Report an error if the position is out of bounds,
- but we can still check the rest of the arguments. */
- if (arg_idx >= decl_nargs)
- {
- error_at (DECL_SOURCE_LOCATION (decl),
- "callback argument index %d is out of range", arg_idx + 1);
- *no_add_attrs = true;
- continue;
- }
-
tree arg_type = chain_index (arg_idx, decl_type_args);
gcc_checking_assert (arg_type != NULL_TREE);
arg_type = TREE_VALUE (arg_type);
- tree expected_type = TREE_VALUE (it);
/* Check the type of the value we are about to pass ("arg_type")
for compatibility with the actual type the callback function
expects ("expected_type"). */
if (!types_compatible_p (expected_type, arg_type))
{
- error_at (DECL_SOURCE_LOCATION (decl),
- "argument type at index %d is not compatible with callback "
- "argument type at index %d",
- arg_idx + 1, curr + 1);
+ warning_at (
+ DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+ "argument type at index %d is not compatible with callback "
+ "argument type at index %d",
+ arg_idx + 1, curr + 1);
*no_add_attrs = true;
continue;
}
@@ -4825,10 +4812,10 @@ handle_callback_only_attribute (tree *node, tree name,
tree args,
for (; it; it = lookup_attribute ("callback_only", TREE_CHAIN (it)))
if (callback_get_fn_index (it) == callback_fn_idx)
{
- error_at (DECL_SOURCE_LOCATION (decl),
- "function declaration has multiple callback attributes "
- "describing argument no. %d",
- callback_fn_idx + 1);
+ warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+ "function declaration has multiple callback attributes "
+ "describing argument no. %d",
+ callback_fn_idx + 1);
*no_add_attrs = true;
break;
}
diff --git a/gcc/testsuite/gcc.dg/attr-callback.c
b/gcc/testsuite/gcc.dg/attr-callback.c
index d9cc67c3aff..74c8287471b 100755
--- a/gcc/testsuite/gcc.dg/attr-callback.c
+++ b/gcc/testsuite/gcc.dg/attr-callback.c
@@ -25,15 +25,15 @@ unknown_2(void (*)(int*, double*), int*, double*, char*);
[[gnu::callback_only(1, 0, 3, 3)]]
void
-too_many(void (*)(int*, double*), int*, double*); /* { dg-error "argument
number mismatch, 2 expected, got 3" }*/
+too_many(void (*)(int*, double*), int*, double*); /* { dg-warning "argument
number mismatch, 2 expected, got 3" }*/
[[gnu::callback_only(1, 2)]]
void
-too_few_1(void (*)(int*, double*), int*, double*); /* { dg-error "argument
number mismatch, 2 expected, got 1" }*/
+too_few_1(void (*)(int*, double*), int*, double*); /* { dg-warning "argument
number mismatch, 2 expected, got 1" }*/
[[gnu::callback_only(1)]]
void
-too_few_2(void (*)(int*, double*), int*, double*); /* { dg-error "argument
number mismatch, 2 expected, got 0" }*/
+too_few_2(void (*)(int*, double*), int*, double*); /* { dg-warning "argument
number mismatch, 2 expected, got 0" }*/
[[gnu::callback_only(3, 1)]]
void
@@ -45,52 +45,70 @@ downcast(char*, void* (*)(float*), double*);
[[gnu::callback_only(1, 2, 5)]]
void
-out_of_range_1(char (*)(float*, double*), float*, double*, int*); /* {
dg-error "callback argument index 5 is out of range" } */
+out_of_range_1(char (*)(float*, double*), float*, double*, int*); /* {
dg-warning "exceeds the number of function parameters" } */
[[gnu::callback_only(1, -2, 3)]]
void
-out_of_range_2(char (*)(float*, double*), float*, double*, int*); /* {
dg-error "callback argument index -2 is out of range" } */
+out_of_range_2(char (*)(float*, double*), float*, double*, int*); /* {
dg-warning "exceeds the number of function parameters" } */
[[gnu::callback_only(-1, 2, 3)]]
void
-out_of_range_3(char (*)(float*, double*), float*, double*, int*); /* {
dg-error "callback function index -1 is out of range" } */
+out_of_range_3(char (*)(float*, double*), float*, double*, int*); /* {
dg-warning "exceeds the number of function parameters" } */
[[gnu::callback_only(67, 2, 3)]]
void
-out_of_range_4(char (*)(float*, double*), float*, double*, int*); /* {
dg-error "callback function index 67 is out of range" } */
+out_of_range_4(char (*)(float*, double*), float*, double*, int*); /* {
dg-warning "exceeds the number of function parameters" } */
[[gnu::callback_only(0, 2, 3)]]
void
-unknown_fn(char (*)(float*, double*), float*, double*, int*); /* { dg-error
"callback function position cannot be marked as unknown" } */
+unknown_fn(char (*)(float*, double*), float*, double*, int*); /* { dg-warning
"callback function position cannot be marked as unknown" } */
[[gnu::callback_only(1, 2)]]
void
-not_a_fn(int, int); /* { dg-error "argument no. 1 is not an address of a
function" } */
+not_a_fn(int, int); /* { dg-warning "refers to" } */
struct S
{
int x;
};
+static struct S placeholder;
+
+static int one = 1;
+
+static const int const_one = 1;
+
[[gnu::callback_only(1, 2)]]
void
-incompatible_types_1(void (*)(struct S*), struct S); /* { dg-error "argument
type at index 2 is not compatible with callback argument type at index 1" } */
+incompatible_types_1(void (*)(struct S*), struct S); /* { dg-warning "refers
to" } */
[[gnu::callback_only(1, 3, 2)]]
void
-incompatible_types_2(void (*)(struct S*, int*), int*, double); /* { dg-error
"argument type at index 3 is not compatible with callback argument type at
index 1" } */
+incompatible_types_2(void (*)(struct S*, int*), int*, double); /* { dg-warning
"refers to" } */
[[gnu::callback_only(1, "2")]]
void
-wrong_arg_type_1(void (*)(void*), void*); /* { dg-error "argument no. 1 is not
an integer constant" } */
+wrong_arg_type_1(void (*)(void*), void*); /* { dg-warning "argument no. 1 is
not an integer constant" } */
[[gnu::callback_only("not a number", 2, 2)]]
void
-wrong_arg_type_2(void (*)(void*, void*), void*); /* { dg-error "argument
specifying callback function position is not an integer constant" } */
+wrong_arg_type_2(void (*)(void*, void*), void*); /* { dg-warning "has type" }
*/
+
+[[gnu::callback_only(placeholder, 2, 2)]]
+void
+wrong_arg_type_3(void (*)(void*, void*), void*); /* { dg-warning "has type" }
*/
+
+[[gnu::callback_only(one, 2, 2)]]
+void
+int_identifier(void (*)(void*, void*), void*); /* { dg-warning "is not an
integer constant" } */
+
+[[gnu::callback_only(one, 2, 2)]]
+void
+int_identifier_1(void (*)(void*, void*), void*); /* { dg-warning "is not an
integer constant" } */
[[gnu::callback_only(1, 2), gnu::callback_only(1, 3)]]
void
-multiple_single_fn(void (*)(int*), int*, int*); /* { dg-error "function
declaration has multiple callback attributes describing argument no. 1" } */
+multiple_single_fn(void (*)(int*), int*, int*); /* { dg-warning "function
declaration has multiple callback attributes describing argument no. 1" } */
/* Check that the attribute won't resolve outside of our namespace. */
--
2.55.0