search

diku-dk / smlfut

Call Futhark programs from Standard ML
GNU General Public License v3.0
10 stars 1 forks source link

MLton/MPL doesn't like the `strlen` and `strcpy` imports for error handling #2

Closed shwestrick closed 1 year ago

shwestrick commented 1 year ago

The generated code uses _import "strlen" public : ... and _import "strcpy" public : ... to manipulate strings returned by futhark_context_get_error.

However, the MLton/MPL FFI isn't happy with these imports:

/tmp/filehOTM72.0.c:199:13: error: conflicting types for ‘strcpy’
 PUBLIC void strcpy (Objptr x1, CPointer x0);
             ^~~~~~
In file included from /home/ec2-user/proj/mpl/hybrid-sched/build/lib/mlton/include/c-chunk.h:17:0,
                 from /tmp/filehOTM72.0.c:2:
/usr/include/string.h:121:14: note: previous declaration of ‘strcpy’ was here
 extern char *strcpy (char *__restrict __dest, const char *__restrict __src)
              ^~~~~~
/tmp/filehOTM72.0.c:200:14: error: conflicting types for ‘strlen’
 PUBLIC Int64 strlen (CPointer x0);
              ^~~~~~
In file included from /home/ec2-user/proj/mpl/hybrid-sched/build/lib/mlton/include/c-chunk.h:17:0,
                 from /tmp/filehOTM72.0.c:2:
/usr/include/string.h:384:15: note: previous declaration of ‘strlen’ was here
 extern size_t strlen (const char *__s)
               ^~~~~~
MLton [mpl] 20230509.184946-g6f2c989b8 raised: Fail: Process.waits: child 10592 failed with exit status 1
make: *** [main.mpl.bin] Error 1

An immediate workaround would be to also generate a small .c file that wraps these functions with new names, but that's a bit awkward. Not sure what the best solution is yet.

athas commented 1 year ago

Can't I just call them the way MLton expects them to be? It looks like I gave them the wrong types.

Incidentally, why does it work for me? I test with the latest release of MLton.

athas commented 1 year ago

I changed the types. Does it work now? I'm still curious why it didn't fail for me.

shwestrick commented 1 year ago

Still not working unfortunately.

I'm able to trigger it with MLton also, using -codegen c at compile time:

mlton -default-ann 'allowFFI true' -codegen c test/test.mlb test/test.c

So, it seems to work with the native codegen, but not with the C codegen. In MPL we only use the C codegen.

Perhaps @MatthewFluet can help... :)

athas commented 1 year ago

It's a little disappointing if I have to generate C wrapper code, but hardly the end of the world. I suspect I will need to do that to support other SML compilers anyway, but MLton looked like it might not have needed it.

MatthewFluet commented 1 year ago

It looks like this issue is arising from https://github.com/diku-dk/smlfut/blob/main/src/smlfut.sml#L127-L147.

Assuming that errors are rare, then is it really necessary to use strlen and strcpy? Would it not be just as easy to use MLton.Pointer.getWord8 and some loops?

In any case, using strcpy to mutate an SML string is undefined behavior (for MLton). MLton assumes that SML strings are immutable; in particular, seeing val s = CharVector.tabulate (Word64.toInt n, fn _ => #" "), it can assume that s only contains #" " elements and optimize accordingly. (Of course, as with many undefined behaviors, it will most likely work the way you want.)

You might as well use val s = CharVector.tabulate (Word64.toInt n, fn i => Char.chr (Word8.toInt (MLton.Pointer.getWord8 (p, i)))).

shwestrick commented 1 year ago

I like Matthew's suggestion!