#12614 closed bug (fixed)

Integer division can overwrite other arguments to foreign call

Reported by: jscholl Owned by:
Priority: high Milestone: 8.0.2
Component: Compiler (NCG) Version: 8.0.1
Keywords: integer division Cc: hsyl20
Operating System: Unknown/Multiple Architecture: x86_64 (amd64)
Type of failure: Incorrect result at runtime Test Case:
Blocked By: Blocking:
Related Tickets: #11792 Differential Rev(s): Phab:D2263
Wiki Page:

Description

If you call a foreign function, GHC can generate incorrect code while passing the arguments to the function, overwriting the 3rd argument if a later argument contains an integer division.

Main.hs:

{-# LANGUAGE ForeignFunctionInterface #-}
module Main where

{-# NOINLINE foo #-}
foo :: Int -> IO ()
foo x = c_foo 0 0 x $ x + x `quot` 10

foreign import ccall "foo" c_foo :: Int -> Int -> Int -> Int -> IO ()

main :: IO ()
main = do
    foo 202
    foo 203
    foo 204

foo.c:

#include <stdio.h>

void foo(int a, int b, int c, int d) {
    printf("%d, %d, %d, %d\n", a, b, c, d);
}

Expected output:

0, 0, 202, 222
0, 0, 203, 223
0, 0, 204, 224

Actual output:

0, 0, 2, 222
0, 0, 3, 223
0, 0, 4, 224

The bug has to be somewhere in the code generator. The cmm reads:

call "ccall" arg hints:  [‘signed’, ‘signed’, ‘signed’, ‘signed’]  result hints:  [] foo(0, 0, _s3nE::I64, _s3nE::I64 + %MO_S_Quot_W64(_s3nE::I64, 10));

This generates the following assembler code:

        xorl %edi,%edi
        xorl %esi,%esi
        movq %rbx,%rdx
        movl $10,%ecx
        movq %rax,%rdx  <-- move 3rd argument into rdx
        movq %rbx,%rax
        movq %rdx,%r8
        cqto
        idivq %rcx      <-- rax := rax / rcx; rdx := rax % rcx
        movq %rbx,%rcx
        addq %rax,%rcx
        subq $8,%rsp
        xorl %eax,%eax
        movq %r8,%rbx
        call foo

Thus rdx is overwritten again before the call, leading to incorrect results.

Change History (7)

comment:1 Changed 14 months ago by hsyl20

This looks very similar to #11792. Isn't the foreign call unsafe in your test case?

comment:2 Changed 14 months ago by jscholl

You are right, it is most likely the same root cause. But no, the foreign call is not unsafe as far as I understand (the default is safe, right?). Also if I change it to an explicit safe or unsafe call the behavior is unchanged, so this does not seem to be a factor in this case.

comment:3 Changed 14 months ago by hsyl20

I have fixed both this issue and #11792 in Phab:D2263. I have added your failing example (extended to test pushed args) as a test too.

comment:4 Changed 14 months ago by hsyl20

Cc: hsyl20 added
Differential Rev(s): Phab:D2263
Milestone: 8.0.2
Priority: normalhigh
Status: newpatch

comment:5 Changed 14 months ago by Ben Gamari <ben@…>

In b61b7c24/ghc:

CodeGen X86: fix unsafe foreign calls wrt inlining

Foreign calls (unsafe and safe) interact badly with inlining and
register passing ABIs (see #11792 and #12614):
the inlined code to compute a parameter of the call may overwrite a
register already set to pass a preceding parameter.

With this patch, we compute all parameters which are not simple
expressions before assigning them to fixed registers required by the
ABI.

Test Plan:
   - Add test (test both reg and stack parameters)
   - Validate

Reviewers: osa1, bgamari, austin, simonmar

Reviewed By: simonmar

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D2263

GHC Trac Issues: #11792, #12614

comment:6 Changed 14 months ago by bgamari

Status: patchmerge

comment:7 Changed 14 months ago by bgamari

Resolution: fixed
Status: mergeclosed

Comment:5 merged to ghc-8.0 as aec4a514857d051f5eeb15b9cbc8e6dd29850c5f

Note: See TracTickets for help on using tickets.