Segfault on build #811

Closed
davidhesselbom opened this Issue Aug 25, 2014 · 25 comments

Projects

None yet

3 participants

@davidhesselbom
Contributor

This results in a segfault when building:

Foo: class {
    done: Bool { get {
        this lock<Bool>(f)
    }}
}
(SIGSEGV) segmentation fault
[fancy backtrace]
0     Backtrace class()                in lang/Backtrace             (at /home/dhesselbom/versioned/ooc/rock/sdk/lang/Backtrace.ooc:285)                     
1     Exception getCurrentBacktrace()  in lang/Exception             (at /home/dhesselbom/versioned/ooc/rock/sdk/lang/Exception.ooc:209)                     
2     _signalHandler()                 in lang/Exception             (at /home/dhesselbom/versioned/ooc/rock/sdk/lang/Exception.ooc:269)                     
3     killpg()                         in                            (at (null):0)                                                                           
4     String toCString()               in lang/String                (at /home/dhesselbom/versioned/ooc/rock/sdk/lang/String.ooc:276)                        
5     ac_X31_hash()                    in structs/HashMap            (at /home/dhesselbom/versioned/ooc/rock/sdk/structs/HashMap.ooc:124)                    
6     AstBuilder gotVarDecl_impl()     in rock/frontend/AstBuilder   (at /home/dhesselbom/versioned/ooc/rock/source/rock/frontend/AstBuilder.ooc:479)        
7     nq_onStatement_impl()            in                            (at /home/dhesselbom/versioned/ooc/rock/source/rock/frontend/AstBuilder.ooc:888)        
8     yyDone()                         in                            (at /home/dhesselbom/versioned/ooc/rock/.libs/rock/source/rock/frontend/NagaQueen.c:705)
9     AstBuilder init()                in rock/frontend/AstBuilder   (at /home/dhesselbom/versioned/ooc/rock/source/rock/frontend/AstBuilder.ooc:77)         
10    AstBuilder new()                 in rock/frontend/AstBuilder   (at /home/dhesselbom/versioned/ooc/rock/source/rock/frontend/AstBuilder.ooc:63)         
11    CommandLine postParsing()        in rock/frontend/CommandLine  (at /home/dhesselbom/versioned/ooc/rock/source/rock/frontend/CommandLine.ooc:746)       
12    CommandLine init()               in rock/frontend/CommandLine  (at /home/dhesselbom/versioned/ooc/rock/source/rock/frontend/CommandLine.ooc:536)       
13    CommandLine new()                in rock/frontend/CommandLine  (at /home/dhesselbom/versioned/ooc/rock/source/rock/frontend/CommandLine.ooc:30)        
14    main()                           in                            (at /home/dhesselbom/versioned/ooc/rock/source/rock/rock.ooc:2)                         
15    libc_start_main()                in                            (at /build/buildd/eglibc-2.19/csu/libc-start.c:321)                                     
16    _start()                         in                            (at (null):0)

I'm running rock on the master branch, if it's any help.

@davidhesselbom davidhesselbom changed the title from Compiler segfault to Segfault on build Aug 25, 2014
@shamanas
Collaborator

Hm, weird.

Is lock a generic type or a generic function here btw?

@davidhesselbom
Contributor

Lock is supposed to be defined as

lock: func ~function <T> (function: Func -> T) -> T

and called like this

Foo: class {
    _done: Bool
    done: Bool { get {
        f := func -> Bool { this _done }
        this lock<Bool>(f)
    }}
}

but the 4 lines of code I posted in the OP is the only thing I'm building and is enough to cause the segfault. The same segfault occurs when trying to build this here snippet as well, but I didn't want to distract from what seems to be a parsing issue more than anything else.

@shamanas
Collaborator

Ah, thanks for the info, I'll fix this tonight

@shamanas shamanas added the Bug label Sep 19, 2014
@shamanas shamanas self-assigned this Sep 19, 2014
@shamanas shamanas added the Middle-end label Sep 19, 2014
@shamanas
Collaborator

Hm, the call is handled as a call combo, this is weird :P

@shamanas
Collaborator

@davidhesselbom
So apparently the f<T>(...) syntax is not implemented in Nagaqueen afaict, I was sure it was valid too.
The way I've seen this done is

f: func <T> (T: Class) -> T { /* ... */ }

//...

ok? := f(Bool)

Actually, in your case, this should be enough

Foo: class {
    _done: Bool
    done: Bool { get {
        f := func -> Bool { this _done }
        this lock(f)
    }}
}

@fasterthanlime Should I implement this syntax or not?
I think it looks fine the way it is and the generated code should be exactly the same (generic parameters already compile to arguments of type Class afaik)

@shamanas
Collaborator

Ofc the segfault should not happen, this should be a syntax error.
For some reason this is built as a Combo call, I don't why it segfaults yet though.

@shamanas
Collaborator

It should be documented somewhere for sure though.

@shamanas shamanas added Front-end and removed Middle-end labels Sep 21, 2014
@fasterthanlime
Collaborator

@shamanas I have no idea what you're referring to in

So apparently the f(...) syntax is not implemented in Nagaqueen

Care to elaborate?

@fasterthanlime
Collaborator

@davidhesselbom Just curious - what resource are you locking/releasing access to? In the SDK we have mutexes (mutii?) and we do stuff like

// this call will block until the mutex is available
mutex with(||
  // in there, the mutex is acquired to this thread
  // after the closure ends, the mutex is released
)
// and now the mutex is available again
@shamanas
Collaborator

Ups sorry I didn't escape < and >, I meant

f <String> (...)

Is not valid syntactically

@fasterthanlime
Collaborator

Ah, indeed. Types are kind of first-class in ooc, so you can just pass them as regular arguments. I've never been fond of the Java-y way of doing things which was to specify types when calling it, with carets. (Also it makes parsing harder). But maybe I was mistaken back then. That would be a breaking change, though...

@shamanas
Collaborator

It doesn't necessarily need to be breaking (the current code should run fine if the only thing modified is resolving generic arguments from the function call < and >)

However, I do think the current syntax is pleasent and frankly, this change would require some prettty deep digging into FunctionCall.ooc which may be the most intimidating part of the rock codebase :P

Also:

f: func <T, V> (val: V) -> T { /* ... */ }

f <String> (42) // Does this compile? If it does, isn't it misleading?
// Or do we need
f <String, Int> (42)

// Versus

f: func <T, V> (val: V, T: Class) -> T { /* ... */ }

f(42, String) // \o/ !
@shamanas
Collaborator

Btw, what happens currenty:

this f <Int> ()

// this f -> VariableAccess
// <Int> shouldn't match to anything but FunctionCallNoName is the fallback
// and it somehow ends up matching to it, although it should expect ( args ) directly
// after the variable access...
@shamanas
Collaborator

Also, it would be nice to error out on definitions like

f: func <T> -> T {
    null as T
}

Where we know the generic type cannot be infered from a function call

@fasterthanlime
Collaborator

@shamanas Yeaaaah rock isn't that smart.

@shamanas
Collaborator

@fasterthanlime But it could be! :D
It isn't actually that hard, it's pretty much the same code FunctionCall uses to infer generic types from its arguments and the FunctionDecl (that could be abstracted in some way) + checking if we infered all of them.

We can move this to algo/GenericInference.ooc and do... stuff... there :)

@fasterthanlime
Collaborator

@shamanas Insert something about pile of hacks here and mention proper inference engine

@shamanas
Collaborator

I think splitting off generic inference tools into another module rather than having them tied into FunctionCall would be less hacky though ^_^

@shamanas
Collaborator

I can't figure out why nagaqueen just ignores the < ... > part :/

@fasterthanlime
Collaborator

Couldn't it be parsed as IDENT OP_LESSTHAN IDENT OP_GREATERTHAN PARENS ?

@shamanas
Collaborator

I thought so but the parenthesis are interpreted as FunctionCallNoName which means we are in the Access rule which is

Access =  ((ident:IDENT_CORE { tokenPos; } '&' ![&=] - { core->token[1] += 1; $$=l=nq_onAddressOf(core->this, nq_onVarAccess(core->this, NULL, ident)); }) # special case: blah& is always a reference. blah & blih is a binary and.
          | l:Value)
                ((
                    - '[' - { tokenPos; nq_onArrayAccessStart(core->this, l); }
                             index:Expr - { nq_onStatement(core->this, index); }
                   (',' WS index:Expr - { nq_onStatement(core->this, index); })*
                    - CLOS_SQUAR ~{ throwError(NQE_EXP_CLOSING_SQUAR, "Malformed array access! Expected ']' to end array access.\n"); }
                    - { $$=l=nq_onArrayAccessEnd(core->this); }

                 )
                | - call:FunctionCall                   { nq_onFunctionCallExpr(core->this, call, l); $$=l=call; }
                | - r:IDENT_CORE { tokenPos; $$=l=nq_onVarAccess(core->this, l, r); }
                | - AS_KW { tokenPos; } -  r:Type       { $$=l=nq_onCast(core->this, l, r); }
                | '&' ![&=] - &([ \t\r\n;,)}] | ']')    { core->token[1] += 1; $$=l=nq_onAddressOf(core->this, l); }
                | '@'                                    { $$=l=nq_onDereference(core->this, l); }
                | - call:FunctionCallNoname             { $$=l=nq_onFunctionCallCombo(core->this, call, l); }
                )* -

(FunctionCallNoName only appears in this rule)

@davidhesselbom
Contributor

After I reported this, I noticed that rock tends to crash with segfaults in many cases where a simple "syntax error" message would be more helpful. I think this started happening at around the same time that abstract operator support was added, but I'm not sure.

@fasterthanlime, can mutex with(|| ) blocks have returns in them? In C# you can do

lock (this.lock) {
    // do stuff, more code
    return whatever
}

but in ooc you have to do

result: Int
mutex lock()
// more code
result = whatever
mutex unlock()
result

afaik and by passing a function to lock() we were hoping to keep our code cleaner, so that's why we tried this the first place. If I can do

with mutex(||
    //do stuff, more code
    return whatever
)

I guess that would work too.

with mutex(|| looks weird, by the way :)

@shamanas
Collaborator

@davidhesselbom
I will try to revert the abstract operatory syntax and see if it still segfaults then.
A || ... is a function so a return statement will return from the closure itself, not th eouter function

@shamanas
Collaborator

I think this would work though:

extend Mutex {
    with: func <T> ~return (f: Func -> T) {
        lock()
        ret := f()
        unlock()
        ret
    }
}
@fasterthanlime
Collaborator

Well, that syntax is still not supported, but at least it throws a rock error now, not a segfault.

@fasterthanlime fasterthanlime modified the milestone: 0.9.10 Jul 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment