Calling C function via NLFFI binding may give result outside range of its C return type

[pclayton]

Version

110.99.3 (Latest)

Operating System

• Any
• Linux
• macOS
• Windows
• Other Unix

OS Version

Linux fedora 6.0.18-200.fc36.x86_64 #1 SMP PREEMPT_DYNAMIC Sat Jan 7 17:08:48 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Processor

• Any
• Arm (using Rosetta)
• PowerPC
• Sparc
• x86 (32-bit)
• x86-64 (64-bit)
• Other

System Component

Foreign-Function Interface (FFI)

Severity

Minor

Description

For a C function whose return type is less than 32 bits, i.e. 8 or 16 bits, NLFFI generates bindings where the ML result type is Int32.int or Word32.word. Although the ML type can represent a greater range of values than the C function returns, I would not expect the returned ML value to be outside the range of values that the C function can return. However, under certain conditions, the ML value is outside the range. The ML value is correct in the bits represented by the C function return type but higher bits may be 1, giving a value out of range. It appears that the code generated by NLFFI does not mask out the higher bits if a return type has fewer than 32 bits. The reason this can occur is because the value in the eax register is returned when only the value in the ax or al register is wanted.

Transcript

The attached example produces the following output where the value of c is not in the range of an unsigned char:

[pclayton@fedora tmp]$ tar -xzf nlffi-return-uchar-issue-20230309.tar.gz 
[pclayton@fedora tmp]$ cd nlffi-return-uchar-issue-20230309/
[pclayton@fedora nlffi-return-uchar-issue-20230309]$ make -f test_1.make 
gcc -m32 -fPIC -g -Wall -c test_1_a.c -o test_1_a.o
gcc -m32 -shared -Wl,-soname,test_1_lib.so -o test_1_lib.so test_1_a.o
[pclayton@fedora nlffi-return-uchar-issue-20230309]$ sml -32 < test-1.sml
Standard ML of New Jersey (32-bit) v110.99.3 [built: Thu Mar 09 12:23:24 2023]
- [autoloading]
[library $smlnj/cm/cm.cm is stable]
[library $smlnj/internal/cm-sig-lib.cm is stable]
[library $/pgraph.cm is stable]
[library $smlnj/internal/srcpath-lib.cm is stable]
[library $SMLNJ-BASIS/basis.cm is stable]
[library $SMLNJ-BASIS/(basis.cm):basis-common.cm is stable]
[autoloading done]
[scanning test-1.cm]
[library $c/c.cm is stable]
[attempting to load plugin $/make-tool.cm]
[library $/make-tool.cm is stable]
[library $smlnj/internal/cm-lib.cm is stable]
$Execute: required privileges are:
  cm-init
[library $smlnj/cm/tools.cm is stable]
[plugin $/make-tool.cm loaded successfully]
[make -f test_1_lib.make SMLNJ_BINDIR=/opt/smlnj/smlnj-110.99.3/bin FFI/test-1-lib.cm]
ml-nlffigen -32 -include ../test-1-lib.sml -libhandle Test1Lib.libh -dir FFI -cmfile test-1-lib.cm test_1_a.h
[scanning (test-1.cm):FFI/test-1-lib.cm]
[library $c/internals/c-int.cm is stable]
[parsing (test-1.cm):FFI/(test-1-lib.cm):f-gt.sml]
[creating directory FFI/.cm/SKEL]
[parsing (test-1.cm):FFI/(test-1-lib.cm):t-bool.sml]
[parsing (test-1.cm):FFI/(test-1-lib.cm):callop-0.sml]
[parsing (test-1.cm):FFI/(test-1-lib.cm):fptr-rtti-0.sml]
[library $c/memory/memory.cm is stable]
[compiling (test-1.cm):FFI/(test-1-lib.cm):t-bool.sml]
[creating directory FFI/.cm/x86-unix]
[code: 110, data: 54, env: 115 bytes]
binfile format error: bad magic number "x86-110.99.1    ", expected "x86-110.99.3    "
[compiling (test-1.cm):FFI/(test-1-lib.cm):../test-1-lib.sml]
[code: 564, data: 44, env: 212 bytes]
[compiling (test-1.cm):FFI/(test-1-lib.cm):callop-0.sml]
[code: 304, env: 597 bytes]
[compiling (test-1.cm):FFI/(test-1-lib.cm):fptr-rtti-0.sml]
[code: 652, env: 248 bytes]
[compiling (test-1.cm):FFI/(test-1-lib.cm):f-gt.sml]
[code: 1119, data: 31, env: 406 bytes]
$Execute: required privileges are:
  c-int
  primitive
[New bindings added.]
[New bindings added.]
= val a = 0wx1 : Word32.word
val b = 0wx0 : Word32.word
val c = 0wx100 : Word32.word

Expected Behavior

In the transcript above, I would expect to see

...
val c = 0wx0 : Word32.word

Steps to Reproduce

The example nlffi-return-uchar-issue-20230309.tar.gz demonstrates this issue as follows:

tar -xzf nlffi-return-uchar-issue-20230309.tar.gz
cd nlffi-return-uchar-issue-20230309/
make -f test_1.make
sml -32 < test-1.sml

In the output (see the transcript above) it can be seen that c has the value 0wx100 which is outside the range of unsigned char. This behavior depends on how gcc compiles the function gt. For details, see the comments in the file test-1.sml.

Note that the transcript above contains

binfile format error: bad magic number "x86-110.99.1    ", expected "x86-110.99.3    "

I don't think this is related.

Additional Information

Here is the output from the my gdb session where I found the issue:

[pclayton@fedora test_2]$ gdb /opt/smlnj/smlnj-110.99.1/bin/.run/run.x86-linux
GNU gdb (GDB) Fedora 12.1-2.fc36
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later [<http://gnu.org/licenses/gpl.html>](http://gnu.org/licenses/gpl.html)
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
[<https://www.gnu.org/software/gdb/bugs/>](https://www.gnu.org/software/gdb/bugs/).
Find the GDB manual and other documentation resources online at:
    [<http://www.gnu.org/software/gdb/documentation/>](http://www.gnu.org/software/gdb/documentation/).

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /opt/smlnj/smlnj-110.99.1/bin/.run/run.x86-linux...

This GDB supports auto-downloading debuginfo from the following URLs:
https://debuginfod.fedoraproject.org/ 
Enable debuginfod for this session? (y or [n]) 
Debuginfod has been disabled.
To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
(No debugging symbols found in /opt/smlnj/smlnj-110.99.1/bin/.run/run.x86-linux)
(gdb) break gt
Function "gt" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (gt) pending.
(gdb) run
Starting program: /opt/smlnj/smlnj-110.99.1/bin/.run/run.x86-linux 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
/opt/smlnj/smlnj-110.99.1/bin/.run/run.x86-linux: Fatal error -- no in-core heap image found

[Inferior 1 (process 667036) exited with code 01]
Missing separate debuginfos, use: dnf debuginfo-install glibc-2.35-22.fc36.i686
(gdb) run @SMLload=/opt/smlnj/smlnj-110.99.1/bin/.heap/sml.x86-linux
Starting program: /opt/smlnj/smlnj-110.99.1/bin/.run/run.x86-linux @SMLload=/opt/smlnj/smlnj-110.99.1/bin/.heap/sml.x86-linux
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Standard ML of New Jersey (32-bit) v110.99.1 [built: Thu Sep 02 11:12:52 2021]
- CM.make "test-1.cm";
[autoloading]
[library $smlnj/cm/cm.cm is stable]
[library $smlnj/internal/cm-sig-lib.cm is stable]
[library $/pgraph.cm is stable]
[library $smlnj/internal/srcpath-lib.cm is stable]
[library $SMLNJ-BASIS/basis.cm is stable]
[library $SMLNJ-BASIS/(basis.cm):basis-common.cm is stable]
[autoloading done]
[scanning test-1.cm]
[library $c/c.cm is stable]
[attempting to load plugin $/make-tool.cm]
[library $/make-tool.cm is stable]
[library $smlnj/internal/cm-lib.cm is stable]
$Execute: required privileges are:
  cm-init
[library $smlnj/cm/tools.cm is stable]
[plugin $/make-tool.cm loaded successfully]
[make -f test_1_lib.make SMLNJ_BINDIR=/opt/smlnj/smlnj-110.99.1/bin FFI/test-1-lib.cm]
[Detaching after fork from child process 667088]
make: 'FFI/test-1-lib.cm' is up to date.
[scanning (test-1.cm):FFI/test-1-lib.cm]
[library $c/internals/c-int.cm is stable]
[library $c/memory/memory.cm is stable]
[loading (test-1.cm):FFI/(test-1-lib.cm):t-bool.sml]
[loading (test-1.cm):FFI/(test-1-lib.cm):../test-1-lib.sml]
[loading (test-1.cm):FFI/(test-1-lib.cm):callop-1.sml]
[loading (test-1.cm):FFI/(test-1-lib.cm):fptr-rtti-1.sml]
[loading (test-1.cm):FFI/(test-1-lib.cm):f-gt.sml]
[loading (test-1.cm):FFI/(test-1-lib.cm):callop-0.sml]
[loading (test-1.cm):FFI/(test-1-lib.cm):fptr-rtti-0.sml]
[loading (test-1.cm):FFI/(test-1-lib.cm):f-dummy.sml]
$Execute: required privileges are:
  c-int
  primitive
[New bindings added.]
val it = true : bool
- CM.make "$c/c.cm";
[New bindings added.]
val it = true : bool
- F_gt.f (12345, 12345);

Breakpoint 1, gt (x=12345, y=12345) at test_1_a.c:6
6	  return x > y;
(gdb) disas
Dump of assembler code for function gt:
   0xf7cd114d <+0>:	push   %ebp
   0xf7cd114e <+1>:	mov    %esp,%ebp
   0xf7cd1150 <+3>:	call   0xf7cd1179 <__x86.get_pc_thunk.ax>
   0xf7cd1155 <+8>:	add    $0x2eab,%eax
=> 0xf7cd115a <+13>:	mov    0x8(%ebp),%eax
   0xf7cd115d <+16>:	cmp    0xc(%ebp),%eax
   0xf7cd1160 <+19>:	setg   %al
   0xf7cd1163 <+22>:	pop    %ebp
   0xf7cd1164 <+23>:	ret    
End of assembler dump.
(gdb) print $eax
$4 = -137543680
(gdb) ni 
0xf7cd115d	6	  return x > y;
(gdb) disas
Dump of assembler code for function gt:
   0xf7cd114d <+0>:	push   %ebp
   0xf7cd114e <+1>:	mov    %esp,%ebp
   0xf7cd1150 <+3>:	call   0xf7cd1179 <__x86.get_pc_thunk.ax>
   0xf7cd1155 <+8>:	add    $0x2eab,%eax
   0xf7cd115a <+13>:	mov    0x8(%ebp),%eax
=> 0xf7cd115d <+16>:	cmp    0xc(%ebp),%eax
   0xf7cd1160 <+19>:	setg   %al
   0xf7cd1163 <+22>:	pop    %ebp
   0xf7cd1164 <+23>:	ret    
End of assembler dump.
(gdb) print $eax
$5 = 12345
(gdb) ni 
0xf7cd1160	6	  return x > y;
(gdb) disas
Dump of assembler code for function gt:
   0xf7cd114d <+0>:	push   %ebp
   0xf7cd114e <+1>:	mov    %esp,%ebp
   0xf7cd1150 <+3>:	call   0xf7cd1179 <__x86.get_pc_thunk.ax>
   0xf7cd1155 <+8>:	add    $0x2eab,%eax
   0xf7cd115a <+13>:	mov    0x8(%ebp),%eax
   0xf7cd115d <+16>:	cmp    0xc(%ebp),%eax
=> 0xf7cd1160 <+19>:	setg   %al
   0xf7cd1163 <+22>:	pop    %ebp
   0xf7cd1164 <+23>:	ret    
End of assembler dump.
(gdb) print $eax
$6 = 12345
(gdb) ni 
7	}
(gdb) disas
Dump of assembler code for function gt:
   0xf7cd114d <+0>:	push   %ebp
   0xf7cd114e <+1>:	mov    %esp,%ebp
   0xf7cd1150 <+3>:	call   0xf7cd1179 <__x86.get_pc_thunk.ax>
   0xf7cd1155 <+8>:	add    $0x2eab,%eax
   0xf7cd115a <+13>:	mov    0x8(%ebp),%eax
   0xf7cd115d <+16>:	cmp    0xc(%ebp),%eax
   0xf7cd1160 <+19>:	setg   %al
=> 0xf7cd1163 <+22>:	pop    %ebp
   0xf7cd1164 <+23>:	ret    
End of assembler dump.
(gdb) print $eax
$7 = 12288
(gdb) c
Continuing.
val it = 0wx3000 : Word32.word
-

Email address

phil.clayton@veonix.com

[JohnReppy]

The bug is in MLRISC/x86/c-calls/ia32-svid.sml, where the ABI is not being honored for 8-bit and 16-bit integer return values. The fix is going to be to zero/sign extend the result, but we will need to change some of the representations to make this work.

[JohnReppy]

I have implemented a fix for this issue that appears to be generating the correct machine code, but it will need to be verified on a 32-bit Linux system (the macOS C compiler does not exhibit the bug). The fix was to modify the CPS C-calls code to add size/zero-extension of the result when the C prototype's return type is a small integer type. As it turned out, MLRISC did not handle the zero-extension case correctly (it assumed that the high bits were guaranteed to be zero), so it was also necessary to change the mltree-gen.sml code.

[pclayton]

I have tested ddccb5d on my Linux machine and this resolves the issue for 32-bit SML/NJ. In the output, I now see:

val c = 0wx0 : Word32.word

as expected.

[JohnReppy]

Fixed for 110.99.4.