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 crash from an incomplete do-while statement #680

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

Daniel-Cortez
Copy link
Contributor

@Daniel-Cortez Daniel-Cortez commented Sep 30, 2021

What this PR does / why we need it:

Fixes the compiler crash from an incomplete do-while statement without the while (...); part (see #678).
Function test() (file sc1.c) was saving and restoring the previous value of sc_intest through PUSHSTK_I()/POPSTK_I(), and this was preventing the compiler from stopping the compilation process when the end of file was reached before closing the do-while statement. Making the function save/restore the value of sc_intest using the actual stack (by saving the value into a local variable) fixes the issue.

Which issue(s) this PR fixes:

Fixes #678

What kind of pull this is:

  • A Bug Fix
  • A New Feature
  • Some repository meta (documentation, etc)
  • Other

Additional Documentation:

@Daniel-Cortez Daniel-Cortez requested a review from a team as a code owner September 30, 2021 15:09
@YashasSamaga YashasSamaga linked an issue Oct 8, 2021 that may be closed by this pull request
@stale
Copy link

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale because it has not had recent activity.

@stale stale bot added the state: stale label Jan 9, 2022
@Y-Less
Copy link
Member

Y-Less commented Mar 8, 2022

Merged locally.

@Y-Less Y-Less closed this Mar 8, 2022
@Y-Less Y-Less reopened this Mar 8, 2022
@stale stale bot removed the state: stale label Mar 8, 2022
@Y-Less
Copy link
Member

Y-Less commented Mar 8, 2022

I get an access violation here after merging this PR:

/* pc_readsrc()
 * Reads a single line from the source file (or up to a maximum number of
 * characters if the line in the input file is too long).
 */
char *pc_readsrc(void *handle,unsigned char *target,int maxchars)
{
  return fgets((char*)target,maxchars,(FILE*)handle);
}
Exception thrown at 0x77A0FF05 (ntdll.dll) in pawncc.exe: 0xC0000005: Access violation writing location 0x000000B8.

I can't see why, looking at the change, but it was definitely introduced by 3e0c5ba

@Y-Less
Copy link
Member

Y-Less commented Mar 8, 2022

When compiling the YSI test script:

https://github.com/pawn-lang/YSI/blob/master/gamemodes/YSI_TEST.pwn

@Daniel-Cortez
Copy link
Contributor Author

Daniel-Cortez commented Mar 8, 2022

My bad, it seems I forgot to replace the use of POPSTK_I here:

https://github.com/Daniel-Cortez/pawn-3.10/blob/f053b83a48f81287c2871a5b00445fb102b73f86/source/compiler/sc1.c#L6020

This caused corruption of the value stack, would only happen when the control expression in if, while, do..while, for, assert or state results in a constant value, which is pretty rare as normally the compiler warns about this. In your case it was assert(false) in YSI_Players\y_groups\y_groups_entry.

@Daniel-Cortez
Copy link
Contributor Author

OK, now it should be fixed, both here and in #681. Not sure if I should add a separate test for this, as the case seems to be very specific.

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.

Crash from an incomplete do-while statement
2 participants