* [PATCH] Fix sanitizer attribute infrastructure to use standard TREE_LIST format [PR113264]
@ 2025-08-25 15:59 Kees Cook
2025-08-25 20:24 ` Qing Zhao
0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2025-08-25 15:59 UTC (permalink / raw)
To: Andrew Pinski; +Cc: Kees Cook, Qing Zhao, gcc-patches, linux-hardening
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.
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 <kees@kernel.org>
---
Cc: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
---
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));
+ 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));
+ 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));
+ }
}
/* 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. */
+ 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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix sanitizer attribute infrastructure to use standard TREE_LIST format [PR113264]
2025-08-25 15:59 [PATCH] Fix sanitizer attribute infrastructure to use standard TREE_LIST format [PR113264] Kees Cook
@ 2025-08-25 20:24 ` Qing Zhao
2025-08-26 5:43 ` Kees Cook
0 siblings, 1 reply; 3+ messages in thread
From: Qing Zhao @ 2025-08-25 20:24 UTC (permalink / raw)
To: Kees Cook
Cc: Andrew Pinski, gcc-patches@gcc.gnu.org,
linux-hardening@vger.kernel.org
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 <kees@kernel.org> 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 <kees@kernel.org>
> ---
> Cc: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix sanitizer attribute infrastructure to use standard TREE_LIST format [PR113264]
2025-08-25 20:24 ` Qing Zhao
@ 2025-08-26 5:43 ` Kees Cook
0 siblings, 0 replies; 3+ messages in thread
From: Kees Cook @ 2025-08-26 5:43 UTC (permalink / raw)
To: Qing Zhao
Cc: Andrew Pinski, gcc-patches@gcc.gnu.org,
linux-hardening@vger.kernel.org
On Mon, Aug 25, 2025 at 08:24:20PM +0000, Qing Zhao wrote:
> Hi, Kees,
>
> Is this patch for GCC14? I noticed that some codes have been changed in the latest trunk GCC already.
Oh, I may be a bit behind. My base commit was
3e4ced9de1f1c6444eae44c1fed493742c345bc6 (Aug 1st).
I will rebase...
> > On Aug 25, 2025, at 11:59, Kees Cook <kees@kernel.org> 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”.
It looks like it's all the sanitizer attributes. I happened across this
bug while testing some alternative solutions for Linux's disabling of
retpolines in certain sections[1]. I was trying to add no_sanitize("kcfi")
as a test and tripped over PR113264. So I figured I should try to fix
it.
[1] https://lore.kernel.org/all/20250825142603.1907143-4-kees@kernel.org/
> Is there any issue with the handling of the attribute “copy”?
The problem appears to be entirely due to attempting an attribute copy,
yes. The sanitizer itself is fine and happy with INTEGER_CST, but when
the attribute copy code runs it was very alarmed to not find TREE_LIST.
> >
> > 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 <kees@kernel.org>
> > ---
> > Cc: Andrew Pinski <andrew.pinski@oss.qualcomm.com>
> > ---
> > 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”?
Oh, this may have slipped through when I tried to separate this from my
other patch that moved from 32-bit to 64-bit bit widths in this code.
That type should be the same type as "result_flags".
I have fixed it locally now:
diff --git a/gcc/asan.h b/gcc/asan.h
index 13cd741306b0..be7be7a067ce 100644
--- a/gcc/asan.h
+++ b/gcc/asan.h
@@ -256,7 +256,7 @@ sanitize_flags_p (unsigned int flag, const_tree fn = current_function_decl)
/* 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));
+ unsigned int no_sanitize_flags = tree_to_uhwi (TREE_VALUE (attr_args));
result_flags &= ~no_sanitize_flags;
}
}
>
> > + 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.
Oh, weird. It looks correct on my end? Did the tabs get lost? It looks
okay here?
https://gcc.gnu.org/pipermail/gcc-patches/2025-August/693250.html
> > + 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.
Where should I do the breaks on this to avoid 80 char limit? I was
copying the existing style. If I bring the assignment up a line, I
get 85 characters:
DECL_ATTRIBUTES (node) = tree_cons (get_identifier ("no_sanitize"), attr_value,
DECL_ATTRIBUTES (node));
> > + }
> > }
> >
> > /* 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?
Right, when trying to add a new sanitizer attribute to a function that
already had sanitizer attributes copied over.
-Kees
--
Kees Cook
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-08-26 5:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-25 15:59 [PATCH] Fix sanitizer attribute infrastructure to use standard TREE_LIST format [PR113264] Kees Cook
2025-08-25 20:24 ` Qing Zhao
2025-08-26 5:43 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).