Re: A first attempt to implement jit_embed

[Marc Nieper-Wißkirchen]

Am Di., 16. Aug. 2022 um 20:35 Uhr schrieb Paulo César Pereira de
Andrade <paulo.cesar.pereira.de.andrade@gmail.com>:
>
> Em dom., 14 de ago. de 2022 às 17:35, Marc Nieper-Wißkirchen
> <marc.nieper+gnu@gmail.com> escreveu:
>
>   Hi,
>
> > With the patch below, I implemented
> >
> > jit_embed(jit_pointer_t data, jit_uint32_t length)
> >
> > At the moment, the implementation is for x86, but porting to other CPUs is 
> > simple.
> >
> > Criticism is welcome.
>
>   It appears you forgot to add the embed.tst file to the patch.
>   It could be useful to add multiple data types, so one could even add
> assembly code in some different way, like encoded bytes.

What do you mean by this? The DATA can point to arbitrary data,
including machine code.

jit_embed, however, has one limitation: the data has to be known
during jitting.  For example, this precludes an address only known
after code generation is embedded.

This, let me suggest an instruction jit_skip instead that works like
jit_align and emits nops (as many as jit_skip's argument says).  The
corresponding memory area can be patched later after the code is
emitted.

So that this works for non-alternative code buffers, two procedures
jit_unprotect and jit_protect have to be offered as well. They can be
called after jit_emit and before jit_clear.  Jit_unprotect makes the
code memory area writable (and possibly non-executable), jit_protect
reverses this.

What do you think? It looks a lot more flexible to me.

Thank you,

Marc



>
>   It should also be possible to add embed data larger than jit_instr_max.
>   Could change lib/jit_size.c:_jit_get_size() to check for jit_embed and
> update the guessed size as appropriate. BTW, jit_get_size() likely
> overestimates the size. It just plays safe, to not risk writing out of bounds.
>   jit_code_embed should be handled in a special way, so that the jit_*sz.c
> files would not add the largest found value when computing the size.
>   The patch also did not update the jit_x86-sz.c file, nor any other files,
> so, it will compute the guessed buffer size incorrectly.
>   Probably  no need to have jit_cc_a1_len, otherwise, could have a
> generic way to print integers. Currently it only prints hexadecimal values
> for any immediate value. Having it print signed integers for offsets would
> be useful.
>   Could also use the word (w) field, no need for an extra uint32 field.
>   I suggested a prototype with a int32 field because I do not expect one
> to embed more than 2G of data in a code buffer :)
>
> > Thanks,
> >
> > Marc
> >
> > diff --git a/bootstrap.conf b/bootstrap.conf
> > index 423491b..c257814 100644
> > --- a/bootstrap.conf
> > +++ b/bootstrap.conf
> > @@ -18,6 +18,7 @@
> >
> >  # gnulib modules used by this package.
> >  gnulib_modules="
> > +  obstack
> >  "
> >
> >  # gnulib library name.
> > diff --git a/check/Makefile.am b/check/Makefile.am
> > index fc9f232..48b6f41 100644
> > --- a/check/Makefile.am
> > +++ b/check/Makefile.am
> > @@ -49,6 +49,7 @@ EXTRA_DIST = \
> >   3to2.tst 3to2.ok \
> >   add.tst add.ok \
> >   align.tst align.ok \
> > + embed.tst embed.ok \
> >   allocai.tst allocai.ok \
> >   allocar.tst allocar.ok \
> >   bp.tst bp.ok \
> > @@ -113,7 +114,7 @@ EXTRA_DIST = \
> >   run-test all.tst
> >
> >  base_TESTS = \
> > - 3to2 add align allocai \
> > + 3to2 add align embed allocai \
> >   allocar bp divi fib rpn \
> >   ldstr ldsti \
> >   ldstxr ldstxi \
> > @@ -192,7 +193,8 @@ endif
> >  if test_arm_arm
> >  #arm_TESTS = $(addsuffix .arm, $(base_TESTS))
> >  arm_TESTS = \
> > - 3to2.arm add.arm align.arm allocai.arm \
> > + 3to2.arm add.arm align.arm embed.arm \
> > + allocai.arm \
> >   allocar.arm bp.arm divi.arm fib.arm \
> >   rpn.arm ldstr.arm ldsti.arm \
> >   ldstxr.arm ldstxi.arm \
> > diff --git a/check/lightning.c b/check/lightning.c
> > index 3cf3e70..0a4b5ca 100644
> > --- a/check/lightning.c
> > +++ b/check/lightning.c
> > @@ -26,6 +26,7 @@
> >  #else
> >  #  include <unistd.h>
> >  #endif
> > +#include <obstack.h>
> >  #include <stdio.h>
> >  #include <stdarg.h>
> >  #include <lightning.h>
> > @@ -529,6 +530,7 @@ static label_t *get_label(skip_t skip);
> >  static token_t regname(void);
> >  static token_t identifier(int ch);
> >  static void get_data(type_t type);
> > +static void get_embedded_data(type_t type);
> >  static void dot(void);
> >  static token_t number(int ch);
> >  static int escape(int ch);
> > @@ -585,6 +587,7 @@ static int  symbol_offset;
> >  static hash_t *instrs;
> >  static char *data;
> >  static size_t  data_offset, data_length;
> > +static struct obstack     obstack[1];
> >  static instr_t  instr_vector[] = {
> >  #define entry(value) { NULL, #value, value }
> >  #define entry2(name, function) { NULL, name, function }
> > @@ -833,6 +836,12 @@ static instr_t  instr_vector[] = {
> >  #undef entry
> >  };
> >
> > +/*
> > + * Obstack allocation
> > + */
> > +#define obstack_chunk_alloc xmalloc
> > +#define obstack_chunk_free free
> > +
> >  /*
> >   * Implementation
> >   */
> > @@ -2176,6 +2185,9 @@ get_data(type_t type)
> >      token_t token;
> >      char *test = data;
> >
> > +    if (parser.parsing == PARSING_CODE)
> > +      return get_embedded_data(type);
> > +
> >      for (;;) {
> >   switch (type) {
> >      case type_c:
> > @@ -2239,6 +2251,32 @@ get_data(type_t type)
> >      }
> >  }
> >
> > +static void
> > +get_embedded_data(type_t type)
> > +{
> > +    void *data;
> > +    int ch;
> > +    jit_uint32_t len;
> > +
> > +    for (;;) {
> > + switch (type) {
> > + case type_i:
> > +    len = sizeof(signed int);
> > +    data = obstack_alloc(obstack, len);
> > +    *(signed int *)data = get_int(skip_ws);
> > +    jit_embed(data, len);
> > +    break;
> > + /* FIXME **more types** */
> > + default:
> > +    abort();
> > + }
> > + ch = skipws();
> > + if (ch == '\n' || ch == ';' || ch == EOF)
> > +    break;
> > + ungetch(ch);
> > +    }
> > +}
> > +
> >  static void
> >  dot(void)
> >  {
> > @@ -4086,6 +4124,8 @@ main(int argc, char *argv[])
> >      optind = 1;
> >  #endif
> >
> > +    obstack_init (obstack);
> > +
> >      progname = argv[0];
> >
> >      init_jit(progname);
> > @@ -4344,5 +4384,7 @@ main(int argc, char *argv[])
> >
> >      finish_jit();
> >
> > +    obstack_free (obstack, NULL);
> > +
> >      return (0);
> >  }
> > diff --git a/gnulib-lib/.gitignore b/gnulib-lib/.gitignore
> > index d9f5394..9d61cde 100644
> > --- a/gnulib-lib/.gitignore
> > +++ b/gnulib-lib/.gitignore
> > @@ -1,2 +1,18 @@
> >  /Makefile.am
> > -/dummy.c
> > +/_Noreturn.h
> > +/alignof.h
> > +/arg-nonnull.h
> > +/c++defs.h
> > +/exitfail.c
> > +/exitfail.h
> > +/gettext.h
> > +/limits.in.h
> > +/obstack.c
> > +/obstack.h
> > +/stddef.in.h
> > +/stdint.in.h
> > +/stdlib.in.h
> > +/sys_types.in.h
> > +/unistd.c
> > +/unistd.in.h
> > +/warn-on-use.h
> > diff --git a/include/lightning.h.in b/include/lightning.h.in
> > index 887a951..a7186a7 100644
> > --- a/include/lightning.h.in
> > +++ b/include/lightning.h.in
> > @@ -187,6 +187,8 @@ typedef enum {
> >  #define jit_align(u) jit_new_node_w(jit_code_align, u)
> >      jit_code_live, jit_code_align,
> >      jit_code_save, jit_code_load,
> > +#define jit_embed(u,v)          jit_new_node_pl(jit_code_embed,u,v)
> > +    jit_code_embed,
> >  #define jit_name(u) _jit_name(_jit,u)
> >      jit_code_name,
> >  #define jit_note(u, v) _jit_note(_jit, u, v)
> > @@ -1096,6 +1098,9 @@ extern jit_node_t *_jit_new_node_pwf(jit_state_t*, 
> > jit_code_t,
> >  #define jit_new_node_pwd(c,u,v,w) _jit_new_node_pwd(_jit,c,u,v,w)
> >  extern jit_node_t *_jit_new_node_pwd(jit_state_t*, jit_code_t,
> >       jit_pointer_t, jit_word_t, jit_float64_t);
> > +#define jit_new_node_pl(c,u,v)    _jit_new_node_pl(_jit,c,u,v)
> > +extern jit_node_t *_jit_new_node_pl(jit_state_t*, jit_code_t,
> > +    jit_pointer_t, jit_uint32_t);
> >
> >  #define jit_arg_register_p(u) _jit_arg_register_p(_jit,u)
> >  extern jit_bool_t _jit_arg_register_p(jit_state_t*, jit_node_t*);
> > diff --git a/include/lightning/jit_private.h 
> > b/include/lightning/jit_private.h
> > index 0af24cb..97822e2 100644
> > --- a/include/lightning/jit_private.h
> > +++ b/include/lightning/jit_private.h
> > @@ -276,6 +276,7 @@ extern jit_node_t *_jit_data(jit_state_t*, const void*,
> >  #define jit_cc_a2_int 0x00100000 /* arg2 is immediate word */
> >  #define jit_cc_a2_flt 0x00200000 /* arg2 is immediate float */
> >  #define jit_cc_a2_dbl 0x00400000 /* arg2 is immediate double */
> > +#define jit_cc_a1_len           0x00800000 /* arg1 is an immediate uint 
> > length */
> >
> >  #if __ia64__ || (__sparc__ && __WORDSIZE == 64)
> >  extern void
> > @@ -381,6 +382,7 @@ union jit_data {
> >      jit_float64_t d;
> >      jit_pointer_t p;
> >      jit_node_t *n;
> > +    jit_uint32_t         l;
> >  };
> >
> >  struct jit_note {
> > diff --git a/lib/jit_names.c b/lib/jit_names.c
> > index ebd3d56..69ae274 100644
> > --- a/lib/jit_names.c
> > +++ b/lib/jit_names.c
> > @@ -21,6 +21,7 @@ static char *code_name[] = {
> >      "data",
> >      "live", "align",
> >      "save", "load",
> > +    "embed",
> >      "#name", "#note",
> >      "label",
> >      "prolog",
> > diff --git a/lib/jit_print.c b/lib/jit_print.c
> > index 61d9650..b9b3ece 100644
> > --- a/lib/jit_print.c
> > +++ b/lib/jit_print.c
> > @@ -107,7 +107,7 @@ _jit_print_node(jit_state_t *_jit, jit_node_t *node)
> >   (jit_cc_a0_int|jit_cc_a0_flt|jit_cc_a0_dbl|jit_cc_a0_jmp|
> >   jit_cc_a0_reg|jit_cc_a0_rlh|jit_cc_a0_arg|
> >   jit_cc_a1_reg|jit_cc_a1_int|jit_cc_a1_flt|jit_cc_a1_dbl|jit_cc_a1_arg|
> > - jit_cc_a2_reg|jit_cc_a2_int|jit_cc_a2_flt|jit_cc_a2_dbl);
> > + jit_cc_a2_reg|jit_cc_a2_int|jit_cc_a2_flt|jit_cc_a2_dbl|jit_cc_a1_len);
> >      if (!(node->flag & jit_flag_synth) && ((value & jit_cc_a0_jmp) ||
> >     node->code == jit_code_finishr ||
> >     node->code == jit_code_finishi))
> > @@ -289,6 +289,10 @@ _jit_print_node(jit_state_t *_jit, jit_node_t *node)
> >      else
> >   print_flt(node->w.d);
> >      return;
> > +        l:
> > +    print_chr(' '); print_ptr(node->u.p);
> > +    print_chr(' '); print_dec(node->v.l);
> > +    return;
> >   case jit_code_name:
> >      print_chr(' ');
> >      if (node->v.p && _jitc->emit)
> > @@ -371,7 +375,9 @@ _jit_print_node(jit_state_t *_jit, jit_node_t *node)
> >      goto n_r_f;
> >   case jit_cc_a0_jmp|jit_cc_a1_reg|jit_cc_a2_dbl:
> >      goto n_r_d;
> > - default:
> > +        case jit_cc_a1_len:
> > +    goto l;
> > +        default:
> >      abort();
> >      }
> >      break;
> > diff --git a/lib/jit_x86.c b/lib/jit_x86.c
> > index e3e1383..b650f7f 100644
> > --- a/lib/jit_x86.c
> > +++ b/lib/jit_x86.c
> > @@ -1598,6 +1598,11 @@ _emit_code(jit_state_t *_jit)
> >   if ((word = _jit->pc.w & (node->u.w - 1)))
> >      nop(node->u.w - word);
> >   break;
> > +    case jit_code_embed:
> > + assert(node->v.l <= jit_get_max_instr());
> > + jit_memcpy(_jit->pc.uc, node->u.p, node->v.l);
> > + _jit->pc.uc += node->v.l;
> > + break;
> >      case jit_code_note: case jit_code_name:
> >   node->u.w = _jit->pc.w;
> >   break;
> > diff --git a/lib/lightning.c b/lib/lightning.c
> > index b78bd07..374aa32 100644
> > --- a/lib/lightning.c
> > +++ b/lib/lightning.c
> > @@ -1198,6 +1198,17 @@ _jit_new_node_pwd(jit_state_t *_jit, jit_code_t code,
> >      return (link_node(node));
> >  }
> >
> > +jit_node_t *
> > +_jit_new_node_pl(jit_state_t *_jit, jit_code_t code,
> > + jit_pointer_t u, jit_uint32_t v)
> > +{
> > +    jit_node_t *node = new_node(code);
> > +    assert(!_jitc->realize);
> > +    node->u.p = u;
> > +    node->v.l = v;
> > +    return (link_node(node));
> > +}
> > +
> >  jit_node_t *
> >  _jit_label(jit_state_t *_jit)
> >  {
> > @@ -1316,6 +1327,9 @@ _jit_classify(jit_state_t *_jit, jit_code_t code)
> >   case jit_code_finishi: /* synthesized will set jit_cc_a0_jmp */
> >      mask = jit_cc_a0_int;
> >      break;
> > +        case jit_code_embed:
> > +    mask = jit_cc_a1_len;
> > +    break;
> >   case jit_code_reti_f: case jit_code_pushargi_f:
> >      mask = jit_cc_a0_flt;
> >      break;
> > diff --git a/m4/.gitignore b/m4/.gitignore
> > index 24e2f3f..870b024 100644
> > --- a/m4/.gitignore
> > +++ b/m4/.gitignore
> > @@ -8,3 +8,21 @@
> >  /gnulib-comp.m4
> >  /gnulib-tool.m4
> >  /zzgnulib.m4
> > +/absolute-header.m4
> > +/extensions.m4
> > +/extern-inline.m4
> > +/include_next.m4
> > +/limits-h.m4
> > +/multiarch.m4
> > +/obstack.m4
> > +/off_t.m4
> > +/pid_t.m4
> > +/ssize_t.m4
> > +/stddef_h.m4
> > +/stdint.m4
> > +/stdlib_h.m4
> > +/sys_types_h.m4
> > +/unistd_h.m4
> > +/warn-on-use.m4
> > +/wchar_t.m4
> > +/wint_t.m4
> > diff --git a/m4/gnulib-cache.m4 b/m4/gnulib-cache.m4
> > index 45be7ba..2a2d48b 100644
> > --- a/m4/gnulib-cache.m4
> > +++ b/m4/gnulib-cache.m4
> > @@ -36,12 +36,13 @@
> >  #  --aux-dir=build-aux \
> >  #  --no-conditional-dependencies \
> >  #  --libtool \
> > -#  --macro-prefix=gl
> > +#  --macro-prefix=gl \
> > +#  obstack
> >
> >  # Specification in the form of a few gnulib-tool.m4 macro invocations:
> >  gl_LOCAL_DIR([gl])
> >  gl_MODULES([
> > -
> > +  obstack
> >  ])
> >  gl_AVOID([])
> >  gl_SOURCE_BASE([gnulib-lib])