From: Li Chen <[email protected]> The cleanup_plugin should warn about both uninitialized cleanup variables and those initialized to NULL, as documented in include/linux/cleanup.h. The "__free(...) = NULL" pattern at function top poses interdependency problems.
Also, plugin warnings should not be converted to errors by -Werror, as they are informational warnings meant to guide developers, not block builds. Signed-off-by: Li Chen <[email protected]> --- scripts/gcc-plugins/Kconfig | 2 +- scripts/gcc-plugins/cleanup_plugin.c | 122 ++++++++++++++++++++++++--- 2 files changed, 111 insertions(+), 13 deletions(-) diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig index 906d50eb5efa6..ae6f79c617149 100644 --- a/scripts/gcc-plugins/Kconfig +++ b/scripts/gcc-plugins/Kconfig @@ -40,6 +40,6 @@ config GCC_PLUGIN_CLEANUP_ATTRIBUTE_WARN def_bool y help Warn when local automatic variables annotated with - __attribute__((cleanup(...))) are declared without an initializer. + __attribute__((cleanup(...))) are declared without an constructor. endif diff --git a/scripts/gcc-plugins/cleanup_plugin.c b/scripts/gcc-plugins/cleanup_plugin.c index d28f8969186de..c0744bbb7ef15 100644 --- a/scripts/gcc-plugins/cleanup_plugin.c +++ b/scripts/gcc-plugins/cleanup_plugin.c @@ -1,7 +1,69 @@ // SPDX-License-Identifier: GPL-2.0 /* - * Warn about uninitialized automatic variables that use the - * __attribute__((cleanup(...))) attribute. + * Copyright 2025 by Li Chen <[email protected]> + * + * This gcc plugin warns about problematic patterns when using variables + * with __attribute__((cleanup(...))). The cleanup attribute helpers + * (__free, DEFINE_FREE, etc.) are designed to automatically clean up + * resources when variables go out of scope, following LIFO ordering. + * However, certain patterns can lead to interdependency issues. + * + * The plugin detects two problematic patterns: + * + * 1. Uninitialized cleanup variables: + * Variables declared with cleanup attributes but not initialized can + * cause issues when cleanup functions are called on undefined values. + * + * Example: + * void func(void) + * { + * struct resource *res __free(cleanup); // Warning: not initialized + * res = acquire_resource(); + * // ... + * } + * + * Should be: + * void func(void) + * { + * struct resource *res __free(cleanup) = acquire_resource(); + * // ... + * } + * + * 2. NULL-initialized cleanup variables: + * The "__free(...) = NULL" pattern at function top can cause + * interdependency problems, especially when combined with guards or + * multiple cleanup variables, as documented in include/linux/cleanup.h. + * + * Example: + * void func(void) + * { + * struct resource *res __free(cleanup) = NULL; // Warning: NULL init + * guard(mutex)(&lock); + * res = acquire_resource(); + * // cleanup may run without lock held! + * } + * + * Should be: + * void func(void) + * { + * guard(mutex)(&lock); + * struct resource *res __free(cleanup) = acquire_resource(); + * // ... + * } + * + * The plugin provides clear warnings to help developers identify these + * patterns during compilation. Importantly, these warnings are not + * converted to errors by -Werror, allowing builds to continue while + * still alerting developers to potential issues. + * + * Options: + * - None currently supported + * + * Attribute: __attribute__((cleanup(...))) + * The cleanup gcc attribute can be used on automatic variables to + * specify a function to be called when the variable goes out of scope. + * This plugin validates that such variables are properly initialized + * at declaration time to avoid interdependency issues. */ #include "gcc-common.h" @@ -41,38 +103,74 @@ static bool is_candidate_decl(tree var) return true; } -static bool has_declaration_initializer(tree var) +static bool is_null_initializer(tree initial) { - if (DECL_INITIAL(var)) + if (!initial) + return false; + + /* Check if the initializer is NULL pointer constant */ + if (initial == null_pointer_node) return true; -#ifdef DECL_INITIALIZED_P - if (DECL_INITIALIZED_P(var)) + /* Check if it's an integer constant zero (which can be NULL) */ + if (TREE_CODE(initial) == INTEGER_CST && integer_zerop(initial)) return true; -#endif return false; } +static bool has_valid_declaration_initializer(tree var) +{ + tree initial = DECL_INITIAL(var); + + /* No initializer at all */ + if (!initial) { +#ifdef DECL_INITIALIZED_P + if (DECL_INITIALIZED_P(var)) + return true; +#endif + return false; + } + + /* NULL initialization is considered invalid for cleanup variables */ + if (is_null_initializer(initial)) + return false; + + /* Any other non-NULL initializer is valid */ + return true; +} + static void warn_if_uninitialized(tree var) { location_t loc; bool saved_warning_as_error; + tree initial = DECL_INITIAL(var); + bool is_null_init = false; - if (has_declaration_initializer(var)) + if (has_valid_declaration_initializer(var)) return; loc = DECL_SOURCE_LOCATION(var); if (loc == UNKNOWN_LOCATION) return; + /* Check if it's a NULL initialization */ + is_null_init = initial && is_null_initializer(initial); + /* Temporarily disable treating warnings as errors for this specific warning */ saved_warning_as_error = global_dc->warning_as_error_requested_p(); global_dc->set_warning_as_error_requested(false); - warning_at( - loc, 0, - "%qD declared with cleanup attribute is not initialized at declaration", - var); + if (is_null_init) { + warning_at( + loc, 0, + "%qD declared with cleanup attribute is initialized to NULL at declaration", + var); + } else { + warning_at( + loc, 0, + "%qD declared with cleanup attribute is not initialized at declaration", + var); + } /* Restore the original setting */ global_dc->set_warning_as_error_requested(saved_warning_as_error); } -- 2.51.0
