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

Adding -Ciro breaks Linux #786

Open
barbeque-squared opened this issue Dec 20, 2023 · 6 comments · Fixed by #788
Open

Adding -Ciro breaks Linux #786

barbeque-squared opened this issue Dec 20, 2023 · 6 comments · Fixed by #788

Comments

@barbeque-squared
Copy link
Collaborator

barbeque-squared commented Dec 20, 2023

EDIT 2023-12-24: keep this open until we're actually using -Ciro -gl -OoNOSTACKTRACE in all builds. This will probably take multiple PR's to fully fix

See also #750 and the patch below (I'm not allowed to attach it for some reason).

diff --git a/src/Makefile.in b/src/Makefile.in
index f5b2b14..fb295b7 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -97,7 +97,7 @@ PFLAGS_EXTRA   += @PFLAGS_EXTRA@
 # - Note that fpc.cfg already defines -vinw, so add -v0 first
 # - The stack check (-Ct) might not work with enabled threading
 # - Do we need -Coi?
-PFLAGS_BASE_DEFAULT    := -Si -Sg- -Sc- -v0Binwe
+PFLAGS_BASE_DEFAULT    := -Si -Sg- -Sc- -v0Binwe -Ciro -gl
 PFLAGS_DEBUG_DEFAULT   := -Xs- -g -gl -dDEBUG_MODE
 PFLAGS_RELEASE_DEFAULT := -Xs- -O2
 PFLAGS_EXTRA_DEFAULT   :=

This works on Windows to expose the underlying error from the aforementioned ticket. However, when used on Linux, the game will crash when going to the song selection screen from the player selection screen with the following error on the console:

Sorry, an error ocurred! Please report this error to the game-developers. Also check the Error.log file in the game folder.
Stacktrace:
Exception class: ERangeError
Message: Range check error
  $000000000045FAFF  RGBFLOATTOINT,  line 291 of menu/UMenu.pas

(Error.log contains no useful information)

This ticket serves more as reasoning why I didn't include this Makefile change in PR #787 . I assume it would be useful to enable this eventually everywhere, but I don't know how many other errors this will expose (which will only occur runtime). We know Windows is fine (the pipeline builds have always used -Ciro), this purely affects Linux (and probably Mac).

@s09bQ5
Copy link
Collaborator

s09bQ5 commented Dec 20, 2023

I have a fix for that one. I was also playing with -Ciro on Linux.

@barbeque-squared
Copy link
Collaborator Author

I've done some experimenting to get usable stacktraces, but I'm not getting anywhere. What I did:

  • in UScreenSingView line 531, just change the 1 to 6 to 1 to 6000 (just do whatever to consistently throw an error)
  • in UMain mess around with the exception handler

I've been dumping some of the variables used in the UMain exception handler, and maybe I'm doing it wrong but ExceptFrameCount is always 0 for me. The closest explanation for this that I was able to find was https://www.freepascal.org/docs-html/rtl/sysutils/exceptframecount.html but this doesn't make any sense?

Why is this 0 at this point in the code?

@s09bQ5
Copy link
Collaborator

s09bQ5 commented Dec 22, 2023

I have no idea why it doesn't work on your system. I have literally just executed

git checkout -f origin/master
./configure --enable-debug
sed -i '/^PFLAGS_BASE_DEFAULT/s/$/ -Ciro/' src/Makefile
make
./game/ultrastardx

to get

Sorry, an error ocurred! Please report this error to the game-developers. Also check the Error.log file in the game folder.
Stacktrace:
Exception class: ERangeError
Message: Range check error
  $000000000046556D  RGBFLOATTOINT,  line 291 of menu/UMenu.pas
  $000000000056D946  CREATE,  line 826 of screens/views/UScreenSingView.pas
  $000000000055FAC4  CREATE,  line 651 of screens/controllers/UScreenSingController.pas
  $000000000053DCA1  PARSEINPUT,  line 409 of screens/UScreenName.pas
  $00000000004634E6  PARSEINPUT,  line 674 of menu/UDisplay.pas
  $0000000000486973  CHECKEVENTS,  line 584 of base/UMain.pas
  $000000000048574F  MAINLOOP,  line 341 of base/UMain.pas
  $00000000004854A8  MAIN,  line 269 of base/UMain.pas
  $000000000040750E  main,  line 414 of ultrastardx.dpr

@barbeque-squared
Copy link
Collaborator Author

./configure --enable-debug

The --enable-debug does something. On Windows I never gave it this flag but it was still reporting full stacktraces. But on Linux apparently I do need to pass it. I'm not sure what's causing this (could be /etc/fpc.cfg, could be some ifdef in USDX, could be just how stuff is supposed to work) but I dislike this sort of inconsistencies.

I'll figure out where it's coming from in the coming days.

@s09bQ5
Copy link
Collaborator

s09bQ5 commented Dec 23, 2023

The only thing --enable-debug does is cause src/Makefile to pass -g -gl -dDEBUG_MODE instead of -O2 to fpc. If I add -O2 the stacktrace stops after the first line. Adding -OoNOSTACKFRAME makes it work again.

@barbeque-squared
Copy link
Collaborator Author

Thank you for this explanation! And you are right: if I add -OoNOSTACKTRACE to the PFLAGS_RELEASE_DEFAULT line, then I do get full stacktraces without any configure flags. I guess (my) Linux builds a release version by default.

Ie my src/Makefile.in now looks like this:

PFLAGS_BASE_DEFAULT    := -Si -Sg- -Sc- -v0Binwe -Ciro -gl
PFLAGS_DEBUG_DEFAULT   := -Xs- -g -gl -dDEBUG_MODE
PFLAGS_RELEASE_DEFAULT := -Xs- -O2 -OoNOSTACKFRAME
PFLAGS_EXTRA_DEFAULT   :=

Some cleanup/deduplication can probably be done to those lines, and we'd need to test if at least the Windows make method and the CI are also still happy with this, but would having stackframes even have a noticeable effect on the performance of release builds? Obviously it will increase the size of the executable a bit, but it doesn't appear to be stupid big and it's incredibly useful in reports.

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 a pull request may close this issue.

2 participants