Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#17492 closed defect (fixed)

wxString %zu format specifier is not recognised using MinGW[64]

Reported by: iwbnwif Owned by: Vadim Zeitlin <vadim@…>
Priority: normal Milestone: 3.1.1
Component: base Version: dev-latest
Keywords: MinGW format %zu __USE_MINGW_ANSI_STDIO Cc:
Blocked By: Blocking:
Patch: yes

Description

The problem can be reproduced by adding the following lines to the MyFrame constructor of the minimal sample:

    size_t big_size = 500L;
    
    wxString test = wxString::Format("Converting: %zu", big_size);
    
    wxPuts (test);

If this is built with at least one popular version of MinGW it will fail as follows:

Converting: zu

Tested on binary install of TDM-GCC 64-bit:

g++ --version
g++ (tdm64-1) 5.1.0
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Attachments (1)

strvararg.diff download (778 bytes) - added by iwbnwif 6 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by iwbnwif

The problem appears to be that this particular version of MinGW does not seem to recognize %zu even though
__USE_MINGW_ANSI_STDIO is defined as 1.

The attached patch provides a work around, however I feel it should look more like the following:

#if defined(__VISUALC__) || (defined(__MINGW32__) && (__USE_MINGW_ANSI_STDIO +0 == 0))

But this does not work for the aforementioned reason.

Changed 6 years ago by iwbnwif

comment:2 Changed 6 years ago by vadz

  • Keywords MinGW added; wxString removed
  • Milestone set to 3.1.1
  • Patch set
  • Status changed from new to confirmed

I think your patch does work because MinGW seems to always accept %Id, even when using ANSI stdio, somehow: if you compile this program

#include <stdio.h>

int main()
{
    printf("Using %%z = %zu (%zx)\n", (size_t)-1, (size_t)-1);
    printf("Using %%I = %Iu (%Ix)\n", (size_t)-1, (size_t)-1);

    return 0;
}

the second line always gives correct results, whether you compile it with -D__USE_MINGW_ANSI_STDIO=1 or without it (although it does give warnings 'I' flag used with '%x' gnu_printf format [-Wformat=]).

However testing for __USE_MINGW_ANSI_STDIO could still be better, so I wonder what exactly is wrong with the version of the patch proposed in the comment, it seems like it really ought to work. Are you sure you (re)compiled everything with -D__USE_MINGW_ANSI_STDIO=1?

Anyhow, the default is to not use ANSI stdio, so currently things are broken by default and we definitely need to fix this, so in the worst case I'll just apply your patch (if you can also add unit test for %z... to it, it would be great). But I think testing for __USE_MINGW_ANSI_STDIO==1 would be even better.

comment:3 Changed 6 years ago by iwbnwif

Anyhow, the default is to not use ANSI stdio ...

This is the strange thing, it does seem to be defined.
If I put the following code immediately before the line in my patch, a pre-processor error is indeed thrown.

#ifdef __USE_MINGW_ANSI_STDIO
#error
#endif

It appears to be defined in _mingw.h:

/* We are activating __USE_MINGW_ANSI_STDIO for various define indicators.
   Note that we enable it also for _GNU_SOURCE in C++, but not for C case. */
#if (defined (_POSIX) || defined (_POSIX_SOURCE) || defined (_POSIX_C_SOURCE) \
     || defined (_ISOC99_SOURCE) \
     || defined (_XOPEN_SOURCE) || defined (_XOPEN_SOURCE_EXTENDED) \
     || (defined (_GNU_SOURCE) && defined (__cplusplus)) \
     || defined (_SVID_SOURCE)) \
    && !defined(__USE_MINGW_ANSI_STDIO)
/* Enable __USE_MINGW_ANSI_STDIO if _POSIX defined
 * and If user did _not_ specify it explicitly... */
#  define __USE_MINGW_ANSI_STDIO			1
#endif

If I put in a #error just before the #endif in the above snippet, then a pre-processor error occurs (compiling png.c in the case of a straightforward make).

For reference, I am using the following command line to compile wxWidgets:

mingw32-make -f makefile.gcc SHARED=1 BUILD=debug from the build/msw subdirectory.

Version 0, edited 6 years ago by iwbnwif (next)

comment:4 Changed 6 years ago by vadz

OK, I finally understand what's going on here. There are 2 not completely obvious things: first, wxPrintf() is defined inline in wx/wxcrtvararg.h, so its behaviour is determined by whether __USE_MINGW_ANSI_STDIO is defined or not when compiling the application code. And, confusingly, in my simple example from the comment:2, __USE_MINGW_ANSI_STDIO is 0 because this is indeed its default value, but it becomes 1 as soon as any C++ header is included, e.g. <string> or <iostream>!

Second, our own code is compiled with __USE_MINGW_ANSI_STDIO=1 with the default build options because we do include <string> from wx/stringimpl.h which is (indirectly) included from src/common/strvararg.cpp.

Combining the two, testing for __USE_MINGW_ANSI_STDIO inside the library is, at best, useless and, at worst, wrong. So all this is just to say that I now think your original patch is correct and I'll apply it and will also remove the existing test for __USE_MINGW_ANSI_STDIO from src/common/strvarg.cpp.

comment:5 Changed 6 years ago by Vadim Zeitlin <vadim@…>

In 2f35bda1714c6860453c45a92963c08385b1d9fb/git-wxWidgets:

Fix handling of "%z" in format strings in 64 bit MSVC builds

ChangeFmtChar(), used only to replace "%z" with "%I" when using MSVC, was
buggy and forgot to increment the pointer to the next format character,
meaning that the "I" we wanted to insert into the format string was simply
lost, overwritten by the next character ("d", "u" or "x").

This actually didn't change anything in 32 bit builds, but it did result in
not using the correct size in 64 bit ones, e.g. using "%zx" with (size_t)-1
output only "ffffffff" instead of the correct "ffffffffffffffff".

See #17492.

comment:6 Changed 6 years ago by Vadim Zeitlin <vadim@…>

  • Owner set to Vadim Zeitlin <vadim@…>
  • Resolution set to fixed
  • Status changed from confirmed to closed

In c30fe114eef2c80884acafde06d51d2dc4a5351b/git-wxWidgets:

Fix handling of "%z" in format strings for MinGW in all cases

MinGW only supports "%z" directly when using ANSI stdio, but this is not
always the case (even if it often is, as just including any standard C++
header enables ANSI stdio as a side effect). Translate "%z" to "%I" for it as
well as for MSVC to ensure that it always works.

Closes #17492.

comment:7 follow-up: Changed 6 years ago by iwbnwif

Thank you for taking to time to investigate this further and for applying the patch.

There are a couple of things that in your comment:4 that still leave some dangling doubts. I will record them here just anything happens in future with regard to this, but probably there is nothing to do right now.

Thing 1.

... here are 2 not completely obvious things: first, wxPrintf() is defined inline in wx/wxcrtvararg.h, so its behaviour is determined by whether __USE_MINGW_ANSI_STDIO is defined or not when compiling the application code.

The first thing I tried when I observed the problem was to add
-D__USE_MINGW_ANSI_STDIO=1 to my application's compiler command line, so it should have been defined when wxPrintf() was compiled?

Thing 2.

Second, our own code is compiled with __USE_MINGW_ANSI_STDIO=1 with the default build options because we do include <string> from wx/stringimpl.h which is (indirectly) included from src/common/strvararg.cpp.

This I can't verify at the moment (no MinGW64 platform) but why is __USE_MINGW_ANSI_STDIO=1 defined in <string>. Surely that means that any std::string will format %zu correctly, so to have non-ANSI behaviour you would need to #undef __USE_MINGW_ANSI_STDIO?

Basically I didn't succeed in getting %zu to work at all, except when wxWidgets translates it to %Iu!

comment:8 in reply to: ↑ 7 Changed 6 years ago by vadz

Replying to iwbnwif:

Thing 1.

... here are 2 not completely obvious things: first, wxPrintf() is defined inline in wx/wxcrtvararg.h, so its behaviour is determined by whether __USE_MINGW_ANSI_STDIO is defined or not when compiling the application code.

The first thing I tried when I observed the problem was to add
-D__USE_MINGW_ANSI_STDIO=1 to my application's compiler command line, so it should have been defined when wxPrintf() was compiled?

Yes, and wxPrintf("%zu") does/did work for me if I defined __USE_MINGW_ANSI_STDIO=1 explicitly when compiling the wx version of the simple test, i.e.

#include <wx/crt.h>

int main()
{
    wxPrintf("Using %%z = %zu (%zx)\n", (size_t)-1, (size_t)-1);
    wxPrintf("Using %%I = %Iu (%Ix)\n", (size_t)-1, (size_t)-1);

    return 0;
}

Thing 2.

Second, our own code is compiled with __USE_MINGW_ANSI_STDIO=1 with the default build options because we do include <string> from wx/stringimpl.h which is (indirectly) included from src/common/strvararg.cpp.

This I can't verify at the moment (no MinGW64 platform) but why is __USE_MINGW_ANSI_STDIO=1 defined in <string>.

I'm not even sure if it's intentional, but the fact is that _mingw.h gets included from any standard C++ header and it does this by default.

Surely that means that any std::string will format %zu correctly,

Sorry, I don't understand what do you mean here, std::string doesn't format anything.

comment:9 follow-up: Changed 6 years ago by iwbnwif

Update: std::printf does work when -D__USE_MINGW_ANSI_STDIO=1 is passed on the compile command line and doesn't (with a warning) when it is not passed.

This is despite the fact that __USE_MINGW_ANSI_STDIO appears to be set to 1 in programs that include wxWidgets headers, even when the command line argument is not present.

I guess it all relates to what is the state of __USE_MINGW_ANSI_STDIO=1 in the relevant header / compilation unit (i.e. printf code is perhaps includes an application compile time unit that obviously doesn't have wxWidgets headers).

So I don't think that this problem is completely fixed, because including wxWidgets shouldn't really cause this unexpected behavior. However, because of the structure of the MinGW includes I am not sure there is anything that can be done about it. So for now the workaround is probably okay.

comment:10 in reply to: ↑ 9 ; follow-up: Changed 6 years ago by vadz

Replying to iwbnwif:

Update: std::printf does work when -D__USE_MINGW_ANSI_STDIO=1 is passed on the compile command line and doesn't (with a warning) when it is not passed.

It should be the same for wxPrintf(), really (except for the warning part because we don't use MS format checks, unlike MinGW standard headers).

This is despite the fact that __USE_MINGW_ANSI_STDIO appears to be set to 1 in programs that include wxWidgets headers, even when the command line argument is not present.

As soon as you include wx/string.h, you get it because this includes <string> with the default options.

comment:11 in reply to: ↑ 10 Changed 6 years ago by iwbnwif

Replying to vadz:
Sorry our comments crossed earlier, also please ignore the bit I wrote about std::string.

This is despite the fact that __USE_MINGW_ANSI_STDIO appears to be set to 1 in programs that include wxWidgets headers, even when the command line argument is not present.

As soon as you include wx/string.h, you get it because this includes <string> with the default options.

Yes, and this has side effects because:

#include <string>
int main( int argc, char** argv )
{
#if (__USE_MINGW_ANSI_STDIO == 1)
    printf ("Using printf %zu\n", static_cast<size_t>(500));
#endif    

    return 0;
}

works as expected, whereas:

#include <wx/string.h>
int main( int argc, char** argv )
{
#if (__USE_MINGW_ANSI_STDIO == 1)
    printf ("Using printf %zu\n", static_cast<size_t>(500));
#endif    

    return 0;
}

Causes some sort of Shrodinger's effect because __USE_MINGW_ANSI_STDIO is both defined and not defined at the same time :)

Last edited 6 years ago by iwbnwif (previous) (diff)
Note: See TracTickets for help on using tickets.