Stream: git-wasmtime

Topic: wasmtime / issue #4828 cranelift-fuzzgen fuzzbug: fcmp gi...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2022 at 00:53):

jameysharp opened issue #4828:

Original report: https://oss-fuzz.com/testcase-detail/5662899036618752

<details>
<summary>Test case input</summary>

<!-- Please base64-encode the input that libFuzzer generated, and paste it in the code-block below. This is required for us to reproduce the issue. -->

YWlzY3RuIG8BXQAAe/HfLJGaiKD4/wAA6bZjQHhAcB7//wAHAAAA6bYgJgAB6fgAAAABAABK//8A
ABgAAAD/////f///////////////////////////////////////////////////////////////
/////////////////ydV1sbWP9aCKgKCtgoFAGUAAAAAAAAAm/4p2gFG8J19fQAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAABlAWzzxkbacCrdpP//AAAAAAD6/wBDQ0P///////////////////////8A
ZcYNRyYxRvSWnfAyAAAAADsgAakAfX0AAP+udf8AAA8AAAAAAPr/AENDQ0NDm5ubm5ubm5ubm5ub
m5ubm5ubm5ubm5ubfX3zfZubfQ==

</details>

I've minimized this to the following CLIF test:

test interpret
test run
target aarch64
target s390x
target x86_64

function %fn(f64 uext) -> b1 system_v {

block0(v3: f64):
    v78 = fcmp le v3, v3
    return v78
}

; run: %fn(-NaN:0x7ffffffffffff) == false

On x86-64, this call returns false; but on the interpreter it returns true. Those two implementations have disagreed at least back to June, so this doesn't seem to be due to any recent changes.

I wasn't sure what the "right" answer is here so I wrote a quick wasm program for which wasmtime generates this CLIF instruction.

<details>
<summary>Equivalent wasm</summary>

(module
  (func $fn (param f64)
    local.get 0
    local.get 0
    f64.le
    if
      unreachable
    end
  )
  (func $test
    f64.const -nan:0x7ffffffffffff
    call $fn
  )
  (export "test" (func $test))
)

</details>

This program doesn't trap either on wasmtime compiling for x86-64, or on wabt's wasm-interp, so I gather the Cranelift interpreter is the one giving the wrong answer here.

@afonso360, can you look into this?

I guess we should extend cranelift/filetests/filetests/runtests/fcmp.clif with NaN tests, and also start running that test file on the interpreter.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2022 at 00:53):

jameysharp labeled issue #4828:

Original report: https://oss-fuzz.com/testcase-detail/5662899036618752

<details>
<summary>Test case input</summary>

<!-- Please base64-encode the input that libFuzzer generated, and paste it in the code-block below. This is required for us to reproduce the issue. -->

YWlzY3RuIG8BXQAAe/HfLJGaiKD4/wAA6bZjQHhAcB7//wAHAAAA6bYgJgAB6fgAAAABAABK//8A
ABgAAAD/////f///////////////////////////////////////////////////////////////
/////////////////ydV1sbWP9aCKgKCtgoFAGUAAAAAAAAAm/4p2gFG8J19fQAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAABlAWzzxkbacCrdpP//AAAAAAD6/wBDQ0P///////////////////////8A
ZcYNRyYxRvSWnfAyAAAAADsgAakAfX0AAP+udf8AAA8AAAAAAPr/AENDQ0NDm5ubm5ubm5ubm5ub
m5ubm5ubm5ubm5ubfX3zfZubfQ==

</details>

I've minimized this to the following CLIF test:

test interpret
test run
target aarch64
target s390x
target x86_64

function %fn(f64 uext) -> b1 system_v {

block0(v3: f64):
    v78 = fcmp le v3, v3
    return v78
}

; run: %fn(-NaN:0x7ffffffffffff) == false

On x86-64, this call returns false; but on the interpreter it returns true. Those two implementations have disagreed at least back to June, so this doesn't seem to be due to any recent changes.

I wasn't sure what the "right" answer is here so I wrote a quick wasm program for which wasmtime generates this CLIF instruction.

<details>
<summary>Equivalent wasm</summary>

(module
  (func $fn (param f64)
    local.get 0
    local.get 0
    f64.le
    if
      unreachable
    end
  )
  (func $test
    f64.const -nan:0x7ffffffffffff
    call $fn
  )
  (export "test" (func $test))
)

</details>

This program doesn't trap either on wasmtime compiling for x86-64, or on wabt's wasm-interp, so I gather the Cranelift interpreter is the one giving the wrong answer here.

@afonso360, can you look into this?

I guess we should extend cranelift/filetests/filetests/runtests/fcmp.clif with NaN tests, and also start running that test file on the interpreter.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2022 at 00:53):

jameysharp labeled issue #4828:

Original report: https://oss-fuzz.com/testcase-detail/5662899036618752

<details>
<summary>Test case input</summary>

<!-- Please base64-encode the input that libFuzzer generated, and paste it in the code-block below. This is required for us to reproduce the issue. -->

YWlzY3RuIG8BXQAAe/HfLJGaiKD4/wAA6bZjQHhAcB7//wAHAAAA6bYgJgAB6fgAAAABAABK//8A
ABgAAAD/////f///////////////////////////////////////////////////////////////
/////////////////ydV1sbWP9aCKgKCtgoFAGUAAAAAAAAAm/4p2gFG8J19fQAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAABlAWzzxkbacCrdpP//AAAAAAD6/wBDQ0P///////////////////////8A
ZcYNRyYxRvSWnfAyAAAAADsgAakAfX0AAP+udf8AAA8AAAAAAPr/AENDQ0NDm5ubm5ubm5ubm5ub
m5ubm5ubm5ubm5ubfX3zfZubfQ==

</details>

I've minimized this to the following CLIF test:

test interpret
test run
target aarch64
target s390x
target x86_64

function %fn(f64 uext) -> b1 system_v {

block0(v3: f64):
    v78 = fcmp le v3, v3
    return v78
}

; run: %fn(-NaN:0x7ffffffffffff) == false

On x86-64, this call returns false; but on the interpreter it returns true. Those two implementations have disagreed at least back to June, so this doesn't seem to be due to any recent changes.

I wasn't sure what the "right" answer is here so I wrote a quick wasm program for which wasmtime generates this CLIF instruction.

<details>
<summary>Equivalent wasm</summary>

(module
  (func $fn (param f64)
    local.get 0
    local.get 0
    f64.le
    if
      unreachable
    end
  )
  (func $test
    f64.const -nan:0x7ffffffffffff
    call $fn
  )
  (export "test" (func $test))
)

</details>

This program doesn't trap either on wasmtime compiling for x86-64, or on wabt's wasm-interp, so I gather the Cranelift interpreter is the one giving the wrong answer here.

@afonso360, can you look into this?

I guess we should extend cranelift/filetests/filetests/runtests/fcmp.clif with NaN tests, and also start running that test file on the interpreter.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2022 at 16:36):

akirilov-arm commented on issue #4828:

AFAIK all comparisons involving a NaN (including those when both operands are NaNs) should return false, except for inequality.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2022 at 16:45):

afonso360 commented on issue #4828:

That is true for the regular comparisons, however there are some FloatCC::Unordered* that invert that and always return true on NaN's.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2022 at 16:49):

afonso360 edited a comment on issue #4828:

That is true for the regular comparisons, however there are some FloatCC::Unordered* that invert that and always return true on NaN's. That isn't the case here, this is a Ordered comparison, so we should be returning false as jamey mentions.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2022 at 16:49):

afonso360 edited a comment on issue #4828:

That is true for the regular comparisons, however there are some FloatCC::Unordered* that invert that and always return true on NaN's. That isn't the case here, this is a Ordered comparison, so we should be returning false as jamey mentions.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2022 at 16:50):

afonso360 edited a comment on issue #4828:

That is true for the regular comparisons, however there are some FloatCC::Unordered* that invert that and always return true on NaN's. That isn't the case here, this is a Ordered comparison, so we should be returning false as jameysharp mentions.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 02 2022 at 15:47):

akirilov-arm labeled issue #4828:

Original report: https://oss-fuzz.com/testcase-detail/5662899036618752

<details>
<summary>Test case input</summary>

<!-- Please base64-encode the input that libFuzzer generated, and paste it in the code-block below. This is required for us to reproduce the issue. -->

YWlzY3RuIG8BXQAAe/HfLJGaiKD4/wAA6bZjQHhAcB7//wAHAAAA6bYgJgAB6fgAAAABAABK//8A
ABgAAAD/////f///////////////////////////////////////////////////////////////
/////////////////ydV1sbWP9aCKgKCtgoFAGUAAAAAAAAAm/4p2gFG8J19fQAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAABlAWzzxkbacCrdpP//AAAAAAD6/wBDQ0P///////////////////////8A
ZcYNRyYxRvSWnfAyAAAAADsgAakAfX0AAP+udf8AAA8AAAAAAPr/AENDQ0NDm5ubm5ubm5ubm5ub
m5ubm5ubm5ubm5ubfX3zfZubfQ==

</details>

I've minimized this to the following CLIF test:

test interpret
test run
target aarch64
target s390x
target x86_64

function %fn(f64 uext) -> b1 system_v {

block0(v3: f64):
    v78 = fcmp le v3, v3
    return v78
}

; run: %fn(-NaN:0x7ffffffffffff) == false

On x86-64, this call returns false; but on the interpreter it returns true. Those two implementations have disagreed at least back to June, so this doesn't seem to be due to any recent changes.

I wasn't sure what the "right" answer is here so I wrote a quick wasm program for which wasmtime generates this CLIF instruction.

<details>
<summary>Equivalent wasm</summary>

(module
  (func $fn (param f64)
    local.get 0
    local.get 0
    f64.le
    if
      unreachable
    end
  )
  (func $test
    f64.const -nan:0x7ffffffffffff
    call $fn
  )
  (export "test" (func $test))
)

</details>

This program doesn't trap either on wasmtime compiling for x86-64, or on wabt's wasm-interp, so I gather the Cranelift interpreter is the one giving the wrong answer here.

@afonso360, can you look into this?

I guess we should extend cranelift/filetests/filetests/runtests/fcmp.clif with NaN tests, and also start running that test file on the interpreter.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 02 2022 at 17:42):

jameysharp closed issue #4828:

Original report: https://oss-fuzz.com/testcase-detail/5662899036618752

<details>
<summary>Test case input</summary>

<!-- Please base64-encode the input that libFuzzer generated, and paste it in the code-block below. This is required for us to reproduce the issue. -->

YWlzY3RuIG8BXQAAe/HfLJGaiKD4/wAA6bZjQHhAcB7//wAHAAAA6bYgJgAB6fgAAAABAABK//8A
ABgAAAD/////f///////////////////////////////////////////////////////////////
/////////////////ydV1sbWP9aCKgKCtgoFAGUAAAAAAAAAm/4p2gFG8J19fQAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAABlAWzzxkbacCrdpP//AAAAAAD6/wBDQ0P///////////////////////8A
ZcYNRyYxRvSWnfAyAAAAADsgAakAfX0AAP+udf8AAA8AAAAAAPr/AENDQ0NDm5ubm5ubm5ubm5ub
m5ubm5ubm5ubm5ubfX3zfZubfQ==

</details>

I've minimized this to the following CLIF test:

test interpret
test run
target aarch64
target s390x
target x86_64

function %fn(f64 uext) -> b1 system_v {

block0(v3: f64):
    v78 = fcmp le v3, v3
    return v78
}

; run: %fn(-NaN:0x7ffffffffffff) == false

On x86-64, this call returns false; but on the interpreter it returns true. Those two implementations have disagreed at least back to June, so this doesn't seem to be due to any recent changes.

I wasn't sure what the "right" answer is here so I wrote a quick wasm program for which wasmtime generates this CLIF instruction.

<details>
<summary>Equivalent wasm</summary>

(module
  (func $fn (param f64)
    local.get 0
    local.get 0
    f64.le
    if
      unreachable
    end
  )
  (func $test
    f64.const -nan:0x7ffffffffffff
    call $fn
  )
  (export "test" (func $test))
)

</details>

This program doesn't trap either on wasmtime compiling for x86-64, or on wabt's wasm-interp, so I gather the Cranelift interpreter is the one giving the wrong answer here.

@afonso360, can you look into this?

I guess we should extend cranelift/filetests/filetests/runtests/fcmp.clif with NaN tests, and also start running that test file on the interpreter.

Last updated: Nov 22 2024 at 16:03 UTC