Re: [PATCH v12 0/11 Add support for CRC64 generation in linker

[Alan Modra <amodra@gmail.com>]

On Wed, Mar 08, 2023 at 12:48:15PM +0100, Ulf Samuelsson via Binutils wrote:
> Building using 32-bit bfd_vma makes life a lot more difficult since a lot of
> the code uses the bfd_vma type.

Right, there are a few obvious places in lddigest.c that need to use
uint64_t rather than bfd_vma.

> The expression calculator implemented in ldexp.c stores all integers in a
> "bigint".
> 
>   struct big_int
>     {
>       bfd_vma integer;
>       char *str;
>     } bigint;
> 
> So if you do DIGEST POLY (64,0xD800000000000000ULL,x,y,z,v,w)
> the expression calculator will return the wrong value for the all the 64 bit
> constants larger than 0xFFFFFFFF.

Yes, that is a pain.

> You  do not get around this, except by redesigning a **lot**.

I think you can do it by changing lang_add_digest to take etree_type*
args.  I'm feeling generous so here's a first pass at it.  I haven't
fixed the debug functions..

It's late here so I'll commit this tomorrow after a little more
testing.

diff --git a/ld/lddigest.c b/ld/lddigest.c
index d0bb4db73ab..846ece177d3 100644
--- a/ld/lddigest.c
+++ b/ld/lddigest.c
@@ -103,10 +103,12 @@ lang_add_crc32_syndrome (algorithm_desc_t * a)
 static void
 lang_add_crc32_table (bool big_endian)
 {
-  uint32_t *crc32_table = algorithm.crc_tab;	/* Use a precomputed, if it exists */
+  /* Use a precomputed table, if it exists.  */
+  uint32_t *crc32_table = algorithm.crc_tab;
   bool local_table = false;
   if (crc32_table == NULL)
-    {				/* No luck, create a table */
+    {
+      /* No luck, create a table.  */
       crc32_table = init_crc32_tab (&algorithm);
       if (crc32_table == NULL)
 	{
@@ -115,7 +117,7 @@ lang_add_crc32_table (bool big_endian)
 	}
       local_table = true;
     }
-  for (bfd_vma i = 0; i < 256; i++)
+  for (int i = 0; i < 256; i++)
     {
       uint32_t elem = crc32_table[i];
       if (big_endian)
@@ -181,10 +183,12 @@ print_hash64_table (algorithm_desc_t * a)
 static void
 lang_add_crc64_table (bool big_endian)
 {
-  bfd_vma *crc64_table = algorithm.crc_tab;	/* Use a precomputed, if it exists */
+  /* Use a precomputed table, if it exists.  */
+  uint64_t *crc64_table = algorithm.crc_tab;
   bool local_table = false;
   if (crc64_table == NULL)
-    {				/* No luck, create a table */
+    {
+      /* No luck, create a table.  */
       crc64_table = init_crc64_tab (&algorithm);
       if (crc64_table == NULL)
 	{
@@ -194,9 +198,9 @@ lang_add_crc64_table (bool big_endian)
       local_table = true;
     }
   print_hash64_table (&algorithm);
-  for (bfd_vma i = 0; i < 256; i++)
+  for (int i = 0; i < 256; i++)
     {
-      bfd_vma elem = crc64_table[i];
+      uint64_t elem = crc64_table[i];
       if (big_endian)
 	{
 	  elem = __builtin_bswap64 (elem);
@@ -214,12 +218,42 @@ lang_add_crc64_table (bool big_endian)
 
 /* ============ DIGEST COMMON functions ======================================*/
 
+static uint64_t
+get_int (etree_type *tree, bool *err)
+{
+  if (tree != NULL)
+    {
+      exp_fold_tree_no_dot (tree);
+
+      if (expld.result.valid_p)
+	{
+	  if (expld.result.str != NULL)
+	    {
+	      char *end;
+	      uint64_t val = strtoull (expld.result.str, &end, 16);
+	      if (!*end)
+		return val;
+	    }
+	  else
+	    {
+	      if (expld.result.section != NULL)
+		expld.result.value += expld.result.section->vma;
+	      return expld.result.value;
+	    }
+	}
+    }
+  *err = true;
+  return 0;
+}
+
 void
-lang_add_digest (bfd_vma size,
-		 bfd_vma poly,
-		 bfd_vma initial,
-		 bfd_vma xor_val,
-		 bfd_vma ireflect, bfd_vma oreflect, bfd_vma reciprocal)
+lang_add_digest (etree_type *size,
+		 etree_type *poly,
+		 etree_type *initial,
+		 etree_type *xor_val,
+		 etree_type *ireflect,
+		 etree_type *oreflect,
+		 etree_type *reciprocal)
 {
   if (algorithm.crc_algo != no_algo)	/* We only allow one CRC <polynom> */
     {
@@ -227,40 +261,44 @@ lang_add_digest (bfd_vma size,
       return;
     }
 
+  bool err = false;
   algorithm.name = "CUSTOM";
   algorithm.big_endian = digest_big_endian;
-  if (size == 64)
+  algorithm.crc_size = get_int (size, &err);
+  algorithm.poly.d64 = get_int (poly, &err);
+  algorithm.initial.d64 = get_int (initial, &err);
+  algorithm.xor_val.d64 = get_int (xor_val, &err);
+  algorithm.ireflect = get_int (ireflect, &err);
+  algorithm.oreflect = get_int (oreflect, &err);
+  algorithm.crc_tab = NULL;
+  algorithm.reciprocal = get_int (reciprocal, &err);
+  algorithm.expected.d64 = 0;
+
+  if (err)
+    {
+      einfo (_("%F%P: Invalid DIGEST arg\n"));
+      return;
+    }
+
+  if (algorithm.crc_size == 64)
     {
       algorithm.crc_algo = crc_algo_64;
-      algorithm.crc_size = size;
-      algorithm.poly.d64 = poly;	/* Set the polynom */
-      algorithm.initial.d64 = initial;	/* Set seed */
-      algorithm.xor_val.d64 = xor_val;	/* final XOR value */
-      algorithm.ireflect = ireflect;
-      algorithm.oreflect = oreflect;
-      algorithm.crc_tab = NULL;
-      algorithm.reciprocal = reciprocal;
-      algorithm.expected.d64 = 0;
-
       lang_add_crc64_syndrome (&algorithm);
     }
-  else if (size == 32)
+  else if (algorithm.crc_size == 32)
     {
       algorithm.crc_algo = crc_algo_32;
-      algorithm.crc_size = size;
-      algorithm.poly.d32._0 = poly;	/* Set the polynom */
-      algorithm.initial.d32._0 = initial;	/* Set seed */
-      algorithm.xor_val.d32._0 = xor_val;	/* final XOR value */
-      algorithm.ireflect = ireflect;
-      algorithm.oreflect = oreflect;
-      algorithm.crc_tab = NULL;
-      algorithm.reciprocal = reciprocal;
-      algorithm.expected.d32._0 = 0;
+      algorithm.poly.d32._0 = algorithm.poly.d64;
+      algorithm.poly.d32._1 = 0;
+      algorithm.initial.d32._0 = algorithm.initial.d64;
+      algorithm.initial.d32._1 = 0;
+      algorithm.xor_val.d32._0 = algorithm.xor_val.d64;
+      algorithm.xor_val.d32._1 = 0;
       lang_add_crc32_syndrome (&algorithm);
     }
   else
     {
-      einfo (_("%F%P: Illegal Size in DIGEST: %E\n"));
+      einfo (_("%F%P: Invalid Size in DIGEST\n"));
       return;
     }
 }
@@ -660,7 +698,7 @@ set_crc64_checksum (uint64_t crc, bfd_vma addr)
 }
 
 static bool
-set_crc_checksum (bfd_vma crc, bfd_vma addr, bfd_vma size)
+set_crc_checksum (uint64_t crc, bfd_vma addr, int size)
 {
   bool status;
   if (size == 64)
@@ -700,7 +738,7 @@ symbol_lookup (char *name, bfd_vma * val)
  * Multiplexing function for calculating CRC with different algorithms
  * 'algorithm.crc_algo'
  */
-static bfd_vma
+static uint64_t
 calculate_crc (const unsigned char *input_str, size_t num_bytes)
 {
   if (algorithm.crc_algo == crc_algo_64)
@@ -782,7 +820,7 @@ void
 lang_generate_crc (void)
 {
   bfd_vma crc_addr, crc_area_start, crc_area_end;
-  bfd_vma crc;
+  uint64_t crc;
   bool can_do_crc;
 
   /* Return immediately, if CRC is not requested */
diff --git a/ld/lddigest.h b/ld/lddigest.h
index 9f5e5f3fbda..f5ae1ddad7e 100755
--- a/ld/lddigest.h
+++ b/ld/lddigest.h
@@ -33,6 +33,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include "bfd.h"
+#include "ldexp.h"
 
 #define	CRC_POLY_64_ECMA	0x42F0E1EBA9EA3693ull	/* Normal */
 #define	CRC_POLY_64_ECMA_EXP	0x6C40DF5F0B497347ull	/* CRC when testing "123456789" */
@@ -163,17 +164,18 @@ extern uint32_t calc_crc32
 extern void lang_add_crc32_syndrome (algorithm_desc_t * a);
 
 /* CR-64 */
-extern bfd_vma *init_crc64_tab (algorithm_desc_t * dsc);
-extern bfd_vma calc_crc64
+extern uint64_t *init_crc64_tab (algorithm_desc_t * dsc);
+extern uint64_t calc_crc64
   (algorithm_desc_t * dsc, const unsigned char *input_str, size_t num_bytes);
 extern void lang_add_crc64_syndrome (algorithm_desc_t * a);
 
-extern void lang_add_digest (bfd_vma size,
-			     bfd_vma poly,
-			     bfd_vma initial,
-			     bfd_vma xor_val,
-			     bfd_vma ireflect,
-			     bfd_vma oreflect, bfd_vma reciprocal);
+extern void lang_add_digest (etree_type *size,
+			     etree_type *poly,
+			     etree_type *initial,
+			     etree_type *xor_val,
+			     etree_type *ireflect,
+			     etree_type *oreflect,
+			     etree_type *reciprocal);
 extern bool lang_set_digest (char *digest);
 extern void lang_add_digest_table (bool big_endian);
 extern const char *lang_get_label (const char *label, bool *big_endian);
diff --git a/ld/lddigest_tab.c b/ld/lddigest_tab.c
index 4f5db10c997..b286a3c0161 100644
--- a/ld/lddigest_tab.c
+++ b/ld/lddigest_tab.c
@@ -99,9 +99,9 @@ const algorithm_desc_t algorithms[MAXALGO+1] =
   [CRC32] =
 	{
 		crc_algo_32, 32, "CRC32",
-		.poly.d32._0 = CRC_POLY_32,
+		.poly.d32 = { CRC_POLY_32, 0 },
 		.initial.d64 = CRC_START_32_INV,
-		.xor_val.d32._0 = CRC_END_32_INV,
+		.xor_val.d32 = { CRC_END_32_INV, 0 },
 		.ireflect = true,
 		.oreflect = true,
 		.reciprocal = false,
diff --git a/ld/ldgram.y b/ld/ldgram.y
index ea0c569279a..2bc2f5e24e2 100644
--- a/ld/ldgram.y
+++ b/ld/ldgram.y
@@ -747,13 +747,13 @@ polynome:
 		   mustbe_exp ',' mustbe_exp ',' mustbe_exp ')'
 		{
 		  lang_add_digest (
-			$3->value.value,	/* size			*/
-			$5->value.value,	/* polynome		*/
-			$7->value.value,	/* initial value	*/
-			$9->value.value,	/* xor     value	*/
-			$11->value.value,	/* input   reflected	*/
-			$13->value.value,	/* output  reflected	*/
-			$15->value.value	/* reciprocal		*/
+			$3,	/* size			*/
+			$5,	/* polynome		*/
+			$7,	/* initial value	*/
+			$9,	/* xor     value	*/
+			$11,	/* input   reflected	*/
+			$13,	/* output  reflected	*/
+			$15	/* reciprocal		*/
 			);
 		  polynome_valid = true;
 		}
diff --git a/ld/testsuite/ld-scripts/crc64-poly-size.d b/ld/testsuite/ld-scripts/crc64-poly-size.d
index 6a3651cbff5..1c7b3c46881 100644
--- a/ld/testsuite/ld-scripts/crc64-poly-size.d
+++ b/ld/testsuite/ld-scripts/crc64-poly-size.d
@@ -1,5 +1,5 @@
 #source: crc64-poly-size.s
 #ld: -T crc64-poly-size.t
-# error: .*: Illegal Size in DIGEST: .*
+# error: .*: Invalid Size in DIGEST
 #target: [is_elf_format] [is_coff_format]
 #skip: tic4x-*-* tic54x-*-*


-- 
Alan Modra
Australia Development Lab, IBM