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

RB_GC_GUARD_PTR on non-GCC compilers fails to link #489

Closed
lamont-granquist opened this issue Mar 17, 2016 · 9 comments · Fixed by #490
Closed

RB_GC_GUARD_PTR on non-GCC compilers fails to link #489

lamont-granquist opened this issue Mar 17, 2016 · 9 comments · Fixed by #490

Comments

@lamont-granquist
Copy link
Contributor

I have no idea if this is an ffi bug or a ruby bug or both...

You use RB_GC_GUARD_PTR in at least a couple of places, e.g.:

RB_GC_GUARD_PTR(argv);

RB_GC_GUARD_PTR(rbParams);

The ruby sources in in ruby.h have a comment which sort of implies maybe you shouldn't?

/* RB_GC_GUARD_PTR() is an intermediate macro, and has no effect by
 * itself.  don't use it directly */

The ruby.h file then defines the various macros in this kinda nasty #ifdef mess here:

#ifdef __GNUC__
#define RB_GC_GUARD_PTR(ptr) \
    __extension__ ({volatile VALUE *rb_gc_guarded_ptr = (ptr); rb_gc_guarded_ptr;})
#else
#ifdef _MSC_VER
#pragma optimize("", off)
static inline volatile VALUE *rb_gc_guarded_ptr(volatile VALUE *ptr) {return ptr;}
#pragma optimize("", on)
#else
volatile VALUE *rb_gc_guarded_ptr_val(volatile VALUE *ptr, VALUE val);
#define HAVE_RB_GC_GUARDED_PTR_VAL 1
#define RB_GC_GUARD(v) (*rb_gc_guarded_ptr_val(&(v),(v)))
#endif
#define RB_GC_GUARD_PTR(ptr) rb_gc_guarded_ptr(ptr)
#endif

I'm compiling on AIX with XLC and not GCC so for me this becomes:

volatile VALUE *rb_gc_guarded_ptr_val(volatile VALUE *ptr, VALUE val);
#define HAVE_RB_GC_GUARDED_PTR_VAL 1
#define RB_GC_GUARD(v) (*rb_gc_guarded_ptr_val(&(v),(v)))
#define RB_GC_GUARD_PTR(ptr) rb_gc_guarded_ptr(ptr)

Now the cool thing here is that the symbol rb_gc_guarded_ptr is only defined in that GNUC block, so this is guaranteed to blow up on any non-GCC/non-MSC install if anyone ever uses RB_GC_GUARD_PTR directly.

So can you just s/RB_GC_GUARD_PTR/RB_GC_GUARD/g and it'll work correctly, or is there a reason why you're trying to access that macro directly?

@tduehr
Copy link
Member

tduehr commented Mar 18, 2016

This is a pretty old section of the code. Do you know how long RB_GC_GUARD has been available?

@lamont-granquist
Copy link
Contributor Author

I was /guessing/ only since ruby-2.2 since I hit this trying to up our builds from ruby-2.1 to 2.2

It looks like you define RB_GC_GUARD to be a no-op if it doesn't exist (I was assuming backcompat there with pre-2.2 GCs)

@lamont-granquist
Copy link
Contributor Author

nope it exists in 2.1.8 as well, which confuses the hell out of me now...

@lamont-granquist
Copy link
Contributor Author

ah i see in 2.1.8 there used to be a rb_gc_guarded_ptr in this codepath so that RB_GC_GUARD_PTR would evaluate. it looks like in 2.2 they accepted a fix for RB_GC_GUARD that broke RB_GC_GUARD_PTR for AIX (possibly because it never worked?).

#ifdef __GNUC__
#define RB_GC_GUARD_PTR(ptr) \
    __extension__ ({volatile VALUE *rb_gc_guarded_ptr = (ptr); rb_gc_guarded_ptr;})
#else
#ifdef _MSC_VER
#pragma optimize("", off)
static inline volatile VALUE *rb_gc_guarded_ptr(volatile VALUE *ptr) {return ptr;}
#pragma optimize("", on)
#else
volatile VALUE *rb_gc_guarded_ptr(volatile VALUE *ptr);                                           <==================  rb_gc_guarded_ptr existed in 2.1.x on AIX/xlc
#define HAVE_RB_GC_GUARDED_PTR 1
#endif
#define RB_GC_GUARD_PTR(ptr) rb_gc_guarded_ptr(ptr)
#endif
#define RB_GC_GUARD(v) (*RB_GC_GUARD_PTR(&(v)))

@lamont-granquist
Copy link
Contributor Author

ruby/ruby@08252d1 is the commit that looks like it fixed RB_GC_GUARD and broke RB_GC_GUARD_PTR

@lamont-granquist
Copy link
Contributor Author

Yeah, fixed a segfault on the solaris compiler: https://bugs.ruby-lang.org/issues/7805

@tduehr
Copy link
Member

tduehr commented Mar 18, 2016

Since 2.0.0 has reached end of life, we should be okay with simply switching to RB_GC_GUARD rather than switching between the two.

@lamont-granquist
Copy link
Contributor Author

I think the fix in the PR should be backcompat as well. Switching to a ruby array should allow RB_GC_GUARD to protect the array for as far back as that's needed, and the hack you've got to no-op RB_GC_GUARD if its not defined should go back even further.

@larskanis
Copy link
Member

@tduehr RB_GC_GUARD was already present in ruby-1.8.7. RB_GC_GUARD_PTR was always used as internal construct to make sure a valid pointer to the given VALUE is build by the compiler, hence the VALUE itself is placed on the stack memory (not just a processor register). However, when ALLOCA_N is used, the memory should be allocated on the stack in any case.

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

Successfully merging a pull request may close this issue.

3 participants