[PATCH 0/3] MR6922: Improve clang configuration.

[eric pouech (＠epo)]

This MR improves configuration scheme when using clang as cross compiler:
- it fixes test for dwarf (-4) support (current test is failing as ldd emits
  a warning when generating the long section names for the dwarf section,
  that configure.ac treats as an error),
- it fixes --enable-build-id option with clang (clang linker uses -build-id
  option, while gcc uses --build-id /mind the extra '-'/)
- it adds support in winegcc for a generic build-id linker option.

(extracted from draft MR!6715)

[PATCH 1/3] configure.ac: Properly test clang for dwarf support.

[Eric Pouech]

From: Eric Pouech

clang emits a warning when linking the dwarf's long sections names, which
lets the configuration test fail.

Signed-off-by: Eric Pouech 
---
 configure.ac | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 74fed2f681e..f3752619521 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1004,7 +1004,10 @@ This is an error since --enable-archs=$wine_arch was requested.])])
     if test "x$ac_debug_format_seen" = x
     then
       case $wine_crossdebug in
-        *dwarf)  WINE_TRY_PE_CFLAGS([-gdwarf-4]) ;;
+        *dwarf)  dnl clang emits a warning without -Wl,-debug:dwarf, so ensure linker option is present
+                 WINE_TRY_PE_CFLAGS([-Wl,-debug:dwarf],[AS_VAR_APPEND([${wine_arch}_LDFLAGS],[" -Wl,-debug:dwarf"])
+                                                        CFLAGS="$CFLAGS -Wl,-debug:dwarf"])
+                 WINE_TRY_PE_CFLAGS([-gdwarf-4]) ;;
         pdb)     WINE_TRY_PE_CFLAGS([-gcodeview]) ;;
       esac
     fi

[PATCH 2/3] winegcc: Remap build-id linker option for clang.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 tools/winegcc/winegcc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/winegcc/winegcc.c b/tools/winegcc/winegcc.c
index 60cc6148158..62a03db53a0 100644
--- a/tools/winegcc/winegcc.c
+++ b/tools/winegcc/winegcc.c
@@ -178,6 +178,7 @@ struct options
     int strip;
     int pic;
     int no_default_config;
+    int build_id;
     const char* wine_objdir;
     const char* winebuild;
     const char* output_name;
@@ -427,6 +428,9 @@ static struct strarray get_link_args( struct options *opts, const char *output_n
         if (opts->debug_file && strendswith(opts->debug_file, ".pdb"))
             strarray_add(&link_args, strmake("-Wl,--pdb=%s", opts->debug_file));

+        if (opts->build_id)
+            strarray_add( &link_args, "-Wl,--build-id");
+
         if (opts->out_implib)
             strarray_add(&link_args, strmake("-Wl,--out-implib,%s", opts->out_implib));

@@ -466,6 +470,9 @@ static struct strarray get_link_args( struct options *opts, const char *output_n
         else if (!opts->strip)
             strarray_add(&link_args, "-Wl,-debug:dwarf");

+        if (opts->build_id)
+            strarray_add( &link_args, "-Wl,-build-id");
+
         if (opts->out_implib)
             strarray_add(&link_args, strmake("-Wl,-implib:%s", opts->out_implib));
         else
@@ -1843,6 +1850,11 @@ int main(int argc, char **argv)
                                 opts.out_implib = xstrdup( Wl.str[++j] );
                                 continue;
                             }
+                            if (!strcmp( Wl.str[j], "--build-id" ))
+                            {
+                                opts.build_id = 1;
+                                continue;
+                            }
                             if (!strcmp(Wl.str[j], "-static")) linking = -1;
                             strarray_add(&opts.linker_args, strmake("-Wl,%s",Wl.str[j]));
                         }

[PATCH 3/3] configure.ac: Properly detect build-id support for clang.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 configure.ac | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index f3752619521..74c7c7c7089 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1014,9 +1014,13 @@ This is an error since --enable-archs=$wine_arch was requested.])])
     AS_VAR_SET([${wine_arch}_DEBUG],[$wine_crossdebug])

test "x$enable_werror" != xyes || WINE_TRY_PE_CFLAGS([-Werror])
-    test "x$enable_build_id" != xyes || WINE_TRY_PE_CFLAGS([-Wl,--build-id],
-                                         [AS_VAR_APPEND([${wine_arch}_CFLAGS],[" -Wl,--build-id"])
-                                          AS_VAR_APPEND([${wine_arch}_LDFLAGS],[" -Wl,--build-id"])])
+    if test "x$enable_build_id" = "xyes"
+    then
+        dnl gcc/mingw
+        WINE_TRY_PE_CFLAGS([-Wl,--build-id], [AS_VAR_APPEND([${wine_arch}_LDFLAGS],[" -Wl,--build-id"])])
+        dnl clang
+        WINE_TRY_PE_CFLAGS([-Wl,-build-id], [AS_VAR_APPEND([${wine_arch}_LDFLAGS],[" -Wl,--build-id"])])
+    fi

done

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about tools/winegcc/winegcc.c:
>      if (opts->debug_file && strendswith(opts->debug_file, ".pdb"))
>          strarray_add(&link_args, strmake("-Wl,--pdb=%s", opts->debug_file));
>
> -
>    if (opts->build_id)
> -
>        strarray_add( &link_args, "-Wl,--build-id");

This looks good, but we should probably add for generic Unix case too.

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about configure.ac:
>  AS_VAR_SET([${wine_arch}_DEBUG],[$wine_crossdebug])
>
>  test "x$enable_werror" != xyes || WINE_TRY_PE_CFLAGS([-Werror])
> - test "x$enable_build_id" != xyes || WINE_TRY_PE_CFLAGS([-Wl,--build-id],
> -
>                                     [AS_VAR_APPEND([${wine_arch}_CFLAGS],[" -Wl,--build-id"])
> -
>                                      AS_VAR_APPEND([${wine_arch}_LDFLAGS],[" -Wl,--build-id"])])
> - if test "x$enable_build_id" = "xyes"
> - then
> -
>    dnl gcc/mingw
> -
>    WINE_TRY_PE_CFLAGS([-Wl,--build-id], [AS_VAR_APPEND([${wine_arch}_LDFLAGS],[" -Wl,--build-id"])])
> -
>    dnl clang
> -
>    WINE_TRY_PE_CFLAGS([-Wl,-build-id], [AS_VAR_APPEND([${wine_arch}_LDFLAGS],[" -Wl,--build-id"])])

With that logic in `winegcc`, I think we could skip `WINE_TRY_PE_CFLAGS` and just append to `*_LDFLAGS` unconditionally. It means that if the toolchain is not capable of adding handling it, the compilation will fail, but that seems justified for a feature that need to be explicitly enabled (and we can revisit that if we decide to change it changes; toolchains lacking it are mostly theoretical anyway).

[Alfred Agrell (＠Alcaro)]

On Mon Dec  2 14:50:40 2024 +0000, Jacek Caban wrote:
> With that logic in `winegcc`, I think we could skip `WINE_TRY_PE_CFLAGS`
> and just append to `*_LDFLAGS` unconditionally. It means that if the
> toolchain is not capable of adding handling it, the compilation will
> fail, but that seems justified for a feature that need to be explicitly
> enabled (and we can revisit that if we decide to change it changes;
> toolchains lacking it are mostly theoretical anyway).

Why is the second build-id on this line spelled with double dash?

[eric pouech (＠epo)]

On Mon Dec  2 15:03:54 2024 +0000, Alfred Agrell wrote:
> Why is the second build-id on this line spelled with double dash?

that's intentional. one single definition (with double dash) and winegcc will adapt to the actual toolchain

[eric pouech (＠epo)]

@jacek: in the case of the toolchain not supporting build id, the ideal would be to have an explicit report at the end of the configuration of the problem. but that's not the case in the current situation.
so your proposal won't make things that worst <g> (just a bit more cryptic to follow)
but --enable-build-id is more targeted at packagers, so I'll update accordingly

<dream on>I'd love to be able to run the TRY_*_FLAGS directly with winegcc</dream>

[eric pouech (＠epo)]

On Mon Dec  2 16:17:26 2024 +0000, Jacek Caban wrote:
> This looks good, but we should probably add for generic Unix case too.

I'll give a look (likely will require adaptation on configure.ac as well)

[eric pouech (＠epo)]

On Mon Dec  2 16:17:26 2024 +0000, eric pouech wrote:
> I'll give a look (likely will require adaptation on configure.ac as well)

actually, clang in generic Unix compilation happily accepts -Wl,--build-id
(just warns about the flag being added to CFLAGS, which can remove IMO)
so winegcc adaptation for Unix target doesn't seem needed, and I'd rather call all of this a clang bug to not accept --build-id for windows's target while it does for unix