Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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