[PATCH] bfd: handle codepage when opening files on MinGW

[PATCH] bfd: handle codepage when opening files on MinGW

[Clément Chigot]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

Hi Nick,

A few days ago, I've tried to open
https://sourceware.org/bugzilla/show_bug.cgi?id=25713
again because the new handling of Windows filename is triggering
issues for GNAT under MinGW.

We've made the following changes attached. We aren't sure this
is the best solution, but it seems to work fine and it's what MinGW is
using internally.
If you find such a patch ok, I guess it would be logical to merge it
for 2.39.

Thanks in advance
Clément

[-- Attachment #2: 0001-bfd-handle-codepage-when-opening-files-on-MinGW.patch --]
[-- Type: text/x-patch, Size: 2482 bytes --]

From e6b00f67c519d6a39a05343436462860d393b078 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Chigot?= <chigot@adacore.com>
Date: Fri, 10 Jun 2022 09:36:43 +0200
Subject: [PATCH] bfd: handle codepage when opening files on MinGW

Even if MS docs say that CP_UTF8 should always be used on newer
applications, forcing it might produce undefined filename if the
encoding isn't UTF-8.
MinGW seems to call ___lc_codepage_func() in order to retrieve the
current thread codepage.

bfd/ChangeLog:

	* bfdio.c (_bfd_real_fopen): Retrieve codepage with
	___lc_codepage_func() on MinGW.

---
 bfd/bfdio.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/bfd/bfdio.c b/bfd/bfdio.c
index 5c9a6555894..58245972f0c 100644
--- a/bfd/bfdio.c
+++ b/bfd/bfdio.c
@@ -28,6 +28,7 @@
 #include "aout/ar.h"
 #if defined (_WIN32)
 #include <windows.h>
+#include <locale.h>
 #endif
 
 #ifndef S_IXUSR
@@ -121,15 +122,21 @@ _bfd_real_fopen (const char *filename, const char *modes)
    const wchar_t  prefix[] = L"\\\\?\\";
    const wchar_t  ccs[] = L", ccs=UNICODE";
    const size_t   partPathLen = strlen (filename) + 1;
+#ifdef __MINGW32__
+   const unsigned int cp = ___lc_codepage_func();
+#else
+   const unsigned int cp = CP_UTF8;
+#endif
+
 
    /* Converting the partial path from ascii to unicode.
       1) Get the length: Calling with lpWideCharStr set to null returns the length.
       2) Convert the string: Calling with cbMultiByte set to -1 includes the terminating null.  */
-   size_t         partPathWSize = MultiByteToWideChar (CP_UTF8, 0, filename, -1, NULL, 0);
+   size_t         partPathWSize = MultiByteToWideChar (cp, 0, filename, -1, NULL, 0);
    wchar_t *      partPath = calloc (partPathWSize, sizeof(wchar_t));
    size_t         ix;
 
-   MultiByteToWideChar (CP_UTF8, 0, filename, -1, partPath, partPathWSize);
+   MultiByteToWideChar (cp, 0, filename, -1, partPath, partPathWSize);
 
    /* Convert any UNIX style path separators into the DOS i.e. backslash separator.  */
    for (ix = 0; ix < partPathLen; ix++)
@@ -153,7 +160,7 @@ _bfd_real_fopen (const char *filename, const char *modes)
    /* It is non-standard for modes to exceed 16 characters.  */
    wchar_t    modesW[16 + sizeof(ccs)];
 
-   MultiByteToWideChar (CP_UTF8, 0, modes, -1, modesW, sizeof(modesW));
+   MultiByteToWideChar (cp, 0, modes, -1, modesW, sizeof(modesW));
    wcscat (modesW, ccs);
 
    FILE *     file = _wfopen (fullPath, modesW);
-- 
2.25.1

Re: [PATCH] bfd: handle codepage when opening files on MinGW

[Clément Chigot]

[-- Attachment #1: Type: text/plain, Size: 687 bytes --]

On Tue, Jun 28, 2022 at 9:27 AM Clément Chigot <chigot@adacore.com> wrote:
>
> Hi Nick,
>
> A few days ago, I've tried to open
> https://sourceware.org/bugzilla/show_bug.cgi?id=25713
> again because the new handling of Windows filename is triggering
> issues for GNAT under MinGW.
>
> We've made the following changes attached. We aren't sure this
> is the best solution, but it seems to work fine and it's what MinGW is
> using internally.
> If you find such a patch ok, I guess it would be logical to merge it
> for 2.39.

Oops, I've pushed a patch which was a bit old and doesn't apply anymore...
This version is better.

Sorry for the noise..

Thanks
Clément

[-- Attachment #2: 0001-bfd-handle-codepage-when-opening-files-on-MinGW.patch --]
[-- Type: text/x-patch, Size: 2467 bytes --]

From 004d2b514691c9e3349d9d16824ce764e093cf15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Chigot?= <chigot@adacore.com>
Date: Tue, 28 Jun 2022 09:42:12 +0200
Subject: [PATCH] bfd: handle codepage when opening files on MinGW

Even if MS docs say that CP_UTF8 should always be used on newer
applications, forcing it might produce undefined filename if the
encoding isn't UTF-8.
MinGW seems to call ___lc_codepage_func() in order to retrieve the
current thread codepage.

bfd/ChangeLog:

        * bfdio.c (_bfd_real_fopen): Retrieve codepage with
        ___lc_codepage_func() on MinGW.
---
 bfd/bfdio.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/bfd/bfdio.c b/bfd/bfdio.c
index dc8d3916509..a7c7d5bd363 100644
--- a/bfd/bfdio.c
+++ b/bfd/bfdio.c
@@ -28,6 +28,7 @@
 #include "aout/ar.h"
 #if defined (_WIN32)
 #include <windows.h>
+#include <locale.h>
 #endif
 
 #ifndef S_IXUSR
@@ -120,15 +121,20 @@ _bfd_real_fopen (const char *filename, const char *modes)
    wchar_t **     lpFilePart = {NULL};
    const wchar_t  prefix[] = L"\\\\?\\";
    const size_t   partPathLen = strlen (filename) + 1;
+#ifdef __MINGW32__
+   const unsigned int cp = ___lc_codepage_func();
+#else
+   const unsigned int cp = CP_UTF8;
+#endif
 
    /* Converting the partial path from ascii to unicode.
       1) Get the length: Calling with lpWideCharStr set to null returns the length.
       2) Convert the string: Calling with cbMultiByte set to -1 includes the terminating null.  */
-   size_t         partPathWSize = MultiByteToWideChar (CP_UTF8, 0, filename, -1, NULL, 0);
+   size_t         partPathWSize = MultiByteToWideChar (cp, 0, filename, -1, NULL, 0);
    wchar_t *      partPath = calloc (partPathWSize, sizeof(wchar_t));
    size_t         ix;
 
-   MultiByteToWideChar (CP_UTF8, 0, filename, -1, partPath, partPathWSize);
+   MultiByteToWideChar (cp, 0, filename, -1, partPath, partPathWSize);
 
    /* Convert any UNIX style path separators into the DOS i.e. backslash separator.  */
    for (ix = 0; ix < partPathLen; ix++)
@@ -152,7 +158,7 @@ _bfd_real_fopen (const char *filename, const char *modes)
    /* It is non-standard for modes to exceed 16 characters.  */
    wchar_t    modesW[16];
 
-   MultiByteToWideChar (CP_UTF8, 0, modes, -1, modesW, sizeof(modesW));
+   MultiByteToWideChar (cp, 0, modes, -1, modesW, sizeof(modesW));
 
    FILE *     file = _wfopen (fullPath, modesW);
    free (fullPath);
-- 
2.25.1

Re: [PATCH] bfd: handle codepage when opening files on MinGW

[Nick Clifton]

Hi Clément,

>> We've made the following changes attached. We aren't sure this
>> is the best solution, but it seems to work fine and it's what MinGW is
>> using internally.
>> If you find such a patch ok, I guess it would be logical to merge it
>> for 2.39.

The patch looks good to me - please apply.

Cheers
   Nick

Re: [PATCH] bfd: handle codepage when opening files on MinGW

[Luis Machado]

Hi Clément,


On 6/28/22 08:47, Clément Chigot via Binutils wrote:
> On Tue, Jun 28, 2022 at 9:27 AM Clément Chigot <chigot@adacore.com> wrote:
>>
>> Hi Nick,
>>
>> A few days ago, I've tried to open
>> https://sourceware.org/bugzilla/show_bug.cgi?id=25713
>> again because the new handling of Windows filename is triggering
>> issues for GNAT under MinGW.
>>
>> We've made the following changes attached. We aren't sure this
>> is the best solution, but it seems to work fine and it's what MinGW is
>> using internally.
>> If you find such a patch ok, I guess it would be logical to merge it
>> for 2.39.
> 
> Oops, I've pushed a patch which was a bit old and doesn't apply anymore...
> This version is better.
> 
> Sorry for the noise..
> 
> Thanks
> Clément

It's been reported to me that this is causing some issues building binutils with mingw.

In fact, trying to build master binutils-gdb with Ubuntu 22.04's mingw-w64 8.0 or Ubuntu 20.04's mingw-w64 7.0.0 runs
into the following:

bfd/bfdio.c: In function ‘_bfd_real_fopen’:
bfd/bfdio.c:125:28: error: implicit declaration of function ‘___lc_codepage_func’ [-Werror=implicit-function-declaration]
   125 |    const unsigned int cp = ___lc_codepage_func();

Re: [PATCH] bfd: handle codepage when opening files on MinGW

[Clément Chigot]

Hi Luis,

> It's been reported to me that this is causing some issues building binutils with mingw.
>
> In fact, trying to build master binutils-gdb with Ubuntu 22.04's mingw-w64 8.0 or Ubuntu 20.04's mingw-w64 7.0.0 runs
> into the following:
>
> bfd/bfdio.c: In function ‘_bfd_real_fopen’:
> bfd/bfdio.c:125:28: error: implicit declaration of function ‘___lc_codepage_func’ [-Werror=implicit-function-declaration]
>    125 |    const unsigned int cp = ___lc_codepage_func();

Looking at the commit exposing this ___lc_codepage_func, it seems to have
been integrated only after 8.0.1 (cf
https://github.com/mirror/mingw-w64/commit/64cb5e8582d9004cfa4d90b189c80c6e1a35d7af)

I don't know if there is any define providing the version of MinGW (I
didn't find it).
Otherwise, I guess it'll have to be checked by the configure.

I won't have time to create a patch for that before next week (probably on
wednesday/thursday). Meanwhile, you should be able to recompile binutils
without this patch. As long as you don't have non-ASCII characters in your
filename, you should be fine.

But thanks for bringing that up !

Clément

Re: [PATCH] bfd: handle codepage when opening files on MinGW

[Luis Machado]

On 8/12/22 16:11, Clément Chigot wrote:
> Hi Luis,
> 
>> It's been reported to me that this is causing some issues building binutils with mingw.
>>
>> In fact, trying to build master binutils-gdb with Ubuntu 22.04's mingw-w64 8.0 or Ubuntu 20.04's mingw-w64 7.0.0 runs
>> into the following:
>>
>> bfd/bfdio.c: In function ‘_bfd_real_fopen’:
>> bfd/bfdio.c:125:28: error: implicit declaration of function ‘___lc_codepage_func’ [-Werror=implicit-function-declaration]
>>     125 |    const unsigned int cp = ___lc_codepage_func();
> 
> Looking at the commit exposing this ___lc_codepage_func, it seems to have
> been integrated only after 8.0.1 (cf
> https://github.com/mirror/mingw-w64/commit/64cb5e8582d9004cfa4d90b189c80c6e1a35d7af)
> 
> I don't know if there is any define providing the version of MinGW (I
> didn't find it).
> Otherwise, I guess it'll have to be checked by the configure.
> 
> I won't have time to create a patch for that before next week (probably on
> wednesday/thursday). Meanwhile, you should be able to recompile binutils
> without this patch. As long as you don't have non-ASCII characters in your
> filename, you should be fine.
> 
> But thanks for bringing that up !
> 
> Clément

No worries. I just thought I'd mention it in case more people are seeing this. We can address this
later.

Re: [PATCH] bfd: handle codepage when opening files on MinGW

[Nick Clifton]

Hi Luis,

> In fact, trying to build master binutils-gdb with Ubuntu 22.04's mingw-w64 8.0 or Ubuntu 20.04's mingw-w64 7.0.0 runs
> into the following:
> 
> bfd/bfdio.c: In function ‘_bfd_real_fopen’:
> bfd/bfdio.c:125:28: error: implicit declaration of function ‘___lc_codepage_func’ [-Werror=implicit-function-declaration]
>    125 |    const unsigned int cp = ___lc_codepage_func();

So that would imply a missing #include of a system header, yes ?

Do you know which header provides the required prototype ?

The code is currently conditional upon __MINGW32__ being defined.  Perhaps
that check needs to be extended/changed to reference a different preprocessor
symbol ?

Cheers
   Nick

Re: [PATCH] bfd: handle codepage when opening files on MinGW

[Torbjorn SVENSSON]

Hello Nick,

On 2022-08-15 13:07, Nick Clifton wrote:
> Hi Luis,
> 
>> In fact, trying to build master binutils-gdb with Ubuntu 22.04's 
>> mingw-w64 8.0 or Ubuntu 20.04's mingw-w64 7.0.0 runs
>> into the following:
>>
>> bfd/bfdio.c: In function ‘_bfd_real_fopen’:
>> bfd/bfdio.c:125:28: error: implicit declaration of function 
>> ‘___lc_codepage_func’ [-Werror=implicit-function-declaration]
>>    125 |    const unsigned int cp = ___lc_codepage_func();
> 
> So that would imply a missing #include of a system header, yes ?
> 
> Do you know which header provides the required prototype ?
> 
> The code is currently conditional upon __MINGW32__ being defined.  Perhaps
> that check needs to be extended/changed to reference a different 
> preprocessor
> symbol ?

The problem is that the function is not exposed in the MinGW headers 
until the 9.0 release.
I sent a patch that fixes the build error 
https://sourceware.org/pipermail/binutils/2022-August/122423.html 
earlier today (with you on CC).

> 
> Cheers
>    Nick
> 
> 

Re: [PATCH] bfd: handle codepage when opening files on MinGW

[Luis Machado]

Hi Nick,

On 8/15/22 12:38, Torbjorn SVENSSON wrote:
> Hello Nick,
> 
> On 2022-08-15 13:07, Nick Clifton wrote:
>> Hi Luis,
>>
>>> In fact, trying to build master binutils-gdb with Ubuntu 22.04's mingw-w64 8.0 or Ubuntu 20.04's mingw-w64 7.0.0 runs
>>> into the following:
>>>
>>> bfd/bfdio.c: In function ‘_bfd_real_fopen’:
>>> bfd/bfdio.c:125:28: error: implicit declaration of function ‘___lc_codepage_func’ [-Werror=implicit-function-declaration]
>>>    125 |    const unsigned int cp = ___lc_codepage_func();
>>
>> So that would imply a missing #include of a system header, yes ?
>>
>> Do you know which header provides the required prototype ?
>>
>> The code is currently conditional upon __MINGW32__ being defined.  Perhaps
>> that check needs to be extended/changed to reference a different preprocessor
>> symbol ?
> 
> The problem is that the function is not exposed in the MinGW headers until the 9.0 release.
> I sent a patch that fixes the build error https://sourceware.org/pipermail/binutils/2022-August/122423.html earlier today (with you on CC).
> 
>>
>> Cheers
>>    Nick
>>
>>

What Torbjörn said. We need to restrict things based on the version of MinGW.

His patch looks good to me. I just wasn't sure about the _CRTIMP in his patch.