Hi, Kees, Is this patch for GCC14? I noticed that some codes have been changed in the latest trunk GCC already.
> On Aug 25, 2025, at 11:59, Kees Cook <[email protected]> wrote: > > The __attribute__((__copy__)) functionality was crashing when copying > sanitizer-related attributes because these attributes violated the standard > GCC attribute infrastructure by storing INTEGER_CST values directly instead > of wrapping them in TREE_LIST like all other attributes. So, from my understanding of your patch, this is a bug in the handling of the attribute “no_sanitize”. Is there any issue with the handling of the attribute “copy”? > > Wrap sanitizer attributes INTEGER_CST values in TREE_LIST structures > to follow the same pattern as other attributes. This eliminates the > copy_list() crashes when copying sanitizer attributes: > > test.c:4:1: internal compiler error: tree check: expected tree that contains > ‘common’ structure, have ‘integer_cst’ in copy_list, at tree.cc:1427 > 4 | __attribute__((__copy__(__tanh))); > | ^~~~~~~~~~~~~ > 0x859d06 tree_contains_struct_check_failed(tree_node const*, > tree_node_structure_enum, char const*, int, char const*) > ../../gcc/gcc/tree.cc:9126 > 0x860f78 contains_struct_check(tree_node*, tree_node_structure_enum, char > const*, int, char const*) > ../../gcc/gcc/tree.h:3748 > 0x860f78 copy_list(tree_node*) > ../../gcc/gcc/tree.cc:1427 > 0xa755a5 handle_copy_attribute > ../../gcc/gcc/c-family/c-attribs.cc:3077 > > gcc/c-family/ChangeLog: > > PR c/113264 > * c-attribs.cc (add_no_sanitize_value): Store INTEGER_CST values > wrapped in TREE_LIST following standard attribute conventions. > (handle_no_sanitize_attribute): Handle both original string > arguments and copied INTEGER_CST values from TREE_LIST format. > > gcc/ChangeLog: > > PR c/113264 > * asan.h (sanitize_flags_p): Extract sanitizer flags from > TREE_LIST wrapper instead of directly from INTEGER_CST. > > gcc/d/ChangeLog: > > PR c/113264 > * d-attribs.cc (d_handle_no_sanitize_attribute): Store INTEGER_CST > values wrapped in TREE_LIST following standard conventions. > > gcc/testsuite/ChangeLog: > > PR c/113264 > * gcc.dg/pr113264.c: New test. > * gcc.dg/pr113264-asan-no-instrumentation.c: New test validating > that copied sanitizer attributes prevent ASAN instrumentation. > > Signed-off-by: Kees Cook <[email protected]> > --- > Cc: Andrew Pinski <[email protected]> > --- > gcc/asan.h | 8 ++- > .../gcc.dg/pr113264-asan-no-instrumentation.c | 43 +++++++++++ > gcc/testsuite/gcc.dg/pr113264.c | 72 +++++++++++++++++++ > gcc/c-family/c-attribs.cc | 49 +++++++++---- > gcc/d/d-attribs.cc | 16 ++++- > 5 files changed, 170 insertions(+), 18 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/pr113264-asan-no-instrumentation.c > create mode 100644 gcc/testsuite/gcc.dg/pr113264.c > > diff --git a/gcc/asan.h b/gcc/asan.h > index 273d6745c58d..13cd741306b0 100644 > --- a/gcc/asan.h > +++ b/gcc/asan.h > @@ -252,7 +252,13 @@ sanitize_flags_p (unsigned int flag, const_tree fn = > current_function_decl) > { > tree value = lookup_attribute ("no_sanitize", DECL_ATTRIBUTES (fn)); > if (value) > - result_flags &= ~tree_to_uhwi (TREE_VALUE (value)); > + { > + /* Extract the INTEGER_CST from the TREE_LIST wrapper. */ > + tree attr_args = TREE_VALUE (value); > + gcc_assert (attr_args && TREE_CODE (attr_args) == TREE_LIST); > + unsigned long long no_sanitize_flags = tree_to_uhwi (TREE_VALUE > (attr_args)); Should the above be “unsigned long long” or “unsigned HOST_WIDE_INT”? > + result_flags &= ~no_sanitize_flags; > + } > } > > return result_flags; > diff --git a/gcc/testsuite/gcc.dg/pr113264-asan-no-instrumentation.c > b/gcc/testsuite/gcc.dg/pr113264-asan-no-instrumentation.c > new file mode 100644 > index 000000000000..d99819b4792f > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr113264-asan-no-instrumentation.c > @@ -0,0 +1,43 @@ > +/* PR c/113264 - __attribute__((copy)) crashes with sanitizer attributes > + Test that copied no_sanitize_address attributes prevent ALL ASAN > instrumentation. > + This file should have NO ASAN calls since all functions have the > attribute. > + { dg-do compile } > + { dg-options "-fsanitize=address" } > + { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */ > + > +/* Original function with no_sanitize_address */ > +__attribute__((no_sanitize_address)) > +int original (int *p, int *q) > +{ > + *p = 42; > + return *q; > +} > + > +/* Copy the attribute - should also have no_sanitize_address */ > +__typeof(original) copy1 __attribute__((__copy__(original))); > +int copy1 (int *p, int *q) > +{ > + *p = 43; > + return *q; > +} > + > +/* Another copy - should also have no_sanitize_address */ > +__typeof(original) copy2 __attribute__((__copy__(original))); > +int copy2 (int *p, int *q) > +{ > + *p = 44; > + return *q; > +} > + > +/* Copy from a copy - should still have no_sanitize_address */ > +__typeof(copy1) copy_of_copy __attribute__((__copy__(copy1))); > +int copy_of_copy (int *p, int *q) > +{ > + *p = 45; > + return *q; > +} > + > +/* Since ALL functions have no_sanitize_address (either directly or copied), > + there should be NO ASAN instrumentation calls in the entire file. */ > +/* { dg-final { scan-assembler-not "__asan_report_store" } } */ > +/* { dg-final { scan-assembler-not "__asan_report_load" } } */ > diff --git a/gcc/testsuite/gcc.dg/pr113264.c b/gcc/testsuite/gcc.dg/pr113264.c > new file mode 100644 > index 000000000000..7a1b128bf203 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr113264.c > @@ -0,0 +1,72 @@ > +/* PR c/113264 - __attribute__((copy)) crashes with sanitizer attributes > + Test that the copy attribute correctly handles sanitizer attributes > + which store their arguments as INTEGER_CST values rather than TREE_LIST. > + { dg-do compile } > + { dg-options "-O2" } */ > + > +/* Test copying no_sanitize_address attribute. */ > +__attribute__((no_sanitize_address)) void f1 (void); > +__typeof(f1) f1_copy __attribute__((__copy__(f1))); > + > +/* Test copying no_sanitize_thread attribute. */ > +__attribute__((no_sanitize_thread)) void f2 (void); > +__typeof(f2) f2_copy __attribute__((__copy__(f2))); > + > +/* Test copying no_sanitize_undefined attribute. */ > +__attribute__((no_sanitize_undefined)) void f3 (void); > +__typeof(f3) f3_copy __attribute__((__copy__(f3))); > + > +/* Test copying no_sanitize_coverage attribute. */ > +__attribute__((no_sanitize_coverage)) void f4 (void); > +__typeof(f4) f4_copy __attribute__((__copy__(f4))); > + > +/* Test copying no_sanitize attribute with string arguments. */ > +__attribute__((no_sanitize("address"))) void f5 (void); > +__typeof(f5) f5_copy __attribute__((__copy__(f5))); > + > +__attribute__((no_sanitize("thread"))) void f6 (void); > +__typeof(f6) f6_copy __attribute__((__copy__(f6))); > + > +__attribute__((no_sanitize("undefined"))) void f7 (void); > +__typeof(f7) f7_copy __attribute__((__copy__(f7))); > + > +/* Test copying multiple sanitizer flags. */ > +__attribute__((no_sanitize("address", "thread"))) void f8 (void); > +__typeof(f8) f8_copy __attribute__((__copy__(f8))); > + > +/* Test the original bug report case. This may trigger a warning > + about conflicting with a built-in function name. */ > +__attribute__((no_sanitize_address)) void h (void); > +__typeof(h) tanhf64 __attribute__((__copy__(h))); > +/* { dg-warning "conflicting types for built-in function" "" { target *-*-* > } .-1 } */ > + > +/* Test copying from a function pointer variable - this should trigger a > warning. */ > +__attribute__((no_sanitize_address)) void f9 (void); > +void (*f9_ptr)(void) = f9; /* { dg-message "symbol 'f9_ptr' referenced" } */ > +__typeof(f9) f9_ptr_copy __attribute__((__copy__(*f9_ptr))); /* { dg-warning > "'copy' attribute ignored" } */ > + > +/* Test with actual function definitions to ensure attributes are properly > applied. */ > +__attribute__((no_sanitize_address)) > +void actual_func (int *p) > +{ > + *p = 100; > +} > + > +__typeof(actual_func) actual_func_copy > __attribute__((__copy__(actual_func))); > + > +void actual_func_copy (int *p) > +{ > + *p = 200; > +} > + > +/* Test copying sanitizer attributes along with other attributes. */ > +__attribute__((no_sanitize_address, noinline, cold)) > +void multi_attr (void); > + > +__typeof(multi_attr) multi_attr_copy __attribute__((__copy__(multi_attr))); > + > +/* Verify that the copy attribute works with function declarations that > + have sanitizer attributes applied via separate declarations. */ > +void separate_decl (void); > +__attribute__((no_sanitize_address)) void separate_decl (void); > +__typeof(separate_decl) separate_decl_copy > __attribute__((__copy__(separate_decl))); > diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc > index 1f4a0df12051..e028d599e3c2 100644 > --- a/gcc/c-family/c-attribs.cc > +++ b/gcc/c-family/c-attribs.cc > @@ -1414,19 +1414,28 @@ add_no_sanitize_value (tree node, unsigned int flags) > tree attr = lookup_attribute ("no_sanitize", DECL_ATTRIBUTES (node)); > if (attr) > { > - unsigned int old_value = tree_to_uhwi (TREE_VALUE (attr)); > + /* Extract the INTEGER_CST from the TREE_LIST wrapper. */ > + tree attr_args = TREE_VALUE (attr); > + gcc_assert (attr_args && TREE_CODE (attr_args) == TREE_LIST); > + unsigned int old_value = tree_to_uhwi (TREE_VALUE (attr_args)); > + > flags |= old_value; > > if (flags == old_value) > return; > > - TREE_VALUE (attr) = build_int_cst (unsigned_type_node, flags); > + tree new_value = build_tree_list (NULL_TREE, > + build_int_cst (unsigned_type_node, flags)); the indentation in the above line has issue. > + TREE_VALUE (attr) = new_value; > } > else > - DECL_ATTRIBUTES (node) > - = tree_cons (get_identifier ("no_sanitize"), > - build_int_cst (unsigned_type_node, flags), > - DECL_ATTRIBUTES (node)); > + { > + tree attr_value = build_tree_list (NULL_TREE, > + build_int_cst (unsigned_type_node, flags)); > + DECL_ATTRIBUTES (node) > + = tree_cons (get_identifier ("no_sanitize"), attr_value, > + DECL_ATTRIBUTES (node)); The indentations in the above have issues. > + } > } > > /* Handle a "no_sanitize" attribute; arguments as in > @@ -1444,17 +1453,29 @@ handle_no_sanitize_attribute (tree *node, tree name, > tree args, int, > return NULL_TREE; > } > > - for (; args; args = TREE_CHAIN (args)) > + /* Handle both original string arguments and copied attributes that > + have already been processed into INTEGER_CST wrapped in TREE_LIST. */ So, when handling the attribute “copy” of the current function, the attribute “no sanitize” of the original function has been handled already? thanks. Qing > + if (args && TREE_CODE (args) == TREE_LIST > + && TREE_VALUE (args) && TREE_CODE (TREE_VALUE (args)) == INTEGER_CST) > { > - tree id = TREE_VALUE (args); > - if (TREE_CODE (id) != STRING_CST) > + /* This is a copied attribute with the flags already processed. */ > + flags = tree_to_uhwi (TREE_VALUE (args)); > + } > + else > + { > + /* Process original string arguments. */ > + for (; args; args = TREE_CHAIN (args)) > { > - error ("%qE argument not a string", name); > - return NULL_TREE; > - } > + tree id = TREE_VALUE (args); > + if (TREE_CODE (id) != STRING_CST) > + { > + error ("%qE argument not a string", name); > + return NULL_TREE; > + } > > - char *string = ASTRDUP (TREE_STRING_POINTER (id)); > - flags |= parse_no_sanitize_attribute (string); > + char *string = ASTRDUP (TREE_STRING_POINTER (id)); > + flags |= parse_no_sanitize_attribute (string); > + } > } > > add_no_sanitize_value (*node, flags); > diff --git a/gcc/d/d-attribs.cc b/gcc/d/d-attribs.cc > index 77315dc5d58d..c8d41add0cb1 100644 > --- a/gcc/d/d-attribs.cc > +++ b/gcc/d/d-attribs.cc > @@ -1424,16 +1424,26 @@ d_handle_no_sanitize_attribute (tree *node, tree > name, tree args, int, > merge existing flags if no_sanitize was previously handled. */ > if (tree attr = lookup_attribute ("no_sanitize", DECL_ATTRIBUTES (*node))) > { > - unsigned int old_value = tree_to_uhwi (TREE_VALUE (attr)); > + /* Extract the INTEGER_CST from the TREE_LIST wrapper. */ > + tree attr_args = TREE_VALUE (attr); > + gcc_assert (attr_args && TREE_CODE (attr_args) == TREE_LIST); > + unsigned int old_value = tree_to_uhwi (TREE_VALUE (attr_args)); > + > flags |= old_value; > > if (flags != old_value) > - TREE_VALUE (attr) = build_int_cst (d_uint_type, flags); > + { > + tree new_value = build_tree_list (NULL_TREE, > + build_int_cst (d_uint_type, flags)); > + TREE_VALUE (attr) = new_value; > + } > } > else > { > + tree attr_value = build_tree_list (NULL_TREE, > + build_int_cst (d_uint_type, flags)); > DECL_ATTRIBUTES (*node) = tree_cons (get_identifier ("no_sanitize"), > - build_int_cst (d_uint_type, flags), > + attr_value, > DECL_ATTRIBUTES (*node)); > } > > -- > 2.34.1 >
