[ktap] Re: [PATCH] FFI support for ktap

  • From: Jovi Zhangwei <jovi.zhangwei@xxxxxxxxx>
  • To: Yicheng Qin <qycqycqycqycqyc@xxxxxxxxx>
  • Date: Tue, 5 Nov 2013 15:23:47 +0800

On Mon, Nov 4, 2013 at 2:30 PM, Yicheng Qin <qycqycqycqycqyc@xxxxxxxxx> wrote:
> This patch is the first version of basic FFI support for ktap.
>
> A brief overview of current design:
> * user define needed C symbols in ktap script (functions, structs, etc)
> * C symbols get compiled into chunk and passed into ktap vm
> * ktap vm generates ktap_ctype according to symbol information from
> chunks and populates the global "C" table
> * when ktap script calls "C.foo()", ktap vm detects that it's a FFI
> function, and does following:
>   - sets up the hardware stack
>   - calls into the C function
>   - put back return value to ktap stack
> * continue executing next instructions
>
> Implementation of function call module is under interpreter/ffi/,
> including argument check, type conversion, stack setup and return value
> handling.
>
> Noted that in this POC, C symbol definitions are not compiled into ktap
> chunk yet. Because we are still working on a C header parser. Currently,
> we work around it by defining C symbols in ktap table and populate
> global "C" table in runtime. That way we can focus on the FFI call
> implementaion. This will be fixed in the third milestone.
>
> Support:
> - x86_64 machine
> - int, long, longlong and void type
> - kernel functions that are defined in C table in script
> - implicit type conversion between ctypes and ktap types
>
> Test:
> ```
> New test script is under test/ffi/
> ```
>
> Plan:
> https://github.com/ktap/ktap/wiki/FFI-Support
>
> Signed-off-by: Yicheng Qin <qycqycqycqycqyc@xxxxxxxxx>
> Signed-off-by: Qingping Hou <qingping.hou@xxxxxxxxx>
> ---
>  Makefile                     |   8 +-
>  include/ffi.h                | 117 ++++++++++++++++++
>  include/ktap.h               |   6 +
>  include/ktap_types.h         |  23 +++-
>  interpreter/ffi/ffi_call.c   | 282 
> +++++++++++++++++++++++++++++++++++++++++++
>  interpreter/ffi/ffi_symbol.c | 195 ++++++++++++++++++++++++++++++
>  interpreter/ffi/ffi_type.c   |  43 +++++++
>  interpreter/library/ffi.c    |  40 ++++++
>  interpreter/table.c          |  17 ++-
>  interpreter/vm.c             |  27 +++++
>  test/ffi/Makefile            |  13 ++
>  test/ffi/cdef.kp             |  40 ++++++
>  test/ffi/ktest.c             |  32 +++++
>  13 files changed, 839 insertions(+), 4 deletions(-)
>  create mode 100644 include/ffi.h
>  create mode 100644 interpreter/ffi/ffi_call.c
>  create mode 100644 interpreter/ffi/ffi_symbol.c
>  create mode 100644 interpreter/ffi/ffi_type.c
>  create mode 100644 interpreter/library/ffi.c
>  create mode 100644 test/ffi/Makefile
>  create mode 100644 test/ffi/cdef.kp
>  create mode 100644 test/ffi/ktest.c
>
[...]
> diff --git a/test/ffi/cdef.kp b/test/ffi/cdef.kp
> new file mode 100644
> index 0000000..27597be
> --- /dev/null
> +++ b/test/ffi/cdef.kp
> @@ -0,0 +1,40 @@
> +# this need to be synchronized with C header
> +KTAP_CDVOID = 0
> +KTAP_CDCHAR = 1
> +KTAP_CDUCHAR = 2
> +KTAP_CDUSHORT = 3
> +KTAP_CDSHORT = 4
> +KTAP_CDUINT = 5
> +KTAP_CDINT = 6
> +KTAP_CDULONG = 7
> +KTAP_CDLONG = 8
> +KTAP_CDULLONG = 9
> +KTAP_CDLLONG = 10
> +KTAP_CDPTR = 11
> +KTAP_CDFUNC = 12
> +KTAP_CDUNKNOWN = 13
> +
> +
> +C.cdef["sched_clock"] = {}
> +C.cdef["sched_clock"]["addr"] = `sched_clock`
> +C.cdef["sched_clock"]["type"] = KTAP_CDFUNC
> +C.cdef["sched_clock"][1] = KTAP_CDULLONG
> +
> +#C.cdef["ktest_int_int"] = {}
> +#C.cdef["ktest_int_int"]["type"] = KTAP_CDFUNC
> +#C.cdef["ktest_int_int"]["addr"] = `ktest_int_int`
> +#C.cdef["ktest_int_int"][1] = KTAP_CDINT
> +#C.cdef["ktest_int_int"][2] = KTAP_CDINT
> +#C.cdef["ktest_int_int"][3] = KTAP_CDINT
> +
> +ffi.load_sym()
> +
> +# end of cdef header
> +
> +
> +print("start of user script...")
> +
> +print("[*] try a registered C function...")
> +#ret = C.ktest_int_int(1337, 1337)
> +ret = C.sched_clock()
> +print("C function returned, value: ", ret)

It seems we put the function prototype by table,
and parse table in ktapvm to get function prototype data structure.
I know this just is a workaround, since cdef parser not finished yet.

This remind me maybe it's not time to merge this code right now,
there have some code change( in table.c) just for workaround cdef
handling, it means we have to revise it later when compiler cdef parser
finish, that's not good for code maintenance.

Also we have to add a ffi sample script in there, but test/ffi/cdef.kp
file in your patch really cannot be merged, because of the hardcoded
c type enum.

Basically the patch doesn't have any other big problem, IMO.
As initial ffi implementation, it's good to let it work.

So how about you continue to moving on to finish cdef parser? then
we don't need those incomplete solution anymore and all workaround
code can be kill, and demo it with a more simpler sample script.
At that time we can deliver a clean code base for ffi feature.

What do you think?

Jovi

Other related posts: