Opened 9 years ago

Closed 9 years ago

#788 closed defect (fixed)

ARM FFI problems, partial patch

Reported by: ivan4th Owned by:
Priority: normal Milestone:
Component: Foreign Function Interface Version: trunk
Keywords: Cc:

Description

I've discovered multiple problems in ARM FFI while trying to run CFFI test suite.

  1. There was a stray apostrophe in lib/ffi-linuxarm.lisp in arm-linux::generate-callback-bindings which caused problems with compilation. Fixed in the patch.
  1. There was an error in s8->fixnum vinsn (val / result confusion, shift in the wrong direction). Attached charfail.lisp demonstrates the problem. Fixed in the patch.
  1. There was a problem in mem-set-c-single-float / mem-set-single-float, val/src confusion that didn't seem to affect anything most of the time by pure accident plus bad index reference in mem-set-single-float. This is demonstrated by the second assert in attached floatfail.lisp. Fixed in the patch.
  1. There is still a problem with mem-set-c-double-float, mem-set-double-float, get-double, get-double? and maybe other functions. Besides ignoring val and using src instead in mem-set(-c)-double-float, there's a more serious problem that I was unable to fix.

Namely, these functions use explicit register specifications for their temps that are used for strd/ldrd instructions that need consective registers. This seems to break register handling somehow, as the following can be observed at the end of disassembly of my-mem-set-double-float in attached floatfail.lisp:

  (ldrd imm0 (:@ arg_z (:$ 2)))
  (fmdrr d0 imm0 imm1)
  (add nargs imm0 imm1)
  (fmrrd imm0 imm1 d0)
  (strd imm0 (:@ nargs (:$ 0)))
  (ldmia (:! sp) (imm0 vsp fn pc))

Here we see imm0 and imm1 getting destroyed by get-double vinsn and later being used to construct address for the following strd as if nothing happened. This leads to memory fault.

The possible workaround is not using strd/ldrd at all but perhaps it's also possible to fix temp handling, but I don't have time to do it myself right now.

Attachments (3)

ccl-arm-ffi-2010-12-08.patch (1.7 KB) - added by ivan4th 9 years ago.
charfail.lisp (250 bytes) - added by ivan4th 9 years ago.
floatfail.lisp (1.2 KB) - added by ivan4th 9 years ago.

Download all attachments as: .zip

Change History (6)

Changed 9 years ago by ivan4th

Changed 9 years ago by ivan4th

Changed 9 years ago by ivan4th

comment:1 Changed 9 years ago by ivan4th

s/Namely, these functions/Namely, these vinsns/

comment:2 Changed 9 years ago by ivan4th

s/maybe other functions/maybe other vinsns/ too

comment:3 Changed 9 years ago by gb

  • Resolution set to fixed
  • Status changed from new to closed

This should all be fixed in the trunk in r14472; thanks.

Part of the fix involved making it legal (GC-wise) for the LR to point at the "pad" word of a DOUBLE-FLOAT or DOUBLE-FLOAT vector, so that we can use floating-point load and store instructions in more cases (without having to use intermediate GPRs). This involves a small change to the GC, so the kernel should be recompiled before exercising these changes.

Note: See TracTickets for help on using tickets.