[ktap] Re: [PATCHv3 1/8] Introduce ffi_type, csymbol and ktap_cdata
- From: Qingping Hou <qingpinghou@xxxxxxxxx>
- To: Jovi Zhangwei <jovi.zhangwei@xxxxxxxxx>
- Date: Sat, 30 Nov 2013 02:43:21 -0500
On Fri 29 Nov 2013 10:56:00 PM EST, Jovi Zhangwei wrote:
> On Sat, Nov 30, 2013 at 4:47 AM, Qingping Hou <dave2008713@xxxxxxxxx> wrote:
>> From: Yicheng Qin <qycqycqycqycqyc@xxxxxxxxx>
>>
>> ffi type is the basic type used in ffi module, which has fixed size and
>> alignment.
>>
>> csymbol means symbol for c type, which is designed to represent all
>> possible c types in its format. It doesn't contain any real value or
>> data.
>>
>> ktap_cdata is the uniform cdata type in ktap. It is an instance of
>> csymbol, which binds a csymbol with the real data. It mainly exists in
>> the vm stack.
>>
>> Signed-off-by: Yicheng Qin <qycqycqycqycqyc@xxxxxxxxx>
>> Signed-off-by: Qingping Hou <qingping.hou@xxxxxxxxx>
>> ---
>> include/ktap_types.h | 42 +++++++-
>> interpreter/ffi/ffi_type.c | 50 +++++++++
>> interpreter/kp_ffi.h | 259
>> +++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 350 insertions(+), 1 deletion(-)
>> create mode 100644 interpreter/ffi/ffi_type.c
>> create mode 100644 interpreter/kp_ffi.h
>>
>> diff --git a/include/ktap_types.h b/include/ktap_types.h
>> index 68ec23c..f92eaf5 100644
>> --- a/include/ktap_types.h
>> +++ b/include/ktap_types.h
>> @@ -8,6 +8,7 @@ typedef char u8;
>> #include <stdlib.h>
>> #include <stdio.h>
>> #include <string.h>
>> +#include <stdint.h>
>> #endif
>>
>> typedef struct ktap_parm {
>> @@ -246,6 +247,29 @@ typedef struct ktap_stringtable {
>> int size;
>> } ktap_stringtable;
>>
>> +#ifdef CONFIG_KTAP_FFI
>> +typedef int csymbol_id;
>> +typedef struct csymbol csymbol;
>> +
>> +/* global ffi state maintained in each ktap vm instance */
>> +typedef struct ffi_state {
>> + ktap_tab *ctable;
>> + int csym_nr;
>> + csymbol *csym_arr;
>> +} ffi_state;
>> +
>> +/* instance of csymbol */
>> +typedef struct ktap_cdata {
>> + CommonHeader;
>> + csymbol_id id;
>> + union {
>> + uint64_t i;
>> + void *p; /* pointer address */
>> + void *st; /* struct member data */
>> + } u;
>> +} ktap_cdata;
>> +#endif
>> +
>> typedef struct ktap_stats {
>> int mem_allocated;
>> int nr_mem_allocate;
>> @@ -300,6 +324,9 @@ typedef struct ktap_global_state {
>> int wait_user;
>> ktap_closure *trace_end_closure;
>> struct ktap_stats __percpu *stats;
>> +#ifdef CONFIG_KTAP_FFI
>> + ffi_state ffis;
>> +#endif
>> #endif
>> int error;
>> } ktap_global_state;
>> @@ -342,6 +369,9 @@ union ktap_gcobject {
>> struct ktap_upval uv;
>> struct ktap_state th; /* thread */
>> struct ktap_btrace bt; /* backtrace object */
>> +#ifdef CONFIG_KTAP_FFI
>> + struct ktap_cdata cd;
>> +#endif
>> };
>>
>> #define gch(o) (&(o)->gch)
>> @@ -376,11 +406,11 @@ union ktap_gcobject {
>> #define KTAP_TTHREAD 7
>> #define KTAP_TPROTO 8
>> #define KTAP_TUPVAL 9
>> -
>> #define KTAP_TEVENT 10
>> #define KTAP_TBTRACE 11
>> #define KTAP_TPTABLE 12
>> #define KTAP_TSTATDATA 13
>> +#define KTAP_TCDATA 14
>> /*
>> * type number is ok so far, but it may collide later between
>> * 16+ and | (1 << 4), so be careful on this.
>> @@ -417,6 +447,7 @@ union ktap_gcobject {
>> #define fvalue(o) (val_(o).f)
>> #define evalue(o) (val_(o).p)
>> #define btvalue(o) (&val_(o).gc->bt)
>> +#define cdvalue(o) (&val_(o).gc->cd)
>>
>> #define is_nil(o) ((o)->type == KTAP_TNIL)
>> #define is_boolean(o) ((o)->type == KTAP_TBOOLEAN)
>> @@ -430,6 +461,9 @@ union ktap_gcobject {
>> #define is_event(o) ((o)->type == KTAP_TEVENT)
>> #define is_btrace(o) ((o)->type == KTAP_TBTRACE)
>> #define is_needclone(o) is_btrace(o)
>> +#ifdef CONFIG_KTAP_FFI
>> +#define is_cdata(o) ((o)->type == KTAP_TCDATA)
>> +#endif
>>
>>
>> #define set_nil(obj) \
>> @@ -476,6 +510,12 @@ union ktap_gcobject {
>> { ktap_value *io = (obj); \
>> val_(io).gc = (ktap_gcobject *)(x); settype(io, KTAP_TBTRACE); }
>>
>> +#ifdef CONFIG_KTAP_FFI
>> +#define set_cdata(obj, x) \
>> + { ktap_value *io=(obj); \
>> + val_(io).gc = (ktap_gcobject *)(x); settype(io, KTAP_TCDATA); }
>> +#endif
>> +
>> #define set_obj(obj1, obj2) \
>> { const ktap_value *io2 = (obj2); ktap_value *io1 = (obj1); \
>> io1->val = io2->val; io1->type = io2->type; }
>> diff --git a/interpreter/ffi/ffi_type.c b/interpreter/ffi/ffi_type.c
>> new file mode 100644
>> index 0000000..608756b
>> --- /dev/null
>> +++ b/interpreter/ffi/ffi_type.c
>> @@ -0,0 +1,50 @@
>> +#include "../kp_ffi.h"
>> +#ifdef __KERNEL__
>> +#include <linux/types.h>
>> +#else
>> +#include <stdint.h>
>> +#endif
>> +
>> +#define CTYPE_MODE_HELPER(name, type) \
>> +struct _##name##_align { \
>> + type t1; \
>> + char c; \
>> + type t2; \
>> +};
>> +
>> +#define CTYPE_MODE(name) \
>> +{ \
>> + offsetof(struct _##name##_align, c), \
>> + offsetof(struct _##name##_align, t2) - \
>> + offsetof(struct _##name##_align, c), \
>> + #name \
>> +}
>> +
>> +#define CTYPE_MODE_NAME(name) _##name##_mode
>> +
>> +/* ffi_ctype_mode should be corresponded to ffi_ctype */
>> +CTYPE_MODE_HELPER(uint8, uint8_t);
>> +CTYPE_MODE_HELPER(int8, int8_t);
>> +CTYPE_MODE_HELPER(uint16, uint16_t);
>> +CTYPE_MODE_HELPER(int16, int16_t);
>> +CTYPE_MODE_HELPER(uint32, uint32_t);
>> +CTYPE_MODE_HELPER(int32, int32_t);
>> +CTYPE_MODE_HELPER(uint64, uint64_t);
>> +CTYPE_MODE_HELPER(int64, int64_t);
>> +CTYPE_MODE_HELPER(pointer, void*);
>> +
>> +const ffi_mode ffi_type_modes[NUM_FFI_TYPE+1] = {
>> + {0, 1, "void"},
>> + CTYPE_MODE(uint8),
>> + CTYPE_MODE(int8),
>> + CTYPE_MODE(uint16),
>> + CTYPE_MODE(int16),
>> + CTYPE_MODE(uint32),
>> + CTYPE_MODE(int32),
>> + CTYPE_MODE(uint64),
>> + CTYPE_MODE(int64),
>> + CTYPE_MODE(pointer),
>> + {0, 1, "function"},
>> + {0, 1, "struct"},
>> + {0, 1, "unknown"},
>> +};
>> diff --git a/interpreter/kp_ffi.h b/interpreter/kp_ffi.h
>> new file mode 100644
>> index 0000000..6a3cb9c
>> --- /dev/null
>> +++ b/interpreter/kp_ffi.h
>
> Please move kp_ffi.h into include/ktap_ffi.h
>
> That ffi header file is used by compiler and interpreter, so it's better place
> at include/ directory.
>
Agree, will do it in v4.
> Also it's better to add more comments into structure definitions, and
> inline access functions, to explain what it does.
>
Sure.
>> @@ -0,0 +1,259 @@
>> +#ifndef __KTAP_FFI_H__
>> +#define __KTAP_FFI_H__
>> +
>> +#ifdef CONFIG_KTAP_FFI
>> +
>> +#include <stddef.h>
>
> stddef.h is included by interpreter?
>
That's a mistake, we will move it to ffi_type.c in v4.
>> +#include "../include/ktap_types.h"
>> +
>> +
>> +typedef enum {
>> + /* 0 - 4 */
>> + FFI_VOID,
>> + FFI_UINT8,
>> + FFI_INT8,
>> + FFI_UINT16,
>> + FFI_INT16,
>> + /* 5 - 9 */
>> + FFI_UINT32,
>> + FFI_INT32,
>> + FFI_UINT64,
>> + FFI_INT64,
>> + FFI_PTR,
>> + /* 10 - 12 */
>> + FFI_FUNC,
>> + FFI_STRUCT,
>> + FFI_UNKNOWN,
>> +} ffi_type;
>> +
>> +#define NUM_FFI_TYPE ((int)FFI_UNKNOWN)
>> +
>> +typedef struct {
>> + size_t size;
>> + size_t align;
>> + const char *name;
>> +} ffi_mode;
>> +extern const ffi_mode const ffi_type_modes[];
>> +
>> +#define ffi_type_size(t) (ffi_type_modes[t].size)
>> +#define ffi_type_align(t) (ffi_type_modes[t].align)
>> +#define ffi_type_name(t) (ffi_type_modes[t].name)
>> +
>> +
>> +#define CSYM_NAME_MAX_LEN 64
>> +
>> +typedef struct csymbol_func {
>> + void *addr;
>> + csymbol_id ret_id;
>> + int arg_nr;
>> + csymbol_id *arg_ids;
>> +} csymbol_func;
>> +
> I tried below ffi script:
>
> cdef("int printk(const char *fmt, ...)")
> C.printk("This is ffi printk\n")
>
> [root@localhost ktap]# ./ktap printk.kp
> cparser error:
> unexpected end
>
> Current cpraser doesn't support function variable argument?
> does luaffi support it? I think this is a basic functionality of ffi,
> and it will be common used in ktap ffi.
> (that's the first ffi script I would try :-) )
>
luaffi does support variable argument functions, we will look into it
this in the following week.
>> +static inline csymbol *id_to_csym(ktap_state *ks, csymbol_id id)
>> +{
>> + return ffi_get_csym_by_id(ks, id);
>> +}
>> +
>> +#define csym_type(cs) ((cs)->type)
>> +
>> +static inline char *csym_name(csymbol *cs)
>> +{
>> + return cs->name;
>> +}
>> +
>> +/*
>> + * start of pointer symbol helper functions
>> + */
>> +static inline csymbol_id csym_ptr_deref_id(csymbol *cs)
>> +{
>> + return cs->u.p;
>> +}
>> +
>> +static inline csymbol *csym_ptr_deref(ktap_state *ks, csymbol *cs)
>> +{
>> + return id_to_csym(ks, csym_ptr_deref_id(cs));
>> +}
>> +
>> +static inline void csym_set_ptr_deref(csymbol *cs, csymbol_id id)
>> +{
>> + cs->u.p = id;
>> +}
>> +
>> +/*
>> + * start of function symbol helper functions
>> + */
>> +static inline csymbol_func *csym_func(csymbol *cs)
>> +{
>> + return &cs->u.f;
>> +}
>> +
>> +static inline csymbol *csymf_arg(ktap_state *ks, csymbol_func *csf, int idx)
>> +{
>> + return id_to_csym(ks, csf->arg_ids[idx]);
>> +}
>> +
>> +static inline csymbol *csymf_ret(ktap_state *ks, csymbol_func *csf)
>> +{
>> + return id_to_csym(ks, csf->ret_id);
>> +}
>> +
>> +static inline csymbol *csym_func_arg(ktap_state *ks, csymbol *cs, int idx)
>> +{
>> + return csymf_arg(ks, csym_func(cs), idx);
>> +}
>> +
>> +static inline csymbol_id *csymf_arg_ids(csymbol_func *csf)
>> +{
>> + return csf->arg_ids;
>> +}
>> +
>> +static inline int csymf_arg_nr(csymbol_func *csf)
>> +{
>> + return csf->arg_nr;
>> +}
>> +
>> +static inline int *csym_func_arg_ids(csymbol *cs)
>> +{
>> + return csymf_arg_ids(csym_func(cs));
>> +}
>> +
>> +static inline csymbol_id csymf_ret_id(csymbol_func *csf)
>> +{
>> + return csf->ret_id;
>> +}
>> +
>> +static inline void *csymf_addr(csymbol_func *csf)
>> +{
>> + return csf->addr;
>> +}
>> +
>> +static inline void *csym_func_addr(csymbol *cs)
>> +{
>> + return csymf_addr(csym_func(cs));
>> +}
>> +
>> +/*
>> + * start of struct symbol helper functions
>> + */
>> +static inline csymbol_struct *csym_struct(csymbol *cs)
>> +{
>> + return &cs->u.st;
>> +}
>> +
>> +static inline csymbol *csymst_mb(ktap_state *ks, csymbol_struct *csst, int
>> idx)
>> +{
>> + return id_to_csym(ks, csst->members[idx].id);
>> +}
>> +
>> +static inline csymbol *csym_struct_mb(ktap_state *ks, csymbol *cs, int idx)
>> +{
>> + return csymst_mb(ks, csym_struct(cs), idx);
>> +}
>> +
>> +static inline int csymst_mb_nr(csymbol_struct *csst)
>> +{
>> + return csst->memb_nr;
>> +}
>> +
> Now I think csym* inline access function is not good at readability now, ;)
> because there have too many "csym" prefix word in code, like structure name
> and field access functions.
>
> I suggest to use CSYM* macro for structure field access, do you like it?
>
Yeah, simple one line macro is gong to make it a lot easier to read. we
will see if it's possible to make it more readable with static inline
as type checking is a good thing to have. If not, we will switch to
macro.
Other related posts:
