[PATCH 0/1] Fix clang build, partially

[PATCH 0/1] Fix clang build, partially

[Darren Kenny]

To fix the build with clang, there are 2 parts needed, the first is to get a
fix back into gnulib to ensure that the regex code can build, that has been
submitted upstream and can be pulled in later since there is now a preference
not to have patches held in GRUB's own sources but fix upstream.

But even with that fix, GRUB won't build, and that is because Clang replaces
the 'abort()' call with '__builtin_trap', but the abort call is surrounded by
an EXPORT_FUNC(), which is valid for GCC.

The core of the issue is that the gensymlist script should be filtering out
any call starting with '__builtin_', so this patch will add that filter.

Thanks,

Darren.

Darren Kenny (1):
  gensymlist: fix clang build with exporting of __builtin_trap

 grub-core/gensymlist.sh | 1 +
 1 file changed, 1 insertion(+)

-- 
2.31.1

[PATCH 1/1] gensymlist: fix clang build with exporting of __builtin_trap

[Darren Kenny]

clang expands the abort function to __builtin_trap, but that cannot be
exported.

The script that generates the symlist.c file should also exclude any
symbols that start with __builtin_.

Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
---
 grub-core/gensymlist.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/grub-core/gensymlist.sh b/grub-core/gensymlist.sh
index 5074ef6aad58..a2e5b85d0a71 100644
--- a/grub-core/gensymlist.sh
+++ b/grub-core/gensymlist.sh
@@ -58,6 +58,7 @@ EOF
 
 (while read LINE; do echo $LINE; done) \
   | grep -v '^#' \
+  | grep -v 'EXPORT_FUNC *(__builtin_[a-zA-Z0-9_]*)' \
   | sed -n \
         -e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC *(\([a-zA-Z0-9_]*\)).*/      {"\1", \1, 1},/;p;}' \
         -e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR *(\([a-zA-Z0-9_]*\)).*/      {"\1", (void *) \&\1, 0},/;p;}' \
-- 
2.31.1

Re: [PATCH 1/1] gensymlist: fix clang build with exporting of __builtin_trap

[Vladimir 'phcoder' Serbinenko]

On Tue, Jun 14, 2022 at 3:37 PM Darren Kenny <darren.kenny@oracle.com> wrote:
>
> clang expands the abort function to __builtin_trap, but that cannot be
> exported.
>
> The script that generates the symlist.c file should also exclude any
> symbols that start with __builtin_.
This patch will break many platforms including PPC which rely on
functions like __builtin_bswap*
>
> Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
> ---
>  grub-core/gensymlist.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/grub-core/gensymlist.sh b/grub-core/gensymlist.sh
> index 5074ef6aad58..a2e5b85d0a71 100644
> --- a/grub-core/gensymlist.sh
> +++ b/grub-core/gensymlist.sh
> @@ -58,6 +58,7 @@ EOF
>
>  (while read LINE; do echo $LINE; done) \
>    | grep -v '^#' \
> +  | grep -v 'EXPORT_FUNC *(__builtin_[a-zA-Z0-9_]*)' \
>    | sed -n \
>          -e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC *(\([a-zA-Z0-9_]*\)).*/      {"\1", \1, 1},/;p;}' \
>          -e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR *(\([a-zA-Z0-9_]*\)).*/      {"\1", (void *) \&\1, 0},/;p;}' \
> --
> 2.31.1
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH 1/1] gensymlist: fix clang build with exporting of __builtin_trap

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]

Correct solution is to provide __builtin_trap ourselves. Likely it would be
a wrapper around grub_abort

Le mar. 14 juin 2022, 15:37, Darren Kenny <darren.kenny@oracle.com> a
écrit :

> clang expands the abort function to __builtin_trap, but that cannot be
> exported.
>
> The script that generates the symlist.c file should also exclude any
> symbols that start with __builtin_.
>
> Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
> ---
>  grub-core/gensymlist.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/grub-core/gensymlist.sh b/grub-core/gensymlist.sh
> index 5074ef6aad58..a2e5b85d0a71 100644
> --- a/grub-core/gensymlist.sh
> +++ b/grub-core/gensymlist.sh
> @@ -58,6 +58,7 @@ EOF
>
>  (while read LINE; do echo $LINE; done) \
>    | grep -v '^#' \
> +  | grep -v 'EXPORT_FUNC *(__builtin_[a-zA-Z0-9_]*)' \
>    | sed -n \
>          -e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC
> *(\([a-zA-Z0-9_]*\)).*/      {"\1", \1, 1},/;p;}' \
>          -e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR
> *(\([a-zA-Z0-9_]*\)).*/      {"\1", (void *) \&\1, 0},/;p;}' \
> --
> 2.31.1
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 1966 bytes --]

Re: [PATCH 1/1] gensymlist: fix clang build with exporting of __builtin_trap

[Glenn Washburn]

On Tue, 14 Jun 2022 18:55:18 +0200
"Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> wrote:

> Correct solution is to provide __builtin_trap ourselves. Likely it would be
> a wrapper around grub_abort

This seems like tacit support for at least the direction of my patch
removing the use of __builtin_trap[1] which was introduced recently in
cd37d3d39 (gnulib: Drop no-abort.patch) and which uses grub_abort()
instead of __builtin_trap. The only other place that could potentially
use __builtin_trap is in grub-core/lib/gnulib/verify.h in the "assume"
macro. However, I don't see anywhere that macro is being used. Though
Daniel was wondering what was wrong with defining abort in
grub-core/lib/posix_wrap/stdlib.h as had been done in db7337a3d
(grub-core/lib/posix_wrap/stdlib.h (abort): Removed).

Glenn

[1] https://lists.gnu.org/archive/html/grub-devel/2022-03/msg00220.html

> 
> Le mar. 14 juin 2022, 15:37, Darren Kenny <darren.kenny@oracle.com> a
> écrit :
> 
> > clang expands the abort function to __builtin_trap, but that cannot be
> > exported.
> >
> > The script that generates the symlist.c file should also exclude any
> > symbols that start with __builtin_.
> >
> > Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
> > ---
> >  grub-core/gensymlist.sh | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/grub-core/gensymlist.sh b/grub-core/gensymlist.sh
> > index 5074ef6aad58..a2e5b85d0a71 100644
> > --- a/grub-core/gensymlist.sh
> > +++ b/grub-core/gensymlist.sh
> > @@ -58,6 +58,7 @@ EOF
> >
> >  (while read LINE; do echo $LINE; done) \
> >    | grep -v '^#' \
> > +  | grep -v 'EXPORT_FUNC *(__builtin_[a-zA-Z0-9_]*)' \
> >    | sed -n \
> >          -e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC
> > *(\([a-zA-Z0-9_]*\)).*/      {"\1", \1, 1},/;p;}' \
> >          -e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR
> > *(\([a-zA-Z0-9_]*\)).*/      {"\1", (void *) \&\1, 0},/;p;}' \
> > --
> > 2.31.1
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >

Re: [PATCH 1/1] gensymlist: fix clang build with exporting of __builtin_trap

[Darren Kenny]

Hi Glenn,

Finally managed to get a look at this again, and I agree Glenn that your
patch fixes this issue too, so I withdraw my patch.

Thanks,

Darren.

On Tuesday, 2022-06-14 at 13:18:43 -05, Glenn Washburn wrote:
> On Tue, 14 Jun 2022 18:55:18 +0200
> "Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> wrote:
>
>> Correct solution is to provide __builtin_trap ourselves. Likely it would be
>> a wrapper around grub_abort
>
> This seems like tacit support for at least the direction of my patch
> removing the use of __builtin_trap[1] which was introduced recently in
> cd37d3d39 (gnulib: Drop no-abort.patch) and which uses grub_abort()
> instead of __builtin_trap. The only other place that could potentially
> use __builtin_trap is in grub-core/lib/gnulib/verify.h in the "assume"
> macro. However, I don't see anywhere that macro is being used. Though
> Daniel was wondering what was wrong with defining abort in
> grub-core/lib/posix_wrap/stdlib.h as had been done in db7337a3d
> (grub-core/lib/posix_wrap/stdlib.h (abort): Removed).
>
> Glenn
>
> [1] https://lists.gnu.org/archive/html/grub-devel/2022-03/msg00220.html
>
>> 
>> Le mar. 14 juin 2022, 15:37, Darren Kenny <darren.kenny@oracle.com> a
>> écrit :
>> 
>> > clang expands the abort function to __builtin_trap, but that cannot be
>> > exported.
>> >
>> > The script that generates the symlist.c file should also exclude any
>> > symbols that start with __builtin_.
>> >
>> > Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
>> > ---
>> >  grub-core/gensymlist.sh | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/grub-core/gensymlist.sh b/grub-core/gensymlist.sh
>> > index 5074ef6aad58..a2e5b85d0a71 100644
>> > --- a/grub-core/gensymlist.sh
>> > +++ b/grub-core/gensymlist.sh
>> > @@ -58,6 +58,7 @@ EOF
>> >
>> >  (while read LINE; do echo $LINE; done) \
>> >    | grep -v '^#' \
>> > +  | grep -v 'EXPORT_FUNC *(__builtin_[a-zA-Z0-9_]*)' \
>> >    | sed -n \
>> >          -e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC
>> > *(\([a-zA-Z0-9_]*\)).*/      {"\1", \1, 1},/;p;}' \
>> >          -e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR
>> > *(\([a-zA-Z0-9_]*\)).*/      {"\1", (void *) \&\1, 0},/;p;}' \
>> > --
>> > 2.31.1
>> >
>> >
>> > _______________________________________________
>> > Grub-devel mailing list
>> > Grub-devel@gnu.org
>> > https://lists.gnu.org/mailman/listinfo/grub-devel
>> >
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel