| Message ID | 20231125103116.797608-3-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] modpost: move __attribute__((format(printf, 2, 3))) to modpost.h | expand |
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 >
On Thu, Nov 30, 2023 at 9:32 AM Nathan Chancellor <nathan@kernel.org> wrote: > > 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: My eyes spotted 2/3, but I can swap the patch order and rephase the patch description because clang can detect the unreachable code. > 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 > > >
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)
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> --- scripts/mod/modpost.c | 4 +--- scripts/mod/modpost.h | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)