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

Merge branch 'version-1-2' into nimbus #3

Closed
wants to merge 19 commits into from
Closed

Merge branch 'version-1-2' into nimbus #3

wants to merge 19 commits into from

Conversation

etan-status
Copy link

@etan-status etan-status commented Oct 22, 2021

This includes a fix to allow converting static vars to openArray.
It is needed to support compile-time optimizations for ongoing merkle
multiproof work in nimbus-eth2.

narimiran and others added 17 commits April 28, 2021 08:40
* CIs: attempt to use csources_v1
* also updated the BSDs
* also updated azure pipelines
* std modules should not itself use the 'std/' import dir...
* compiler has to be careful with std/ for v1 booting

(cherry picked from commit a9b62de)
…18039) [backport:1.2]

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 a1c82c3)
…kport:1.0]

* fix nim-lang#18702
* Apply suggestions from code review

(cherry picked from commit 901c5de)
@etan-status
Copy link
Author

nim-lang#19035

@etan-status etan-status marked this pull request as ready for review October 22, 2021 13:06
@tersec
Copy link

tersec commented Oct 22, 2021

I'm less than enthusiastic about creating a divergence from every specific upstream Nim version. At least in theory, we've been able to target some specific name-able version of upstream Nim, rather than Status's-own-Nim-amalgamation.

@etan-status
Copy link
Author

This has been accepted into upstream devel.

Sent request to backport to the older versions as well.
1.2 PR: nim-lang#19049

When assigning constant output to a seq, and then passing that static
seq to other functions that take `openArray`, the compiler may end up
producing errors, as it does not know how to convert `static[seq[T]]`
to `openArray[T]`. By ignoring the `static` wrapper on the type for
the purpose of determining data memory location and length, this gets
resolved cleanly. Unfortunately, it is relatively tricky to come up
with a minimal example, as there are followup problems from the failing
conversion, e.g., this may lead to `internal error: inconsistent
environment type`, instead of the relevant `openArrayLoc` error message.

(cherry picked from commit 490c422)
@etan-status etan-status changed the title allow converting static vars to openArray Merge branch 'version-1-2' into nimbus Oct 27, 2021
@etan-status
Copy link
Author

This has been accepted into version-1-2 upstream. I have updated this PR to merge from version-1-2 instead of cherry-picking.

This includes a fix to allow converting `static` vars to `openArray`.
It is needed to support compile-time optimizations for ongoing merkle
multiproof work in `nimbus-eth2`.
@stefantalpalaru
Copy link

Unfortunately, we have a long tradition of only using official Nim releases. Exceptions to that need to have a very strong case behind them.

So how badly do we need this?

@etan-status
Copy link
Author

Will update this when the next 1.2 Nim release is ready.

@etan-status etan-status marked this pull request as draft October 28, 2021 06:45
@etan-status
Copy link
Author

New Nim release is ready as a nightly. Closing this so that the usual process can be followed for updating to newer Nim versions.

@etan-status etan-status closed this Nov 5, 2021
etan-status pushed a commit that referenced this pull request Apr 10, 2023
tlsEmulation:on under NetBSD-10Beta and NetBSD-current produces an executable which crashes immediately as follows:

Core was generated by `koch'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000000000047b4c2 in nimZeroMem ()
(gdb) bt
#0  0x000000000047b4c2 in nimZeroMem ()
#1  0x00000000004897b2 in threadVarAlloc__system_2162 ()
#2  0x000000000048980e in initThreadVarsEmulation ()
#3  0x0000000000489848 in PreMain ()
#4  0x000000000048986a in NimMain ()
nim-lang#5  0x00000000004898a9 in main ()

I can't speak about the other BSDs.
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.

8 participants