Problems compiling 'getrandom' with MinGW

Problems compiling 'getrandom' with MinGW

[Eli Zaretskii]

The current version of Gnulib's 'getrandom' has a couple of problems
when compiled with MinGW:

1. It assumes that the header bcrypt.h is always available.  This is
true for MinGW64, but not for mingw.org's MinGW.  A proposed patch to
allow the code be compiled without bcrypt.h is below, it is required
in Emacs because Emacs still supports versions of Windows older than
Vista.

2. It causes the calling program to be linked against bcrypt.dll if
that library is available at build time.  This will cause problems if
the produced program is then copied to a system where bcrypt.dll is
not available, because the program will refuse to start.  Since the
code to dynamically load bcrypt.dll is already in getrandom.c, I
suggest to leave only it, and remove the possibility of linking
against the DLL -- it is IMO more trouble than help.  (In Emacs, I
needed to set gl_cv_lib_assume_bcrypt=no to avoid producing such
problematic binaries, but most people won't look so close at the code,
and will not understand the consequences.)

Here's the patch I propose to let getrandom.c compile for versions of
Windows older than Vista:

diff --git a/lib/getrandom.c b/lib/getrandom.c
index f0b3f53..112c100 100644
--- a/lib/getrandom.c
+++ b/lib/getrandom.c
@@ -29,7 +29,13 @@
 #if defined _WIN32 && ! defined __CYGWIN__
 # define WIN32_LEAN_AND_MEAN
 # include <windows.h>
-# include <bcrypt.h>
+# if _WIN32_WINNT >= 0x0600
+#  include <bcrypt.h>
+# else
+   typedef LONG NTSTATUS;
+   typedef void *BCRYPT_ALG_HANDLE;
+#  define BCRYPT_USE_SYSTEM_PREFERRED_RNG 0x00000002
+# endif
 # if !HAVE_LIB_BCRYPT
 #  include <wincrypt.h>
 #  ifndef CRYPT_VERIFY_CONTEXT

Re: Problems compiling 'getrandom' with MinGW

[Bruno Haible]

Hi Eli,

> 1. It assumes that the header bcrypt.h is always available.  This is
> true for MinGW64, but not for mingw.org's MinGW.  A proposed patch to
> allow the code be compiled without bcrypt.h is below

Thanks for the report and draft patch. I'm applying the patch below.

My patch tests for the presence of <bcrypt.h>. _WIN32_WINNT >= 0x0600
does not guarantee that <bcrypt.h> is present, because someone might,
on a Windows XP machine, compile with -D_WIN32_WINNT=_WIN32_WINNT_WIN7.

> 2. It causes the calling program to be linked against bcrypt.dll if
> that library is available at build time.

No, it links the program against bcrypt.dll if you are compiling with
a _WIN32_WINNT value >= _WIN32_WINNT_WIN7 - consistently with
<https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom>
and
<https://docs.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt>.

> This will cause problems if
> the produced program is then copied to a system where bcrypt.dll is
> not available, because the program will refuse to start.
> ...
> In Emacs, I
> needed to set gl_cv_lib_assume_bcrypt=no to avoid producing such
> problematic binaries

You can get into such a scenario if you did not specify _WIN32_WINNT
as documented in
<https://docs.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt>.

Specifying values for *_cv_* works too, but is a makeshift. Specifying
_WIN32_WINNT is more future-proof.

> Since the
> code to dynamically load bcrypt.dll is already in getrandom.c, I
> suggest to leave only it, and remove the possibility of linking
> against the DLL

Declined. The code is optimal for both the future and the past versions
of Windows. Your proposal would make it less optimal for all versions
starting with Windows 7.

Bruno


2020-06-28  Bruno Haible  <bruno@clisp.org>

	getrandom: Fix compilation errors on older versions of mingw.
	Reported by Eli Zaretskii <eliz@gnu.org> in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-06/msg00059.html>.
	* m4/getrandom.m4 (gl_FUNC_GETRANDOM): Test whether <bcrypt.h> exists.
	* lib/getrandom.c: If <bcrypt.h> is not available, include <ntdef.h> and
	define/declare BCRYPT_ALG_HANDLE, BCRYPT_USE_SYSTEM_PREFERRED_RNG,
	BCryptGenRandom ourselves.

diff --git a/lib/getrandom.c b/lib/getrandom.c
index f0b3f53..030a78b 100644
--- a/lib/getrandom.c
+++ b/lib/getrandom.c
@@ -29,7 +29,16 @@
 #if defined _WIN32 && ! defined __CYGWIN__
 # define WIN32_LEAN_AND_MEAN
 # include <windows.h>
-# include <bcrypt.h>
+# if HAVE_BCRYPT_H
+#  include <bcrypt.h>
+# else
+#  include <ntdef.h> /* NTSTATUS */
+typedef void * BCRYPT_ALG_HANDLE;
+#  define BCRYPT_USE_SYSTEM_PREFERRED_RNG 0x00000002
+#  if HAVE_LIB_BCRYPT
+extern NTSTATUS WINAPI BCryptGenRandom (BCRYPT_ALG_HANDLE, UCHAR *, ULONG, ULONG);
+#  endif
+# endif
 # if !HAVE_LIB_BCRYPT
 #  include <wincrypt.h>
 #  ifndef CRYPT_VERIFY_CONTEXT
diff --git a/m4/getrandom.m4 b/m4/getrandom.m4
index 37fb100..2a0034b 100644
--- a/m4/getrandom.m4
+++ b/m4/getrandom.m4
@@ -1,4 +1,4 @@
-# getrandom.m4 serial 5
+# getrandom.m4 serial 6
 dnl Copyright 2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -36,6 +36,7 @@ AC_DEFUN([gl_FUNC_GETRANDOM],
 
   case "$host_os" in
     mingw*)
+      AC_CHECK_HEADERS([bcrypt.h])
       AC_CACHE_CHECK([whether the bcrypt library is guaranteed to be present],
         [gl_cv_lib_assume_bcrypt],
         [AC_COMPILE_IFELSE(

Re: Problems compiling 'getrandom' with MinGW

[Eli Zaretskii]

> From: Bruno Haible <bruno@clisp.org>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Sun, 28 Jun 2020 19:19:10 +0200
> 
> > 1. It assumes that the header bcrypt.h is always available.  This is
> > true for MinGW64, but not for mingw.org's MinGW.  A proposed patch to
> > allow the code be compiled without bcrypt.h is below
> 
> Thanks for the report and draft patch. I'm applying the patch below.

Thanks.

> My patch tests for the presence of <bcrypt.h>. _WIN32_WINNT >= 0x0600
> does not guarantee that <bcrypt.h> is present, because someone might,
> on a Windows XP machine, compile with -D_WIN32_WINNT=_WIN32_WINNT_WIN7.

I'm okay with testing for bcrypt.h's presence, of course.

> > 2. It causes the calling program to be linked against bcrypt.dll if
> > that library is available at build time.
> 
> No, it links the program against bcrypt.dll if you are compiling with
> a _WIN32_WINNT value >= _WIN32_WINNT_WIN7 - consistently with
> <https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom>
> and
> <https://docs.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt>.

Yes.  And that's the problem I was talking about: if some header sets
_WIN32_WINNT to such a value, for whatever reason, the program becomes
dependent on bcrypt.dll.  I very much doubt that people who do that,
or inherit this setting from someone else (perhaps the default of the
system headers) will be aware of this subtlety.

I'm asking why have that subtlety, when code to load the DLL at run
time already exists and is tested.  What does Gnulib and its users
gain by linking statically against the DLL?

> > Since the code to dynamically load bcrypt.dll is already in
> > getrandom.c, I suggest to leave only it, and remove the
> > possibility of linking against the DLL
> 
> Declined. The code is optimal for both the future and the past versions
> of Windows. Your proposal would make it less optimal for all versions
> starting with Windows 7.

Less optimal in what sense, exactly?

And what about being kind to a fellow GNU project, in this case Emacs?
Why should Emacs jump through hoops to make sure the Emacs binary on
Windows can be installed on any Windows version, regardless of how and
where it was built?  I was once told by RMS that GNU projects are
supposed to support one another.

I urge you to please reconsider.

Re: Problems compiling 'getrandom' with MinGW

[Bruno Haible]

Hi Eli,

> > > 2. It causes the calling program to be linked against bcrypt.dll if
> > > that library is available at build time.
> > 
> > No, it links the program against bcrypt.dll if you are compiling with
> > a _WIN32_WINNT value >= _WIN32_WINNT_WIN7 - consistently with
> > <https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom>
> > and
> > <https://docs.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt>.
> 
> Yes.  And that's the problem I was talking about: if some header sets
> _WIN32_WINNT to such a value, for whatever reason, the program becomes
> dependent on bcrypt.dll.  I very much doubt that people who do that,
> or inherit this setting from someone else (perhaps the default of the
> system headers) will be aware of this subtlety.
> 
> I'm asking why have that subtlety

This "subtlety" is *the* documented way to build a binary for a different
Windows version than the default for the tool set that you have installed.
Quoting:

  "The preprocessor macros WINVER and _WIN32_WINNT specify the minimum
   operating system version your code supports."

A web search for "windows compile vista" turns up this page among the
first 10 hits.

This convention is also supported by the mingw headers.

I don't see the point of not obeying this convention.

> What does Gnulib and its users
> gain by linking statically against the DLL?

1) Testability: With a static dependency, you notice immediately if a binary
is not working on the set of desired target platforms. With dynamic loading,
you have to exercise a particular functionality in order to notice it.

2) You will have noticed in the code that if linking directly with bcrypt.dll,
the code does not even attempt to invoke the other deprecated Microsoft DLL.

> And what about being kind to a fellow GNU project, in this case Emacs?
> Why should Emacs jump through hoops to make sure the Emacs binary on
> Windows can be installed on any Windows version, regardless of how and
> where it was built?

Come on. I'm not telling the Emacs Windows porters to "jump through hoops".
I'm telling them to add "-D_WIN32_WINNT=_WIN32_WINNT_WINXP" to $CC or
$CPPFLAGS, if Windows XP is their minimum target Windows version.

> I was once told by RMS that GNU projects are
> supposed to support one another.

Sure. That's what I'm doing by replying to your mail within less than 2 hours
- interrupting my work on other GNU projects - and giving you an advice that
is consistent with the Microsoft documentation and the stackoverflow.com
know-how.

Bruno

Re: Problems compiling 'getrandom' with MinGW

[Bruno Haible]

> 2020-06-28  Bruno Haible  <bruno@clisp.org>
> 
> 	getrandom: Fix compilation errors on older versions of mingw.

Oops, this patch produced compilation errors. Here's the fix.


2020-07-03  Bruno Haible  <bruno@clisp.org>

	getrandom: Fix compilation error on native Windows (regr. 2020-06-28).
	* lib/getrandom.c: Don't include <ntdef.h>. Instead, define NTSTATUS.
	* m4/getrandom.m4 (gl_FUNC_GETRANDOM): Include <windows.h> before
	<bcrypt.h>.

diff --git a/lib/getrandom.c b/lib/getrandom.c
index 030a78b..f8695ab 100644
--- a/lib/getrandom.c
+++ b/lib/getrandom.c
@@ -32,7 +32,7 @@
 # if HAVE_BCRYPT_H
 #  include <bcrypt.h>
 # else
-#  include <ntdef.h> /* NTSTATUS */
+#  define NTSTATUS LONG
 typedef void * BCRYPT_ALG_HANDLE;
 #  define BCRYPT_USE_SYSTEM_PREFERRED_RNG 0x00000002
 #  if HAVE_LIB_BCRYPT
diff --git a/m4/getrandom.m4 b/m4/getrandom.m4
index 2a0034b..424c2fa 100644
--- a/m4/getrandom.m4
+++ b/m4/getrandom.m4
@@ -1,4 +1,4 @@
-# getrandom.m4 serial 6
+# getrandom.m4 serial 7
 dnl Copyright 2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -36,7 +36,9 @@ AC_DEFUN([gl_FUNC_GETRANDOM],
 
   case "$host_os" in
     mingw*)
-      AC_CHECK_HEADERS([bcrypt.h])
+      AC_CHECK_HEADERS([bcrypt.h], [], [],
+        [[#include <windows.h>
+        ]])
       AC_CACHE_CHECK([whether the bcrypt library is guaranteed to be present],
         [gl_cv_lib_assume_bcrypt],
         [AC_COMPILE_IFELSE(