diff mbox

[Ada] Latent bug in Uintp.Most_Sig_2_Digits

Message ID 20170425082313.GA73746@adacore.com
State New
Headers show

Commit Message

Arnaud Charlet April 25, 2017, 8:23 a.m. UTC
This patch fixes a latent bug in Uintp.Most_Sig_2_Digits, that would
cause a crash if Left is indirect and Right is direct. In fact, that
combination never occurs in any existing calls. No test is available,
because the bug is latent.

Tested on x86_64-pc-linux-gnu, committed on trunk

2017-04-25  Bob Duff  <duff@adacore.com>

	* uintp.adb (Most_Sig_2_Digits): In case Direct (Right), fetch
	Direct_Val (Right), instead of the incorrect Direct_Val (Left).
	(UI_GCD): Remove ??? comment involving possible efficiency
	improvements. This just isn't important after all these years.
	Also minor cleanup.
	* uintp.ads: Minor cleanup.
diff mbox

Patch

Index: uintp.adb
===================================================================
--- uintp.adb	(revision 247135)
+++ uintp.adb	(working copy)
@@ -52,7 +52,7 @@ 
 
    UI_Power_2 : array (Int range 0 .. 64) of Uint;
    --  This table is used to memoize exponentiations by powers of 2. The Nth
-   --  entry, if set, contains the Uint value 2 ** N. Initially UI_Power_2_Set
+   --  entry, if set, contains the Uint value 2**N. Initially UI_Power_2_Set
    --  is zero and only the 0'th entry is set, the invariant being that all
    --  entries in the range 0 .. UI_Power_2_Set are initialized.
 
@@ -149,9 +149,9 @@ 
       Left_Hat  : out Int;
       Right_Hat : out Int);
    --  Returns leading two significant digits from the given pair of Uint's.
-   --  Mathematically: returns Left / (Base ** K) and Right / (Base ** K) where
+   --  Mathematically: returns Left / (Base**K) and Right / (Base**K) where
    --  K is as small as possible S.T. Right_Hat < Base * Base. It is required
-   --  that Left > Right for the algorithm to work.
+   --  that Left >= Right for the algorithm to work.
 
    function N_Digits (Input : Uint) return Int;
    pragma Inline (N_Digits);
@@ -264,7 +264,7 @@ 
       -------------------
 
       function Better_In_Hex return Boolean is
-         T16 : constant Uint := Uint_2 ** Int'(16);
+         T16 : constant Uint := Uint_2**Int'(16);
          A   : Uint;
 
       begin
@@ -506,6 +506,7 @@ 
       pragma Assert (Left >= Right);
 
       if Direct (Left) then
+         pragma Assert (Direct (Right));
          Left_Hat  := Direct_Val (Left);
          Right_Hat := Direct_Val (Right);
          return;
@@ -533,7 +534,7 @@ 
 
       begin
          if Direct (Right) then
-            T := Direct_Val (Left);
+            T := Direct_Val (Right);
             R1 := abs (T / Base);
             R2 := T rem Base;
             Length_R := 2;
@@ -1370,7 +1371,7 @@ 
 
       elsif Right <= Uint_64 then
 
-         --  2 ** N for N in 2 .. 64
+         --  2**N for N in 2 .. 64
 
          if Left = Uint_2 then
             declare
@@ -1390,7 +1391,7 @@ 
                return UI_Power_2 (Right_Int);
             end;
 
-         --  10 ** N for N in 2 .. 64
+         --  10**N for N in 2 .. 64
 
          elsif Left = Uint_10 then
             declare
@@ -1585,20 +1586,6 @@ 
          else
             --  Use prior single precision steps to compute this Euclid step
 
-            --  For constructs such as:
-            --  sqrt_2: constant := 1.41421_35623_73095_04880_16887_24209_698;
-            --  sqrt_eps: constant long_float := long_float( 1.0 / sqrt_2)
-            --    ** long_float'machine_mantissa;
-            --
-            --  we spend 80% of our time working on this step. Perhaps we need
-            --  a special case Int / Uint dot product to speed things up. ???
-
-            --  Alternatively we could increase the single precision iterations
-            --  to handle Uint's of some small size ( <5 digits?). Then we
-            --  would have more iterations on small Uint. On the code above, we
-            --  only get 5 (on average) single precision iterations per large
-            --  iteration. ???
-
             Tmp_UI := (UI_From_Int (A) * U) + (UI_From_Int (B) * V);
             V := (UI_From_Int (C) * U) + (UI_From_Int (D) * V);
             U := Tmp_UI;
Index: uintp.ads
===================================================================
--- uintp.ads	(revision 247135)
+++ uintp.ads	(working copy)
@@ -238,7 +238,7 @@ 
      (B      : Uint;
       E      : Uint;
       Modulo : Uint) return Uint;
-   --  Efficiently compute (B ** E) rem Modulo
+   --  Efficiently compute (B**E) rem Modulo
 
    function UI_Modular_Inverse (N : Uint; Modulo : Uint) return Uint;
    --  Compute the multiplicative inverse of N in modular arithmetics with the
@@ -438,7 +438,7 @@ 
    Base_Bits : constant := 15;
    --  Number of bits in base value
 
-   Base : constant Int := 2 ** Base_Bits;
+   Base : constant Int := 2**Base_Bits;
 
    --  Values in the range -(Base-1) .. Max_Direct are encoded directly as
    --  Uint values by adding a bias value. The value of Max_Direct is chosen
@@ -454,13 +454,13 @@ 
    --  avoid accidental use of Uint arithmetic on these values, which is never
    --  correct.
 
-   type Ctrl is range Int'First .. Int'Last;
+   type Ctrl is new Int;
 
    Uint_Direct_Bias  : constant Ctrl := Ctrl (Uint_Low_Bound) + Ctrl (Base);
    Uint_Direct_First : constant Ctrl := Uint_Direct_Bias + Ctrl (Min_Direct);
    Uint_Direct_Last  : constant Ctrl := Uint_Direct_Bias + Ctrl (Max_Direct);
 
-   Uint_0   : constant Uint := Uint (Uint_Direct_Bias);
+   Uint_0   : constant Uint := Uint (Uint_Direct_Bias + 0);
    Uint_1   : constant Uint := Uint (Uint_Direct_Bias + 1);
    Uint_2   : constant Uint := Uint (Uint_Direct_Bias + 2);
    Uint_3   : constant Uint := Uint (Uint_Direct_Bias + 3);
@@ -499,7 +499,7 @@ 
    Uint_Minus_80  : constant Uint := Uint (Uint_Direct_Bias - 80);
    Uint_Minus_128 : constant Uint := Uint (Uint_Direct_Bias - 128);
 
-   Uint_Max_Simple_Mul : constant := Uint_Direct_Bias + 2 ** 15;
+   Uint_Max_Simple_Mul : constant := Uint_Direct_Bias + 2**15;
    --  If two values are directly represented and less than or equal to this
    --  value, then we know the product fits in a 32-bit integer. This allows
    --  UI_Mul to efficiently compute the product in this case.