Hi, this patch allows the callback attribute to be used outside of GCC under the 'gnu::callback_only'. It also fixes some missing bounds checks in the attribute handler. The attribute is not recognized outside of the gnu namespace, I did this so that there is no accidental mixup with the LLVM callback attribute, which has different syntax. I am not great at user-facing documentation, I hope the docs are clear about the attribute's usage, suggestions are of course welcome.
I'd like to have this commited after this patch series: https://gcc.gnu.org/pipermail/gcc-patches/2025-October/697994.html (or at least patches 1 and 3), which is why this patch includes some changes to that code as well. Bootstrapped and regtested on x86_64-linux WITH the aforementioned series applied, no issues. Best regards, Josef gcc/ChangeLog: * attr-callback.cc (callback_edge_callee_has_attr): Specify gnu namespace in attribute lookup. (callback_fetch_attr_by_edge): Likewise. (handle_callback_attribute): Fix bugs in bounds checks, add gnu namespace to attribute lookup. * attr-callback.h (CALLBACK_ATTR_IDENT): Change identifier to 'callback_only' * builtin-attrs.def (ATTR_CALLBACK): Likewise. * cgraph.cc (cgraph_edge::redirect_call_stmt_to_callee): Specify gnu namespace in attribute lookup. (cgraph_node::verify_node): Likewise. * doc/extend.texi: Add gnu::callback_only attribute documentation. * ipa-cp.cc (purge_useless_callback_edges): Specify gnu namespace attribute lookup. * ipa-prop.cc (ipa_compute_jump_functions_for_edge): Likewise. * tree-core.h: Change ECF_CB_* comment from 'callback' to 'callback_only'. gcc/c-family/ChangeLog: * c-attribs.cc: Define the 'callback_only' attribute in gnu namespace only. * c-common.h: Add extern decl for the callback attribute table. gcc/c/ChangeLog: * c-objc-common.h: Add the callback attribute table to allow it to be registered. gcc/cp/ChangeLog: * cp-objcp-common.h: Likewise. gcc/testsuite/ChangeLog: * gcc.dg/attr-callback.c: New test. * gcc.dg/ipa/ipcp-cb2.c: New test. Signed-off-by: Josef Melcr <[email protected]> --- gcc/attr-callback.cc | 16 ++-- gcc/attr-callback.h | 2 +- gcc/builtin-attrs.def | 2 +- gcc/c-family/c-attribs.cc | 16 +++- gcc/c-family/c-common.h | 1 + gcc/c/c-objc-common.h | 3 +- gcc/cgraph.cc | 6 +- gcc/cp/cp-objcp-common.h | 1 + gcc/doc/extend.texi | 35 +++++++++ gcc/ipa-cp.cc | 2 +- gcc/ipa-prop.cc | 4 +- gcc/testsuite/gcc.dg/attr-callback.c | 106 +++++++++++++++++++++++++++ gcc/testsuite/gcc.dg/ipa/ipcp-cb2.c | 51 +++++++++++++ gcc/tree-core.h | 2 +- 14 files changed, 227 insertions(+), 20 deletions(-) create mode 100755 gcc/testsuite/gcc.dg/attr-callback.c create mode 100644 gcc/testsuite/gcc.dg/ipa/ipcp-cb2.c diff --git a/gcc/attr-callback.cc b/gcc/attr-callback.cc index ff1145cbce1..6685d8b2fbe 100644 --- a/gcc/attr-callback.cc +++ b/gcc/attr-callback.cc @@ -87,7 +87,7 @@ callback_special_case_attr (tree decl) bool callback_edge_callee_has_attr (cgraph_edge *e) { - return lookup_attribute (CALLBACK_ATTR_IDENT, + return lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, DECL_ATTRIBUTES (e->callee->decl)) || callback_is_special_cased (e->callee->decl, e->call_stmt); } @@ -113,12 +113,12 @@ callback_fetch_attr_by_edge (cgraph_edge *e, cgraph_edge *carrying) if (callback_is_special_cased (carrying->callee->decl, e->call_stmt)) return callback_special_case_attr (carrying->callee->decl); - tree cb_attr = lookup_attribute (CALLBACK_ATTR_IDENT, + tree cb_attr = lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, DECL_ATTRIBUTES (carrying->callee->decl)); gcc_checking_assert (cb_attr); tree res = NULL_TREE; for (; cb_attr; - cb_attr = lookup_attribute (CALLBACK_ATTR_IDENT, TREE_CHAIN (cb_attr))) + cb_attr = lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, TREE_CHAIN (cb_attr))) { unsigned id = callback_get_fn_index (cb_attr); if (id == e->callback_id) @@ -229,10 +229,10 @@ handle_callback_attribute (tree *node, tree name, tree args, return NULL_TREE; } --callback_fn_idx; - if (callback_fn_idx >= decl_nargs) + if (callback_fn_idx < 0 || callback_fn_idx >= decl_nargs) { error_at (DECL_SOURCE_LOCATION (decl), - "callback function position out of range"); + "callback function index %d is out of range", callback_fn_idx + 1); *no_add_attrs = true; return NULL_TREE; } @@ -305,7 +305,7 @@ handle_callback_attribute (tree *node, tree name, tree args, 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) + if (arg_idx < 0 || arg_idx >= decl_nargs) { error_at (DECL_SOURCE_LOCATION (decl), "callback argument index %d is out of range", arg_idx + 1); @@ -331,8 +331,8 @@ handle_callback_attribute (tree *node, tree name, tree args, /* Check that the decl does not already have a callback attribute describing the same argument. */ - it = lookup_attribute (CALLBACK_ATTR_IDENT, DECL_ATTRIBUTES (decl)); - for (; it; it = lookup_attribute (CALLBACK_ATTR_IDENT, TREE_CHAIN (it))) + it = lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, DECL_ATTRIBUTES (decl)); + for (; it; it = lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, TREE_CHAIN (it))) if (callback_get_fn_index (it) == callback_fn_idx) { error_at (DECL_SOURCE_LOCATION (decl), diff --git a/gcc/attr-callback.h b/gcc/attr-callback.h index 39a4fe8ac94..fd0c7d90f52 100644 --- a/gcc/attr-callback.h +++ b/gcc/attr-callback.h @@ -28,7 +28,7 @@ enum callback_position CB_UNKNOWN_POS = 0 }; -#define CALLBACK_ATTR_IDENT " callback" +#define CALLBACK_ATTR_IDENT "callback_only" /* Returns a callback attribute with callback index FN_IDX, and ARG_COUNT arguments specified by VA_ARGS. */ diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def index dedb841364d..e37bff38a8c 100644 --- a/gcc/builtin-attrs.def +++ b/gcc/builtin-attrs.def @@ -130,7 +130,7 @@ DEF_ATTR_IDENT (ATTR_TM_TMPURE, "transaction_pure") DEF_ATTR_IDENT (ATTR_RETURNS_TWICE, "returns_twice") DEF_ATTR_IDENT (ATTR_RETURNS_NONNULL, "returns_nonnull") DEF_ATTR_IDENT (ATTR_WARN_UNUSED_RESULT, "warn_unused_result") -DEF_ATTR_IDENT (ATTR_CALLBACK, " callback") +DEF_ATTR_IDENT (ATTR_CALLBACK, "callback_only") DEF_ATTR_TREE_LIST (ATTR_NOVOPS_LIST, ATTR_NOVOPS, ATTR_NULL, ATTR_NULL) diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc index 8ca767abbeb..187e74ae0b2 100644 --- a/gcc/c-family/c-attribs.cc +++ b/gcc/c-family/c-attribs.cc @@ -485,8 +485,6 @@ const struct attribute_spec c_common_gnu_attributes[] = handle_tm_attribute, NULL }, { "transaction_may_cancel_outer", 0, 0, false, true, false, false, handle_tm_attribute, NULL }, - { CALLBACK_ATTR_IDENT, 1, -1, true, false, false, false, - handle_callback_attribute, NULL }, /* ??? These two attributes didn't make the transition from the Intel language document to the multi-vendor language document. */ { "transaction_pure", 0, 0, false, true, false, false, @@ -707,6 +705,20 @@ const struct scoped_attribute_specs c_common_format_attribute_table = "gnu", { c_common_format_attributes } }; +/* Attribute table for the callback attribute to be used by the C frontends. We + don't want to expose the attribute outside of the GNU namespace, so it has to + be separated out. */ +const struct attribute_spec c_common_callback_attribute[] = +{ + { CALLBACK_ATTR_IDENT, 1, -1, true, false, false, false, + handle_callback_attribute, NULL }, +}; + +const struct scoped_attribute_specs c_common_callback_attribute_table = +{ + "gnu", { c_common_callback_attribute } +}; + /* Returns TRUE iff the attribute indicated by ATTR_ID takes a plain identifier as an argument, so the front end shouldn't look it up. */ diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index bedbd4a94b0..5662d936669 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -827,6 +827,7 @@ extern struct visibility_flags visibility_options; extern const struct scoped_attribute_specs c_common_gnu_attribute_table; extern const struct scoped_attribute_specs c_common_clang_attribute_table; extern const struct scoped_attribute_specs c_common_format_attribute_table; +extern const struct scoped_attribute_specs c_common_callback_attribute_table; /* Pointer to function to lazily generate the VAR_DECL for __FUNCTION__ etc. ID is the identifier to use, NAME is the string. diff --git a/gcc/c/c-objc-common.h b/gcc/c/c-objc-common.h index 84f6fd34b28..e12479be33f 100644 --- a/gcc/c/c-objc-common.h +++ b/gcc/c/c-objc-common.h @@ -82,7 +82,8 @@ static const scoped_attribute_specs *const c_objc_attribute_table[] = &std_attribute_table, &c_common_gnu_attribute_table, &c_common_clang_attribute_table, - &c_common_format_attribute_table + &c_common_format_attribute_table, + &c_common_callback_attribute_table }; #undef LANG_HOOKS_ATTRIBUTE_TABLE diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc index 782c4d87b63..07c3080bf06 100644 --- a/gcc/cgraph.cc +++ b/gcc/cgraph.cc @@ -1823,7 +1823,7 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e, { cgraph_edge *carrying = e->get_callback_carrying_edge (); if (!callback_is_special_cased (carrying->callee->decl, e->call_stmt) - && !lookup_attribute (CALLBACK_ATTR_IDENT, + && !lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, DECL_ATTRIBUTES (carrying->callee->decl))) /* Callback attribute is removed if the dispatching function changes signature, as the indices wouldn't be correct anymore. These edges @@ -4359,9 +4359,9 @@ cgraph_node::verify_node (void) { int ncallbacks = 0; int nfound_edges = 0; - for (tree cb = lookup_attribute (CALLBACK_ATTR_IDENT, DECL_ATTRIBUTES ( + for (tree cb = lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, DECL_ATTRIBUTES ( e->callee->decl)); - cb; cb = lookup_attribute (CALLBACK_ATTR_IDENT, TREE_CHAIN (cb)), + cb; cb = lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, TREE_CHAIN (cb)), ncallbacks++) ; for (cgraph_edge *cbe = callees; cbe; cbe = cbe->next_callee) diff --git a/gcc/cp/cp-objcp-common.h b/gcc/cp/cp-objcp-common.h index ff354285d2e..b694fc414db 100644 --- a/gcc/cp/cp-objcp-common.h +++ b/gcc/cp/cp-objcp-common.h @@ -130,6 +130,7 @@ static const scoped_attribute_specs *const cp_objcp_attribute_table[] = &c_common_gnu_attribute_table, &c_common_clang_attribute_table, &c_common_format_attribute_table, + &c_common_callback_attribute_table, &internal_attribute_table }; diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index fb117f59665..6ee1e73b9a7 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -2399,6 +2399,41 @@ Darwin (OSX) installations. On such installations, the XCode and system documentation provide descriptions of @code{CFString}, @code{CFStringRefs} and associated functions. +@cindex @code{gnu::callback_only} function attribute +@item gnu::callback_only +The @code{gnu::callback_only} attribute specifies that the annotated +function may call the specified callback function. The first parameter +identifies the index of the callback function, the rest of the arguments specify the +indices of the arguments of the indirect call. All indices start from 1. If the function +takes the implicit @code{this} pointer, it is referred to by the index 1, with +the rest of the arguments starting at index 2. The index 0 marks an argument not +present in the arguments of the annotated function, an argument which is +modified before calling the callback function. The annotated function must pass the +specified arguments in the specified order to the callback function, which must be +callable with the number, order and type of the arguments. The specified pointer +to the callback may not escape the translation unit of the annotated function and it +may not be captured. The annotated function is required to pass the arguments +through, it may not change or dereference them. The arguments also may not escape. + +The attribute exposes the potentially hidden callsite in the annotated function, +enabling interprocedural optimizations which may not be possible without the +attribute. It is most useful for annotating functions from dynamically linked libraries, +as their bodies are not available during compilation. + +This attribute is the counterpart of the @code{callback} attribute present in clang, +though the attributes are not mutually compatible. The clang implementation allows +identifiers as arguments, marks an unknown argument with -1 and the @code{this} pointer +with the index 0. The @code{gnu::callback_only} attribute may be used multiple +times per function, which is not permitted by the clang implementation. + +An example usage: + +@smallexample +[[gnu::callback_only(1, 3, 2)]] +void foo(void (*bar)(int*, double*), double* y, int* x); +@end smallexample + +means that there is a call @code{bar (x, y)} inside @code{foo}. @cindex @code{gnu_inline} function attribute @item gnu_inline diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc index 2105c9a2ef7..94c952a3bbb 100644 --- a/gcc/ipa-cp.cc +++ b/gcc/ipa-cp.cc @@ -6236,7 +6236,7 @@ purge_useless_callback_edges () if (dump_file) fprintf (dump_file, "\tExamining callbacks of edge %s -> %s:\n", e->caller->dump_name (), e->callee->dump_name ()); - if (!lookup_attribute (CALLBACK_ATTR_IDENT, + if (!lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, DECL_ATTRIBUTES (e->callee->decl)) && !callback_is_special_cased (e->callee->decl, e->call_stmt)) { diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index 72a0f814fcd..21e1f9afcb1 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -2550,11 +2550,11 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi, /* Argument is a pointer to a function. Look for a callback attribute describing this argument. */ tree callback_attr - = lookup_attribute (CALLBACK_ATTR_IDENT, + = lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, DECL_ATTRIBUTES (cs->callee->decl)); for (; callback_attr; callback_attr - = lookup_attribute (CALLBACK_ATTR_IDENT, + = lookup_attribute ("gnu", CALLBACK_ATTR_IDENT, TREE_CHAIN (callback_attr))) if (callback_get_fn_index (callback_attr) == n) break; diff --git a/gcc/testsuite/gcc.dg/attr-callback.c b/gcc/testsuite/gcc.dg/attr-callback.c new file mode 100755 index 00000000000..4ebd8eefff2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/attr-callback.c @@ -0,0 +1,106 @@ +/* Test callback attribute error checking. */ + +/* { dg-do compile } */ +/* { dg-options "-std=gnu17 -Wattributes" } */ + +[[gnu::callback_only(1, 2)]] +void +correct_1(void (*)(int*), int*); + +[[gnu::callback_only(1, 2, 3)]] +void +correct_2(void (*)(int*, double*), int*, double*); + +[[gnu::callback_only(1, 2, 3), gnu::callback_only(4, 5)]] +void +correct_3(void (*)(int*, double*), int*, double*, int (*)(void*), void*); + +[[gnu::callback_only(1, 0)]] +void +unknown_1(void (*)(int*)); + +[[gnu::callback_only(1, 2, 0)]] +void +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" }*/ + +[[gnu::callback_only(1, 2)]] +void +too_few_1(void (*)(int*, double*), int*, double*); /* { dg-error "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" }*/ + +[[gnu::callback_only(3, 1)]] +void +promotion(char*, float, int (*)(int*)); + +[[gnu::callback_only(2, 3)]] +void +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" } */ + +[[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" } */ + +[[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" } */ + +[[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" } */ + +[[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" } */ + +[[gnu::callback_only(1, 2)]] +void +not_a_fn(int, int); /* { dg-error "argument no. 1 is not an address of a function" } */ + +struct S{ + int x; +}; + +[[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" } */ + +[[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" } */ + +[[gnu::callback_only(1, "2")]] +void +wrong_arg_type_1(void (*)(void*), void*); /* { dg-error "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" } */ + +[[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" } */ + +/* Check that the attribute won't resolve outside of our namespace. */ + +[[callback_only(1, 2)]] /* { dg-warning "ignored" } */ +void +ignore_1(void (*)(int*), int*); + +[[callback(1, 2)]] /* { dg-warning "ignored" } */ +void +ignore_2(void (*)(int*), int*); + +[[gnu::callback(1, 2)]] +void +ignore_3(void (*)(int*), int*); /* { dg-warning "ignored" } */ diff --git a/gcc/testsuite/gcc.dg/ipa/ipcp-cb2.c b/gcc/testsuite/gcc.dg/ipa/ipcp-cb2.c new file mode 100644 index 00000000000..f7ff9868b55 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/ipcp-cb2.c @@ -0,0 +1,51 @@ +/* Test that we can handle multiple callback attributes and use them to + propagate into callbacks. 'cb1' body borrowed from a ipa-cp test to get the + pass to work. */ + +/* { dg-do compile } */ +/* { dg-options "-O3 -fdump-ipa-cp" } */ + +struct S { + int a, b, c; +}; + +extern void *blah(int, void *); + +[[gnu::callback_only(1, 2), gnu::callback_only(3, 4, 5)]] extern void +call(void (*fn1)(struct S *), struct S *a, void (*fn2)(struct S *, struct S *), + struct S *b, struct S *c); + +void cb1(struct S *p) { + int i, c = p->c; + int b = p->b; + void *v = (void *)p; + + for (i = 0; i < c; i++) + v = blah(b + i, v); +} + +void cb2(struct S *a, struct S *b) { + cb1(a); + cb1(b); +} + +void test(int a, int b, int c) { + struct S s; + s.a = a; + s.b = b; + s.c = c; + struct S ss; + ss.a = s.c; + ss.b = s.b; + ss.c = s.a; + call(cb1, &s, cb2, &s, &ss); +} + +int main() { + test(1, 64, 32); + return 0; +} + +/* { dg-final { scan-ipa-dump "Creating a specialized node of cb1" "cp" } } */ +/* { dg-final { scan-ipa-dump "Creating a specialized node of cb2" "cp" } } */ +/* { dg-final { scan-ipa-dump-times "Aggregate replacements: " 2 "cp" } } */ diff --git a/gcc/tree-core.h b/gcc/tree-core.h index 145e758600e..c6428dced93 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -102,7 +102,7 @@ struct die_struct; meant to be used for the construction of builtin functions. They were only added because Fortran uses them for attributes of builtins. */ -/* callback(1, 2) */ +/* callback_only(1, 2) */ #define ECF_CB_1_2 (1 << 17) /* Call argument flags. */ -- 2.51.0
