Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Wed, 29 Nov 2023 17:32:07 -0700
From:   Nathan Chancellor <nathan@kernel.org>
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
        Nicolas Schier <nicolas@fjasle.eu>
Subject: Re: [PATCH 3/3] modpost: move exit(1) for fatal() to modpost.h
Message-ID: <20231130003207.GD2513828@dev-arch.thelio-3990X>
References: <20231125103116.797608-1-masahiroy@kernel.org>
 <20231125103116.797608-3-masahiroy@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231125103116.797608-3-masahiroy@kernel.org>
Precedence: bulk

On Sat, Nov 25, 2023 at 07:31:16PM +0900, Masahiro Yamada wrote:
> fatal() never returns, but compilers are not aware of this fact because
> exit(1) is called within the modpost_log() definition.
> 
> Move exit(1) to the fatal() macro so that compilers can identify
> unreachable code flows.
> 
> Remove the initializer for 'taddr' in section_rel(), as compilers now
> recognize this is not an uninitialized bug.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

While I did not see any warnings when testing, it seems like this patch
should come before patch 2 to avoid any potential problems from dropping
the unreachable code (since the compiler won't be able to infer that
fatal() is __noreturn without this change); doesn't really matter though
so:

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> 
>  scripts/mod/modpost.c | 4 +---
>  scripts/mod/modpost.h | 2 +-
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 013fc5031bc7..696c583a14ec 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -84,8 +84,6 @@ void modpost_log(enum loglevel loglevel, const char *fmt, ...)
>  	vfprintf(stderr, fmt, arglist);
>  	va_end(arglist);
>  
> -	if (loglevel == LOG_FATAL)
> -		exit(1);
>  	if (loglevel == LOG_ERROR)
>  		error_occurred = true;
>  }
> @@ -1415,7 +1413,7 @@ static void section_rel(struct module *mod, struct elf_info *elf,
>  
>  	for (rel = start; rel < stop; rel++) {
>  		Elf_Sym *tsym;
> -		Elf_Addr taddr = 0, r_offset;
> +		Elf_Addr taddr, r_offset;
>  		unsigned int r_type, r_sym;
>  		void *loc;
>  
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 9fe974dc1a52..aaa67b24775e 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -215,4 +215,4 @@ modpost_log(enum loglevel loglevel, const char *fmt, ...);
>   */
>  #define warn(fmt, args...)	modpost_log(LOG_WARN, fmt, ##args)
>  #define error(fmt, args...)	modpost_log(LOG_ERROR, fmt, ##args)
> -#define fatal(fmt, args...)	modpost_log(LOG_FATAL, fmt, ##args)
> +#define fatal(fmt, args...)	do { modpost_log(LOG_FATAL, fmt, ##args); exit(1); } while (1)
> -- 
> 2.40.1
>