vm: simplify calldests

- Replaces the map of valid call destinations with an fd_set. - Removes dead code from the CALL_REG instruction handler. - Adopts fd_pchash_inverse to resolve CALL_IMM targets.
firedancer-io · Jan 30, 2024 · 501bb24 · 501bb24
1 parent bfd35f5
commit 501bb24
Show file tree

Hide file tree

Showing 17 changed files with 117 additions and 237 deletions.
diff --git a/ffi/rust/firedancer-sys/src/ballet/sbpf.rs b/ffi/rust/firedancer-sys/src/ballet/sbpf.rs
@@ -1,5 +1,4 @@
 pub use crate::genballet::{
-    fd_sbpf_calldests_t,
     fd_sbpf_elf_info_t,
     fd_sbpf_elf_peek,
     fd_sbpf_program_align,

diff --git a/ffi/rust/firedancer-sys/wrapper_ballet.h b/ffi/rust/firedancer-sys/wrapper_ballet.h
@@ -1,6 +1,5 @@
 #include "src/ballet/fd_ballet.h"
 #include "src/ballet/pack/fd_pack.h"
 #include "src/ballet/sbpf/fd_sbpf_loader.h"
-#include "src/ballet/sbpf/fd_sbpf_maps.c"
 #include "src/ballet/shred/fd_shred.h"
 #include "src/ballet/txn/fd_txn.h"
diff --git a/src/ballet/sbpf/fd_sbpf_loader.c b/src/ballet/sbpf/fd_sbpf_loader.c
@@ -7,8 +7,6 @@
 #include <assert.h>
 #include <stdio.h>
 
-#include "fd_sbpf_maps.c"
-
 /* Error handling *****************************************************/
 
 /* Thread local storage last error value */
@@ -513,7 +511,7 @@ fd_sbpf_program_footprint( fd_sbpf_elf_info_t const * info ) {
   FD_COMPILER_UNPREDICTABLE( info ); /* Make this appear as FD_FN_PURE (e.g. footprint might depened on info contents in future) */
   return FD_LAYOUT_FINI( FD_LAYOUT_APPEND( FD_LAYOUT_APPEND( FD_LAYOUT_INIT,
     alignof(fd_sbpf_program_t), sizeof(fd_sbpf_program_t) ),
-    fd_sbpf_calldests_align(),  fd_sbpf_calldests_footprint() ),
+    fd_sbpf_calldests_align(), fd_sbpf_calldests_footprint( info->rodata_sz / 8UL ) ),  /* calldests bitmap */
     alignof(fd_sbpf_program_t) );
 }
 
@@ -539,36 +537,36 @@ fd_sbpf_program_new( void *                     prog_mem,
 
   /* Initialize program struct */
 
-  ulong laddr = (ulong)prog_mem;
-  laddr+=FD_LAYOUT_INIT;
-  memset( (void *)laddr, 0, sizeof(fd_sbpf_program_t) );
-  fd_sbpf_program_t * prog = (fd_sbpf_program_t *)fd_type_pun( (void *)laddr );
+  FD_SCRATCH_ALLOC_INIT( laddr, prog_mem );
+  fd_sbpf_program_t * prog = FD_SCRATCH_ALLOC_APPEND( laddr, alignof(fd_sbpf_program_t), sizeof(fd_sbpf_program_t) );
+
+  *prog = (fd_sbpf_program_t) {
+    .info      = *elf_info,
+    .rodata    = rodata,
+    .rodata_sz = elf_info->rodata_sz,
+    .text      = (ulong *)((ulong)rodata + elf_info->text_off),
+    .text_cnt  = elf_info->text_cnt,
+    .entry_pc  = elf_info->entry_pc
+  };
 
-  memcpy( &prog->info, elf_info, sizeof(fd_sbpf_elf_info_t) );
-  prog->rodata    = rodata;
-  prog->rodata_sz = elf_info->rodata_sz;
-  prog->text      = (ulong *)((ulong)rodata + elf_info->text_off);
-  prog->text_cnt  = elf_info->text_cnt;
-  prog->entry_pc  = elf_info->entry_pc;
 
   /* Initialize calldests map */
 
-  laddr=FD_LAYOUT_APPEND( laddr, alignof( fd_sbpf_program_t ),
-                                 sizeof ( fd_sbpf_program_t ) );
-  /* fd_sbpf_calldests_align() < alignof( fd_sbpf_program_t ) */
-  prog->calldests = fd_sbpf_calldests_join( fd_sbpf_calldests_new( (void *)laddr ) );
+  ulong pc_max = elf_info->rodata_sz / 8UL;
+  prog->calldests =
+    fd_sbpf_calldests_join( fd_sbpf_calldests_new(
+        FD_SCRATCH_ALLOC_APPEND( laddr, fd_sbpf_calldests_align(),
+                                        fd_sbpf_calldests_footprint( pc_max ) ),
+        pc_max ) );
 
   return prog;
 }
 
 void *
 fd_sbpf_program_delete( fd_sbpf_program_t * mem ) {
 
-  ulong laddr = (ulong)fd_type_pun( mem );
-  laddr+=FD_LAYOUT_INIT;
-  memset( (void *)laddr, 0, sizeof(fd_sbpf_program_t) );
-
   fd_sbpf_calldests_delete( fd_sbpf_calldests_leave( mem->calldests ) );
+  fd_memset( mem, 0, sizeof(fd_sbpf_program_t) );
 
   return (void *)mem;
 }
@@ -577,8 +575,8 @@ fd_sbpf_program_delete( fd_sbpf_program_t * mem ) {
 
 struct fd_sbpf_loader {
   /* External objects */
-  fd_sbpf_calldests_t * calldests;  /* owned by program */
-  fd_sbpf_syscalls_t  * syscalls;   /* owned by caller */
+  ulong *              calldests;  /* owned by program */
+  fd_sbpf_syscalls_t * syscalls;   /* owned by caller */
 
   /* Dynamic table */
   uint dyn_off;  /* File offset of dynamic table (UINT_MAX=missing) */
@@ -930,17 +928,20 @@ fd_sbpf_r_bpf_64_32( fd_sbpf_loader_t   const * loader,
     /* Register function call */
     ulong target_pc = (S-sh_addr) / 8UL;
 
+    /* TODO bounds check the target? */
+
     /* Check for collision with syscall ID */
     REQUIRE( !fd_sbpf_syscalls_query( loader->syscalls, (uint)target_pc, NULL ) );
 
     /* Register new entry */
     uint hash;
     if( name_len >= 10UL && 0==strncmp( name, "entrypoint", name_len ) ) {
+      /* TODO register entrypoint */
       hash = 0x71e3cf81;
     } else {
       hash = fd_murmur3_32( &target_pc, 8UL, 0U );
     }
-    REQUIRE( fd_sbpf_calldests_upsert( loader->calldests, hash, target_pc ) );
+    fd_sbpf_calldests_insert( loader->calldests, target_pc );
 
     V = (uint)hash;
   } else {
@@ -1025,12 +1026,10 @@ fd_sbpf_hash_calls( fd_sbpf_loader_t *    loader,
     REQUIRE( target_pc<insn_cnt );  /* bounds check target */
 
     /* Derive hash and insert */
-    /* FIXME encrypt target_pc before insert */
-    uint hash = fd_murmur3_32( &target_pc, 8UL, 0U );
-    REQUIRE( fd_sbpf_calldests_upsert( calldests, hash, target_pc ) );
+    fd_sbpf_calldests_insert( calldests, target_pc );
 
     /* Replace immediate with hash */
-    FD_STORE( uint, ptr+4UL, hash );
+    FD_STORE( uint, ptr+4UL, fd_pchash( (uint)target_pc ) );
   }
 
   return 0;
@@ -1199,15 +1198,6 @@ fd_sbpf_program_load( fd_sbpf_program_t *  prog,
   if( FD_UNLIKELY( (err=fd_sbpf_relocate    ( &loader, elf, elf_sz, prog->rodata, &prog->info ))!=0 ) )
     return err;
 
-  /* Override entrypoint */
-  do {
-    fd_sbpf_calldests_t * entry = fd_sbpf_calldests_query( prog->calldests, 0x71e3cf81, NULL );
-    if( !entry )
-      entry = fd_sbpf_calldests_insert( prog->calldests, 0x71e3cf81 );
-    REQUIRE( entry );
-    entry->pc = prog->entry_pc;
-  } while(0);
-
   /* Create read-only segment */
   if( FD_UNLIKELY( (err=fd_sbpf_zero_rodata( elf, prog->rodata, &prog->info ))!=0 ) )
     return err;

diff --git a/src/ballet/sbpf/fd_sbpf_loader.h b/src/ballet/sbpf/fd_sbpf_loader.h
@@ -19,28 +19,49 @@
 
 /* Program struct *****************************************************/
 
-/* fd_sbpf_calldests_t is a map type used to resolve sBPF call targets.
-   This is required because loaded sBPF bytecode does not directly call
-   relative addresses, but instead calls the Murmur3 hash of the
-   destination program counter.  This hash is not trivially reversible
-   thus we store all Murmur3(PC) => PC mappings in this map. */
-
-struct __attribute__((aligned(16UL))) fd_sbpf_calldests {
-  ulong key;  /* hash of PC */
-  /* FIXME salt map key with an add-rotate-xor */
-  ulong pc;
-};
-typedef struct fd_sbpf_calldests fd_sbpf_calldests_t;
+/* fd_sbpf_calldests is a bit vector of valid call destinations.
+   Should be configured to fit any possible program counter.  The max
+   program counter is <size of ELF binary> divided by 8. */
+
+#define SET_NAME fd_sbpf_calldests
+#include "../../util/tmpl/fd_set_dynamic.c"
+
+/* fd_sbpf_syscall_fn_t is a callback implementing an sBPF syscall.
+   ctx is the executor context. */
+
+typedef ulong
+(* fd_sbpf_syscall_fn_t)( void *  ctx,
+                          ulong   r1,
+                          ulong   r2,
+                          ulong   r3,
+                          ulong   r4,
+                          ulong   r5,
+                          ulong * r0 );
 
 /* fd_sbpf_syscalls_t maps syscall IDs => local function pointers. */
-typedef ulong (*fd_sbpf_syscall_fn_ptr_t)(void * ctx, ulong arg0, ulong arg1, ulong arg2, ulong arg3, ulong arg4, ulong * ret);
+
 struct __attribute__((aligned(16UL))) fd_sbpf_syscalls {
-  uint                     key;       /* Murmur3-32 hash of function name */
-  fd_sbpf_syscall_fn_ptr_t func_ptr;  /* Function pointer */
-  char const *             name;
+  uint                 key;       /* Murmur3-32 hash of function name */
+  fd_sbpf_syscall_fn_t func_ptr;  /* Function pointer */
+  char const *         name;
 };
+
 typedef struct fd_sbpf_syscalls fd_sbpf_syscalls_t;
 
+/* fd_sbpf_syscalls_t maps syscall IDs => local function pointers. */
+
+#define MAP_NAME              fd_sbpf_syscalls
+#define MAP_T                 fd_sbpf_syscalls_t
+#define MAP_KEY_T             uint
+#define MAP_KEY_NULL          0U
+#define MAP_KEY_INVAL(k)      !(k)
+#define MAP_KEY_EQUAL(k0,k1)  (k0)==(k1)
+#define MAP_KEY_EQUAL_IS_SLOW 0
+#define MAP_KEY_HASH(k)       (k)
+#define MAP_MEMOIZE           0
+#define MAP_LG_SLOT_CNT       12
+#include "../../util/tmpl/fd_map.c"
+
 /* fd_sbpf_elf_info_t contains basic information extracted from an ELF
    binary. Indicates how much scratch memory and buffer size is required
    to fully load the program. */
@@ -95,7 +116,7 @@ struct __attribute__((aligned(32UL))) fd_sbpf_program {
   ulong   text_cnt;  /* instruction count */
   ulong   entry_pc;  /* entrypoint PC (at text[ entry_pc - start_pc ]) */
 
-  /* Map of valid call destinations */
+  /* Bit vector of valid call destinations (bit count is rodata_sz) */
   fd_sbpf_calldests_t * calldests;
 };
 typedef struct fd_sbpf_program fd_sbpf_program_t;

diff --git a/src/ballet/sbpf/fd_sbpf_maps.c b/src/ballet/sbpf/fd_sbpf_maps.c
diff --git a/src/ballet/sbpf/fd_sbpf_maps.h b/src/ballet/sbpf/fd_sbpf_maps.h
diff --git a/src/ballet/sbpf/fuzz_sbpf_loader.c b/src/ballet/sbpf/fuzz_sbpf_loader.c
@@ -4,7 +4,6 @@
 
 #include "../../util/sanitize/fd_fuzz.h"
 #include "fd_sbpf_loader.h"
-#include "fd_sbpf_maps.c"
 
 #include <stdlib.h>
 

diff --git a/src/ballet/sbpf/test_sbpf_load_prog.c b/src/ballet/sbpf/test_sbpf_load_prog.c
@@ -10,8 +10,6 @@
 #include <stdlib.h>
 #include <sys/stat.h>
 
-#include "fd_sbpf_maps.c"
-
 uint const _syscalls[] = {
   0xb6fc1a11, 0x686093bb, 0x207559bd, 0x5c2a3178, 0x52ba5096,
   0x7ef088ca, 0x9377323c, 0x48504a38, 0x11f49d86, 0xd7793abb,

diff --git a/src/flamenco/vm/fd_vm_context.c b/src/flamenco/vm/fd_vm_context.c
@@ -1,7 +1,7 @@
 #include "fd_vm_context.h"
 
 #include "../../ballet/sbpf/fd_sbpf_opcodes.h"
-#include "../../ballet/sbpf/fd_sbpf_maps.c"
+#include "../../ballet/sbpf/fd_sbpf_loader.h"
 #include "../runtime/fd_runtime.h"
 
 ulong
@@ -132,8 +132,8 @@ fd_vm_context_validate( fd_vm_exec_context_t const * ctx ) {
       case FD_CHECK_CALL:
         // TODO: Check to make sure we are really doing this right!
         if( instr.imm >= ctx->instrs_sz
-            && fd_sbpf_syscalls_query ( ctx->syscall_map,    instr.imm, NULL ) == NULL
-            && fd_sbpf_calldests_query( ctx->local_call_map, instr.imm, NULL ) == NULL ) {
+            && fd_sbpf_syscalls_query ( ctx->syscall_map, instr.imm, NULL ) == NULL
+            && !fd_sbpf_calldests_test( ctx->calldests,  instr.imm ) ) {
               return FD_VM_SBPF_VALIDATE_ERR_NO_SUCH_EXT_CALL;
         }
         break;
@@ -196,6 +196,5 @@ fd_vm_translate_vm_to_host_private( fd_vm_exec_context_t *  ctx,
     default:
       return 0UL;
   }
-
   return host_addr;
 }
diff --git a/src/flamenco/vm/fd_vm_context.h b/src/flamenco/vm/fd_vm_context.h
@@ -191,7 +191,7 @@ struct fd_vm_exec_context {
   /* Read-only VM parameters: */
   long                        entrypoint;     /* The initial program counter to start at */
   fd_sbpf_syscalls_t *        syscall_map;    /* The map of syscalls that can be called into */
-  fd_sbpf_calldests_t *       local_call_map; /* The map of local functions that can be called into */
+  ulong *                     calldests;      /* The bit vector of local functions that can be called into */
   fd_sbpf_instr_t const *     instrs;         /* The program instructions */
   ulong                       instrs_sz;      /* The number of program instructions FIXME this should be _cnt, not _sz */
   ulong                       instrs_offset;  /* This is the relocation offset we must apply to indirect calls (callx/CALL_REGs) */