[PATCH v11 1/3] Initial support for LuaJIT scripting

[Victor Krapivensky]

On Wed, Aug 23, 2017 at 09:47:22PM +0200, Eugene Syromiatnikov wrote:
> > * strace.c (alloctcb): update the condition of presence of currpers
> > field.
> Btw, looks like there are other places that touch that field, wouldn't
> it better to provide setter/getter for this field which would resolve to
> null function in case no currpers field present?  Otherwise it's quite
> difficult to maintain currpers-touching code and decide which macros
> should be checked in each case.
> 
The rule is simple: places others than its initialization should not
touch it at all, in case of defined(USE_LUAJIT) &&
SUPPORTED_PERSONALITIES == 1. So they should only check for
SUPPORTED_PERSONALITIES > 1, as they did before.

> > diff --git a/defs_shared.h b/defs_shared.h
> > new file mode 100644
> > index 00000000..1fb7a92d
> > --- /dev/null
> > +++ b/defs_shared.h
> > @@ -0,0 +1,66 @@
> > +/*
> > + * Should only be included without FFI_CDEF from defs.h, so no include guards.
> > + */
> BTW, you can add something like
> 
>     #if !defined(FFI_CDEF) || !defined(STRACE_DEFS_SHARED_H)
>     #ifndef FFI_CDEF
>     # define STRACE_DEFS_SHARED_H
>     #endif /* !FFI_CDEF */
> 
>     <...>
> 
>     #endif /* !FFI_CDEF || !STRACE_DEFS_SHARED_H */
> 
> to avoid commenting about absence of include guards.
> 
Well, defs_shared.h is not a proper header file in that it does not
include anything it needs, and such include guards would imply it can be
included from elsewhere.

> > diff --git a/luajit.h b/luajit.h
> > new file mode 100644
> > index 00000000..db8a9474
> > --- /dev/null
> > +++ b/luajit.h
> > @@ -0,0 +1,311 @@
> > +/*
> > + * Should only be included from strace.c, so no include guards.
> > + */
> Same here, regarding include guard.
luajit.h is not a proper header file either, in that it 1) does not
include anything it needs; and 2) contains (non-static inline)
functions. Again, such include guards would imply it can be included
from elsewhere.

> The implementation variant of the FFI library used looks like build-time choice
> and I can't see any difference in the interface of these library versions,
> is the library with which strace is built important for the strace users?
> You may want to elaborate on this, however, in configure.ac or install.texi.
strace is built with a Lua implementation, not a FFI library. LuaJIT
comes with its own FFI library, non-LuaJIT users have to install a
standalone one. A compile-time difference is that LuaJIT defines
LUA_FFILIBNAME, and, in absence of it, strace tries to load a library
named "ffi".
The only run-time difference important to us is how a comparison against
a null pointer should be done: LuaJIT requires comparing a boxed pointer
against nil; luaffi with ffi.C.NULL; and luaffifb with ffi.NULL.
Oh, and Lua 5.1 (non-LuaJIT) is not supported as for now, due to that
standalone implementations don't support arithmetics on pointers.
Honestly, I am not sure if supporting non-LuaJIT is worth it.

> So, the script as simple as
> 
> 	tcp = strace.C.next_sc()
> 	print(tcp.scno)
> 
> would crash strace process itself. What do you think about protecting
> against those simplest ways to crash strace?

Dunno. Put a warning into the man page?