public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* RISCV changes broke 32-bit --enable-targets=all
@ 2020-06-26  1:34 Alan Modra
  2020-06-26  4:08 ` Nelson Chu
  2020-07-11 20:03 ` Palmer Dabbelt
  0 siblings, 2 replies; 6+ messages in thread
From: Alan Modra @ 2020-06-26  1:34 UTC (permalink / raw)
  To: binutils

By the look of it, git commit 39ff0b812324 broke 32-bit host
--enable-targets=all binutils builds.

/usr/local/bin/ld: ../opcodes/.libs/libopcodes.a(riscv-dis.o): in function `parse_riscv_dis_option':
/home/alan/src/binutils-gdb/opcodes/riscv-dis.c:102: undefined reference to `riscv_get_priv_spec_class'
collect2: error: ld returned 1 exit status
Makefile:925: recipe for target 'objdump' failed

The problem is that elfxx-riscv.c is not built for a 32-bit host
without --enable-64-bit-bfd or unless RISCV is given specifically as a
target.  No such trimming of 64-bit only targets is done in opcodes.

One solution is to move these support functions to cpu-riscv.c, which
runs into "error: implicit declaration of function ‘xmalloc’".  Now,
xmalloc is not supposed to be used in libbfd or libopcodes - it's rude
to crash out of an application that calls libbfd or libopcodes
functions without giving it a chance to deal with out-of-memory
itself.  So I removed the xmalloc and instead used a fixed size
buffer.  If you are worried about adding 36 bytes for the buffer to
the riscv_get_priv_spec_class_from_numbers stack frame size, then you
have no idea of the likely xmalloc + malloc stack frame size!  Trying
to reduce memory usage is commendable, but in this instance
riscv_estimate_digit and malloc for a temp buffer uses a lot more
memory than a fixed max-size buffer.

	* elfxx-riscv.c (struct priv_spec_t, priv_specs),
	(riscv_get_priv_spec_class, riscv_get_priv_spec_class_from_numbers),
	(riscv_get_priv_spec_name): Move to..
	* cpu-riscv.c: ..here.
	(riscv_get_priv_spec_class_from_numbers): Don't xmalloc temp buffer.
	Use %u to print unsigned numbers.

diff --git a/bfd/cpu-riscv.c b/bfd/cpu-riscv.c
index 22067ab29b..2e9e9eb9d2 100644
--- a/bfd/cpu-riscv.c
+++ b/bfd/cpu-riscv.c
@@ -23,6 +23,86 @@
 #include "sysdep.h"
 #include "bfd.h"
 #include "libbfd.h"
+#include "elfxx-riscv.h"
+
+/* Record the priv spec version string and the corresponding class.  */
+
+struct priv_spec_t
+{
+  const char *name;
+  enum riscv_priv_spec_class class;
+};
+
+/* List for all supported privilege versions.  */
+
+static const struct priv_spec_t priv_specs[] =
+{
+  {"1.9.1", PRIV_SPEC_CLASS_1P9P1},
+  {"1.10",  PRIV_SPEC_CLASS_1P10},
+  {"1.11",  PRIV_SPEC_CLASS_1P11},
+
+/* Terminate the list.  */
+  {NULL, 0}
+};
+
+/* Get the corresponding CSR version class by giving a privilege
+   version string.  */
+
+int
+riscv_get_priv_spec_class (const char *s,
+			   enum riscv_priv_spec_class *class)
+{
+  const struct priv_spec_t *version;
+
+  if (s == NULL)
+    return 0;
+
+  for (version = &priv_specs[0]; version->name != NULL; ++version)
+    if (strcmp (version->name, s) == 0)
+      {
+	*class = version->class;
+	return 1;
+      }
+
+  /* Can not find the supported privilege version.  */
+  return 0;
+}
+
+/* Get the corresponding CSR version class by giving privilege
+   version numbers.  It is usually used to convert the priv
+   attribute numbers into the corresponding class.  */
+
+int
+riscv_get_priv_spec_class_from_numbers (unsigned int major,
+					unsigned int minor,
+					unsigned int revision,
+					enum riscv_priv_spec_class *class)
+{
+  char buf[36];
+
+  if (major == 0 && minor == 0 && revision == 0)
+    {
+      *class = PRIV_SPEC_CLASS_NONE;
+      return 1;
+    }
+
+  if (revision != 0)
+    snprintf (buf, sizeof (buf), "%u.%u.%u", major, minor, revision);
+  else
+    snprintf (buf, sizeof (buf), "%u.%u", major, minor);
+
+  return riscv_get_priv_spec_class (buf, class);
+}
+
+/* Get the corresponding privilege version string by giving a CSR
+   version class.  */
+
+const char *
+riscv_get_priv_spec_name (enum riscv_priv_spec_class class)
+{
+  /* The first enum is PRIV_SPEC_CLASS_NONE.  */
+  return priv_specs[class - 1].name;
+}
 
 /* This routine is provided two arch_infos and returns an arch_info
    that is compatible with both, or NULL if none exists.  */
diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index fa46b06f8d..1570f1d862 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -1749,98 +1749,3 @@ riscv_arch_str (unsigned xlen, const riscv_subset_list_t *subset)
 
   return attr_str;
 }
-
-/* Record the priv spec version string and the corresponding class.  */
-
-struct priv_spec_t
-{
-  const char *name;
-  enum riscv_priv_spec_class class;
-};
-
-/* List for all supported privilege versions.  */
-
-static const struct priv_spec_t priv_specs[] =
-{
-  {"1.9.1", PRIV_SPEC_CLASS_1P9P1},
-  {"1.10",  PRIV_SPEC_CLASS_1P10},
-  {"1.11",  PRIV_SPEC_CLASS_1P11},
-
-/* Terminate the list.  */
-  {NULL, 0}
-};
-
-/* Get the corresponding CSR version class by giving a privilege
-   version string.  */
-
-int
-riscv_get_priv_spec_class (const char *s,
-			   enum riscv_priv_spec_class *class)
-{
-  const struct priv_spec_t *version;
-
-  if (s == NULL)
-    return 0;
-
-  for (version = &priv_specs[0]; version->name != NULL; ++version)
-    if (strcmp (version->name, s) == 0)
-      {
-	*class = version->class;
-	return 1;
-      }
-
-  /* Can not find the supported privilege version.  */
-  return 0;
-}
-
-/* Get the corresponding CSR version class by giving privilege
-   version numbers.  It is usually used to convert the priv
-   attribute numbers into the corresponding class.  */
-
-int
-riscv_get_priv_spec_class_from_numbers (unsigned int major,
-					unsigned int minor,
-					unsigned int revision,
-					enum riscv_priv_spec_class *class)
-{
-  size_t buf_size;
-  char *buf;
-  int result = 1;
-
-  if (major == 0 && minor == 0 && revision == 0)
-    {
-      *class = PRIV_SPEC_CLASS_NONE;
-      return result;
-    }
-
-  buf_size = riscv_estimate_digit (major)
-	     + 1 /* '.' */
-	     + riscv_estimate_digit (minor)
-	     + 1; /* string terminator */
-  if (revision != 0)
-    {
-      buf_size += 1 /* '.' */
-		  + riscv_estimate_digit (revision);
-      buf = xmalloc (buf_size);
-      snprintf (buf, buf_size, "%d.%d.%d", major, minor, revision);
-    }
-  else
-    {
-      buf = xmalloc (buf_size);
-      snprintf (buf, buf_size, "%d.%d", major, minor);
-    }
-
-  result = riscv_get_priv_spec_class (buf, class);
-  free (buf);
-  return result;
-}
-
-/* Get the corresponding privilege version string by giving a CSR
-   version class.  */
-
-const char *
-riscv_get_priv_spec_name (enum riscv_priv_spec_class class)
-{
-  /* The first enum is PRIV_SPEC_CLASS_NONE.  */
-  return priv_specs[class - 1].name;
-}

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RISCV changes broke 32-bit --enable-targets=all
  2020-06-26  1:34 RISCV changes broke 32-bit --enable-targets=all Alan Modra
@ 2020-06-26  4:08 ` Nelson Chu
  2020-07-11 20:03 ` Palmer Dabbelt
  1 sibling, 0 replies; 6+ messages in thread
From: Nelson Chu @ 2020-06-26  4:08 UTC (permalink / raw)
  To: Alan Modra; +Cc: Binutils

Hi Alan,

Thank you very much for fixing this.

Nelson

On Fri, Jun 26, 2020 at 9:34 AM Alan Modra via Binutils
<binutils@sourceware.org> wrote:
>
> By the look of it, git commit 39ff0b812324 broke 32-bit host
> --enable-targets=all binutils builds.
>
> /usr/local/bin/ld: ../opcodes/.libs/libopcodes.a(riscv-dis.o): in function `parse_riscv_dis_option':
> /home/alan/src/binutils-gdb/opcodes/riscv-dis.c:102: undefined reference to `riscv_get_priv_spec_class'
> collect2: error: ld returned 1 exit status
> Makefile:925: recipe for target 'objdump' failed
>
> The problem is that elfxx-riscv.c is not built for a 32-bit host
> without --enable-64-bit-bfd or unless RISCV is given specifically as a
> target.  No such trimming of 64-bit only targets is done in opcodes.
>
> One solution is to move these support functions to cpu-riscv.c, which
> runs into "error: implicit declaration of function ‘xmalloc’".  Now,
> xmalloc is not supposed to be used in libbfd or libopcodes - it's rude
> to crash out of an application that calls libbfd or libopcodes
> functions without giving it a chance to deal with out-of-memory
> itself.  So I removed the xmalloc and instead used a fixed size
> buffer.  If you are worried about adding 36 bytes for the buffer to
> the riscv_get_priv_spec_class_from_numbers stack frame size, then you
> have no idea of the likely xmalloc + malloc stack frame size!  Trying
> to reduce memory usage is commendable, but in this instance
> riscv_estimate_digit and malloc for a temp buffer uses a lot more
> memory than a fixed max-size buffer.
>
>         * elfxx-riscv.c (struct priv_spec_t, priv_specs),
>         (riscv_get_priv_spec_class, riscv_get_priv_spec_class_from_numbers),
>         (riscv_get_priv_spec_name): Move to..
>         * cpu-riscv.c: ..here.
>         (riscv_get_priv_spec_class_from_numbers): Don't xmalloc temp buffer.
>         Use %u to print unsigned numbers.
>
> diff --git a/bfd/cpu-riscv.c b/bfd/cpu-riscv.c
> index 22067ab29b..2e9e9eb9d2 100644
> --- a/bfd/cpu-riscv.c
> +++ b/bfd/cpu-riscv.c
> @@ -23,6 +23,86 @@
>  #include "sysdep.h"
>  #include "bfd.h"
>  #include "libbfd.h"
> +#include "elfxx-riscv.h"
> +
> +/* Record the priv spec version string and the corresponding class.  */
> +
> +struct priv_spec_t
> +{
> +  const char *name;
> +  enum riscv_priv_spec_class class;
> +};
> +
> +/* List for all supported privilege versions.  */
> +
> +static const struct priv_spec_t priv_specs[] =
> +{
> +  {"1.9.1", PRIV_SPEC_CLASS_1P9P1},
> +  {"1.10",  PRIV_SPEC_CLASS_1P10},
> +  {"1.11",  PRIV_SPEC_CLASS_1P11},
> +
> +/* Terminate the list.  */
> +  {NULL, 0}
> +};
> +
> +/* Get the corresponding CSR version class by giving a privilege
> +   version string.  */
> +
> +int
> +riscv_get_priv_spec_class (const char *s,
> +                          enum riscv_priv_spec_class *class)
> +{
> +  const struct priv_spec_t *version;
> +
> +  if (s == NULL)
> +    return 0;
> +
> +  for (version = &priv_specs[0]; version->name != NULL; ++version)
> +    if (strcmp (version->name, s) == 0)
> +      {
> +       *class = version->class;
> +       return 1;
> +      }
> +
> +  /* Can not find the supported privilege version.  */
> +  return 0;
> +}
> +
> +/* Get the corresponding CSR version class by giving privilege
> +   version numbers.  It is usually used to convert the priv
> +   attribute numbers into the corresponding class.  */
> +
> +int
> +riscv_get_priv_spec_class_from_numbers (unsigned int major,
> +                                       unsigned int minor,
> +                                       unsigned int revision,
> +                                       enum riscv_priv_spec_class *class)
> +{
> +  char buf[36];
> +
> +  if (major == 0 && minor == 0 && revision == 0)
> +    {
> +      *class = PRIV_SPEC_CLASS_NONE;
> +      return 1;
> +    }
> +
> +  if (revision != 0)
> +    snprintf (buf, sizeof (buf), "%u.%u.%u", major, minor, revision);
> +  else
> +    snprintf (buf, sizeof (buf), "%u.%u", major, minor);
> +
> +  return riscv_get_priv_spec_class (buf, class);
> +}
> +
> +/* Get the corresponding privilege version string by giving a CSR
> +   version class.  */
> +
> +const char *
> +riscv_get_priv_spec_name (enum riscv_priv_spec_class class)
> +{
> +  /* The first enum is PRIV_SPEC_CLASS_NONE.  */
> +  return priv_specs[class - 1].name;
> +}
>
>  /* This routine is provided two arch_infos and returns an arch_info
>     that is compatible with both, or NULL if none exists.  */
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index fa46b06f8d..1570f1d862 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1749,98 +1749,3 @@ riscv_arch_str (unsigned xlen, const riscv_subset_list_t *subset)
>
>    return attr_str;
>  }
> -
> -/* Record the priv spec version string and the corresponding class.  */
> -
> -struct priv_spec_t
> -{
> -  const char *name;
> -  enum riscv_priv_spec_class class;
> -};
> -
> -/* List for all supported privilege versions.  */
> -
> -static const struct priv_spec_t priv_specs[] =
> -{
> -  {"1.9.1", PRIV_SPEC_CLASS_1P9P1},
> -  {"1.10",  PRIV_SPEC_CLASS_1P10},
> -  {"1.11",  PRIV_SPEC_CLASS_1P11},
> -
> -/* Terminate the list.  */
> -  {NULL, 0}
> -};
> -
> -/* Get the corresponding CSR version class by giving a privilege
> -   version string.  */
> -
> -int
> -riscv_get_priv_spec_class (const char *s,
> -                          enum riscv_priv_spec_class *class)
> -{
> -  const struct priv_spec_t *version;
> -
> -  if (s == NULL)
> -    return 0;
> -
> -  for (version = &priv_specs[0]; version->name != NULL; ++version)
> -    if (strcmp (version->name, s) == 0)
> -      {
> -       *class = version->class;
> -       return 1;
> -      }
> -
> -  /* Can not find the supported privilege version.  */
> -  return 0;
> -}
> -
> -/* Get the corresponding CSR version class by giving privilege
> -   version numbers.  It is usually used to convert the priv
> -   attribute numbers into the corresponding class.  */
> -
> -int
> -riscv_get_priv_spec_class_from_numbers (unsigned int major,
> -                                       unsigned int minor,
> -                                       unsigned int revision,
> -                                       enum riscv_priv_spec_class *class)
> -{
> -  size_t buf_size;
> -  char *buf;
> -  int result = 1;
> -
> -  if (major == 0 && minor == 0 && revision == 0)
> -    {
> -      *class = PRIV_SPEC_CLASS_NONE;
> -      return result;
> -    }
> -
> -  buf_size = riscv_estimate_digit (major)
> -            + 1 /* '.' */
> -            + riscv_estimate_digit (minor)
> -            + 1; /* string terminator */
> -  if (revision != 0)
> -    {
> -      buf_size += 1 /* '.' */
> -                 + riscv_estimate_digit (revision);
> -      buf = xmalloc (buf_size);
> -      snprintf (buf, buf_size, "%d.%d.%d", major, minor, revision);
> -    }
> -  else
> -    {
> -      buf = xmalloc (buf_size);
> -      snprintf (buf, buf_size, "%d.%d", major, minor);
> -    }
> -
> -  result = riscv_get_priv_spec_class (buf, class);
> -  free (buf);
> -  return result;
> -}
> -
> -/* Get the corresponding privilege version string by giving a CSR
> -   version class.  */
> -
> -const char *
> -riscv_get_priv_spec_name (enum riscv_priv_spec_class class)
> -{
> -  /* The first enum is PRIV_SPEC_CLASS_NONE.  */
> -  return priv_specs[class - 1].name;
> -}
>
> --
> Alan Modra
> Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RISCV changes broke 32-bit --enable-targets=all
  2020-06-26  1:34 RISCV changes broke 32-bit --enable-targets=all Alan Modra
  2020-06-26  4:08 ` Nelson Chu
@ 2020-07-11 20:03 ` Palmer Dabbelt
  2020-07-31 21:56   ` Maciej W. Rozycki
  1 sibling, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2020-07-11 20:03 UTC (permalink / raw)
  To: binutils

On Thu, 25 Jun 2020 18:34:01 PDT (-0700), binutils@sourceware.org wrote:
> By the look of it, git commit 39ff0b812324 broke 32-bit host
> --enable-targets=all binutils builds.
>
> /usr/local/bin/ld: ../opcodes/.libs/libopcodes.a(riscv-dis.o): in function `parse_riscv_dis_option':
> /home/alan/src/binutils-gdb/opcodes/riscv-dis.c:102: undefined reference to `riscv_get_priv_spec_class'
> collect2: error: ld returned 1 exit status
> Makefile:925: recipe for target 'objdump' failed

Sorry about that.  I know I haven't been checking --enable-targets=all builds,
and I'm not sure if anyone else has either.

>
> The problem is that elfxx-riscv.c is not built for a 32-bit host
> without --enable-64-bit-bfd or unless RISCV is given specifically as a
> target.  No such trimming of 64-bit only targets is done in opcodes.
>
> One solution is to move these support functions to cpu-riscv.c, which
> runs into "error: implicit declaration of function ‘xmalloc’".  Now,
> xmalloc is not supposed to be used in libbfd or libopcodes - it's rude
> to crash out of an application that calls libbfd or libopcodes
> functions without giving it a chance to deal with out-of-memory
> itself.  So I removed the xmalloc and instead used a fixed size
> buffer.  If you are worried about adding 36 bytes for the buffer to
> the riscv_get_priv_spec_class_from_numbers stack frame size, then you
> have no idea of the likely xmalloc + malloc stack frame size!  Trying
> to reduce memory usage is commendable, but in this instance
> riscv_estimate_digit and malloc for a temp buffer uses a lot more
> memory than a fixed max-size buffer.

Ya, that all seems fine.  I see this has been committed.

Thanks for fixing it!

>
> 	* elfxx-riscv.c (struct priv_spec_t, priv_specs),
> 	(riscv_get_priv_spec_class, riscv_get_priv_spec_class_from_numbers),
> 	(riscv_get_priv_spec_name): Move to..
> 	* cpu-riscv.c: ..here.
> 	(riscv_get_priv_spec_class_from_numbers): Don't xmalloc temp buffer.
> 	Use %u to print unsigned numbers.
>
> diff --git a/bfd/cpu-riscv.c b/bfd/cpu-riscv.c
> index 22067ab29b..2e9e9eb9d2 100644
> --- a/bfd/cpu-riscv.c
> +++ b/bfd/cpu-riscv.c
> @@ -23,6 +23,86 @@
>  #include "sysdep.h"
>  #include "bfd.h"
>  #include "libbfd.h"
> +#include "elfxx-riscv.h"
> +
> +/* Record the priv spec version string and the corresponding class.  */
> +
> +struct priv_spec_t
> +{
> +  const char *name;
> +  enum riscv_priv_spec_class class;
> +};
> +
> +/* List for all supported privilege versions.  */
> +
> +static const struct priv_spec_t priv_specs[] =
> +{
> +  {"1.9.1", PRIV_SPEC_CLASS_1P9P1},
> +  {"1.10",  PRIV_SPEC_CLASS_1P10},
> +  {"1.11",  PRIV_SPEC_CLASS_1P11},
> +
> +/* Terminate the list.  */
> +  {NULL, 0}
> +};
> +
> +/* Get the corresponding CSR version class by giving a privilege
> +   version string.  */
> +
> +int
> +riscv_get_priv_spec_class (const char *s,
> +			   enum riscv_priv_spec_class *class)
> +{
> +  const struct priv_spec_t *version;
> +
> +  if (s == NULL)
> +    return 0;
> +
> +  for (version = &priv_specs[0]; version->name != NULL; ++version)
> +    if (strcmp (version->name, s) == 0)
> +      {
> +	*class = version->class;
> +	return 1;
> +      }
> +
> +  /* Can not find the supported privilege version.  */
> +  return 0;
> +}
> +
> +/* Get the corresponding CSR version class by giving privilege
> +   version numbers.  It is usually used to convert the priv
> +   attribute numbers into the corresponding class.  */
> +
> +int
> +riscv_get_priv_spec_class_from_numbers (unsigned int major,
> +					unsigned int minor,
> +					unsigned int revision,
> +					enum riscv_priv_spec_class *class)
> +{
> +  char buf[36];
> +
> +  if (major == 0 && minor == 0 && revision == 0)
> +    {
> +      *class = PRIV_SPEC_CLASS_NONE;
> +      return 1;
> +    }
> +
> +  if (revision != 0)
> +    snprintf (buf, sizeof (buf), "%u.%u.%u", major, minor, revision);
> +  else
> +    snprintf (buf, sizeof (buf), "%u.%u", major, minor);
> +
> +  return riscv_get_priv_spec_class (buf, class);
> +}
> +
> +/* Get the corresponding privilege version string by giving a CSR
> +   version class.  */
> +
> +const char *
> +riscv_get_priv_spec_name (enum riscv_priv_spec_class class)
> +{
> +  /* The first enum is PRIV_SPEC_CLASS_NONE.  */
> +  return priv_specs[class - 1].name;
> +}
>
>  /* This routine is provided two arch_infos and returns an arch_info
>     that is compatible with both, or NULL if none exists.  */
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index fa46b06f8d..1570f1d862 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1749,98 +1749,3 @@ riscv_arch_str (unsigned xlen, const riscv_subset_list_t *subset)
>
>    return attr_str;
>  }
> -
> -/* Record the priv spec version string and the corresponding class.  */
> -
> -struct priv_spec_t
> -{
> -  const char *name;
> -  enum riscv_priv_spec_class class;
> -};
> -
> -/* List for all supported privilege versions.  */
> -
> -static const struct priv_spec_t priv_specs[] =
> -{
> -  {"1.9.1", PRIV_SPEC_CLASS_1P9P1},
> -  {"1.10",  PRIV_SPEC_CLASS_1P10},
> -  {"1.11",  PRIV_SPEC_CLASS_1P11},
> -
> -/* Terminate the list.  */
> -  {NULL, 0}
> -};
> -
> -/* Get the corresponding CSR version class by giving a privilege
> -   version string.  */
> -
> -int
> -riscv_get_priv_spec_class (const char *s,
> -			   enum riscv_priv_spec_class *class)
> -{
> -  const struct priv_spec_t *version;
> -
> -  if (s == NULL)
> -    return 0;
> -
> -  for (version = &priv_specs[0]; version->name != NULL; ++version)
> -    if (strcmp (version->name, s) == 0)
> -      {
> -	*class = version->class;
> -	return 1;
> -      }
> -
> -  /* Can not find the supported privilege version.  */
> -  return 0;
> -}
> -
> -/* Get the corresponding CSR version class by giving privilege
> -   version numbers.  It is usually used to convert the priv
> -   attribute numbers into the corresponding class.  */
> -
> -int
> -riscv_get_priv_spec_class_from_numbers (unsigned int major,
> -					unsigned int minor,
> -					unsigned int revision,
> -					enum riscv_priv_spec_class *class)
> -{
> -  size_t buf_size;
> -  char *buf;
> -  int result = 1;
> -
> -  if (major == 0 && minor == 0 && revision == 0)
> -    {
> -      *class = PRIV_SPEC_CLASS_NONE;
> -      return result;
> -    }
> -
> -  buf_size = riscv_estimate_digit (major)
> -	     + 1 /* '.' */
> -	     + riscv_estimate_digit (minor)
> -	     + 1; /* string terminator */
> -  if (revision != 0)
> -    {
> -      buf_size += 1 /* '.' */
> -		  + riscv_estimate_digit (revision);
> -      buf = xmalloc (buf_size);
> -      snprintf (buf, buf_size, "%d.%d.%d", major, minor, revision);
> -    }
> -  else
> -    {
> -      buf = xmalloc (buf_size);
> -      snprintf (buf, buf_size, "%d.%d", major, minor);
> -    }
> -
> -  result = riscv_get_priv_spec_class (buf, class);
> -  free (buf);
> -  return result;
> -}
> -
> -/* Get the corresponding privilege version string by giving a CSR
> -   version class.  */
> -
> -const char *
> -riscv_get_priv_spec_name (enum riscv_priv_spec_class class)
> -{
> -  /* The first enum is PRIV_SPEC_CLASS_NONE.  */
> -  return priv_specs[class - 1].name;
> -}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RISCV changes broke 32-bit --enable-targets=all
  2020-07-11 20:03 ` Palmer Dabbelt
@ 2020-07-31 21:56   ` Maciej W. Rozycki
  2020-07-31 22:40     ` Nick Alcock
  0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2020-07-31 21:56 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: binutils

On Sat, 11 Jul 2020, Palmer Dabbelt wrote:

> > By the look of it, git commit 39ff0b812324 broke 32-bit host
> > --enable-targets=all binutils builds.
> >
> > /usr/local/bin/ld: ../opcodes/.libs/libopcodes.a(riscv-dis.o): in function `parse_riscv_dis_option':
> > /home/alan/src/binutils-gdb/opcodes/riscv-dis.c:102: undefined reference to `riscv_get_priv_spec_class'
> > collect2: error: ld returned 1 exit status
> > Makefile:925: recipe for target 'objdump' failed
> 
> Sorry about that.  I know I haven't been checking --enable-targets=all builds,
> and I'm not sure if anyone else has either.

 FWIW I routinely have (also with GDB), though not with a 32-bit host due 
to their mostly diminished role as processing work force.  In reality I 
think it makes sense to verify both dedicated and `--enable-targets=all' 
configurations, as different kind of breakage may happen depending on the 
presence or absence of specific other code.  I have been hit with that in 
the past.

  Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RISCV changes broke 32-bit --enable-targets=all
  2020-07-31 21:56   ` Maciej W. Rozycki
@ 2020-07-31 22:40     ` Nick Alcock
  2020-09-02  7:20       ` Nelson Chu
  0 siblings, 1 reply; 6+ messages in thread
From: Nick Alcock @ 2020-07-31 22:40 UTC (permalink / raw)
  To: Maciej W. Rozycki via Binutils; +Cc: Palmer Dabbelt, Maciej W. Rozycki

On 31 Jul 2020, Maciej W. Rozycki via Binutils uttered the following:

> On Sat, 11 Jul 2020, Palmer Dabbelt wrote:
>
>> > By the look of it, git commit 39ff0b812324 broke 32-bit host
>> > --enable-targets=all binutils builds.
>> >
>> > /usr/local/bin/ld: ../opcodes/.libs/libopcodes.a(riscv-dis.o): in function `parse_riscv_dis_option':
>> > /home/alan/src/binutils-gdb/opcodes/riscv-dis.c:102: undefined reference to `riscv_get_priv_spec_class'
>> > collect2: error: ld returned 1 exit status
>> > Makefile:925: recipe for target 'objdump' failed
>> 
>> Sorry about that.  I know I haven't been checking --enable-targets=all builds,
>> and I'm not sure if anyone else has either.
>
>  FWIW I routinely have (also with GDB), though not with a 32-bit host due 
> to their mostly diminished role as processing work force.  In reality I 

It's also part of my standard test matrix before pushing CTF stuff (both
on 32- and 64-bit x86 hosts because I broke non-ELF in the past and I
don't want to do it again, and becaue the CTF deduplicator contains some
32-bit-specific code...)

(I also test that 'make -k check-binutils check-gas check-ld' on a long
list of cross targets (which is meant to be more or less every target we
implement) doesn't regress across the range of each series I work on. It
takes about an hour even parallelized, but the number of times it's told
me that I regressed 'make check' or simply broke compilation on some
obscure target I'd never otherwise have thought of testing makes it time
well spent. This is not my idea: hat tip to Jose Marchesi for both the
idea and 90% of the code...)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RISCV changes broke 32-bit --enable-targets=all
  2020-07-31 22:40     ` Nick Alcock
@ 2020-09-02  7:20       ` Nelson Chu
  0 siblings, 0 replies; 6+ messages in thread
From: Nelson Chu @ 2020-09-02  7:20 UTC (permalink / raw)
  To: Nick Alcock; +Cc: Maciej W. Rozycki via Binutils, Maciej W. Rozycki

Thanks for all.  These information and experiences are very helpful :)

On Sat, Aug 1, 2020 at 6:40 AM Nick Alcock via Binutils
<binutils@sourceware.org> wrote:
>
> On 31 Jul 2020, Maciej W. Rozycki via Binutils uttered the following:
>
> > On Sat, 11 Jul 2020, Palmer Dabbelt wrote:
> >
> >> > By the look of it, git commit 39ff0b812324 broke 32-bit host
> >> > --enable-targets=all binutils builds.
> >> >
> >> > /usr/local/bin/ld: ../opcodes/.libs/libopcodes.a(riscv-dis.o): in function `parse_riscv_dis_option':
> >> > /home/alan/src/binutils-gdb/opcodes/riscv-dis.c:102: undefined reference to `riscv_get_priv_spec_class'
> >> > collect2: error: ld returned 1 exit status
> >> > Makefile:925: recipe for target 'objdump' failed
> >>
> >> Sorry about that.  I know I haven't been checking --enable-targets=all builds,
> >> and I'm not sure if anyone else has either.
> >
> >  FWIW I routinely have (also with GDB), though not with a 32-bit host due
> > to their mostly diminished role as processing work force.  In reality I
>
> It's also part of my standard test matrix before pushing CTF stuff (both
> on 32- and 64-bit x86 hosts because I broke non-ELF in the past and I
> don't want to do it again, and becaue the CTF deduplicator contains some
> 32-bit-specific code...)
>
> (I also test that 'make -k check-binutils check-gas check-ld' on a long
> list of cross targets (which is meant to be more or less every target we
> implement) doesn't regress across the range of each series I work on. It
> takes about an hour even parallelized, but the number of times it's told
> me that I regressed 'make check' or simply broke compilation on some
> obscure target I'd never otherwise have thought of testing makes it time
> well spent. This is not my idea: hat tip to Jose Marchesi for both the
> idea and 90% of the code...)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-02  7:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-26  1:34 RISCV changes broke 32-bit --enable-targets=all Alan Modra
2020-06-26  4:08 ` Nelson Chu
2020-07-11 20:03 ` Palmer Dabbelt
2020-07-31 21:56   ` Maciej W. Rozycki
2020-07-31 22:40     ` Nick Alcock
2020-09-02  7:20       ` Nelson Chu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).