2023-12-03 09:50:08

by Masahiro Yamada

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

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 <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
---

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


2024-01-22 04:04:10

by Aiden Leong

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


> 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 <[email protected]>
> Reviewed-by: Nathan Chancellor <[email protected]>
> ---
>
> 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

2024-01-22 13:30:18

by Masahiro Yamada

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

+CC: clang-built-linux list, Fangrui

On Mon, Jan 22, 2024 at 1:04 PM Aiden Leong <[email protected]> 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 <[email protected]>
> > Reviewed-by: Nathan Chancellor <[email protected]>
> > ---
> >
> > 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

2024-01-22 23:03:04

by Nathan Chancellor

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

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 <[email protected]> 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 <[email protected]>
> > > Reviewed-by: Nathan Chancellor <[email protected]>
> > > ---
> > >
> > > 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

2024-01-27 13:41:58

by Masahiro Yamada

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

On Tue, Jan 23, 2024 at 8:02 AM Nathan Chancellor <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > Reviewed-by: Nathan Chancellor <[email protected]>
> > > > ---
> > > >
> > > > 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/[email protected]/T/#u




--
Best Regards
Masahiro Yamada