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

Funny issues with generics and varargs #346

Closed
fredreichbier opened this issue Nov 30, 2011 · 11 comments
Closed

Funny issues with generics and varargs #346

fredreichbier opened this issue Nov 30, 2011 · 11 comments
Labels

Comments

@fredreichbier
Copy link
Member

Fun stuff!

import structs/HashBag

Pair: class <K, V> {
    k: K
    v: V

    init: func (=k, =v) {}
}

//operator => <K, V> (k: K, v: V) -> Pair { Pair<K, V> new(k, v) }

hashbag: func (args: ...) {
    args each(|arg|
        pair := arg as Pair
        VV := pair V
        val := pair v as VV
    )
}

hashbag(Pair new("Hello", "World!"), Pair new("leet", 1337))

This segfaults. If we change the last line to

hashbag(Pair new("Hello", "World!"))

it doesn't. Just adding this here for the records, I'll have a look at it and report more details later.

@fredreichbier
Copy link
Member Author

This seems to be related to V. If it's a pointer type (String, Pointer, Object, ...), it works fine, if it's a basic(?) type (like SSizeT, Bool, Cover), it breaks.

Update: I think I found the problem. This is just a theory though. Please let me know if you find something fishy!

The closure gets compiled to:

void t2____t2_closure242(lang_types__Class* T, uint8_t* arg) {
    t2__Pair* pair = (* (t2__Pair**)arg);
    lang_types__Class* VV = pair->V;
    uint8_t* val = lang_Memory__gc_malloc(VV->size);
    lang_Memory__memcpy(val, (* (uint8_t**)pair->v), VV->size);
    lang_String__String_println(VV->name);
} 

The problem is the memcpy line. It dereferences pair->v, but since pair->v is already a pointer to the actual value, it must not be dereferenced to be passed to memcpy. The reason because it doesn't segfault for pointer types is that v is actually this for pointer types:

Pointer -> Pointer -> Some Struct

So when v is dereferenced and passed to memcpy as a pointer to the actual value, the first 8 bytes of the Struct (on 64bit, 4 on 32bit) get memcpy'ed. This doesn't segfault immediately, but would of course leave an invalid val value. For basic types, v is this:

Pointer -> Basic type

So when we dereference that and pass it as a pointer to memcpy, it'll most likely segfault because 1337 (in this case) isn't a valid pointer value.

Any idea why we're doing that dereference there?

@alexnask
Copy link
Collaborator

alexnask commented Dec 4, 2011

So rock should only dereference if pair v is a pointer type (anyhting but a cover from a C type right?). The thing is, how is rock supposed to knwo this, as it does not hold information on generic types passed to the function at compile-time?

@fredreichbier
Copy link
Member Author

Nope, the dereferencing is wrong anyway, but I was just trying to explain why it doesn't segfault for String values. The generated code is still invalid.

@alexnask
Copy link
Collaborator

alexnask commented Dec 4, 2011

Oh ok read your post kinda fast my bad sorry.

@fredreichbier
Copy link
Member Author

Oh no problem, I wasn't very clear I think. And the bold text made my comment sound a bit harsh. :D

@alexnask
Copy link
Collaborator

Just for the record, this is some sample code that works on my copy of rock right now:

import structs/HashBag

Pair: class <K, V> {
    k: K
    v: V

    init: func (=k, =v)
}

operator => <K, V> (k: K, v: V) -> Pair<K,V> { Pair<K, V> new(k, v) }
operator == <K, V> (l,r: Pair<K, V>) -> Bool {
    l k == r k && l v == r v
}

hashbag: func (args: ...) -> HashBag {
    ret := HashBag new()
    args each(|arg|
        if(!arg class inheritsFrom?(Pair)) raise("You need to pass pairs to hashbag, got %s" format(arg class name))
        pair := arg as Pair
        if(!pair K inheritsFrom?(String)) raise("You need to pass pairs with a String left value to hashbag, got %s" format(pair K name))
        VV := pair V
        val := pair v as VV
        ret add(pair k as String, val)
    )
    ret
}

pair := "foo" => "bar"
hashbag(match pair {
    case ("foo" => "bar") => 42 => "ORLY?!"
    case => "baz" => "bar"
})

The only problem is that the equality does not return true. I guess it doesn't use the overloaded operators for String but actually compares the memory addresses of the fields because of the way generics work, so this error does not raise an exception as it should if pair was matched against "foo" => "bar"

@alexnask
Copy link
Collaborator

alexnask commented May 2, 2012

So I was fooling around with rock and the new => operator and I ran into a similar-ish bug.
Here is the ooc code:

operator => <T> (param: T, call: Func(T)) {
    call(param)
}

42 => (func (s: SSizeT) { s toString() println() })
"foo" => (func (s: String) { s println() })

And the generated "problematic" C code:

// The calls
__genArg191 = 42;
test____OP_DOUBLE_ARR_T___FUNC___T((lang_types__Class*)lang_Numbers__SSizeT_class(), (uint8_t*) &(__genArg191), ((lang_types__Closure) { 
        test____test_closure211, 
        NULL
    }));
    __genArg192 = (void*) lang_String__makeStringLiteral("foo", 3);
    test____OP_DOUBLE_ARR_T___FUNC___T((lang_types__Class*)lang_String__String_class(), (uint8_t*) &(__genArg192), ((lang_types__Closure) { 
        test____test_closure212, 
        NULL
    }));

// The operator
void test____OP_DOUBLE_ARR_T___FUNC___T(lang_types__Class* T, uint8_t* param, lang_types__Closure call) {
    ((void (*)(uint8_t*, void*)) call.thunk)((uint8_t*) param, call.context);
}

As you can see, a pointer to the value is passed to the operator but the operator calls the closure with this pointer rather than dereferencing it to the true value of the variable.

@alexnask
Copy link
Collaborator

Both issues are still open.

@fasterthanlime
Copy link
Collaborator

Oh my, an issue that starts with 3xx. Should we start handing out bounties?

@fasterthanlime
Copy link
Collaborator

@fredreichbier @Shamanas well you both lost! sucks to be y'alllllllllll

drops 🎤

@fasterthanlime
Copy link
Collaborator

(Note: Type owner is still veeery experimental but it does solve that exact case. I'm pretty sure by poking around we can find other similar problems, and solve them accordingly)

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

No branches or pull requests

3 participants