Analyzing destructor #4371

Closed
andreaferretti opened this Issue Jun 20, 2016 · 10 comments

Projects

None yet

3 participants

@andreaferretti
Contributor

I am not sure what this is, but the introduction of this line in commit 95b4a9c has completely broken rosencrantz, which now does not even build anymore (it does not use destructors at all).

Any chance someone could give an explanation what this change is about and what possible fixes I can introduce?

@Varriount
Contributor

Destructors may be implicitly created by the compiler for certain types, such as object types that contain sequences (I think).

That change was my attempt at fixing a runway recursion within the compiler's destructor generation process when confronted with an object type containing a sequence that also contains the same object type (#4363).

What error do you get when building rosencrantz?

@andreaferretti
Contributor
andreaferretti commented Jun 20, 2016 edited
$ nimble server
Executing task server in /home/papillon/esperimenti/rosencrantz/rosencrantz.nimble
Compiling tests/server (rosencrantz) using c backend...
Traceback from system (most recent call last)
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
FAILURE: Execution failed with exit code 1

This is when building the example server, that contains all the features. The same happens when building a smaller application (nimble todo).

I have not been able to isolate where the failure comes from yet.

Both examples can be tried by cloning the repo and running nimble server or nimble todo respectively.

@andreaferretti
Contributor

I tried to compile the server with a non-release-mode compiler and got the following stacktrace:

Compiling tests/server (rosencrantz) using c backend...
Traceback (most recent call last)
nim.nim(115)             nim
nim.nim(71)              handleCmdLine
main.nim(253)            mainCommand
main.nim(64)             commandCompileToC
modules.nim(227)         compileProject
modules.nim(172)         compileModule
passes.nim(203)          processModule
passes.nim(137)          processTopLevelStmt
sem.nim(461)             myProcess
sem.nim(433)             semStmtAndGenerateGenerics
semstmts.nim(1536)       semStmt
semexprs.nim(873)        semExprNoType
semexprs.nim(2420)       semExpr
importer.nim(176)        evalImport
importer.nim(165)        myImportModule
modules.nim(185)         importModule
modules.nim(172)         compileModule
passes.nim(203)          processModule
passes.nim(137)          processTopLevelStmt
sem.nim(461)             myProcess
sem.nim(433)             semStmtAndGenerateGenerics
semstmts.nim(1536)       semStmt
semexprs.nim(873)        semExprNoType
semexprs.nim(2420)       semExpr
importer.nim(176)        evalImport
importer.nim(165)        myImportModule
modules.nim(185)         importModule
modules.nim(172)         compileModule
passes.nim(203)          processModule
passes.nim(137)          processTopLevelStmt
cgen.nim(1220)           myProcess
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2152)       expr
cgen.nim(789)            genProc
cgen.nim(759)            genProcNoForward
cgen.nim(679)            genProcAux
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2083)       expr
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2083)       expr
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2109)       expr
ccgstmts.nim(366)        genReturnStmt
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2111)       expr
ccgstmts.nim(1114)       genAsgn
ccgstmts.nim(86)         loadInto
ccgexprs.nim(2101)       expr
ccgexprs.nim(1854)       genClosure
cgen.nim(479)            initLocExpr
ccgexprs.nim(1988)       expr
cgen.nim(789)            genProc
cgen.nim(759)            genProcNoForward
cgen.nim(679)            genProcAux
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2083)       expr
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2083)       expr
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2083)       expr
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2105)       expr
ccgstmts.nim(261)        genVarStmt
ccgstmts.nim(250)        genClosureVar
ccgstmts.nim(86)         loadInto
ccgexprs.nim(2081)       expr
ccgexprs.nim(1886)       genStmtListExpr
ccgexprs.nim(2101)       expr
ccgexprs.nim(1854)       genClosure
cgen.nim(479)            initLocExpr
ccgexprs.nim(1988)       expr
cgen.nim(789)            genProc
cgen.nim(759)            genProcNoForward
cgen.nim(679)            genProcAux
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2083)       expr
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2083)       expr
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2083)       expr
ccgstmts.nim(1120)       genStmts
ccgexprs.nim(2048)       expr
ccgcalls.nim(533)        genCall
ccgcalls.nim(176)        genPrefixCall
ccgcalls.nim(145)        genArg
cgen.nim(232)            addrLoc
ccgtypes.nim(116)        mapType
@andreaferretti
Contributor

I have isolated the error. It arises from having defined (even if it is not used) the following procedure:

proc logging*(handler: Handler): auto =
  proc h(req: ref Request, ctx: Context): Future[Context] {.async.} =
    let ret = handler(req, ctx)
    debugEcho "$3 $1 $2".format(req.reqMethod, req.url.path, req.hostname)
    return await ret

  return h

where the types are defined by

type
  List[A] = ref object
    value: A
    next: List[A]
  StrPair* = tuple[k, v: string]
  Context* = object
    position*: int
    accept*: bool
    headers*: List[StrPair]
  Handler* = proc(req: ref Request, ctx: Context): Future[Context]

and Request is imported from asynchttpserver.

Turns out, my issue is solved by changing the definition to

proc logging*(handler: Handler): auto =
  proc h(req: ref Request, ctx: Context): Future[Context] {.async.} =
    debugEcho "$3 $1 $2".format(req.reqMethod, req.url.path, req.hostname)
    return await handler(req, ctx)

  return h

Still, it may be the case that some bug is hiding

@Araq
Member
Araq commented Jun 22, 2016

Just fyi (I'm sure it has nothing to do with the bug), for me Rosencrantz fails with

rosencrantz/handlers.nim(2, 19) Error: cannot open 'rosencrantz/core'

@andreaferretti
Contributor

Yeah, it has to do with the new import that does not add the project dir as root path.

It works using the task nimble server (that sets --path: ".") or when importing rosencrantz as a nimble dependency (in which case the rosencrantz dir is added to the search paths by nimble)

@Araq
Member
Araq commented Jun 22, 2016

Well I can reproduce the crash, but it crashes with or without the line t.destructor = analyzingDestructor.

@andreaferretti
Contributor

You are right, probably other modifications have happened in the meantime. When I first reported the bug, that line was the only thing that made the difference.

For me, I am fine to change the definition of logging in Rosencrantz, but there may be still a bug :-?

@Araq
Member
Araq commented Jun 22, 2016

Well it's a compiler crash. Of course there is a serious bug in the compiler somewhere. I'm working on it.

@andreaferretti
Contributor

Just wanted to mention that I updated Rosencrantz, so if you want to reproduce the bug, be sure to check out commit 0c4da1e4e1ddb76563c6947c13d445a36c7cf51d

@Araq Araq added a commit that closed this issue Jul 8, 2016
@Araq Araq fixes #4371 b47d9b7
@Araq Araq closed this in b47d9b7 Jul 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment