Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gems mrblib does not have c extension available #3

Closed
ppibburr opened this issue Feb 4, 2013 · 35 comments
Closed

gems mrblib does not have c extension available #3

ppibburr opened this issue Feb 4, 2013 · 35 comments

Comments

@ppibburr
Copy link
Contributor

ppibburr commented Feb 4, 2013

# Fails in gems mrblib
require 'mruby-cfunc'  #=> uninitialized constant CFunc
@mattn
Copy link
Owner

mattn commented Feb 4, 2013

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:
#3

  • Yasuhiro Matsumoto

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

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
Copy link
Owner

mattn commented Feb 5, 2013

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

$ export MRUBY_REQUIRE=mruby-cfunc
$ gdb mruby
> break abort
> run

@mattn
Copy link
Owner

mattn commented Feb 5, 2013

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

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

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
Copy link
Owner

mattn commented Feb 5, 2013

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
Copy link
Owner

mattn commented Feb 5, 2013

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

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

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

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

  • if mruby-require is supposed to handles all the gems, this makes sense. However no documentation stating the need to do this exists.
  • if gems after mruby-require are to be 'included' as oppesed to 'bundled'. then this is an issue?

@mattn
Copy link
Owner

mattn commented Feb 5, 2013

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

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

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
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

this works however, note the ::

module ::ExampleExtension
  def ExampleExtension.ruby_method
    puts "A Ruby Extension"
  end
end

@mattn
Copy link
Owner

mattn commented Feb 5, 2013

maybe, It's a mruby's bug.

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

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

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

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
Copy link
Owner

mattn commented Feb 5, 2013

can you hook mrb_raise on gdb?

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

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

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

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

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

seems the overflow is present when loaded from ENV

@mattn
Copy link
Owner

mattn commented Feb 5, 2013

seems the overflow is present when loaded from ENV

agreed

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

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
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

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

@mattn
Copy link
Owner

mattn commented Feb 5, 2013

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

good night.

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 5, 2013

i forked at https://github.com/ppibburr/mruby-require/
and modified load_so_file() to load mrb_gem_%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
Copy link
Owner

mattn commented Feb 6, 2013

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
Copy link
Owner

mattn commented Feb 6, 2013

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

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 6, 2013

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

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 6, 2013

i revert fork to only commit of mrb_require.c
here is the commit diff
ppibburr@88455b8

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
Copy link
Owner

mattn commented Feb 6, 2013

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
Copy link
Contributor Author

ppibburr commented Feb 6, 2013

Thanks :)

@mattn
Copy link
Owner

mattn commented Feb 6, 2013

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

@mattn
Copy link
Owner

mattn commented Feb 6, 2013

I sent a pull-req mruby/mruby#826

@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 6, 2013

nice and thank you! :)

@mattn
Copy link
Owner

mattn commented Feb 7, 2013

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

@mattn
Copy link
Owner

mattn commented Feb 7, 2013

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.

@mattn mattn closed this as completed Feb 7, 2013
@ppibburr
Copy link
Contributor Author

ppibburr commented Feb 8, 2013

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
Copy link
Owner

mattn commented Feb 8, 2013

crash with MRUBY_REQUIRE is known issue.

mruby/mruby#808

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants