Re: Assertion error when building in Debug mode with MSVC

2022-01-25 Thread Eric Blake
[adding bug-gnulib]

On Mon, Jan 24, 2022 at 05:00:01PM +0100, Julien Marrec wrote:
> Hello,
> 
> I realize this is probably not a combination of platform, compiler and
> build type that you anticipated. m4 is used in a conan - the C++ package
> manager - recipe, and it serves as the basis for many other recipes (such
> as bison).
> 
> We encountered an issue when building it with Visual Studio (2019 for eg)
> in *Debug* mode (runtime MDd). At runtime, even `m4.exe --version` will
> throw the following assertion in ucrt.dll
> 
> ```
> minkernel\crts\ucrt\src\appcrt\lowio\close.cpp(49) : Assertion failed:
> (_osfile(fh) & FOPEN)
> ```
> 
> It seems that there is a double close happening, apparently from something
> gnulib overrides, as this small example produces exactly the same error:
> 
> ```
> 
> #include 
> #include int main() {
> int no =  _fileno (stdin);
> _close(no);
> fclose(stdin);
> }
> 
> ```

fclose() asserting because the underlying fd has been previously
closed seems odd; is it something that gnulib should work around by
overriding fclose() to be guaranteed that it has an open fd?

This may be fallout from the gnulib module closeout (and the function
close_stdout), trying hard to ensure that any write failures to stdout
are properly detected.  That may be the spot where we would add a
workaround to your platform's assertion failure in fclose().

> 
> An issue can be found there:
> https://github.com/conan-io/conan-center-index/issues/8920
> An older discussion with more information is here:
> https://github.com/conan-io/conan-center-index/pull/7369#issuecomment-927159346
> 
> I have implemented a poor-man's workaround for the time being which
> consists in calling _CrtSetReportMode early in the main function of m4.c so
> that the popup isn't triggered and instead it prints to the console, but I
> would ideally like to see if there isn't a small patch we could do to avoid
> the double close to begin with.
> 
> Could you help please?
> 
> Thank you for your time and contributions!
> Best,
> Julien
> --
> Julien Marrec, EBCP, BPI MFBA
> Owner at EffiBEM 
> T: +33 6 95 14 42 13
> 
> LinkedIn (en ) *| *(fr
> ) :
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




Re: Assertion error when building in Debug mode with MSVC

2022-01-25 Thread Paul Eggert

On 1/25/22 08:13, Eric Blake wrote:

it something that gnulib should work around by
overriding fclose() to be guaranteed that it has an open fd?


This is a bug that Gnulib's fclose module is already supposed to be 
working around. Are the programs in question using the fclose module, 
either directly or indirectly? If so, we should investigate why the 
fclose module isn't working around the MSVC bug. If not, perhaps we 
should change the close-stream module to start depending on the fclose 
module.




Re: Assertion error when building in Debug mode with MSVC

2022-01-25 Thread Julien Marrec
hello,

Thanks for the answers, I'm trying to keep up / follow.

> Are the programs in question using the fclose module, either directly or
indirectly?

Well, simply doing `m4.exe --version` does exhibit the issue. Not sure if
I'm answering the question correctly or not.


I'm looking at the close-out.c, which calls close-stream.c

In close_stream, shouldn't the stdout FILE * be set to NULL if the close
worked, so that in the event it's called a second time you can first check
if stream == NULL?

Something like this:

diff --git a/lib/close-stream.c b/lib/close-stream.c
index 86f6d6e1d..432907a0c 100644--- a/lib/close-stream.c+++
b/lib/close-stream.c@@ -55,6 +55,7 @@
 int
 close_stream (FILE *stream)
 {+  if (stream != NULL) {
   const bool some_pending = (__fpending (stream) != 0);
   const bool prev_fail = (ferror (stream) != 0);
   const bool fclose_fail = (fclose (stream) != 0);
@@ -69,10 +70,12 @@ close_stream (FILE *stream)

   if (prev_fail || (fclose_fail && (some_pending || errno != EBADF)))
 {-  if (! fclose_fail)+  if (! fclose_fail) {
 errno = 0;+stream = NULL;+  }
   return EOF;
 }
+  }
   return 0;
 }


Best,
Julien

--
Julien Marrec, EBCP, BPI MFBA
Owner at EffiBEM 
T: +33 6 95 14 42 13

LinkedIn (en ) *| *(fr
) :



Le mar. 25 janv. 2022 à 22:33, Paul Eggert  a écrit :

> On 1/25/22 08:13, Eric Blake wrote:
> > it something that gnulib should work around by
> > overriding fclose() to be guaranteed that it has an open fd?
>
> This is a bug that Gnulib's fclose module is already supposed to be
> working around. Are the programs in question using the fclose module,
> either directly or indirectly? If so, we should investigate why the
> fclose module isn't working around the MSVC bug. If not, perhaps we
> should change the close-stream module to start depending on the fclose
> module.
>


"bootstrap" changed without adjusting version string

2022-01-25 Thread Bjarni Ingi Gislason
File: git/gnulib/build-aux/bootstrap

scriptversion=2021-04-11.09;

  According to "git blame bootstrap", material has been added at dates
"2021-12-12" and "2022-01-05" without updating the announced string
"scriptversion" in the output of "bootstrap --version".

  This also affects the string "copyright_year".

-- 
Bjarni I. Gislason



Re: "bootstrap" changed without adjusting version string

2022-01-25 Thread Paul Eggert

Thanks, I updated the version string.



Re: Assertion error when building in Debug mode with MSVC

2022-01-25 Thread Paul Eggert

On 1/25/22 14:33, Julien Marrec wrote:


Are the programs in question using the fclose module, either directly or

indirectly?

Well, simply doing `m4.exe --version` does exhibit the issue.


For the module info we need to look at something other than that.

Does your m4 source code contain the files fclose.c and fclose.m4 somewhere?

When you configured m4, did it say "checking whether fclose works on 
input streams" and if so what was the result?


When you built m4, did the build compile fclose.c and create an object 
file fclose.o?



In close_stream, shouldn't the stdout FILE * be set to NULL if the close
worked


No, as that's just a local variable so setting it to NULL won't persist 
until the next call.