Skip to content

Conversation

@ringabout
Copy link
Member

fixes #25306

type
  StackTraceEntry* = object ## In debug mode exceptions store the stack trace that led
                            ## to them. A `StackTraceEntry` is a single entry of the
                            ## stack trace.
    procname*: cstring      ## Name of the proc that is currently executing.
    line*: int              ## Line number of the proc that is currently executing.
    filename*: cstring      ## Filename of the proc that is currently executing.
    when NimStackTraceMsgs:
      frameMsg*: string     ## When a stacktrace is generated in a given frame and
                            ## rendered at a later time, we should ensure the stacktrace
                            ## data isn't invalidated; any pointer into PFrame is
                            ## subject to being invalidated so shouldn't be stored.
    when defined(nimStackTraceOverride):
      programCounter*: uint ## Program counter - will be used to get the rest of the info,
                            ## when `$` is called on this type. We can't use
                            ## "cuintptr_t" in here.
      procnameStr*, filenameStr*: string ## GC-ed alternatives to "procname" and "filename"

@arnetheduck
Copy link
Contributor

maybe add =copy...{.error.} somewhere?

@ringabout
Copy link
Member Author

ringabout commented Nov 25, 2025

=copy...{.error.}

I can add it for nimStackTraceOverride . Though I hope it won't break something

@ringabout ringabout marked this pull request as draft November 25, 2025 14:11
@arnetheduck
Copy link
Contributor

I can add it for nimStackTraceOverride . Though I hope it won't break something

I think there's more value adding it always to remove as many differences as possible between override and non-override mode

@ringabout ringabout marked this pull request as ready for review November 25, 2025 14:24
@ringabout
Copy link
Member Author

ringabout commented Nov 26, 2025

That would potentially break external libraries that use StackTraceEntry.

type
  StackTraceEntry* = object
    when defined(nimStackTraceOverride):
      programCounter*: uint ## Program counter - will be used to get the rest of the info,
                            ## when `$` is called on this type. We can't use
                            ## "cuintptr_t" in here.
      procnameStr*, filenameStr*: string ## GC-ed alternatives to "procname" and "filename"
    else:
      procname*: cstring      ## Name of the proc that is currently executing.
      filename*: cstring      ## Filename of the proc that is currently executing.

when defined(nimStackTraceOverride):
  proc `procname`*(s: StackTraceEntry): cstring =
    s.procnameStr.cstring

  proc `filename`*(s: StackTraceEntry): cstring =
    s.filenameStr.cstring

Ideally, we only provide accessors for procname and filename for nimStackTraceOverride. The differences over filename and procname with/without nimStackTraceOverride already makes these options diverge

@arnetheduck
Copy link
Contributor

only provide accessors

come to think of it, this is tricky because in current versions, libbacktrace has to set these values to a valid pointer as part of constructing the trace entry.

another way I thought of was to implement an actual =copy implementation so the compiler would inject copies in the right places - that copy operator would do the necessary pointer adjustments - not sure if that's possible at this early stage of the nim compilation pipeline though

for i in 0..<s.len:
let entry = addr s[i]
result.add(copyStackTraceEntry entry)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to copy anything here, all you need to do it to patch every entry after an add operation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strictly speaking, in this particular case, one would want to move the existing entries...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add operation triggers a copy, which is marked {.error.}. Perhaps I need to use nodestroy or something?

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 this pull request may close these issues.

Dangling pointers in stack traces with -d:nimStackTraceOverride

4 participants