Re: [PATCH] gas value truncation warning reports truncated values, doesn't look at signedness.

[Bart Samwel]

Alan Modra wrote:
> On Mon, Jan 12, 2004 at 11:57:05AM +0100, Bart Samwel wrote:
>
> > Then it's funny that as uses 64-bit arithmetic, while it's clearly 
> > supposed to be a 32-bit assembler (I have an i386 system without any 
> > 64-bit parts -- a P3, to be exact). The arithmetic gas uses depends on 
> > bfd, which is 64-bit, but I think it might have to depend on the target 
> > architecture instead, i.e., N-bit target architecture = N-bit 
> > arithmetic. What do you think?
>
> It's a reasonable idea.  Do you care to contribute patches?  :)

OK, I've given it a shot in the attached patch. The effect of the patch 
is that all intermediate values are truncated to the platform's bit 
width regardless of the BFD's bit width, so that the result of any 
arithmetic is the same regardless of the BFD's bit width. I'm interested 
in your feedback, so: fire away!

A description of the changes:

* Add value tc_arithmetic_cutoff_mask, tc_arithmetic_hibit and 
ENABLE_ARITHMETIC_CUTOFF to config/tc-i386.h, and initialize them 
appropriately in tc-i386.c (64-bits mask for flag_code == CODE_64BIT, 
32-bits mask if not). The cutoff mask is basically a mask for the bit 
width of the architecture, that can be used to adjust calculations. All 
the other architectures are masked from this change because they don't 
define ENABLE_ARITHMETIC_CUTOFF.

* Change expr() and operand() to call truncate_constant() for any result 
expression that has X_op == O_constant. This effectively causes all 
intermediate values to be truncated.

* Add truncate_constant() to expr.c. This function is based on the 
truncation+warning code in read.c, but always truncates to the bits in 
tc_arithmetic_cutoff_mask. It generates a warning when a value is cut 
off, "intermediate value X truncated to Y". (Remark: it also says 
"intermediate" for the outer expression. We should really suppress the 
warning for the outer expression and leave it to the truncation code in 
read.c -- but that's complicated, because we don't know whether we're at 
the end of the expression.)

* Change the '~' operator implementation in expr.c to cut off bits from 
the result according to the arithmetic cutoff. The input is already 
within range (because of the other changes) so the output definitely 
will be within range as well when we cut it off.

* Change the truncation warnings to report using unsigned long
longs instead of unsigned longs.

* Change the truncation warnings to report truncations of
overflows in the negative using negative values. This is to prevent
things like truncation for -0x80000001 to be reported as truncations
from 0xffffffff7fffffff to 0x7fffffff (this shows the 64-bit arithmetic
too much). Instead, this would be reported as a truncation from
-0x80000001 to 0x7fffffff, which conveys the message that the overflow
was in the negative much more clearly.

* Change the truncation warnings to take into account the sign of the 
value. An unsigned value will now ALWAYS generate a truncation warning 
when one of the high bits is set, instead of only when it cannot be 
interpreted as a within-range negative number.

-- Bart

diff -baur binutils-2.14.90.0.7-original/gas/config/tc-i386.c 
binutils-2.14.90.0.7/gas/config/tc-i386.c
--- binutils-2.14.90.0.7-original/gas/config/tc-i386.c  2003-08-21 
17:28:48.000000000 +0200
+++ binutils-2.14.90.0.7/gas/config/tc-i386.c   2004-01-15 15:37:11.000000000 
+0100
@@ -322,6 +322,10 @@
 /* The dwarf2 data alignment, adjusted for 32 or 64 bit.  */
 int x86_cie_data_alignment;
 
+/* The arithmetic cutoff and hibit, adjusted for 32 or 64 bit.   */
+bfd_vma x86_arithmetic_cutoff_mask;
+bfd_vma x86_arithmetic_hibit;
+
 /* Interface to relax_segment.
    There are 3 major relax states for 386 jump insns because the
    different types of jumps add different sizes to frags when we're
@@ -986,11 +990,15 @@
     {
       x86_dwarf2_return_column = 16;
       x86_cie_data_alignment = -8;
+      x86_arithmetic_cutoff_mask = (bfd_vma)0xFFFFFFFFFFFFFFFFULL;
+      x86_arithmetic_hibit = (bfd_vma)0x8000000000000000ULL;
     }
   else
     {
       x86_dwarf2_return_column = 8;
       x86_cie_data_alignment = -4;
+      x86_arithmetic_cutoff_mask = (bfd_vma)0x00000000FFFFFFFFULL;
+      x86_arithmetic_hibit = (bfd_vma)0x0000000080000000ULL;
     }
 }
 
diff -baur binutils-2.14.90.0.7-original/gas/config/tc-i386.h 
binutils-2.14.90.0.7/gas/config/tc-i386.h
--- binutils-2.14.90.0.7-original/gas/config/tc-i386.h  2003-08-21 
17:28:48.000000000 +0200
+++ binutils-2.14.90.0.7/gas/config/tc-i386.h   2004-01-15 15:42:21.000000000 
+0100
@@ -499,4 +499,10 @@
 #define tc_cfi_frame_initial_instructions tc_x86_frame_initial_instructions
 extern void tc_x86_frame_initial_instructions PARAMS ((void));
 
+extern bfd_vma x86_arithmetic_cutoff_mask;
+extern bfd_vma x86_arithmetic_hibit;
+#define tc_arithmetic_cutoff_mask (x86_arithmetic_cutoff_mask)
+#define tc_arithmetic_hibit (x86_arithmetic_hibit)
+#define TC_ENABLE_ARITHMETIC_CUTOFF
+
 #endif /* TC_I386 */
diff -baur binutils-2.14.90.0.7-original/gas/expr.c 
binutils-2.14.90.0.7/gas/expr.c
--- binutils-2.14.90.0.7-original/gas/expr.c    2003-07-23 17:08:10.000000000 
+0200
+++ binutils-2.14.90.0.7/gas/expr.c     2004-01-15 16:32:57.000000000 +0100
@@ -40,6 +40,7 @@
 static void integer_constant PARAMS ((int radix, expressionS * expressionP));
 static void mri_char_constant PARAMS ((expressionS *));
 static void current_location PARAMS ((expressionS *));
+static void truncate_constant PARAMS((expressionS *exp));
 static void clean_up_expression PARAMS ((expressionS * expressionP));
 static segT operand PARAMS ((expressionS *));
 static operatorT operator PARAMS ((int *));
@@ -748,6 +749,37 @@
     }
 }
 
+/* Truncate a constant expression to the architecture's bit width,
+   if necessary. The width may be less than the BFD's bit width!
+   This should be done at every step in a constant calculation,
+   to ensure that the same results are achieved when compiling
+   with a 32-bit and a 64-bit BFD. */
+
+static void
+truncate_constant (exp)
+     expressionS *exp;
+{
+#ifdef TC_ENABLE_ARITHMETIC_CUTOFF
+  valueT get;
+  valueT use;
+  get = exp->X_add_number;
+  use = get & tc_arithmetic_cutoff_mask;
+  if ((get & ~tc_arithmetic_cutoff_mask) != 0
+       && (exp->X_unsigned
+       || (get & ~tc_arithmetic_cutoff_mask) != ~tc_arithmetic_cutoff_mask
+       || (get & tc_arithmetic_hibit) == 0))
+       {               /* Leading bits contain both 0s & 1s.  */
+         if (exp->X_unsigned || (offsetT)get >= 0)
+           as_warn (_("intermediate value 0x%llx truncated to 0x%llx"),
+                   (unsigned long long) get, (unsigned long long) use);
+         else
+           as_warn (_("intermediate value -0x%llx truncated to 0x%llx"),
+                   (unsigned long long) -get, (unsigned long long) use);
+       }
+  exp->X_add_number = use;
+#endif
+}
+
 /* In: Input_line_pointer points to 1st char of operand, which may
        be a space.
 
@@ -1067,7 +1099,16 @@
                expressionP->X_unsigned = 0;
              }
            else if (c == '~' || c == '"')
+             {
              expressionP->X_add_number = ~ expressionP->X_add_number;
+#ifdef TC_ENABLE_ARITHMETIC_CUTOFF
+               /* With a M-bit BFD on an N-bit architecture (M > N), '~' will
+                  also flip all high bits, which is undesirable because they
+                  will be truncated anyway, and they will generate truncation
+                  warnings. So, we preserve only the lower bits of the result. 
*/
+               expressionP->X_add_number = expressionP->X_add_number & 
tc_arithmetic_cutoff_mask;
+#endif
+             }
            else
              expressionP->X_add_number = ! expressionP->X_add_number;
          }
@@ -1335,6 +1376,8 @@
 
   switch (expressionP->X_op)
     {
+    case O_constant:
+      truncate_constant(expressionP);
     default:
       return absolute_section;
     case O_symbol:
@@ -1868,6 +1911,9 @@
            as_bad (_("operation combines symbols in different segments"));
        }
 
+      if (resultP->X_op == O_constant)
+        truncate_constant(resultP);
+
       op_left = op_right;
     }                          /* While next operator is >= this rank.  */
 
diff -baur binutils-2.14.90.0.7-original/gas/expr.h 
binutils-2.14.90.0.7/gas/expr.h
--- binutils-2.14.90.0.7-original/gas/expr.h    2003-05-05 23:46:47.000000000 
+0200
+++ binutils-2.14.90.0.7/gas/expr.h     2004-01-15 16:02:39.000000000 +0100
@@ -131,9 +131,7 @@
 #endif
 
   /* Non-zero if X_add_number should be regarded as unsigned.  This is
-     only valid for O_constant expressions.  It is only used when an
-     O_constant must be extended into a bignum (i.e., it is not used
-     when performing arithmetic on these values).
+     only valid for O_constant expressions.
      FIXME: This field is not set very reliably.  */
   unsigned int X_unsigned : 1;
 
diff -baur binutils-2.14.90.0.7-original/gas/read.c 
binutils-2.14.90.0.7/gas/read.c
--- binutils-2.14.90.0.7-original/gas/read.c    2003-10-29 18:37:48.000000000 
+0100
+++ binutils-2.14.90.0.7/gas/read.c     2004-01-15 14:40:51.000000000 +0100
@@ -3667,11 +3667,16 @@
       get = exp->X_add_number;
       use = get & unmask;
       if ((get & mask) != 0
-         && ((get & mask) != mask
+         && (exp->X_unsigned
+             || (get & mask) != mask
              || (get & hibit) == 0))
        {               /* Leading bits contain both 0s & 1s.  */
-         as_warn (_("value 0x%lx truncated to 0x%lx"),
-                  (unsigned long) get, (unsigned long) use);
+         if (exp->X_unsigned || (offsetT)get >= 0)
+           as_warn (_("value 0x%llx truncated to 0x%llx"),
+                    (unsigned long long) get, (unsigned long long) use);
+         else
+           as_warn (_("value -0x%llx truncated to 0x%llx"),
+                    (unsigned long long) -get, (unsigned long long) use);
        }
       /* Put bytes in right order.  */
       md_number_to_chars (p, use, (int) nbytes);