Skip to content

Commit

Permalink
remove -Os option for gcc, it breaks TRUENAME check for JOIN/SUBST/AS…
Browse files Browse the repository at this point in the history 
…SIGNed drives
  • Loading branch information
@PerditionC
PerditionC committed Nov 20, 2021
1 parent 5a34eb7 commit 3754daa
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions src/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,9 @@ endif
#EXEFLAGS=-ffreestanding -mrtd
EXEFLAGS?=
LDFLAGS?=-li86
CFLAGS?=-I../kitten -mcmodel=small -fpack-struct -Werror -Os -fno-strict-aliasing $(EXEFLAGS) -o
CFLAGS?=-I../kitten -mcmodel=small -fpack-struct -Werror -fno-strict-aliasing $(EXEFLAGS) -o
endif


ifeq ($(COMPILER),cl)
# Microsoft compatible, Digital Mars (dmc)
# -0s favor small code
Expand Down

5 comments on commit 3754daa

@andrewbird
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should report this to @tkchia , it seems very odd.

@PerditionC
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to, but wanted to see if I could make a minimal test case first.

@tkchia
Copy link
Contributor

@tkchia tkchia commented on 3754daa Nov 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @PerditionC, hello @andrewbird,

The problem is in the label.c code — in fact the same problem might potentially crop up in the Watcom port as well. In

    segread(&sregs);
    regs.x.si = (unsigned)buf1; /* needs NEAR POINTER */
    regs.x.di = (unsigned)buf2; /* needs NEAR POINTER */
    regs.x.ax = 0x6000;
    intdosx(&regs, &regs, &sregs);
    if (*buf1 != *buf2) ...

the segread (·) call may not set sregs.es to point to the program's data segment. This is because the ABI (whether under gcc-ia16 or Open Watcom) does not guarantee the es register to have any particular value at that point. And we need to set both sregs.ds and sregs.es properly for the truename syscall.

So you probably want to do something like

    segread(&sregs);
    sregs.es = sregs.ds; /* <== */
    regs.x.si = (unsigned)buf1; /* needs NEAR POINTER */
    ...

(Personally I would prefer to use FP_SEG and FP_OFF to initialize sregs.ds, sregs.es, regs.x.si and regs.x.di. That though is just a matter of style.)

Thank you!

@tkchia
Copy link
Contributor

@tkchia tkchia commented on 3754daa Nov 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @PerditionC,

Anyway, I would not recommend removing all optimization options for GCC. The default is no optimization (-O0), which results in extremely dumb code.

Thank you!

@PerditionC
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I did a quick read over the code and saw es was being read (segread), but forgot to consider es not being valid itself, should have known given the comment about needing small or tiny model which is a good indicator segments handling may not be right in all settings. Andrew has already submitted a patch with the suggested fix so I will test and commit it after I get up. Thank you both.

Please sign in to comment.