[PATCH v2 1/4] modpost: move __attribute__((format(printf, 2, 3))) to modpost.h

[PATCH v2 1/4] modpost: move __attribute__((format(printf, 2, 3))) to modpost.h

[Masahiro Yamada]

This attribute must be added to the function declaration in a header
for comprehensive checking of all the callsites.

Fixes: 6d9a89ea4b06 ("kbuild: declare the modpost error functions as printf like")
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
---

(no changes since v1)

 scripts/mod/modpost.c | 3 +--
 scripts/mod/modpost.h | 3 ++-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index cb6406f485a9..ca0a90158f85 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -60,8 +60,7 @@ static unsigned int nr_unresolved;
 
 #define MODULE_NAME_LEN (64 - sizeof(Elf_Addr))
 
-void __attribute__((format(printf, 2, 3)))
-modpost_log(enum loglevel loglevel, const char *fmt, ...)
+void modpost_log(enum loglevel loglevel, const char *fmt, ...)
 {
 	va_list arglist;
 
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 69baf014da4f..9fe974dc1a52 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -197,7 +197,8 @@ enum loglevel {
 	LOG_FATAL
 };
 
-void modpost_log(enum loglevel loglevel, const char *fmt, ...);
+void __attribute__((format(printf, 2, 3)))
+modpost_log(enum loglevel loglevel, const char *fmt, ...);
 
 /*
  * warn - show the given message, then let modpost continue running, still
-- 
2.40.1

[PATCH v2 2/4] modpost: inform compilers that fatal() never returns

[Masahiro Yamada]

The function fatal() never returns because modpost_log() calls exit(1)
when LOG_FATAL is passed.

Inform compilers of this fact so that unreachable code flow can be
identified at compile time.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
---

Changes in v2:
  - Use noreturn attribute together with alias

 scripts/mod/modpost.c | 3 +++
 scripts/mod/modpost.h | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index ca0a90158f85..c13bc9095df3 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -90,6 +90,9 @@ void modpost_log(enum loglevel loglevel, const char *fmt, ...)
 		error_occurred = true;
 }
 
+void __attribute__((alias("modpost_log")))
+modpost_log_noret(enum loglevel loglevel, const char *fmt, ...);
+
 static inline bool strends(const char *str, const char *postfix)
 {
 	if (strlen(str) < strlen(postfix))
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 9fe974dc1a52..835cababf1b0 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -200,6 +200,9 @@ enum loglevel {
 void __attribute__((format(printf, 2, 3)))
 modpost_log(enum loglevel loglevel, const char *fmt, ...);
 
+void __attribute__((format(printf, 2, 3), noreturn))
+modpost_log_noret(enum loglevel loglevel, const char *fmt, ...);
+
 /*
  * warn - show the given message, then let modpost continue running, still
  *        allowing modpost to exit successfully. This should be used when
@@ -215,4 +218,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...)	modpost_log_noret(LOG_FATAL, fmt, ##args)
-- 
2.40.1

[PATCH v2 3/4] modpost: remove unneeded initializer in section_rel()

[Masahiro Yamada]

This initializer was added to avoid -Wmaybe-uninitialized (gcc) or
-Wsometimes-uninitialized (clang) warning.

Now that compilers recognize fatal() never returns, it is unneeded.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - Split into a separate patch

 scripts/mod/modpost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index c13bc9095df3..3233946fa5f6 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1421,7 +1421,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;
 
-- 
2.40.1

[PATCH v2 4/4] modpost: remove unreachable code after fatal()

[Masahiro Yamada]

Now compilers can recognize fatal() never returns.

While GCC 4.5 dropped support for -Wunreachable-code, Clang is capable
of detecting the unreachable code.

  $ make HOSTCC=clang HOSTCFLAGS=-Wunreachable-code-return
      [snip]
    HOSTCC  scripts/mod/modpost.o
  scripts/mod/modpost.c:520:11: warning: 'return' will never be executed [-Wunreachable-code-return]
                          return 0;
                                 ^
  scripts/mod/modpost.c:477:10: warning: 'return' will never be executed [-Wunreachable-code-return]
                  return 0;
                         ^
  2 warnings generated.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
---

Changes in v2:
  - Change the patch order
  - Update the commit description and add warning example.

 scripts/mod/modpost.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 3233946fa5f6..e2bc180cecc8 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -476,11 +476,9 @@ static int parse_elf(struct elf_info *info, const char *filename)
 		fatal("%s: not relocatable object.", filename);
 
 	/* Check if file offset is correct */
-	if (hdr->e_shoff > info->size) {
+	if (hdr->e_shoff > info->size)
 		fatal("section header offset=%lu in file '%s' is bigger than filesize=%zu\n",
 		      (unsigned long)hdr->e_shoff, filename, info->size);
-		return 0;
-	}
 
 	if (hdr->e_shnum == SHN_UNDEF) {
 		/*
@@ -518,12 +516,11 @@ static int parse_elf(struct elf_info *info, const char *filename)
 		const char *secname;
 		int nobits = sechdrs[i].sh_type == SHT_NOBITS;
 
-		if (!nobits && sechdrs[i].sh_offset > info->size) {
+		if (!nobits && sechdrs[i].sh_offset > info->size)
 			fatal("%s is truncated. sechdrs[i].sh_offset=%lu > sizeof(*hrd)=%zu\n",
 			      filename, (unsigned long)sechdrs[i].sh_offset,
 			      sizeof(*hdr));
-			return 0;
-		}
+
 		secname = secstrings + sechdrs[i].sh_name;
 		if (strcmp(secname, ".modinfo") == 0) {
 			if (nobits)
-- 
2.40.1

Re: [PATCH v2 2/4] modpost: inform compilers that fatal() never returns

[Aiden Leong]

 > The function fatal() never returns because modpost_log() calls exit(1)

 > when LOG_FATAL is passed.
 >
 > Inform compilers of this fact so that unreachable code flow can be
 > identified at compile time.
 >
 > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
 > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
 > ---
 >
 > Changes in v2:
 >   - Use noreturn attribute together with alias
 >
 >  scripts/mod/modpost.c | 3 +++
 >  scripts/mod/modpost.h | 5 ++++-
 >  2 files changed, 7 insertions(+), 1 deletion(-)
 >
 > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
 > index ca0a90158f85..c13bc9095df3 100644
 > --- a/scripts/mod/modpost.c
 > +++ b/scripts/mod/modpost.c
 > @@ -90,6 +90,9 @@ void modpost_log(enum loglevel loglevel, const char 
*fmt, ...)
 >          error_occurred = true;
 >  }
 >
 > +void __attribute__((alias("modpost_log")))

Hi Masahiro,
I cross-compile kernel on Apple Silicon MacBook Pro
and every thing works well until this patch.

My build command:
make ARCH=arm CROSS_COMPILE=arm-none-eabi- \
HOSTCFLAGS="-I/opt/homebrew/opt/openssl/include" \
HOSTLDFLAGS="-L/opt/homebrew/opt/openssl/lib"

Error message:
scripts/mod/modpost.c:93:21: error: aliases are not supported on darwin

Aiden Leong

 > +modpost_log_noret(enum loglevel loglevel, const char *fmt, ...);
 > +
 >  static inline bool strends(const char *str, const char *postfix)
 >  {
 >      if (strlen(str) < strlen(postfix))
 > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
 > index 9fe974dc1a52..835cababf1b0 100644
 > --- a/scripts/mod/modpost.h
 > +++ b/scripts/mod/modpost.h
 > @@ -200,6 +200,9 @@ enum loglevel {
 >  void __attribute__((format(printf, 2, 3)))
 >  modpost_log(enum loglevel loglevel, const char *fmt, ...);
 >
 > +void __attribute__((format(printf, 2, 3), noreturn))
 > +modpost_log_noret(enum loglevel loglevel, const char *fmt, ...);
 > +
 >  /*
 >   * warn - show the given message, then let modpost continue running, 
still
 >   *        allowing modpost to exit successfully. This should be used 
when
 > @@ -215,4 +218,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...)    modpost_log_noret(LOG_FATAL, fmt, ##args)
 > --
 > 2.40.1

Re: [PATCH v2 2/4] modpost: inform compilers that fatal() never returns

[Masahiro Yamada]

+CC: clang-built-linux list, Fangrui

On Mon, Jan 22, 2024 at 1:04 PM Aiden Leong <aiden.leong@aibsd.com> wrote:
>
>
>  > The function fatal() never returns because modpost_log() calls exit(1)
>
>  > when LOG_FATAL is passed.
>  >
>  > Inform compilers of this fact so that unreachable code flow can be
>  > identified at compile time.
>  >
>  > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>  > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>  > ---
>  >
>  > Changes in v2:
>  >   - Use noreturn attribute together with alias
>  >
>  >  scripts/mod/modpost.c | 3 +++
>  >  scripts/mod/modpost.h | 5 ++++-
>  >  2 files changed, 7 insertions(+), 1 deletion(-)
>  >
>  > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>  > index ca0a90158f85..c13bc9095df3 100644
>  > --- a/scripts/mod/modpost.c
>  > +++ b/scripts/mod/modpost.c
>  > @@ -90,6 +90,9 @@ void modpost_log(enum loglevel loglevel, const char
> *fmt, ...)
>  >          error_occurred = true;
>  >  }
>  >
>  > +void __attribute__((alias("modpost_log")))
>
> Hi Masahiro,
> I cross-compile kernel on Apple Silicon MacBook Pro
> and every thing works well until this patch.
>
> My build command:
> make ARCH=arm CROSS_COMPILE=arm-none-eabi- \
> HOSTCFLAGS="-I/opt/homebrew/opt/openssl/include" \
> HOSTLDFLAGS="-L/opt/homebrew/opt/openssl/lib"
>
> Error message:
> scripts/mod/modpost.c:93:21: error: aliases are not supported on darwin


It is unfortunate.  Indeed, I see this message in:

clang/include/clang/Basic/DiagnosticSemaKinds.td


Is this limitation due to macOS executable (PEF),
or is it Clang-specific?

Perhaps, "brew install gcc" can be a solution?










>
> Aiden Leong
>
>  > +modpost_log_noret(enum loglevel loglevel, const char *fmt, ...);
>  > +
>  >  static inline bool strends(const char *str, const char *postfix)
>  >  {
>  >      if (strlen(str) < strlen(postfix))
>  > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
>  > index 9fe974dc1a52..835cababf1b0 100644
>  > --- a/scripts/mod/modpost.h
>  > +++ b/scripts/mod/modpost.h
>  > @@ -200,6 +200,9 @@ enum loglevel {
>  >  void __attribute__((format(printf, 2, 3)))
>  >  modpost_log(enum loglevel loglevel, const char *fmt, ...);
>  >
>  > +void __attribute__((format(printf, 2, 3), noreturn))
>  > +modpost_log_noret(enum loglevel loglevel, const char *fmt, ...);
>  > +
>  >  /*
>  >   * warn - show the given message, then let modpost continue running,
> still
>  >   *        allowing modpost to exit successfully. This should be used
> when
>  > @@ -215,4 +218,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...)    modpost_log_noret(LOG_FATAL, fmt, ##args)
>  > --
>  > 2.40.1



--
Best Regards
Masahiro Yamada

Re: [PATCH v2 2/4] modpost: inform compilers that fatal() never returns

[Nathan Chancellor]

On Mon, Jan 22, 2024 at 10:29:32PM +0900, Masahiro Yamada wrote:
> +CC: clang-built-linux list, Fangrui
> 
> On Mon, Jan 22, 2024 at 1:04 PM Aiden Leong <aiden.leong@aibsd.com> wrote:
> >
> >
> >  > The function fatal() never returns because modpost_log() calls exit(1)
> >
> >  > when LOG_FATAL is passed.
> >  >
> >  > Inform compilers of this fact so that unreachable code flow can be
> >  > identified at compile time.
> >  >
> >  > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> >  > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> >  > ---
> >  >
> >  > Changes in v2:
> >  >   - Use noreturn attribute together with alias
> >  >
> >  >  scripts/mod/modpost.c | 3 +++
> >  >  scripts/mod/modpost.h | 5 ++++-
> >  >  2 files changed, 7 insertions(+), 1 deletion(-)
> >  >
> >  > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> >  > index ca0a90158f85..c13bc9095df3 100644
> >  > --- a/scripts/mod/modpost.c
> >  > +++ b/scripts/mod/modpost.c
> >  > @@ -90,6 +90,9 @@ void modpost_log(enum loglevel loglevel, const char
> > *fmt, ...)
> >  >          error_occurred = true;
> >  >  }
> >  >
> >  > +void __attribute__((alias("modpost_log")))
> >
> > Hi Masahiro,
> > I cross-compile kernel on Apple Silicon MacBook Pro
> > and every thing works well until this patch.
> >
> > My build command:
> > make ARCH=arm CROSS_COMPILE=arm-none-eabi- \
> > HOSTCFLAGS="-I/opt/homebrew/opt/openssl/include" \
> > HOSTLDFLAGS="-L/opt/homebrew/opt/openssl/lib"
> >
> > Error message:
> > scripts/mod/modpost.c:93:21: error: aliases are not supported on darwin
> 
> 
> It is unfortunate.  Indeed, I see this message in:
> 
> clang/include/clang/Basic/DiagnosticSemaKinds.td
> 
> 
> Is this limitation due to macOS executable (PEF),
> or is it Clang-specific?

Based on my admittedly brief research, this seems related to the Mach-O
format. That message was added by [1] in response to [2] but the message
mentioned weak aliases being supported. A further clarification was made
in [3] to state that all aliases are unsupported as a result of some
internal Apple bug it seems but I do see a couple of bug reports stating
that may not be true [4][5] (although that does not seem relevant for
this report).

[1]: https://github.com/llvm/llvm-project/commit/0017c5fa92ad3b10e15fd34f3865e8e5b850a5ed
[2]: https://llvm.org/bz8720
[3]: https://github.com/llvm/llvm-project/commit/4e30b96834cea5682a8e9e024dda06319825000a
[4]: https://github.com/llvm/llvm-project/issues/11488
[5]: https://github.com/llvm/llvm-project/issues/71001

Cheers,
Nathan

Re: [PATCH v2 2/4] modpost: inform compilers that fatal() never returns

[Masahiro Yamada]

On Tue, Jan 23, 2024 at 8:02 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Mon, Jan 22, 2024 at 10:29:32PM +0900, Masahiro Yamada wrote:
> > +CC: clang-built-linux list, Fangrui
> >
> > On Mon, Jan 22, 2024 at 1:04 PM Aiden Leong <aiden.leong@aibsd.com> wrote:
> > >
> > >
> > >  > The function fatal() never returns because modpost_log() calls exit(1)
> > >
> > >  > when LOG_FATAL is passed.
> > >  >
> > >  > Inform compilers of this fact so that unreachable code flow can be
> > >  > identified at compile time.
> > >  >
> > >  > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > >  > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> > >  > ---
> > >  >
> > >  > Changes in v2:
> > >  >   - Use noreturn attribute together with alias
> > >  >
> > >  >  scripts/mod/modpost.c | 3 +++
> > >  >  scripts/mod/modpost.h | 5 ++++-
> > >  >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >  >
> > >  > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > >  > index ca0a90158f85..c13bc9095df3 100644
> > >  > --- a/scripts/mod/modpost.c
> > >  > +++ b/scripts/mod/modpost.c
> > >  > @@ -90,6 +90,9 @@ void modpost_log(enum loglevel loglevel, const char
> > > *fmt, ...)
> > >  >          error_occurred = true;
> > >  >  }
> > >  >
> > >  > +void __attribute__((alias("modpost_log")))
> > >
> > > Hi Masahiro,
> > > I cross-compile kernel on Apple Silicon MacBook Pro
> > > and every thing works well until this patch.
> > >
> > > My build command:
> > > make ARCH=arm CROSS_COMPILE=arm-none-eabi- \
> > > HOSTCFLAGS="-I/opt/homebrew/opt/openssl/include" \
> > > HOSTLDFLAGS="-L/opt/homebrew/opt/openssl/lib"
> > >
> > > Error message:
> > > scripts/mod/modpost.c:93:21: error: aliases are not supported on darwin
> >
> >
> > It is unfortunate.  Indeed, I see this message in:
> >
> > clang/include/clang/Basic/DiagnosticSemaKinds.td
> >
> >
> > Is this limitation due to macOS executable (PEF),
> > or is it Clang-specific?
>
> Based on my admittedly brief research, this seems related to the Mach-O
> format. That message was added by [1] in response to [2] but the message
> mentioned weak aliases being supported. A further clarification was made
> in [3] to state that all aliases are unsupported as a result of some
> internal Apple bug it seems but I do see a couple of bug reports stating
> that may not be true [4][5] (although that does not seem relevant for
> this report).
>
> [1]: https://github.com/llvm/llvm-project/commit/0017c5fa92ad3b10e15fd34f3865e8e5b850a5ed
> [2]: https://llvm.org/bz8720
> [3]: https://github.com/llvm/llvm-project/commit/4e30b96834cea5682a8e9e024dda06319825000a
> [4]: https://github.com/llvm/llvm-project/issues/11488
> [5]: https://github.com/llvm/llvm-project/issues/71001



Thank you.
As far as I understood, macOS seems to support weak aliases.

With [5] fixed, perhaps we could do the following:

-void __attribute__((alias("modpost_log")))
+void __attribute__((weak, alias("modpost_log")))


But, we do not need to wait for it.
We can do similar without aliases at all.

fix submitted:
https://lore.kernel.org/all/20240127132811.726504-1-masahiroy@kernel.org/T/#u




-- 
Best Regards
Masahiro Yamada