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 reused integers from being optimized out #14

Merged
merged 1 commit into from
Mar 2, 2021

Conversation

jwflory
Copy link
Contributor

@jwflory jwflory commented Feb 17, 2021

Summary

Fix segfault when reused integers are optimized out by compiler

Background

This patch was contributed by @Jan200101 to fix a reported segfault in the Fedora package from compiler optimizations. We can carry a custom patch for the Fedora-specific package, but we generally try to work with an upstream so everyone may benefit from a change, not only Fedora.

This change was tested with a local scratch build on at least two Fedora machines. It is likely other distributions could face similar issues when upgrading their compilers.

Details

It is a simple change, and honestly I am not a C developer. If you really want an explanation, @Jan200101 could probably provide one if you ask.

If this change is accepted, it would be helpful to cut a new release with a git tag. This will trigger the first step in the Fedora package distribution process for shipping an update. 🙂

Outcome

Support optimizations from newer versions of compilers without segfaulting after opening the mailbox

@Jan200101
Copy link
Contributor

The problem was with how gcc optimized the program
(gcc versions before 10.2.1 might not have this problem)
compiling zork with -O2, as one does for Fedora packages, made the compiler try to optimize i__2 out
(gdb output copied from the previously mentioned report)

#0  sparse_ (lbuf=0x7fffffffd54c, lbuf@entry=0x7fffffffd550, llnt=3, vbflag=vbflag@entry=1) at np1.c:128
        ret_val = -1
        i__1 = 3
        i__2 = <optimized out>
        i = 3
        j = 44617
        obj = <optimized out>
        prep = 0
        lbuf1 = 3258
        lbuf2 = 0

This being the code that triggers this sigsegv

zork/np1.c

Lines 126 to 131 in 95b1fd1

L200:
i__2 = prplnt;
for (j = 1; j <= i__2; j += 3) {
/* !LOOK FOR PREPOSITION. */
if (lbuf1 == prpvoc_1.pvoc[j - 1] && lbuf2 == prpvoc_1.pvoc[j]) {
goto L4000;

if we look at prplnt

zork/np1.c

Line 58 in 95b1fd1

prplnt = 48;

we can tell something went horribly wrong in that gdb backtrace since j is far larger than prplnt
the way to solve this is to tell the compiler not to optimize i__2 out, and in the case of my patch I made all system generated locals that are reused over the file volatile

@jwflory
Copy link
Contributor Author

jwflory commented Mar 2, 2021

Thanks for clarifying the change @Jan200101.

@devshane Do you have a moment to review this patch? It would be great to add this here in the upstream project instead of maintaining a downstream patch only available in the Fedora package.

@devshane devshane merged commit c44f046 into devshane:master Mar 2, 2021
@devshane
Copy link
Owner

devshane commented Mar 2, 2021

Thanks for the fix!

https://github.com/devshane/zork/releases/tag/v1.0.3

@jwflory jwflory deleted the fix/compiler-optimizations branch March 31, 2021 22:50
wesleyneal346 added a commit to wesleyneal346/zork that referenced this pull request Sep 19, 2024
stop reused integers from being optimized out (devshane#14)
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.

3 participants