[llvm-commits] [PATCH] StringRef::getAsUnsignedInteger() fix for review

[Chris Lattner]

On Oct 1, 2012, at 2:01 PM, Nick Kledzik <kledzik at apple.com> wrote:

> While working on some of my own test cases, I noticed that getAsUnsignedInteger() was failing to return an error on some overflows.  The function did catch some overflows, but not all.  The following patch improves the overflow detection:

Looks good to me, please commit!

-Chris

> 
> Index: lib/Support/StringRef.cpp
> ===================================================================
> --- lib/Support/StringRef.cpp	(revision 164968)
> +++ lib/Support/StringRef.cpp	(working copy)
> @@ -350,8 +350,8 @@
>     unsigned long long PrevResult = Result;
>     Result = Result*Radix+CharVal;
> 
> -    // Check for overflow.
> -    if (Result < PrevResult)
> +    // Check for overflow by shifting back and seeing if bits were lost.
> +    if (Result/Radix < PrevResult)
>       return true;
> 
>     Str = Str.substr(1);
> 
> 
> The case I saw a problem with is:  "0x1234567890ABCDEF0".    The existing code just checks if after the shift-and-add, the new value is less than the previous.  The problem with that is if the most signification digit is less than the second most significant digit, then after the shift (which shifts the most significant digit into the bit bucket), the new value is larger (because the second most signification digit is now the most significant).
> 
> My proposed fix is to unshift the new value and see if that is smaller that the previous value (meaning some bits were lost to the carry).
> 
> -Nick
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits