Copying StackTraceEntry instances when nimStackTraceOverride is defined
breaks the link between a cstring field that's supposed to point at
another string field in the same object.
Sometimes, the original object is garbage collected, that memory region
reused for storing other strings, so when the StackTraceEntry copy tries
to use its cstring pointer to construct a traceback message, it accesses
unrelated strings.
This only happens for async tracebacks and this patch prevents that by
making sure we only use the string fields when nimStackTraceOverride is
defined.
Async tracebacks also beautified slightly by getting rid of an extra line
that was supposed to be commented out, along with the corresponding debugging output.
There's also a micro-optimisation to avoid concatenating two strings just
to get their combined length.
(cherry picked from commit a1c82c39af)
* Remove the use of usrToCell in gcMark [backport:1.2]
Recently, we've discovered a GC crash resulting from inlining of
the memory allocation procs that allowed the compiler to avoid
maintaining any references to the "user pointer" on the stack.
Instead, a "cell pointer" appeared there and all field accesses
were performed with adjusted offsets. This interfered with the
ability of the GC to mark the correct cell in the conservative
stack scans which lead to premature collection of objects.
More details here:
af69b3ceae
This commit closes another theoretical loophole that may lead to
the same problem. If a short proc is accessing both the object and
its reference count in a short sequence of instructions, the compiler
may be enticed to reduce the number of registers being used by storing
only a single pointer to the object and using offsets when reading
and writing fields. A perfectly good strategy would be to store only
the cell pointer, so the reference count updates can be performed
without applying offsets. Accessing the fields of the object requires
offsets anyway, but these can be adjusted at compile-time without any
loss. Following this strategy will lead to the same problem of marking
a wrong cell during the conservative stack scan, leading to premature
collection.
The problem is avoided by not using `usrToCell` in `gcMark`. Since
the cell discovery logic can already handle interior pointers, the
user pointers don't need to be adjusted for the GC to function correctly.
(cherry picked from commit 3b47a689cf)
This is taken from:
af69b3ceae
Full original comment:
This is to avoid heavy inlining happening when two allocation calls
would occur shortly after each other.
This inlining would sometimes be accompanied with an optimisation
as the compiler is able to see that cellToUsr ending the first
allocation call is shortly followed by an usrToCell call. The
pointer arithmetic is redundant and the compiler can eliminate it,
leaving only the cell address in a register (and later the stack)
instead of the actual pointer to the user data, as one would expect.
This combined with a GC collect cycle will cause the stack scan to
only notice the cell address, which is of no good due to a usrToCell
in the gcMark call which shifts that address to an adjacent cell.
This means that the actual cell of importance will not get marked
and thus cause a premature collection of that cell. BOOM.
(cherry picked from commit 40093b4a93)
* nimStackTraceOverride: enable stack traces in exceptions
This is a two-step stack trace collection scheme, because re-raised
exceptions will collect multiple stack traces but use them rarely, when
printing info about an uncaught exception, so it makes sense to only do
the cheap stack unwinding all the time and the relatively expensive
debugging information collection on-demand.
`asyncfutures` implements its own `$` proc for printing
`seq[StackTraceEntry]`, so we have to add the debugging info there, just
like we do for the private `$` proc in `system/excpt`.
* cleaned up PR #15284
Co-authored-by: Ștefan Talpalaru <stefantalpalaru@yahoo.com>
(cherry picked from commit 1fae66e4df)
* progress
* make tests green
* maybe we also want to reset pointers, dunno
* progress
* cleanup; fixes#13879 [backport:1.2]
(cherry picked from commit b35d370d88)
* code cleanups and feature additions
* added basic test and koch/CI integration
* make it build on Unix
* DrNim: now buildable on Unix, only takes 10 minutes, enjoy
* added basic documentation for DrNim which can also be seen as the RFC we're following
* drnim: change the build setup so that drnim.exe ends up in bin/
* makes simple floating point ranges work
* added basic float range check
* drnim: teach Z3 about Nim's range types plus code refactoring
* drnim: make unsigned numbers work
* added and fixed index checking under setLen
* first implementation of .ensures, .invariant and .assume (.requires still missing and so is proc type compatibility checking
* drnim: .requires checking implemented
* drnim: implemented .ensures properly
* more impressive test involving min()
* drnim: check for proc type compatibility and base method compatibility wrt .requires and .ensures
* testament: support for 'pattern <directory>
* koch: uses new <directory> feature of testament
* drnim: added tiny musings about 'old'
* Make testament work with old SSL versions
* koch: add support for 'koch drnim -d:release'
* drnim: preparations for the param.old notation
* stacktraces can now show custom runtime msgs
* improve tests/stdlib/tstackframes.nim
* fix test for --gc:arc
* test --stacktraceMsgs:on and --stacktraceMsgs:off
* --stacktracemsgs:off by default
* config update
* disable a questionable test
* remove c++ exception handling IDs, new impl doesn't require it anymore
* C++ based exceptions finally work
* fixes bootstrapping problem in C++ mode
* teach GCC it's 2020 now
* more bugfixes for C++ based exception handling
* apply cooldome's patch
* another attempt to enable C++11
* bug fix
Co-authored-by: Araq <rumpf_a@web.de>
Co-authored-by: cooldome <ariabushenko@bk.ru>
* allow defects to be caught even for --exceptions:goto (WIP)
* implemented the new --panics:on|off switch; refs https://github.com/nim-lang/RFCs/issues/180
* new implementation for integer overflow checking
* produce a warning if a user-defined exception type inherits from Exception directly
* applied Timothee's suggestions; improved the documentation and replace the term 'checked runtime check' by 'panic'
* fixes#13627
* don't inherit from Exception directly
* ensure capitalize doesn't take an inferred sink parameter
* sink parameter inference: first version, for now disabled. Changed that sink parameters can be consumed multiple times in order to adhere to our spec.
* sink inference can now be disabled with .nosinks; sometimes for proc type interop this is required
* fixes yet another critical DFA bug
* better implementation that also understands if expressions etc
* document sink parameter inference and allow for global disabling
* fix#9634 debugging a program using execCmdEx now works
* only apply EINTR to c_gets for now
This reverts commit c0f5305b5a0b46983dfd27e3d77ecbf4f8744dcc.
On some OSes (such as FreeBSD or Solaris), pthread_attr_init allocate
memory. So it is necessary to deallocate that memory by using
pthread_attr_destroy.
