New Foreign Function Interface (FFI) #25

alastairreid · 2025-02-04T18:47:10Z

This new interface makes the types of imported/exported
functions independent of the particular representation
choices made by the runtime system.

Instead, values of arguments and results are converted
between the type used for ASL code and a standard
representation used for C code.

The representation of types is as follows

if the ASL type is boolean then "bool" (from stdbool.h)
if the ASL type is string then "const char*"
if the ASL type is integer then "int"
if the ASL type is bits(N) then either
- uint8_t, uint16_t, uint32_t, uint64_t if N IN {8, 16, 32, 64}
- array uint64_t _[N'] where N' is ceiling(N / 64)
if the ASL type is an enumeration, then that enumeration
provided that the enumeration type has been explicitly
exported
if a function returns a tuple of N values, then the C function
expects N extra arguments which should be pointers for
the C function to write results to.

Other types cannot be used in imported/exported functions.

In addition, exported functions must not raise exceptions
and, for extra safety, we insert a runtime check that
they do not raise an exception

In addition, for each configuration variable 'v : ty', we generate
a pair of getter/setter functions

ASL_set_config_{v}({cty} value);
{cty} ASL_get_config_{v}();

Note that (based on the ASL semantics of configuration variables)
all changes to the configuration variable should be made
before executing any ASL code.

All the interesting (and harder to review changes) are in libASL/backend_c_new.ml - the rest is just changes to the runtime API to support the new FFI or to support testing.

nikolaykosarev · 2025-02-12T15:19:27Z

Signed-off-by is missing in the 1st two commits.

alastairreid · 2025-02-14T16:22:36Z

Missing signoffs added.
I need to find a better approach to doing this since I often forget to do this.

nikolaykosarev

Looks good.
There is an unused file in the tests. And exit(1) is missing in one place.

libASL/utils.ml

nikolaykosarev · 2025-02-12T15:40:20Z

libASL/utils.ml

@@ -92,6 +92,20 @@ let split3 (xyzs : ('x * 'y * 'z) list) : ('x list * 'y list * 'z list) =
    xyzs
    ([], [], [])

+(** split4 [(a1, b1, c1, d1); ... (an, bn, cn, dn)] = ([a1; ... an], ... [d1; ... dn]) *)
+let split4 (abcds : ('a * 'b * 'c * 'd) list) : ('a list * 'b list * 'c list * 'd list) =
+  List.fold_right


Have you considered fold_left, as it is tail-recursive?
It will require reversing the resulting lists in the tuple though.

Interesting idea. Definitely worthwhile if the lists are long.
These functions are only used with lists of arguments - which don't get longer than about 5-10 so not being tail recursive should not be a problem. So I will stick with the simple version.

A quick look at the source code for List.split shows that it is not tail recursive either. This might be more of a problem since it is used in more places and maybe some of those involve long lists. (eg a list of all functions).

tests/backends/ffi_config_00.json

tests/backends/ffi_import_01.c

nikolaykosarev · 2025-02-17T14:10:47Z

tests/backends/ffi_import_01.c

+uint32_t FFI_null_bits32(uint32_t x) { return x; }
+uint64_t FFI_null_bits64(uint64_t x) { return x; }
+
+void FFI_null_bits17(uint64_t x[1], uint64_t r[1]) { memcpy(r, x, 1*sizeof(*r)); }


Suggested change

void FFI_null_bits17(uint64_t x[1], uint64_t r[1]) { memcpy(r, x, 1*sizeof(*r)); }

void FFI_null_bits17(uint64_t x[1], uint64_t r[1]) { memcpy(r, x, 1*sizeof(uint64_t)); }

would be cleaner.

Yes - good idea

nikolaykosarev · 2025-02-17T14:21:11Z

bin/asl2c.py

@@ -463,6 +491,11 @@ def main() -> int:
    if args.num_c_files != 1 and args.build:
        print("Error: don't specify --num-c-files if building or running")
        exit(1)
+    if (args.imports or args.exports or args.extra_c) and not args.build:
+        print("Error: don't specify --imports or --exports or --extra-c if not building or not running")


Suggested change

print("Error: don't specify --imports or --exports or --extra-c if not building or not running")

print("Error: don't specify --imports or --exports or --extra-c if not building or not running")

exit(1)

Missing exit(1)

oops - fixed

bin/asl2c.py

nikolaykosarev · 2025-02-17T15:06:51Z

tests/backends/ffi_import_00.c

+// Wide bitvectors are represented as arrays of uint64_t
+// If returning a wide bitvector, an extra array argument is
+// added as the last argument
+void FFI_Invert128(uint64_t x[2], uint64_t y[2])


Suggested change

void FFI_Invert128(uint64_t x[2], uint64_t y[2])

void FFI_Invert128(uint64_t x[], uint64_t y[])

Strictly speaking, array sizes are not needed here. I assume you add them for readability.

Yes, I want the size of the arrays to be clear since the size of the array is important for writing correct code.
I am not sure whether gcc/clang generate warnings if you are obviously writing past the end of an array - but hopefully a human code reviewer would spot a problem.

Signed-off-by: Alastair Reid <[email protected]>

This provides a way to test whether a type was generated by the :xform_tuples transformation - which is needed to support tuple-return in the foreign function interface Signed-off-by: Alastair Reid <[email protected]>

This interface makes the types of imported/exported functions independent of the particular representation choices made by the runtime system. Instead, values of arguments and results are converted between the type used for ASL code and a standard representation used for C code. The representation of types is as follows - if the ASL type is boolean then "bool" (from stdbool.h) - if the ASL type is string then "const char*" - if the ASL type is integer then "int" - if the ASL type is bits(N) then either - uint8_t, uint16_t, uint32_t, uint64_t if N IN {8, 16, 32, 64} - array uint64_t _[N'] where N' is ceiling(N / 64) - if the ASL type is an enumeration, then that enumeration provided that the enumeration type has been explicitly exported Other types cannot be used in imported/exported functions. In addition, exported functions must not raise exceptions and, for extra safety, we insert a runtime check that they do not raise an exception In addition, for each configuration variable 'v : ty', we generate a pair of getter/setter functions ASL_set_config_{v}({cty} value); {cty} ASL_get_config_{v}(); Note that (based on the ASL semantics of configuration variables) all changes to the configuration variable should be made before executing any ASL code. Signed-off-by: Alastair Reid <[email protected]>

- Add --new-ffi flag This causes the new FFI to be used instead of the old FFI where the type of imported/exported functions was determined by the data representations of a particular runtime. - Add --configuration=<json file> Specify a configuration control file typically containing import and export lists to control the FFI - Add --import=<C function name> and --export=<C function name> These specify a C function to be imported/exported. The ASL function name needs to be the same as the C function name. This is a shortcut to creating a configuration file that is mostly intended to make it easier to write tests. - Add --extra-c=<C file> flag This specifies a C file to be compiled and linked against the ASL code when using --build or --run. Mostly intended to make it easier to write tests. Signed-off-by: Alastair Reid <[email protected]>

Signed-off-by: Alastair Reid <[email protected]>

alastairreid force-pushed the areid-new-ffi2 branch from 5984ef4 to 1e36895 Compare February 5, 2025 16:12

alastairreid requested a review from nikolaykosarev February 6, 2025 20:37

alastairreid force-pushed the areid-new-ffi2 branch from 1e36895 to f423c7f Compare February 14, 2025 16:21

nikolaykosarev approved these changes Feb 17, 2025

View reviewed changes

nikolaykosarev reviewed Feb 17, 2025

View reviewed changes

alastairreid added 5 commits February 18, 2025 11:43

Utils: add split4 and split5

f171b6d

Signed-off-by: Alastair Reid <[email protected]>

Xform_tuples: add a isReturnType

003325b

This provides a way to test whether a type was generated by the :xform_tuples transformation - which is needed to support tuple-return in the foreign function interface Signed-off-by: Alastair Reid <[email protected]>

FFI Tests

59499d3

Signed-off-by: Alastair Reid <[email protected]>

alastairreid force-pushed the areid-new-ffi2 branch from f423c7f to 59499d3 Compare February 18, 2025 11:59

alastairreid merged commit 006f70c into master Feb 18, 2025
1 check passed

alastairreid deleted the areid-new-ffi2 branch February 18, 2025 17:54

alastairreid mentioned this pull request Feb 18, 2025

Known issues of the compiler #3

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Foreign Function Interface (FFI) #25

New Foreign Function Interface (FFI) #25

alastairreid commented Feb 4, 2025

nikolaykosarev commented Feb 12, 2025

alastairreid commented Feb 14, 2025

nikolaykosarev left a comment

nikolaykosarev Feb 12, 2025

alastairreid Feb 18, 2025

nikolaykosarev Feb 17, 2025

alastairreid Feb 18, 2025

nikolaykosarev Feb 17, 2025

alastairreid Feb 18, 2025

nikolaykosarev Feb 17, 2025

alastairreid Feb 18, 2025

	void FFI_null_bits17(uint64_t x[1], uint64_t r[1]) { memcpy(r, x, 1sizeof(r)); }
	void FFI_null_bits17(uint64_t x[1], uint64_t r[1]) { memcpy(r, x, 1*sizeof(uint64_t)); }

	print("Error: don't specify --imports or --exports or --extra-c if not building or not running")
	print("Error: don't specify --imports or --exports or --extra-c if not building or not running")
	exit(1)

	void FFI_Invert128(uint64_t x[2], uint64_t y[2])
	void FFI_Invert128(uint64_t x[], uint64_t y[])

New Foreign Function Interface (FFI) #25

New Foreign Function Interface (FFI) #25

Conversation

alastairreid commented Feb 4, 2025

nikolaykosarev commented Feb 12, 2025

alastairreid commented Feb 14, 2025

nikolaykosarev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment