bugGNU gettext - Bugs: bug #57406, mbsinit.c fails to compile with...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #57406: mbsinit.c fails to compile with GCC 9.2.0 on MinGW-w64 (i686)

Submitted by:  Tom Kacvinsky <tomkacvinsky>
Submitted on:  Fri 13 Dec 2019 03:32:45 PM UTC  
 
Category:  Build Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  Bruno Haible <haible>
Open/Closed:  Closed

( Jump to the original submission)

Tue 17 Dec 2019 01:30:21 PM UTC, comment #6: 

Hi Bruno,

comment #5:

>
> OK, I've looked at this release and pushed a (probable) fix into gnulib:
> https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=f61573abd91da7f722b8b4a1dc97bea9b81b17a3
>
> It will propagate into packages over time. So that you can test it now, I've built a testdir from the gnulib sources:
> https://haible.de/bruno/gnu/testdir-posix-mingw.tar.gz


Awesome, thanks! The fix for this matter seems to work fine for me.

I'm running into some other build issues in the gltests subdirectory though, in my clang/libc++/ucrt setup, but I'll investigate that closer so I understand the root cause before making any more noise about it, and I'll file that issue directly with gnulib when I understand it.

> > Since a couple years, mingw-w64 can be configured to use the new Windows 10 UCRT
>
> I didn't know of this. I'm always using and testing what the mingw-w64 packages from Cygwin give me by default.


The UCRT support is tricky as it effectively alters the ABI for most things that interacts with the crt, so it's hard to gradually enable it in an existing setup, it's possible to use it primarily when building a new sysroot from scratch and only building things from source.

> > Currently with UCRT, there's only a header inline definition of mbsinit
>
> Since
> 1) POSIX mandates and gnulib expects that functions are actually available as function pointers,
> 2) Gnulib cannot attach a warning message to an inline function, only to functions declared with an 'extern' prototype,
> this is likely to cause trouble.
> Possibly symptoms of this trouble are:
> - a syntax error in gnulib's wchar.h override,
> - a compilation failure in test-wchar.c or test-wchar-c++.cc.
> This trouble can be worked around in gnulib, of course, but it would be better if it were not present in the first place.
>
> > If I make mingw-w64 provide an mbsinit function in libraries for UCRT (the second patch above), the gnulib build breaks ...
> > I don't know off-hand what the best path forward would be.
>
> How about simply waiting one year or two, then only add the mbsinit function in mingw-w64? This would give the packages (bison, coreutils, cpio, diffutils, gettext, grep, sed, tar, ...) time to pick up the gnulib change from today and prepare a new release.
>
> The 7.0.0 release is out of the door; therefore third-party software must interoperate with it, regardless whether you add the mbsinit function now in 7.0.1 or later in 7.x.


That sounds good! I tested the mbsinit/ucrt change in mingw-w64 with the test package you provided, and it seems to work fine, so I'll hold off of it for now, but try to get it integrated a bit later, when more packages have picked up newer gnulib versions.

Martin Storsjö <mstorsjo>
Tue 17 Dec 2019 10:10:14 AM UTC, comment #5: 

Hi Martin,

Thank you for your explanations; it saved me a couple of hours of investigation work.

> the first patch is included in the v7.0.0 stable release from November.


OK, I've looked at this release and pushed a (probable) fix into gnulib:
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=f61573abd91da7f722b8b4a1dc97bea9b81b17a3

It will propagate into packages over time. So that you can test it now, I've built a testdir from the gnulib sources:
https://haible.de/bruno/gnu/testdir-posix-mingw.tar.gz

> Since a couple years, mingw-w64 can be configured to use the new Windows 10 UCRT


I didn't know of this. I'm always using and testing what the mingw-w64 packages from Cygwin give me by default.

> Currently with UCRT, there's only a header inline definition of mbsinit


Since
1) POSIX mandates and gnulib expects that functions are actually available as function pointers,
2) Gnulib cannot attach a warning message to an inline function, only to functions declared with an 'extern' prototype,
this is likely to cause trouble.
Possibly symptoms of this trouble are:
- a syntax error in gnulib's wchar.h override,
- a compilation failure in test-wchar.c or test-wchar-c++.cc.
This trouble can be worked around in gnulib, of course, but it would be better if it were not present in the first place.

> If I make mingw-w64 provide an mbsinit function in libraries for UCRT (the second patch above), the gnulib build breaks ...
> I don't know off-hand what the best path forward would be.


How about simply waiting one year or two, then only add the mbsinit function in mingw-w64? This would give the packages (bison, coreutils, cpio, diffutils, gettext, grep, sed, tar, ...) time to pick up the gnulib change from today and prepare a new release.

The 7.0.0 release is out of the door; therefore third-party software must interoperate with it, regardless whether you add the mbsinit function now in 7.0.1 or later in 7.x.

Bruno Haible <haible>
Project AdministratorIn charge of this item.
Mon 16 Dec 2019 09:44:50 PM UTC, comment #4: 

I'm continuing this discussion here, even though this matter is entirely between gnulib and mingw-w64 now, not related to gettext itself.

comment #1:

> I see recent relevant patches here:
>
> https://sourceforge.net/p/mingw-w64/mailman/message/36795394/
> https://sourceforge.net/p/mingw-w64/mailman/message/36878685/
>
> This area seems to be in active development on the mingw side.
>
> Please tell us when there is a new release of mingw-w64 that contains these changes (stabilized).


Well, the first patch is included in the v7.0.0 stable release from November.

The second patch was a response to this issue, but it isn't merged yet.

I'll try to summarize the situation:

- Since a couple years, mingw-w64 can be configured to use the new Windows 10 UCRT instead of the old (actively discouraged by microsoft) msvcrt.dll

- In msvcrt.dll, mbstate_t is a plain int, but in UCRT it's a struct

- In mingw.org distributions, mbsinit always returns 1, while mingw-w64 since the inception (or at least from the initial commit in the current VCS, from 2007), have a functional mbsinit similar to what gnulib provides

- Currently, if building gettext and the bundled gnulib with UCRT headers/libs, the build does succeed. (I don't know what actually differs in Tom's setup compared to mine.) Currently with UCRT, there's only a header inline definition of mbsinit, while with msvcrt.dll, there's a mingw-provided extern mbsinit in one of the libraries that are linked in by default.

- gnulib decides to always replace the mbsinit function when targeting mingw, as it historically used to be broken (always returning 1)

- As the UCRT setup lacks an mbsinit function in the libraries, gnulib's mbrtowc.m4 will also decide to replace the mbstate_t type with a typedef int, http://git.savannah.gnu.org/cgit/gnulib.git/tree/m4/mbrtowc.m4#n104. This saves the situation, as then gnulib's mbsinit replacement works with gnulib's mbstate_t type, which works.

- If I make mingw-w64 provide an mbsinit function in libraries for UCRT (the second patch above), the gnulib build breaks, as mbrtowc.m4 above won't decide to replace mbstate_t any longer, but still building gnulib's mbsinit (which assumes that mbstate_t is a plain integer).

I don't know off-hand what the best path forward would be. If replacing mbsinit would be tied to actually detecting it (i.e. ignoring the situation with mingw.org toolchains where it always returns 1), I believe things would work, as mbrtowc.m4 either would force replacing mbstate_t along with mbsinit (which works), or rely on the mingw-w64 provided mbsinit (not trying to build gnulib's mbsinit).

If you want to try it out for yourselves, I can provide a prebuilt toolchain with new enough mingw headers/libs to showcase the issue. If you're interested in trying that, would you want a linux or windows based one?

Martin Storsjö <mstorsjo>
Mon 16 Dec 2019 05:33:34 PM UTC, comment #3: 

> I am looking at the source code for 0.19.8.1


The newest gettext release is 0.20.1.

> is hardcoded into the mbsinit.c file


It is not a problem if a mingw-w64 change has the effect that some gnulib code or test - that obviously made too adventurous assumptions - breaks or fails. Gnulib will be updated when there is a new mingw-w64 release, and the fixed gnulib files will be propagated into the various packages - from coreutils to gettext.

glibc has done such things as well (e.g. the removal of the 'gets' function). Yes, it causes breakage for a certain time. But this shouldn't block the mingw-w64 people from making important, sensible changes.

Bruno Haible <haible>
Project AdministratorIn charge of this item.
Mon 16 Dec 2019 03:02:18 PM UTC, comment #2: 

WE have been having discussions on the list and it turns out the trick that was proposed won't work.  mbsinit() is not auto-detected by the gettext configure command, but is hardcoded into the mbsinit.c file (gettext-runtime/gnulib-lib/mbsinit.)::

#if (defined WIN32 || defined __WIN32_) && !defined _CYGWIN_

/* On native Windows, 'mbstate_t' is defined as 'int'.  */

int
mbsinit (const mbstate_t *ps)
{
  return ps == NULL || *ps == 0;
}

#else

There are modifications for this for later versions gettext right now, I am looking at the source code for 0.19.8.1. related to whether the build system is MINGW based.

Tom Kacvinsky <tomkacvinsky>
Sat 14 Dec 2019 01:20:41 AM UTC, comment #1: 

I see recent relevant patches here:

https://sourceforge.net/p/mingw-w64/mailman/message/36795394/
https://sourceforge.net/p/mingw-w64/mailman/message/36878685/

This area seems to be in active development on the mingw side.

Please tell us when there is a new release of mingw-w64 that contains these changes (stabilized).

Bruno Haible <haible>
Project AdministratorIn charge of this item.
Fri 13 Dec 2019 03:32:45 PM UTC, original submission:  

I see this error

gettext-runtime/gnulib-lib/mbsinit.c:61:28: error: invalid operands to binary == (have 'mbstate_t' {aka 'const struct _Mbstatet'} and 'int')

When building gettext 0.20.1 on MinGW-w64 with GCC 9.2.0 for i686.  I know the problem is that we are trying to compare an int with a struct, which is not supported in C.  The interesting thing is that this does not happen on Linux (possibly because the system gnulib is used).  So I am not sure what to do.

The configure options were pretty bland (just --prefix and --with-libiconv-prefix).

I did not run autogen.sh.

Tom Kacvinsky <tomkacvinsky>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mstorsjo (Posted a comment)
  • -email is unavailable- added by haible (Posted a comment)
  • -email is unavailable- added by tomkacvinsky (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-12-17 haible StatusNeed Info => Fixed
        Assigned toNone => haible
        Open/ClosedOpen => Closed
    2019-12-14 haible StatusNone => Need Info

    Back to the top


    Powered by Savane 3.6