[PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

[PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

[Igor Kudrin]

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

Hi,

This patch adds support for the --build-id=uuid option on Windows
when ld is build in MSYS2 (MinGW) environment. It uses function
UuidCreate() from rpcrt4.dll instead of reading from /dev/urandom,
because the latter is not random enough in that case.

BTW, without the patch, using this option leads to AV.

Best regards,
Igor Kudrin

---
ld/ChangeLog

	* ldbuildid.c [__MINGW32__]: Include windows.h and rpcdce.h.
	(validate_build_id_style) [__MINGW32__]: Allow "uuid" style.
	(generate_build_id) [__MINGW32__]: Fill id_bits using UuidCreate().

[-- Attachment #2: ld-build-id-uuid.patch.txt --]
[-- Type: text/plain, Size: 1826 bytes --]

diff --git a/ld/ldbuildid.c b/ld/ldbuildid.c
index 7d4b9c3..32b7179 100644
--- a/ld/ldbuildid.c
+++ b/ld/ldbuildid.c
@@ -24,6 +24,10 @@
 #include "md5.h"
 #include "sha1.h"
 #include "ldbuildid.h"
+#ifdef __MINGW32__
+#include <windows.h>
+#include <rpcdce.h>
+#endif
 
 #define streq(a,b)     strcmp ((a), (b)) == 0
 #define strneq(a,b,n)  strncmp ((a), (b), (n)) == 0
@@ -32,10 +36,7 @@ bfd_boolean
 validate_build_id_style (const char *style)
 {
  if ((streq (style, "md5")) || (streq (style, "sha1"))
-#ifndef __MINGW32__
-     || (streq (style, "uuid"))
-#endif
-     || (strneq (style, "0x", 2)))
+     || (streq (style, "uuid")) || (strneq (style, "0x", 2)))
    return TRUE;
 
  return FALSE;
@@ -118,9 +119,9 @@ generate_build_id (bfd *abfd,
 	return FALSE;
       sha1_finish_ctx (&ctx, id_bits);
     }
-#ifndef __MINGW32__
   else if (streq (style, "uuid"))
     {
+#ifndef __MINGW32__
       int n;
       int fd = open ("/dev/urandom", O_RDONLY);
 
@@ -130,8 +131,30 @@ generate_build_id (bfd *abfd,
       close (fd);
       if (n < size)
 	return FALSE;
+#else /* __MINGW32__ */
+      UUID uuid;
+      typedef RPC_STATUS (RPC_ENTRY *UuidCreateFn)(UUID *Uuid);
+      UuidCreateFn uuid_create = 0;
+
+      HMODULE rpc_library = LoadLibrary("rpcrt4.dll");
+      if (!rpc_library)
+	return FALSE;
+      uuid_create = (UuidCreateFn)GetProcAddress(rpc_library, "UuidCreate");
+      if (!uuid_create)
+	{
+	  FreeLibrary(rpc_library);
+	  return FALSE;
+	}
+
+      if (uuid_create(&uuid) != RPC_S_OK)
+	{
+	  FreeLibrary(rpc_library);
+	  return FALSE;
+	}
+      FreeLibrary(rpc_library);
+      memcpy(id_bits, &uuid, size < sizeof(UUID) ? size : sizeof(UUID));
+#endif /* __MINGW32__ */
     }
-#endif
   else if (strneq (style, "0x", 2))
     {
       /* ID is in string form (hex).  Convert to bits.  */

Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

[Alan Modra]

On Mon, Jul 25, 2016 at 10:15:34AM +0000, Igor Kudrin wrote:
> +#else /* __MINGW32__ */
> +      UUID uuid;
> +      typedef RPC_STATUS (RPC_ENTRY *UuidCreateFn)(UUID *Uuid);

Formatting.  Space before open parens of function args.
      typedef RPC_STATUS (RPC_ENTRY *UuidCreateFn) (UUID *Uuid);

> +      UuidCreateFn uuid_create = 0;
> +
> +      HMODULE rpc_library = LoadLibrary("rpcrt4.dll");

Same.
      HMODULE rpc_library = LoadLibrary ("rpcrt4.dll");

> +      if (!rpc_library)
> +	return FALSE;
> +      uuid_create = (UuidCreateFn)GetProcAddress(rpc_library, "UuidCreate");

Again, and space after cast.
      uuid_create = (UuidCreateFn) GetProcAddress (rpc_library, "UuidCreate");
and more below.

> +      if (!uuid_create)
> +	{
> +	  FreeLibrary(rpc_library);
> +	  return FALSE;
> +	}
> +
> +      if (uuid_create(&uuid) != RPC_S_OK)
> +	{
> +	  FreeLibrary(rpc_library);
> +	  return FALSE;
> +	}
> +      FreeLibrary(rpc_library);
> +      memcpy(id_bits, &uuid, size < sizeof(UUID) ? size : sizeof(UUID));
> +#endif /* __MINGW32__ */

Other than the formatting issues, I really don't have enough
familiarity with mingw to properly review the patch.  So I'm passing
the buck to Nick.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

[Igor Kudrin]

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

Hi Alan,

Thank you for the comments.
Sorry about the formatting issues, here is an updated version of the patch.

By the way, is there any formatting tool or official settings for tools like
astyle or uncrustify?

Best regards,
Igor Kudrin
________________________________________
From: Alan Modra <amodra@gmail.com>
Sent: Tuesday, July 26, 2016 7:00 PM
To: Igor Kudrin
Cc: binutils@sourceware.org; Nick Clifton
Subject: Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

On Mon, Jul 25, 2016 at 10:15:34AM +0000, Igor Kudrin wrote:
> +#else /* __MINGW32__ */
> +      UUID uuid;
> +      typedef RPC_STATUS (RPC_ENTRY *UuidCreateFn)(UUID *Uuid);

Formatting.  Space before open parens of function args.
      typedef RPC_STATUS (RPC_ENTRY *UuidCreateFn) (UUID *Uuid);

> +      UuidCreateFn uuid_create = 0;
> +
> +      HMODULE rpc_library = LoadLibrary("rpcrt4.dll");

Same.
      HMODULE rpc_library = LoadLibrary ("rpcrt4.dll");

> +      if (!rpc_library)
> +     return FALSE;
> +      uuid_create = (UuidCreateFn)GetProcAddress(rpc_library, "UuidCreate");

Again, and space after cast.
      uuid_create = (UuidCreateFn) GetProcAddress (rpc_library, "UuidCreate");
and more below.

> +      if (!uuid_create)
> +     {
> +       FreeLibrary(rpc_library);
> +       return FALSE;
> +     }
> +
> +      if (uuid_create(&uuid) != RPC_S_OK)
> +     {
> +       FreeLibrary(rpc_library);
> +       return FALSE;
> +     }
> +      FreeLibrary(rpc_library);
> +      memcpy(id_bits, &uuid, size < sizeof(UUID) ? size : sizeof(UUID));
> +#endif /* __MINGW32__ */

Other than the formatting issues, I really don't have enough
familiarity with mingw to properly review the patch.  So I'm passing
the buck to Nick.

--
Alan Modra
Australia Development Lab, IBM

[-- Attachment #2: ld-build-id-uuid-2.patch.txt --]
[-- Type: text/plain, Size: 1837 bytes --]

diff --git a/ld/ldbuildid.c b/ld/ldbuildid.c
index 7d4b9c3..7417503 100644
--- a/ld/ldbuildid.c
+++ b/ld/ldbuildid.c
@@ -24,6 +24,10 @@
 #include "md5.h"
 #include "sha1.h"
 #include "ldbuildid.h"
+#ifdef __MINGW32__
+#include <windows.h>
+#include <rpcdce.h>
+#endif
 
 #define streq(a,b)     strcmp ((a), (b)) == 0
 #define strneq(a,b,n)  strncmp ((a), (b), (n)) == 0
@@ -32,10 +36,7 @@ bfd_boolean
 validate_build_id_style (const char *style)
 {
  if ((streq (style, "md5")) || (streq (style, "sha1"))
-#ifndef __MINGW32__
-     || (streq (style, "uuid"))
-#endif
-     || (strneq (style, "0x", 2)))
+     || (streq (style, "uuid")) || (strneq (style, "0x", 2)))
    return TRUE;
 
  return FALSE;
@@ -118,9 +119,9 @@ generate_build_id (bfd *abfd,
 	return FALSE;
       sha1_finish_ctx (&ctx, id_bits);
     }
-#ifndef __MINGW32__
   else if (streq (style, "uuid"))
     {
+#ifndef __MINGW32__
       int n;
       int fd = open ("/dev/urandom", O_RDONLY);
 
@@ -130,8 +131,30 @@ generate_build_id (bfd *abfd,
       close (fd);
       if (n < size)
 	return FALSE;
+#else /* __MINGW32__ */
+      UUID uuid;
+      typedef RPC_STATUS (RPC_ENTRY *UuidCreateFn) (UUID *Uuid);
+      UuidCreateFn uuid_create = 0;
+
+      HMODULE rpc_library = LoadLibrary ("rpcrt4.dll");
+      if (!rpc_library)
+	return FALSE;
+      uuid_create = (UuidCreateFn) GetProcAddress (rpc_library, "UuidCreate");
+      if (!uuid_create)
+	{
+	  FreeLibrary (rpc_library);
+	  return FALSE;
+	}
+
+      if (uuid_create (&uuid) != RPC_S_OK)
+	{
+	  FreeLibrary (rpc_library);
+	  return FALSE;
+	}
+      FreeLibrary (rpc_library);
+      memcpy (id_bits, &uuid, size < sizeof (UUID) ? size : sizeof (UUID));
+#endif /* __MINGW32__ */
     }
-#endif
   else if (strneq (style, "0x", 2))
     {
       /* ID is in string form (hex).  Convert to bits.  */

Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

[Nick Clifton]

Hi Igor,

> By the way, is there any formatting tool or official settings for tools like
> astyle or uncrustify?

Not as far as I know, but is mainly because I have not heard of those tools.

There is a formatting for mode Emacs which does a good job, should you happen
to use that editor.


> ld/ChangeLog
> 
> 	* ldbuildid.c [__MINGW32__]: Include windows.h and rpcdce.h.
> 	(validate_build_id_style) [__MINGW32__]: Allow "uuid" style.
> 	(generate_build_id) [__MINGW32__]: Fill id_bits using UuidCreate().

Approved and applied - thanks for contributing this patch.

Cheers
  Nick

Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

[Thomas Preudhomme]

Hi Igor,

Your commit d0d4152fa5c87532bf05007def680b5a536e1827 for this patch fails to 
build because of the following warnings treated as errors:

src/binutils/ld/ldbuildid.c:155:36: error: comparison between signed and 
unsigned integer expressions [-Werror=sign-compare]
src/binutils/ld/ldbuildid.c:155:59: error: signed and unsigned type in 
conditional expression [-Werror=sign-compare]

Indeed, size is signed while sizeof is unsigned.

Best regards,

Thomas


On Tuesday 26 July 2016 13:31:37 Igor Kudrin wrote:
> Hi Alan,
> 
> Thank you for the comments.
> Sorry about the formatting issues, here is an updated version of the patch.
> 
> By the way, is there any formatting tool or official settings for tools like
> astyle or uncrustify?
> 
> Best regards,
> Igor Kudrin
> ________________________________________
> From: Alan Modra <amodra@gmail.com>
> Sent: Tuesday, July 26, 2016 7:00 PM
> To: Igor Kudrin
> Cc: binutils@sourceware.org; Nick Clifton
> Subject: Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.
> 
> On Mon, Jul 25, 2016 at 10:15:34AM +0000, Igor Kudrin wrote:
> > +#else /* __MINGW32__ */
> > +      UUID uuid;
> > +      typedef RPC_STATUS (RPC_ENTRY *UuidCreateFn)(UUID *Uuid);
> 
> Formatting.  Space before open parens of function args.
>       typedef RPC_STATUS (RPC_ENTRY *UuidCreateFn) (UUID *Uuid);
> 
> > +      UuidCreateFn uuid_create = 0;
> > +
> > +      HMODULE rpc_library = LoadLibrary("rpcrt4.dll");
> 
> Same.
>       HMODULE rpc_library = LoadLibrary ("rpcrt4.dll");
> 
> > +      if (!rpc_library)
> > +     return FALSE;
> > +      uuid_create = (UuidCreateFn)GetProcAddress(rpc_library,
> > "UuidCreate");
> Again, and space after cast.
>       uuid_create = (UuidCreateFn) GetProcAddress (rpc_library,
> "UuidCreate"); and more below.
> 
> > +      if (!uuid_create)
> > +     {
> > +       FreeLibrary(rpc_library);
> > +       return FALSE;
> > +     }
> > +
> > +      if (uuid_create(&uuid) != RPC_S_OK)
> > +     {
> > +       FreeLibrary(rpc_library);
> > +       return FALSE;
> > +     }
> > +      FreeLibrary(rpc_library);
> > +      memcpy(id_bits, &uuid, size < sizeof(UUID) ? size : sizeof(UUID));
> > +#endif /* __MINGW32__ */
> 
> Other than the formatting issues, I really don't have enough
> familiarity with mingw to properly review the patch.  So I'm passing
> the buck to Nick.
> 
> --
> Alan Modra
> Australia Development Lab, IBM

Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

[Igor Kudrin]

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

Hi Thomas,

Thank you and sorry about that.
Here is the patch to fix the warnings, but I cannot apply it myself.

Best regards,
Igor Kudrin

---
ld/ChangeLog

	* ldbuildid.c (generate_build_id): Fix warning.

________________________________________
From: binutils-owner@sourceware.org <binutils-owner@sourceware.org> on behalf of Thomas Preudhomme <thomas.preudhomme@foss.arm.com>
Sent: Wednesday, July 27, 2016 2:40 PM
To: Igor Kudrin
Cc: binutils@sourceware.org; Alan Modra; Nick Clifton
Subject: Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

Hi Igor,

Your commit d0d4152fa5c87532bf05007def680b5a536e1827 for this patch fails to
build because of the following warnings treated as errors:

src/binutils/ld/ldbuildid.c:155:36: error: comparison between signed and
unsigned integer expressions [-Werror=sign-compare]
src/binutils/ld/ldbuildid.c:155:59: error: signed and unsigned type in
conditional expression [-Werror=sign-compare]

Indeed, size is signed while sizeof is unsigned.

Best regards,

Thomas


On Tuesday 26 July 2016 13:31:37 Igor Kudrin wrote:
> Hi Alan,
>
> Thank you for the comments.
> Sorry about the formatting issues, here is an updated version of the patch.
>
> By the way, is there any formatting tool or official settings for tools like
> astyle or uncrustify?
>
> Best regards,
> Igor Kudrin
> ________________________________________
> From: Alan Modra <amodra@gmail.com>
> Sent: Tuesday, July 26, 2016 7:00 PM
> To: Igor Kudrin
> Cc: binutils@sourceware.org; Nick Clifton
> Subject: Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.
>
> On Mon, Jul 25, 2016 at 10:15:34AM +0000, Igor Kudrin wrote:
> > +#else /* __MINGW32__ */
> > +      UUID uuid;
> > +      typedef RPC_STATUS (RPC_ENTRY *UuidCreateFn)(UUID *Uuid);
>
> Formatting.  Space before open parens of function args.
>       typedef RPC_STATUS (RPC_ENTRY *UuidCreateFn) (UUID *Uuid);
>
> > +      UuidCreateFn uuid_create = 0;
> > +
> > +      HMODULE rpc_library = LoadLibrary("rpcrt4.dll");
>
> Same.
>       HMODULE rpc_library = LoadLibrary ("rpcrt4.dll");
>
> > +      if (!rpc_library)
> > +     return FALSE;
> > +      uuid_create = (UuidCreateFn)GetProcAddress(rpc_library,
> > "UuidCreate");
> Again, and space after cast.
>       uuid_create = (UuidCreateFn) GetProcAddress (rpc_library,
> "UuidCreate"); and more below.
>
> > +      if (!uuid_create)
> > +     {
> > +       FreeLibrary(rpc_library);
> > +       return FALSE;
> > +     }
> > +
> > +      if (uuid_create(&uuid) != RPC_S_OK)
> > +     {
> > +       FreeLibrary(rpc_library);
> > +       return FALSE;
> > +     }
> > +      FreeLibrary(rpc_library);
> > +      memcpy(id_bits, &uuid, size < sizeof(UUID) ? size : sizeof(UUID));
> > +#endif /* __MINGW32__ */
>
> Other than the formatting issues, I really don't have enough
> familiarity with mingw to properly review the patch.  So I'm passing
> the buck to Nick.
>
> --
> Alan Modra
> Australia Development Lab, IBM


[-- Attachment #2: ld-build-id-uuid-warn.patch.txt --]
[-- Type: text/plain, Size: 473 bytes --]

diff --git a/ld/ldbuildid.c b/ld/ldbuildid.c
index d2dccc5..76a0b9e 100644
--- a/ld/ldbuildid.c
+++ b/ld/ldbuildid.c
@@ -152,7 +152,8 @@ generate_build_id (bfd *abfd,
 	  return FALSE;
 	}
       FreeLibrary (rpc_library);
-      memcpy (id_bits, &uuid, size < sizeof (UUID) ? size : sizeof (UUID));
+      memcpy (id_bits, &uuid,
+	      (size_t) size < sizeof (UUID) ? (size_t) size : sizeof (UUID));
 #endif /* __MINGW32__ */
     }
   else if (strneq (style, "0x", 2))

Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

[Alan Modra]

On Wed, Jul 27, 2016 at 09:13:23AM +0000, Igor Kudrin wrote:
> 	* ldbuildid.c (generate_build_id): Fix warning.

Thanks, committed.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH][ld] Implement --build-id=uuid on MinGW/MSYS2.

[Thomas Preudhomme]

On Wednesday 27 July 2016 19:04:46 Alan Modra wrote:
> On Wed, Jul 27, 2016 at 09:13:23AM +0000, Igor Kudrin wrote:
> > 	* ldbuildid.c (generate_build_id): Fix warning.
> 
> Thanks, committed.

Mingw build is fixed indeed. Thank you both.

Best regards,

Thomas