load.c: report a refused irep from mrb_load_irep_cxt() - #7202

Three functions in src/load.c turn a byte stream into a running proc, and all of them go through load_irep(), which is where a refused irep is reported:

static mrb_value
load_irep(mrb_state *mrb, struct RProc *proc, mrb_ccontext *c)
{
  if (!proc || !proc->body.irep) {
    irep_error(mrb);
    return mrb_nil_value();
  }

Two of them hand it whatever the read returned:

MRB_API mrb_value
mrb_load_irep_buf_cxt(mrb_state *mrb, const void *buf, size_t bufsize, mrb_ccontext *c)
{
  return load_irep(mrb, mrb_proc_read_irep_buf(mrb, buf, bufsize), c);
}

MRB_API mrb_value
mrb_load_irep_file_cxt(mrb_state *mrb, FILE* fp, mrb_ccontext *c)
{
  return load_irep(mrb, mrb_proc_read_irep_file(mrb, fp), c);
}

mrb_load_irep_cxt() returns before it:

MRB_API mrb_value
mrb_load_irep_cxt(mrb_state *mrb, const uint8_t *bin, mrb_ccontext *c)
{
  struct RProc *proc = mrb_proc_read_irep(mrb, bin);
  if (!proc) return mrb_undef_value();
  return load_irep(mrb, proc, c);
}

so its caller gets undef with mrb->exc still clear. mrb_load_irep(), which every embedder reaches for and which the generated test drivers use, is that function with a NULL context.

It used to report. The early return came in with e55abd2 (2020-11-24), to skip allocating the temporary irep object when the read fails; before that commit NULL went into load_irep() and raised there:

-  struct RData *irep_obj = mrb_data_object_alloc(mrb, mrb->object_class, NULL, &tempirep_type);
+  struct RData *irep_obj;
   mrb_irep *irep = mrb_read_irep(mrb, bin);
   mrb_value ret;
 
+  if (!irep) return mrb_undef_value();
+  irep_obj = mrb_data_object_alloc(mrb, mrb->object_class, NULL, &tempirep_type);

Passing NULL into load_irep() is what the other two already do, so the allocation the commit was avoiding is not reintroduced by it.

What it costs to be silent

mrbgems/mruby-test/mrbgem.rake writes one mrb_load_irep() per test file and checks the state after each:

  mrb_load_irep(mrb2, gem_test_irep_mruby_bigint_0);
  if (mrb2->exc) {
    mrb_print_error(mrb2);
    mrb_close(mrb2);
    exit(EXIT_FAILURE);
  }

The check never fires, so a test file the VM cannot load is not a failure: it is subtracted from the run. mrbtest counts what it managed to load and reports 0 KO.

On an MRB_INT32 build that is not hypothetical. Today's master cannot load mrbgems/mruby-bigint/test/bigint.rb there, and says so only if the file is fed to mruby -b, which reaches the buf sibling. rake test over the same build says nothing:

$ build/host-m32/bin/mrbc -o b.mrb mrbgems/mruby-bigint/test/bigint.rb
$ build/host-m32/bin/mruby -b b.mrb
trace (most recent call last):
mrbgems/mruby-toplevel-ext/mrblib/toplevel.rb:1: irep load error (ScriptError)
$ rake test
...
  Total: 2292
     KO: 0

With this commit the same build stops on it:

$ rake test
...
mrbgems/mruby-numeric-ext/mrblib/numeric_ext.rb:64: irep load error (ScriptError)
rake aborted!
Command failed with status (1): [build/host-m32/bin/mrbtest]

What made that irep unloadable is #7201, and with both applied the same build is green at 2313 tests, 0 KO. This commit is what would have made it a failure rather than 20 assertions quietly not run.

host-m32 there is an i686-linux-gnu-gcc cross toolchain with -m32 and conf.gembox 'full-core', spelled out in #7201; build_config/host-m32.rb needs a multilib gcc this machine has no -m32 runtime for.

Return value

undef becomes nil on the failing path, which is what mrb_load_irep_buf() and mrb_load_irep_file() already return there. Nothing in the tree reads the value of a failed mrb_load_irep(); the callers are mrbgems/mruby-test/driver.c and the drivers mrbgem.rake generates, and all of them look at mrb->exc. include/mruby/irep.h documents no return value for the failing case.

Tests

None. mrb_load_irep() takes a pointer to a literal and no Ruby-visible path reaches it, so what this changes can only be seen from C or through mrbtest's own drivers, which is the transcript above.

Verification

rake -m test over ci/gcc-clang, every build green, 0 KO, 0 crash, no new warnings. The counts are master's to the test, since nothing here is reached unless a load fails.

On an MRB_INT32 build the run stops, as above, until #7201 is applied; with both, host-m32 (-m32, full-core) is 2313 tests, 0 KO, 4 skip, and host-i32 (-DMRB_INT32 -DMRB_NO_BOXING, full-core) is 2313 tests, 0 KO, 12 skip.

Environment

# full-debug, -O0 because enable_debug appends -g3 -O0
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_GC_STRESS -DMRB_USE_DEBUG_HOOK -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER src/load.c

# bintest
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_GC_FIXED_ARENA -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -DMRB_USE_DEBUG_HOOK src/load.c

# cxx_abi, gcc -x c++ rather than g++, which only links
gcc -g -O3 -Wall -Wundef -Wwrite-strings -x c++ -std=gnu++03 -DMRB_GC_FIXED_ARENA -DMRB_USE_CXX_EXCEPTION -DMRB_USE_CXX_ABI -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER src/load.c

# byte-string
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER src/load.c

# ascii-case
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_USE_ASCII_CASE -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER src/load.c

Summary by CodeRabbit

• Bug Fixes
  • Improved handling when loading executable code fails, ensuring failures are processed consistently.
  • Removed an unnecessary fallback result that could mask loading errors.