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

Fix issue related to pointer storage. #86

Merged
merged 1 commit into from
May 6, 2024
Merged

Conversation

kalmard0
Copy link
Contributor

@kalmard0 kalmard0 commented May 5, 2024

I'm not sure what the "ci" field does, but I noticed while testing inkcpp on various platforms that an unexpected crash happens related to it (it having a value of 255).

After some investigation I realized that in every case other than storage, ci is treated as an int. As a matter of fact, various pieces of code expect it to be able to have the values of "255", "0" and "-1", which is impossible for an 8-bit type.

This change fixes my crash and I believe is correct.

@kalmard0 kalmard0 force-pushed the ci branch 2 times, most recently from 8857e9f to d688cc9 Compare May 5, 2024 17:17
I'm not sure what the "ci" field does, but I noticed while testing
inkcpp on various platforms that an unexpected crash happens related to
it (it having a value of 255).

After some investigation I realized that in every case other than
storage, ci is treated as an int. As a matter of fact, various pieces
of code expect it to be able to have the values of "255", "0" and "-1",
which is impossible for an 8-bit type.

This change fixes my crash and I believe is correct.
@JBenda
Copy link
Owner

JBenda commented May 5, 2024

Thanks for spotting it, your fix looks correct.
Could you provide your crash?
Then I can add a new test case ^^

@kalmard0
Copy link
Contributor Author

kalmard0 commented May 5, 2024

The crash was due to an assertion in stack.cpp line 86: inkAssert(ci == -1 || ci == 0, "only support ci == -1, for now!");
It could be 100% reproduced by building inkcpp with emscripten, and playing through The Intercept always picking the first choice. It happened after 3-4 choices.

I could not reproduce it on an easy-to-debug platform. I suspect it may be possible by finding a compiler flag that changes the signedness of char.

@JBenda
Copy link
Owner

JBenda commented May 6, 2024

Ah, jea, funny that it was working until now. I'm currently thinking about using a int8_t but I think this does not matter much.

The ci is a call stack reference to distinguish between variables with the same name

@JBenda JBenda merged commit df6dfdf into JBenda:master May 6, 2024
8 of 9 checks passed
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.

2 participants