Changed thread_create() to take the new thread entry point as parameter #30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also cleaned up whitespace (system_calls.rb was using tabs instead of spaces in some places).
…-create-take-the-new-thread-entry-point-as-parameter
… preceeding it).
Background: I removed a return value in storm/include/storm/return_values.h. Then I started getting compilation errors like this: main.c:258:13: error: duplicate case value main.c:249:13: error: previously used here The errors were rather strange since I hadn't done any modification to the main.c file! So I started looking at it. It turned out I was using completely bogus values here when checking the return values (C:s lack of "true" strict typing a la Java or C# comes to mind...). Because of this, and the fact that the enums had now changed values, I started getting these compile-time errors. The first return value (STORM_RETURN_UNALIGNED_SECTION) was never ever returned from the called method (in elf.c). The second (STORM_RETURN_SUCCESS) just "happened" to be the same as RETURN_SUCCESS but I fixed it to be right now anyway.
… assume the new method signature.
It is discouraged by the gcc guys also.
This makes it easier when using system_thread_create(), since you almost always want to cast the parameter to this type anyway (to be able to use functions that receives a parameter other than void *).
…elow. The problem at hand here was, in the end, quite simple. We were trying to copy the stuff into the right location by creating a new pointer based on the ESP value minus the "stack base address". This was all in all correct, but the problem was that we were doing this while we had the *PL3* stack mapped. And that was the problem: we were using a memory address in the PL0 stack (since that's what ESP pointed to at the moment), so we were in essence writing to the wrong memory location... Hard to find, but easy to fix once I managed to find it. Joy! :)
No need for an intermediate variable here.
|
(Sorry for the noisy diff. I should get better at cherrypicking out stuff which better belong elsewhere, or maybe even doing a separate branch/PR for such stuff. It just gets unreadable like this.) |
Thank you, Travis, for noticing this. :)
perlun
added a commit
that referenced
this pull request
May 20, 2015
…e-the-new-thread-entry-point-as-parameter Changed thread_create() to take the new thread entry point as parameter
perlun
deleted the
feature/make-thread-create-take-the-new-thread-entry-point-as-parameter
branch
perlun
added a commit
that referenced
this pull request
Sep 12, 2015
perlun
added a commit
that referenced
this pull request
Oct 8, 2018
) It would previously use the PL0 stack for the newly created thread. This commit fixes this, so that the PL3 stack (which is much less limited in size) is used instead. The use of the PL0 stack was a bug I introduced while fixing #21 via pull request #30, but since it was three years ago I can honestly not say with certainty whether this was "by design" or "by accident". Either way, doing this fix is much better than (ab)using the PL0 stack since the latter is limited in size to a single 4 KiB page => way too little for certain recursive programs and larger programs in general. (This change was a drive-by change while I was researching the root cause for some issues mentioned in #120)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is here now to fix #21. The way we used to do things simply don't cut it with more recent versions of
gcc. Hence, I changedthread_createto take two parameters instead:Coincidentally, this is very similar to how
pthread_createalready does it. 😄The servers have also been fixed to work now with this approach; at least the
consoleand thevgaservers seem to be operational. Hence, merging.