Skip to content

fesetround() when called from Rust on Linux doesn't work correctly, but does on Windows, gcc, and clang. #118102

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
sarchar opened this issue Nov 20, 2023 · 8 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues.

Comments

@sarchar
Copy link

sarchar commented Nov 20, 2023

I know this is C code, but I promise this is about Rust. I tried this C code using extern "C" rust calls:

void test()
{
    fesetround(FE_UPWARD);
    printf("Should be -4: %d\n", (int)rint((double)-4.4));
}

I expected to see this happen: Should be -4: -4

Instead, this happened: Should be -4: -5

Meta

I have built a repository at https://github.com/sarchar/fesetroundbug but generally, this behavior only exists in the Linux build. With clang, gcc, and windows Rust, the correct value is displayed.

Please help.

@sarchar sarchar added the C-bug Category: This is a bug. label Nov 20, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 20, 2023
@ChrisDenton
Copy link
Member

What's the behaviour if you compile a pure C program?

@sarchar
Copy link
Author

sarchar commented Nov 20, 2023

What's the behaviour if you compile a pure C program?

I included the output of clang and gcc in my README

@sarchar
Copy link
Author

sarchar commented Nov 20, 2023

I just noticed that without the --release flag, the Linux build produces the correct result.

@sarchar
Copy link
Author

sarchar commented Nov 20, 2023

On Linux, with --release, objdump -d shows this code for the test function:

0000000000007700 <test>:
    7700:       48 83 ec 08             sub    $0x8,%rsp
    7704:       bf 00 08 00 00          mov    $0x800,%edi
    7709:       e8 22 d9 ff ff          callq  5030 <fesetround@plt>
    770e:       f2 0f 10 0d 02 d9 03    movsd  0x3d902(%rip),%xmm1        # 45018 <_fini+0x548>
    7715:       00
    7716:       f2 0f 10 15 12 d9 03    movsd  0x3d912(%rip),%xmm2        # 45030 <_fini+0x560>
    771d:       00
    771e:       f2 0f 10 1d fa d8 03    movsd  0x3d8fa(%rip),%xmm3        # 45020 <_fini+0x550>
    7725:       00
    7726:       66 0f 28 c1             movapd %xmm1,%xmm0
    772a:       66 0f 54 c2             andpd  %xmm2,%xmm0
    772e:       66 0f 2e d8             ucomisd %xmm0,%xmm3
    7732:       76 14                   jbe    7748 <test+0x48>
    7734:       f2 0f 58 c3             addsd  %xmm3,%xmm0
    7738:       66 0f 55 d1             andnpd %xmm1,%xmm2
    773c:       f2 0f 5c c3             subsd  %xmm3,%xmm0
    7740:       66 0f 28 c8             movapd %xmm0,%xmm1
    7744:       66 0f 56 ca             orpd   %xmm2,%xmm1
    7748:       f2 0f 2c f1             cvttsd2si %xmm1,%esi
    774c:       48 8d 3d ad d8 03 00    lea    0x3d8ad(%rip),%rdi        # 45000 <_fini+0x530>
    7753:       31 c0                   xor    %eax,%eax
    7755:       48 83 c4 08             add    $0x8,%rsp
    7759:       e9 f2 d8 ff ff          jmpq   5050 <printf@plt>
    775e:       66 90                   xchg   %ax,%ax

Without --release

00000000000077d5 <test>:
    77d5:       55                      push   %rbp
    77d6:       48 89 e5                mov    %rsp,%rbp
    77d9:       bf 00 08 00 00          mov    $0x800,%edi
    77de:       e8 4d d8 ff ff          callq  5030 <fesetround@plt>
    77e3:       48 8b 05 2e d8 03 00    mov    0x3d82e(%rip),%rax        # 45018 <_fini+0x498>
    77ea:       66 48 0f 6e c0          movq   %rax,%xmm0
    77ef:       e8 8c d8 ff ff          callq  5080 <rint@plt>
    77f4:       f2 0f 2c c0             cvttsd2si %xmm0,%eax
    77f8:       89 c6                   mov    %eax,%esi
    77fa:       48 8d 3d ff d7 03 00    lea    0x3d7ff(%rip),%rdi        # 45000 <_fini+0x480>
    7801:       b8 00 00 00 00          mov    $0x0,%eax
    7806:       e8 45 d8 ff ff          callq  5050 <printf@plt>
    780b:       90                      nop
    780c:       5d                      pop    %rbp
    780d:       c3                      retq
    780e:       66 90                   xchg   %ax,%ax

@saethlin
Copy link
Member

saethlin commented Nov 20, 2023

I think you are just rediscovering #72252 and the issue would be resolved by rust-lang/rfcs#3514 which says your program executes UB. If you think your program should not be UB, I encourage you to read the RFC and the discussion then engage with it.

@saethlin saethlin added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 21, 2023
@sarchar
Copy link
Author

sarchar commented Nov 21, 2023

A bit of extra information here is that optimized gcc also breaks, but clang doesn't:

sarchar@my-pc:~/src/fesetroundbug$ clang -o /tmp/test ./src/fenv.c -DTEST -lm -O3 && /tmp/test
Should be -4: -4
sarchar@my-pc:~/src/fesetroundbug$ gcc -o /tmp/test ./src/fenv.c -DTEST -lm -O3 && /tmp/test
Should be -4: -5
sarchar@my-pc:~/src/fesetroundbug$ gcc -o /tmp/test ./src/fenv.c -DTEST -lm && /tmp/test
Should be -4: -4

If this code is UB then fesetround (and the other intrinsics) should come with a huge notice that they don't always work correctly.

@sarchar
Copy link
Author

sarchar commented Nov 21, 2023

To prevent compiler optimization with rint(), I call it from assembly:

    double a = ...;
    fesetround(FE_UPWARD);
    asm __volatile__(
            "movsd %1,%%xmm0\n\t"
            "call rint\n\t"
            "movsd %%xmm0,%0\n\t"
            : "=m"(a) : "m"(a) : "xmm0", "memory");

And this seems to produce correct results on Rust, clang, and gcc all with and without optimizations.

@sarchar sarchar closed this as completed Nov 21, 2023
@RalfJung
Copy link
Member

If this code is UB then fesetround (and the other intrinsics) should come with a huge notice that they don't always work correctly.

Yes they should. This code is UB in most C compilers as well. You need to set some #pragma to tell the compiler that the rounding mode may have changed. Also see this.

This should be filed as a bug against the libm man pages.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues.
Projects
None yet
Development

No branches or pull requests

5 participants