mattn / mruby-require

39 stars 23 forks source link

gems mrblib does not have c extension available #3

Closed ppibburr closed 11 years ago

ppibburr commented 11 years ago
# Fails in gems mrblib
require 'mruby-cfunc'  #=> uninitialized constant CFunc
mattn commented 11 years ago

Did you do: rake clean all?

On 2/5/13, ppibburr notifications@github.com wrote:

# Fails in gems mrblib
require 'mruby-cfunc'  #=> uninitialized constant CFunc

Reply to this email directly or view it on GitHub: https://github.com/mattn/mruby-require/issues/3

ppibburr commented 11 years ago

actually, i get arena overflow. i get the above problem after i enclosed mrblib loading in save/restore

I suppose arena overflow is the issue and my attempt to fix it was incorrect.

mattn commented 11 years ago

The overflow may not occur in mruby-require. Can you handle it?

$ export MRUBY_REQUIRE=mruby-cfunc
$ gdb mruby
> break abort
> run
mattn commented 11 years ago

Ah, it seens that break abort can't handle arena overflow.

ppibburr commented 11 years ago

ppibburr@localhost:~/git/mruby$ export MRUBY_REQUIRE=mruby-cfunc ppibburr@localhost:~/git/mruby$ gdb bin/mruby GNU gdb (Ubuntu/Linaro 7.4-2012.04-0ubuntu2.1) 7.4-2012.04 ... Reading symbols from /home/ppibburr/git/mruby/bin/mruby...done. (gdb) break abort Breakpoint 1 at 0x804e140 (gdb) run Starting program: /home/ppibburr/git/mruby/bin/mruby [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1".

Breakpoint 1, __GI_abort () at abort.c:53 53 abort.c: No such file or directory. (gdb)

mattn commented 11 years ago

However, mruby-require is doing arena save/restore already.

https://github.com/mattn/mruby-require/blob/master/src/mrb_require.c#L343-L345

Also, generated gem_init.c file. build/host/mrbgems/mruby-cfunc/gem_init.c:

...
void mrb_mruby_cfunc_gem_init(mrb_state *mrb);
void mrb_mruby_cfunc_gem_final(mrb_state *mrb);

void GENERATED_TMP_mrb_mruby_cfunc_gem_init(mrb_state *mrb) {
  int ai = mrb_gc_arena_save(mrb);
  mrb_mruby_cfunc_gem_init(mrb);
  mrb_load_irep(mrb, gem_mrblib_irep_mruby_cfunc);
  if (mrb->exc) {
    mrb_p(mrb, mrb_obj_value(mrb->exc));
    exit(0);
  }
  mrb_gc_arena_restore(mrb, ai);
}

void GENERATED_TMP_mrb_mruby_cfunc_gem_final(mrb_state *mrb) {
  mrb_mruby_cfunc_gem_final(mrb);
}
mattn commented 11 years ago

So I guess, probably, it occur in mruby-cfunc.

/cc @masuidrive Do you know something to handle this issue?

ppibburr commented 11 years ago

fail

$ bin/mruby -e "require 'mrbgems-example'"
-e:1: constant look-up for non class/module (TypeError)

success

export MRUBY_REQUIRE=mrbgems-example
bin/mruby -e "p ExampleExtension"

is the use of the environment variable explicit?

and as a side note. i noticed in build_config.rb that gems added after mruby-require will cause build to fail

mattn commented 11 years ago

Could you please show me the code of mrbgems-example?

ppibburr commented 11 years ago

the full gem is at: https://github.com/masuidrive/mrbgems-example This is the content of the mrblib

module ExampleExtension
  def ExampleExtension.ruby_method
    puts "A Ruby Extension"
  end
end
ppibburr commented 11 years ago

this works however, note the ::

module ::ExampleExtension
  def ExampleExtension.ruby_method
    puts "A Ruby Extension"
  end
end
mattn commented 11 years ago

maybe, It's a mruby's bug.

https://twitter.com/yukihiro_matz/statuses/298712377623465985

sorry, probably, you don't understand japanese...

ppibburr commented 11 years ago

i translated it, kind of :) i modified mruby-cfunc generated gem_init

headers

#include <mruby.h>
#include <mruby/string.h>
#include <mruby/proc.h>
#include <mruby/array.h>
#include <mruby/hash.h>
#include <mruby/variable.h>
#include <mruby/dump.h>

#include <mruby/opcode.h>

#include <setjmp.h>

added these functions

mrb_value
mrb_yield_internal(mrb_state *mrb, mrb_value b, int argc, mrb_value *argv, mrb_value self, struct RClass *c);

static void
replace_stop_with_return(mrb_state *mrb, mrb_irep *irep)
{
  if (irep->iseq[irep->ilen - 1] == MKOP_A(OP_STOP, 0)) {
    irep->iseq = mrb_realloc(mrb, irep->iseq, (irep->ilen + 1) * sizeof(mrb_code));
    irep->iseq[irep->ilen - 1] = MKOP_A(OP_LOADNIL, 0);
    irep->iseq[irep->ilen] = MKOP_AB(OP_RETURN, 0, OP_R_NORMAL);
    irep->ilen++;
  }
}

changed the gem init

void GENERATED_TMP_mrb_mruby_cfunc_gem_init(mrb_state *mrb) {
  int ai = mrb_gc_arena_save(mrb);
  mrb_mruby_cfunc_gem_init(mrb);
  mrb_gc_arena_restore(mrb, ai);

  ai = mrb_gc_arena_save(mrb);
  int n;
  n = mrb_read_irep(mrb, gem_mrblib_irep_mruby_cfunc);

  mrb_gc_arena_restore(mrb, ai);

  if (n >= 0) {
    struct RProc *proc;
    mrb_irep *irep = mrb->irep[n];

    replace_stop_with_return(mrb, irep);
    proc = mrb_proc_new(mrb, irep);
    proc->target_class = mrb->object_class;
    mrb_yield_internal(mrb, mrb_obj_value(proc), 0, NULL, mrb_top_self(mrb), mrb->object_class);
  }
  else if (mrb->exc) {
    // fail to load.
    longjmp(*(jmp_buf*)mrb->jmp, 1);
  }
}

then ran make again

and it works without the envirionment variable. bin/mruby -e "require 'mruby-cfunc'"

but not with the varaible

export MRUBY_REQUIRE=mruby-cfunc
bin/mruby -e "p 1" 
aborted

i use code from mobiruby-common's require

mattn commented 11 years ago

can you hook mrb_raise on gdb?

ppibburr commented 11 years ago

i also copied opcode.h to include/mruby/opcode.h

ppibburr commented 11 years ago

Breakpoint 1, mrb_raise (mrb=0x80ca458, c=0x80cfa60, msg=0x80a33b0 "arena overflow error") at src/error.c:219 219

ppibburr commented 11 years ago

seems the overflow is present when loaded from ENV

mattn commented 11 years ago

seems the overflow is present when loaded from ENV

agreed

ppibburr commented 11 years ago

mrbgems-example works too when i applied the same gem_init.c changes to its generated file (both from ENV and in code)

Should maybe mrbgem temp file genration be modified?

ppibburr commented 11 years ago

I'm going to bed for now. Thanks for your help!

mattn commented 11 years ago

This is problem in mruby. So I'll file this into https://github.com/mruby/mruby/issues

good night.

ppibburr commented 11 years ago

i forked at https://github.com/ppibburr/mruby-require/ and modified load_so_file() to load mrbgem%s_init then check for mrblib data and make irep and load.

this at least lets more mrbgems load successfully, albeit from ENV as described above on larger mrblibs.

mattn commented 11 years ago

Thanks for your activity. I hope to it works on windows also. But dlsym can't get address of variable. Currently, I'm asking to mruby's boss could you change the mrb_load_irep to be that call with switching context?.

mattn commented 11 years ago

And I noticed your fork remove & add files, so I can't see the diff

ppibburr commented 11 years ago

dlsym cannot get address of variable... you mean the the irep data? i'll prepare pastebin of diff

ppibburr commented 11 years ago

i revert fork to only commit of mrb_require.c here is the commit diff https://github.com/ppibburr/mruby-require/commit/88455b8f3f0d958b324453bcc1d2cf85b69e789c

could you change the mrb_load_irep to be that call with switching context? could you explain more? I would like to know some of the mechanics :)

mattn commented 11 years ago

It's a same that you did. mrb->target_class does not point current instance. it point MRB_TT_ICLASS. I think, it should be called via something that call with instance like mrb_yield_intern.

ppibburr commented 11 years ago

Thanks :)

mattn commented 11 years ago

I'm asking to matz: could you please do the patch above officially. :)

mattn commented 11 years ago

I sent a pull-req https://github.com/mruby/mruby/pull/826

ppibburr commented 11 years ago

nice and thank you! :)

mattn commented 11 years ago

However, I wonder whether we should fix mrb_load_irep it self or not. Do you have any opinion?

mattn commented 11 years ago

Sorry, I changed policy. And close the pull-req. Whether mrb_load_irep must not fixed with changing GENERATE_TMP_XXX. So I merged your fork. And fortunately, I could get address of variable of irep with modify of def file. Thank you.

ppibburr commented 11 years ago

Welcome and thank you as well.

Loading ireps seems buggy in general. i have libraries that as regular MRBGEM segfaullt (especially printing to stdout, and at times not printing (a degub print sometimes lets the code run, sometimes it crash), probally memory issues as well, i use cfunc alot.

but will work if required at runtime from main program. , this patch also fixes most of what i need to.

mattn commented 11 years ago

crash with MRUBY_REQUIRE is known issue.

https://github.com/mruby/mruby/pull/808