Cast to double being removed

Cast to double being removed

[Dibyendu Majumdar]

Hi,

In this simple example:

extern int printf(const char *, ...);
int main(void)
{
    printf("%f\n", (double)-1);
    return 0;
}

The linearized output is:

main:
.L0:
        <entry-point>
        symaddr.64  %r1 <- <anon symbol:000001BAAB487258>
        call.32     %r2 <- printf, %r1, $-1
        ret.32      $0

The cast to double has been removed. I am trying to understand where
this is happening as it seems to happen quite early on. Any tips on
where I should look?

Thanks and Regards
Dibyendu

Re: Cast to double being removed

[Linus Torvalds]

On Thu, Mar 16, 2017 at 5:00 PM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
>
> The cast to double has been removed. I am trying to understand where
> this is happening as it seems to happen quite early on. Any tips on
> where I should look?

Since sparse was mostly done for kernel stuff, there's almost no
floating point support anywhere.

There's something seriously screwed up with explicit double casting.
See the differences between

  double f1(void) { return -1; }
  double f2(void) { return (double)-1; }
  double f3(void) { return -1.0; }

and notice how the first one and third ones actually kind-of make
sense. The second one? Not so much, it just uses a 64-bit integer.

The bug is already at the parse tree stage, the linearization doesn't
even see the cast.

I think it's this, in simplify_cast():

        /* A cast of a constant? */
        if (constant(src)) {
                int sign = orig_type->ctype.modifiers & MOD_SIGNED;
                long long val = get_cast_value(src->value, orig_size,
size, sign);
                src = value_pseudo(val);
                goto simplify;
        }

which doesn't even check whether it's a FP value, it just looks if
it's a constant and assumes it's an integer. At least it checks the
sign ;)

               Linus

[PATCH] fix: expansion of integers to floats

[Luc Van Oostenryck]

The test wasn't using is_float_type() and thus missed
SYM_NODE->SUM_BASETYPE::fp_type.

Use is_float_type() now.

Note: there is probably some issue if the expansion of a cast
overflow (at compile-time thus) but that's another story.

Reported-by: Dibyendu Majumdar <mobile@majumdar.org.uk>
CC: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 expand.c                            |   3 +-
 validation/cast-constant-to-float.c |  35 ++++
 validation/cast-constants.c         | 357 ++++++++++++++++++++++++++++++++++++
 3 files changed, 393 insertions(+), 2 deletions(-)
 create mode 100644 validation/cast-constant-to-float.c
 create mode 100644 validation/cast-constants.c

diff --git a/expand.c b/expand.c
index 11f7255bf..5f908c971 100644
--- a/expand.c
+++ b/expand.c
@@ -88,8 +88,7 @@ void cast_value(struct expression *expr, struct symbol *newtype,
 	long long value, mask, signmask;
 	long long oldmask, oldsignmask, dropped;
 
-	if (newtype->ctype.base_type == &fp_type ||
-	    oldtype->ctype.base_type == &fp_type)
+	if (is_float_type(newtype) || is_float_type(oldtype))
 		goto Float;
 
 	// For pointers and integers, we can just move the value around
diff --git a/validation/cast-constant-to-float.c b/validation/cast-constant-to-float.c
new file mode 100644
index 000000000..86b7ac0f7
--- /dev/null
+++ b/validation/cast-constant-to-float.c
@@ -0,0 +1,35 @@
+typedef unsigned int uint;
+typedef unsigned long ulong;
+
+double f1(void) { return -1; }
+double f2(void) { return (double)-1; }
+double f3(void) { return -1.0; }
+
+/*
+ * check-name: cast-constant-to-float
+ * check-command: test-linearize -Wno-decl $file
+ *
+ * check-output-start
+f1:
+.L0:
+	<entry-point>
+	set.64      %r1 <- -1.000000
+	ret.64      %r1
+
+
+f2:
+.L2:
+	<entry-point>
+	set.64      %r3 <- -1.000000
+	ret.64      %r3
+
+
+f3:
+.L4:
+	<entry-point>
+	set.64      %r5 <- -1.000000
+	ret.64      %r5
+
+
+ * check-output-end
+ */
diff --git a/validation/cast-constants.c b/validation/cast-constants.c
new file mode 100644
index 000000000..100424c22
--- /dev/null
+++ b/validation/cast-constants.c
@@ -0,0 +1,357 @@
+typedef unsigned int uint;
+typedef unsigned long ulong;
+
+static int uint_2_int(void) { return (int)123U; }
+static int long_2_int(void) { return (int)123L; }
+static int ulong_2_int(void) { return (int)123UL; }
+static int vptr_2_int(void) { return (int)((void*)123); }
+static int iptr_2_int(void) { return (int)((int*)128); }
+static int float_2_int(void) { return (int)1.123F; }
+static int double_2_int(void) { return (int)1.123L; }
+static uint int_2_uint(void) { return (uint)123; }
+static uint long_2_uint(void) { return (uint)123L; }
+static uint ulong_2_uint(void) { return (uint)123UL; }
+static uint vptr_2_uint(void) { return (uint)((void*)123); }
+static uint iptr_2_uint(void) { return (uint)((int*)128); }
+static uint float_2_uint(void) { return (uint)1.123F; }
+static uint double_2_uint(void) { return (uint)1.123L; }
+static long int_2_long(void) { return (long)123; }
+static long uint_2_long(void) { return (long)123U; }
+static long ulong_2_long(void) { return (long)123UL; }
+static long vptr_2_long(void) { return (long)((void*)123); }
+static long iptr_2_long(void) { return (long)((int*)128); }
+static long float_2_long(void) { return (long)1.123F; }
+static long double_2_long(void) { return (long)1.123L; }
+static ulong int_2_ulong(void) { return (ulong)123; }
+static ulong uint_2_ulong(void) { return (ulong)123U; }
+static ulong long_2_ulong(void) { return (ulong)123L; }
+static ulong vptr_2_ulong(void) { return (ulong)((void*)123); }
+static ulong iptr_2_ulong(void) { return (ulong)((int*)128); }
+static ulong float_2_ulong(void) { return (ulong)1.123F; }
+static ulong double_2_ulong(void) { return (ulong)1.123L; }
+static void * int_2_vptr(void) { return (void *)123; }
+static void * uint_2_vptr(void) { return (void *)123U; }
+static void * long_2_vptr(void) { return (void *)123L; }
+static void * ulong_2_vptr(void) { return (void *)123UL; }
+static void * iptr_2_vptr(void) { return (void *)((int*)128); }
+static int * int_2_iptr(void) { return (int *)123; }
+static int * uint_2_iptr(void) { return (int *)123U; }
+static int * long_2_iptr(void) { return (int *)123L; }
+static int * ulong_2_iptr(void) { return (int *)123UL; }
+static int * vptr_2_iptr(void) { return (int *)((void*)123); }
+static float int_2_float(void) { return (float)123; }
+static float uint_2_float(void) { return (float)123U; }
+static float long_2_float(void) { return (float)123L; }
+static float ulong_2_float(void) { return (float)123UL; }
+static float double_2_float(void) { return (float)1.123L; }
+static double int_2_double(void) { return (double)123; }
+static double uint_2_double(void) { return (double)123U; }
+static double long_2_double(void) { return (double)123L; }
+static double ulong_2_double(void) { return (double)123UL; }
+static double float_2_double(void) { return (double)1.123F; }
+
+/*
+ * check-name: cast-constants.c
+ * check-command: test-linearize $file
+ *
+ * check-output-start
+uint_2_int:
+.L0:
+	<entry-point>
+	ret.32      $123
+
+
+long_2_int:
+.L2:
+	<entry-point>
+	ret.32      $123
+
+
+ulong_2_int:
+.L4:
+	<entry-point>
+	ret.32      $123
+
+
+vptr_2_int:
+.L6:
+	<entry-point>
+	ret.32      $123
+
+
+iptr_2_int:
+.L8:
+	<entry-point>
+	ret.32      $128
+
+
+float_2_int:
+.L10:
+	<entry-point>
+	ret.32      $1
+
+
+double_2_int:
+.L12:
+	<entry-point>
+	ret.32      $1
+
+
+int_2_uint:
+.L14:
+	<entry-point>
+	ret.32      $123
+
+
+long_2_uint:
+.L16:
+	<entry-point>
+	ret.32      $123
+
+
+ulong_2_uint:
+.L18:
+	<entry-point>
+	ret.32      $123
+
+
+vptr_2_uint:
+.L20:
+	<entry-point>
+	ret.32      $123
+
+
+iptr_2_uint:
+.L22:
+	<entry-point>
+	ret.32      $128
+
+
+float_2_uint:
+.L24:
+	<entry-point>
+	ret.32      $1
+
+
+double_2_uint:
+.L26:
+	<entry-point>
+	ret.32      $1
+
+
+int_2_long:
+.L28:
+	<entry-point>
+	ret.64      $123
+
+
+uint_2_long:
+.L30:
+	<entry-point>
+	ret.64      $123
+
+
+ulong_2_long:
+.L32:
+	<entry-point>
+	ret.64      $123
+
+
+vptr_2_long:
+.L34:
+	<entry-point>
+	ret.64      $123
+
+
+iptr_2_long:
+.L36:
+	<entry-point>
+	ret.64      $128
+
+
+float_2_long:
+.L38:
+	<entry-point>
+	ret.64      $1
+
+
+double_2_long:
+.L40:
+	<entry-point>
+	ret.64      $1
+
+
+int_2_ulong:
+.L42:
+	<entry-point>
+	ret.64      $123
+
+
+uint_2_ulong:
+.L44:
+	<entry-point>
+	ret.64      $123
+
+
+long_2_ulong:
+.L46:
+	<entry-point>
+	ret.64      $123
+
+
+vptr_2_ulong:
+.L48:
+	<entry-point>
+	ret.64      $123
+
+
+iptr_2_ulong:
+.L50:
+	<entry-point>
+	ret.64      $128
+
+
+float_2_ulong:
+.L52:
+	<entry-point>
+	ret.64      $1
+
+
+double_2_ulong:
+.L54:
+	<entry-point>
+	ret.64      $1
+
+
+int_2_vptr:
+.L56:
+	<entry-point>
+	ret.64      $123
+
+
+uint_2_vptr:
+.L58:
+	<entry-point>
+	ret.64      $123
+
+
+long_2_vptr:
+.L60:
+	<entry-point>
+	ret.64      $123
+
+
+ulong_2_vptr:
+.L62:
+	<entry-point>
+	ret.64      $123
+
+
+iptr_2_vptr:
+.L64:
+	<entry-point>
+	ret.64      $128
+
+
+int_2_iptr:
+.L66:
+	<entry-point>
+	ret.64      $123
+
+
+uint_2_iptr:
+.L68:
+	<entry-point>
+	ret.64      $123
+
+
+long_2_iptr:
+.L70:
+	<entry-point>
+	ret.64      $123
+
+
+ulong_2_iptr:
+.L72:
+	<entry-point>
+	ret.64      $123
+
+
+vptr_2_iptr:
+.L74:
+	<entry-point>
+	ret.64      $123
+
+
+int_2_float:
+.L76:
+	<entry-point>
+	set.32      %r39 <- 123.000000
+	ret.32      %r39
+
+
+uint_2_float:
+.L78:
+	<entry-point>
+	set.32      %r41 <- 123.000000
+	ret.32      %r41
+
+
+long_2_float:
+.L80:
+	<entry-point>
+	set.32      %r43 <- 123.000000
+	ret.32      %r43
+
+
+ulong_2_float:
+.L82:
+	<entry-point>
+	set.32      %r45 <- 123.000000
+	ret.32      %r45
+
+
+double_2_float:
+.L84:
+	<entry-point>
+	set.32      %r47 <- 1.123000
+	ret.32      %r47
+
+
+int_2_double:
+.L86:
+	<entry-point>
+	set.64      %r49 <- 123.000000
+	ret.64      %r49
+
+
+uint_2_double:
+.L88:
+	<entry-point>
+	set.64      %r51 <- 123.000000
+	ret.64      %r51
+
+
+long_2_double:
+.L90:
+	<entry-point>
+	set.64      %r53 <- 123.000000
+	ret.64      %r53
+
+
+ulong_2_double:
+.L92:
+	<entry-point>
+	set.64      %r55 <- 123.000000
+	ret.64      %r55
+
+
+float_2_double:
+.L94:
+	<entry-point>
+	set.64      %r57 <- 1.123000
+	ret.64      %r57
+
+
+ * check-output-end
+ */
-- 
2.12.0

Re: Cast to double being removed

[Dibyendu Majumdar]

Hi Linus,

On 17 March 2017 at 00:24, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, Mar 16, 2017 at 5:00 PM, Dibyendu Majumdar
> <mobile@majumdar.org.uk> wrote:
>>
>> The cast to double has been removed. I am trying to understand where
>> this is happening as it seems to happen quite early on. Any tips on
>> where I should look?
>
> Since sparse was mostly done for kernel stuff, there's almost no
> floating point support anywhere.
>

So far the tests with floating points I have been running appear to
work after some fixes to sparse-llvm. This is the first one that seems
to be an upstream issue. I have found a floating point paranoia test
in the lcc compiler test suite - hopefully once I adapt this to run
with sparse-llvm it will help tease out any other bugs.

> There's something seriously screwed up with explicit double casting.
> See the differences between
>
>   double f1(void) { return -1; }
>   double f2(void) { return (double)-1; }
>   double f3(void) { return -1.0; }
>
> and notice how the first one and third ones actually kind-of make
> sense. The second one? Not so much, it just uses a 64-bit integer.
>
> The bug is already at the parse tree stage, the linearization doesn't
> even see the cast.
>
> I think it's this, in simplify_cast():
>
>         /* A cast of a constant? */
>         if (constant(src)) {
>                 int sign = orig_type->ctype.modifiers & MOD_SIGNED;
>                 long long val = get_cast_value(src->value, orig_size,
> size, sign);
>                 src = value_pseudo(val);
>                 goto simplify;
>         }
>
> which doesn't even check whether it's a FP value, it just looks if
> it's a constant and assumes it's an integer. At least it checks the
> sign ;)
>

Thank you for pointing me to the location where the issue might be.

Regards
Dibyendu

Re: Cast to double being removed

[Dibyendu Majumdar]

Hi Linus,

On 17 March 2017 at 10:38, Dibyendu Majumdar <mobile@majumdar.org.uk> wrote:
> On 17 March 2017 at 00:24, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>> There's something seriously screwed up with explicit double casting.
>> See the differences between
>>
>>   double f1(void) { return -1; }
>>   double f2(void) { return (double)-1; }
>>   double f3(void) { return -1.0; }
>>
>> and notice how the first one and third ones actually kind-of make
>> sense. The second one? Not so much, it just uses a 64-bit integer.
>>
>> The bug is already at the parse tree stage, the linearization doesn't
>> even see the cast.
>>
>> I think it's this, in simplify_cast():
>>
>>         /* A cast of a constant? */
>>         if (constant(src)) {
>>                 int sign = orig_type->ctype.modifiers & MOD_SIGNED;
>>                 long long val = get_cast_value(src->value, orig_size,
>> size, sign);
>>                 src = value_pseudo(val);
>>                 goto simplify;
>>         }
>>
>> which doesn't even check whether it's a FP value, it just looks if
>> it's a constant and assumes it's an integer. At least it checks the
>> sign ;)
>>
>
> Thank you for pointing me to the location where the issue might be.
>

It seems that not only here but elsewhere also during simplification
of expressions involving constants there is no check whether the
instruction type is floating point. As a test I simply disabled the
simplification if the instruction type is floating point. Good news is
that while before the paranoia test was hanging and giving many
errors, but now it says:

No failures, defects nor flaws have been discovered.
Rounding appears to conform to the proposed IEEE standard P754.
The arithmetic diagnosed appears to be Excellent!
END OF TEST.

Regards
Dibyendu

Re: Cast to double being removed

[Luc Van Oostenryck]

On Fri, Mar 17, 2017 at 12:00:00AM +0000, Dibyendu Majumdar wrote:
> Hi,
> 
> In this simple example:
> 
> extern int printf(const char *, ...);
> int main(void)
> {
>     printf("%f\n", (double)-1);
>     return 0;
> }
> 
> The linearized output is:
> 
> main:
> .L0:
>         <entry-point>
>         symaddr.64  %r1 <- <anon symbol:000001BAAB487258>
>         call.32     %r2 <- printf, %r1, $-1
>         ret.32      $0
> 
> The cast to double has been removed. I am trying to understand where
> this is happening as it seems to happen quite early on. Any tips on
> where I should look?

This is fixed by the patch I sent two days ago:
	https://patchwork.kernel.org/patch/9618765/

-- Luc 

Re: Cast to double being removed

[Dibyendu Majumdar]

Hi Luc,

On 19 March 2017 at 15:22, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Fri, Mar 17, 2017 at 12:00:00AM +0000, Dibyendu Majumdar wrote:
>> Hi,
>>
>> In this simple example:
>>
>> extern int printf(const char *, ...);
>> int main(void)
>> {
>>     printf("%f\n", (double)-1);
>>     return 0;
>> }
>>
>> The linearized output is:
>>
>> main:
>> .L0:
>>         <entry-point>
>>         symaddr.64  %r1 <- <anon symbol:000001BAAB487258>
>>         call.32     %r2 <- printf, %r1, $-1
>>         ret.32      $0
>>
>> The cast to double has been removed. I am trying to understand where
>> this is happening as it seems to happen quite early on. Any tips on
>> where I should look?
>
> This is fixed by the patch I sent two days ago:
>         https://patchwork.kernel.org/patch/9618765/
>

I found that in the simplification phase no account is taken of the
fact that a constant is floating point. Please try to run this test
and let me know if it works.

https://github.com/dibyendumajumdar/dmr_c/blob/master/tests/netlib/paranoia.c

I had to switch off simplifications of instructions that have a
floating point type.

Regards
Dibyendu

Re: Cast to double being removed

[Luc Van Oostenryck]

On Sun, Mar 19, 2017 at 4:43 PM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
> Hi Luc,
>>> The cast to double has been removed. I am trying to understand where
>>> this is happening as it seems to happen quite early on. Any tips on
>>> where I should look?
>>
>> This is fixed by the patch I sent two days ago:
>>         https://patchwork.kernel.org/patch/9618765/
>>
>
> I found that in the simplification phase no account is taken of the
> fact that a constant is floating point. Please try to run this test
> and let me know if it works.

I know. It's why I already sais several times that there is several issues with
floating point numbers.
I'll certainly look at these issues but currently I have around 40
pending patch series
for others problems ...

> https://github.com/dibyendumajumdar/dmr_c/blob/master/tests/netlib/paranoia.c

Interesting.
Do you know the license of this file?

> I had to switch off simplifications of instructions that have a
> floating point type.

OK, but the specific problem exposed by your example was something else,
a tiny braino most probably.

-- Luc

Re: Cast to double being removed

[Dibyendu Majumdar]

On 19 March 2017 at 15:54, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Sun, Mar 19, 2017 at 4:43 PM, Dibyendu Majumdar
> <mobile@majumdar.org.uk> wrote:
>> Hi Luc,
>>>> The cast to double has been removed. I am trying to understand where
>>>> this is happening as it seems to happen quite early on. Any tips on
>>>> where I should look?
>>>
>>> This is fixed by the patch I sent two days ago:
>>>         https://patchwork.kernel.org/patch/9618765/
>>>
>>
>> I found that in the simplification phase no account is taken of the
>> fact that a constant is floating point. Please try to run this test
>> and let me know if it works.
>
> I know. It's why I already sais several times that there is several issues with
> floating point numbers.
> I'll certainly look at these issues but currently I have around 40
> pending patch series
> for others problems ...
>
>> https://github.com/dibyendumajumdar/dmr_c/blob/master/tests/netlib/paranoia.c
>
> Interesting.
> Do you know the license of this file?
>

http://www.netlib.org/paranoia/index.html

In the source it says:

You may copy this program freely if you acknowledge its source.

I have seen it distributed in other places so I believe it is not restricted.

Regards
Dibyendu