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

Functions that have inline assembly that refer to local variables or parameters will refer to them by EBP, but function prologue does not set up EBP stack frame #805

Closed
joncampbell123 opened this issue Jan 29, 2022 · 5 comments
Labels
CG Code generator enhancement

Comments

@joncampbell123
Copy link
Contributor

Take this contrived example from DOSLIB for example:

void blahblah(int vx,int *ay) {
        unsigned int x;

#if TARGET_MSDOS == 32
        __asm {
                mov     eax,vx
                int     2fh
                mov     x,ebx
        }
#else
        __asm {
                mov     ax,vx
                int     2fh
                mov     x,bx
        }
#endif
}

Output according to WDIS

Routine Size: 600 bytes,    Routine Base: _TEXT + 0000

0258                            blahblah_:
0258  53                                push            ebx
0259  51                                push            ecx
025A  56                                push            esi
025B  57                                push            edi
025C  83 EC 08                          sub             esp,0x00000008
025F  89 04 24                          mov             [esp],eax
0262  8B 85 00 00 00 00                 mov             eax,[ebp]
0268  CD 2F                             int             0x2f
026A  89 9D 04 00 00 00                 mov             0x4[ebp],ebx
0270  83 C4 08                          add             esp,0x00000008
0273  5F                                pop             edi
0274  5E                                pop             esi
0275  59                                pop             ecx
0276  5B                                pop             ebx
0277  C3                                ret

Values going into and out of the inline assembly are wrong because it reads and writes through EBP, but the function prologue/epilogue never did the usual PUSH EBP, MOV EBP, ESP and LEAVE routine. Stack frame checking is disabled when compiling this code. So when the inline asm writes to what it thinks is a local variable, it's writing to whatever stack location the parent function happened to leave EBP set to.

A considerable amount of code in DOSLIB uses functions with inline assembly and those functions are causing stack corruption and malfunctioning because of this bug.

I found this while tracking down why the various test programs in DOSLIB were crashing when compiled with recent Open Watcom code. I found this when tracking down why the Sound Blaster test program crashed shortly after probing the DMA controller, why the crash had something to do with the CPU detection code blanking a 32-bit word well above it's own stack frame that caused SNDSB test.c to crash when main() returned, and so on.

@joncampbell123
Copy link
Contributor Author

This only seems to be a major problem with the 32-bit C/C++ compiler.
The 16-bit compiler is correctly setting up the stack frame with BP and SP as expected.

@jmalak jmalak added the bug label Jan 29, 2022
@jmalak
Copy link
Member

jmalak commented Jan 29, 2022

Thank you for your bug report.

@jmalak
Copy link
Member

jmalak commented Feb 1, 2022

The CG appears to use indirect settings to generate a caller stack frame when the __asm block is used.
This was signaled by a change of [e]sp register for the __asm block.
This is not signaled now, I will temporarily return it back.
This should be done by a more transparent method than by signaling the change [e]sp for each __asm block as before.
The caller stack frame should only be generated if the __asm block uses the automatic C variable.

jmalak added a commit that referenced this issue Feb 1, 2022
@jmalak
Copy link
Member

jmalak commented Feb 1, 2022

Now it generates correct code.

Segment: _TEXT BYTE USE32 00000021 bytes
0000				blahblah_:
0000  53				push		ebx
0001  51				push		ecx
0002  56				push		esi
0003  57				push		edi
0004  55				push		ebp
0005  89 E5				mov		ebp,esp
0007  83 EC 04				sub		esp,0x00000004
000A  50				push		eax
000B  8B 85 F8 FF FF FF			mov		eax,-0x8[ebp]
0011  CD 2F				int		0x2f
0013  89 9D FC FF FF FF			mov		-0x4[ebp],ebx
0019  89 EC				mov		esp,ebp
001B  5D				pop		ebp
001C  5F				pop		edi
001D  5E				pop		esi
001E  59				pop		ecx
001F  5B				pop		ebx
0020  C3				ret

@jmalak
Copy link
Member

jmalak commented Feb 1, 2022

CG should be extended to propagate usage of C caller auto variable flag from in-line code back to caller to decide to use standard stack frame in C caller with [e]bp pointer to C auto variables
now it is signaled by changed [e]sp in pragma aux even if it is not true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CG Code generator enhancement
Projects
None yet
Development

No branches or pull requests

2 participants