#613 mingwex/stdio/vsscanf.c clobbers %edi

WSL
closed
nobody
None
fixed
Aged_issue
2013-02-04
2004-07-16
No

The vsscanf function in mingwex/stdio/vsscanf.c
clobbers the %edi register with inline assembler, but
doesn't list edi in the clobber list.

This creates elusive bugs that are optimizer (register
allocation) dependent, and is very annoying. I spent
two days tracking this one.

It is an easy fix, just add "edi" to the clobber list:

mingwex/stdio/vsscanf.c:38
- : "ebx");
+ : "ebx", "edi");

I don't have time to check the other inline assembler
functions but I recommend it. It isn't too hard to
check for this. (It sure beats finding them at runtime!)

Thanks,
Jeff

Discussion

  • Jeff Bonggren

    Jeff Bonggren - 2004-07-16

    Logged In: YES
    user_id=709953

    I went ahead and checked a few more of the inline asm's.
    It seems all of the mingwex/stdio functions that use inline
    assembler are missing edi from their clobber list. The
    files are

    vsscanf.c
    vfscanf.c
    vswscanf.c
    vswscanf.c

     
  • Aaron W. LaFramboise

    Logged In: YES
    user_id=1040098

    Verified. This is my oogly code, sorry you had to waste all
    that time finding this careless bug. =\

    I don't remember why I hardwired in edi at all, at the
    moment. I really should have included better comments. =(

    Btw, should code like this be -masm=intel compatible, or do
    we only care about code in headers?

     
  • Aaron W. LaFramboise

    Logged In: YES
    user_id=1040098

    I also noticed that gdb backtraces cant make it through this
    (it doesn't set ebp, for one) and I doubt dw2 exceptions
    will work through it either (eg from a signal handler). I'm
    not sure how much of a problem this is. Educating myself
    about 'unwinding' is on my TODO list. All of this is going
    to become important with -fomit-frame-pointer becoming the
    default.

     
  • Jeff Bonggren

    Jeff Bonggren - 2004-07-16

    Logged In: YES
    user_id=709953

    As far as I'm concerned, as long as it works in the latest
    GCC I'm happy.

    I made the changes to the clobber list on my local version
    so I could move on, and they worked fine.

    I don't think you need to hardwire the edi. There is a way
    in the inline asm to have gcc allocate a temporary register.
    I found
    http://osdev.neopages.net/tutorials/gccasmtut.php?the_id=68
    to be a useful reference.

    I don't have much experience with this, but I don't
    understand why you would need to do things like setting ebp
    or worry about unwinding. You are just calling a function
    like anyone else. I don't think -fomit-frame-pointer
    changes the function call ABI. GCC should take care of the
    ebp stuff in the function entry/exit boilerplate and
    Microsoft has already taken care of this on the
    msvcrt.dll/sscanf side. The -fomit-frame-pointer will
    remove some of the vsprintf entry/exit code but that is
    transparent to you (it happens before/after the inline asm).

    I'm not sure why backtraces don't make it through, unless
    perhaps msvcrt.dll doesn't use frame pointers or makes some
    other kind of strange optimization (I wouldn't be surprised).

     
  • Aaron W. LaFramboise

    Logged In: YES
    user_id=1040098

    Well theres a big hole in my knowledge right now (basically,
    I only have the most superficial understanding of how
    unwinding works, and I dont know what gdb does, and what GCC
    does or can do, or what glibc2 does), so I'm not going to
    examine this too closely right now, but I will say this is a
    problem for me:

    Testcase:
    #include <stdio.h>
    #include <stdarg.h>

    void stuff(int joe, ...) {
    va_list args;
    va_start(args, joe);
    vsscanf(0, "%d", args);
    va_end(args);
    }

    int main() {
    int i;
    stuff(1, &i);
    return 0;
    }
    EOF

    gcc -std=c99 -Wall -W -pedantic -gdwarf-2 -g3 -o stuff.exe
    stuff.c

    ./stuff
    Program received signal SIGSEGV, Segmentation fault.
    0x77c3e958 in sscanf ()
    (gdb) bt
    #0 0x77c3e958 in sscanf ()
    #1 0x00401449 in vsscanf (s=0x403004 "", format=0x4012ae "",
    arg=0x77c38a55
    "\033_^[áwW3\205u\005\203_SV\030\200=t\001G\213\215q\001\212\021A\204u+\215D\b\001\212\b\204u\215\004\004")
    at ./stdio/vsscanf.c:11
    #2 0x00000000 in ?? () from
    #3 0x00403000 in ?? ()
    #4 0x0022ff7c in ?? ()
    #5 0x00403004 in _data_end__ ()
    #6 0x004012ae in main () at stuff.c:11

    I'd like the backtrace to print something sensible, seeing
    as how I've enabled full debugging information.

     
  • Danny Smith

    Danny Smith - 2004-07-27

    Logged In: YES
    user_id=11494

    I did the quick fix and just added "edi" to thje clobbers. I still
    don't understand why the "movl %%esp, %%ebx\n\t"
    and later restore is needed. Wouldn't it suffice just to add -
    fno-omit-frame-pointer to CFLAGS when compiling these four
    files.

    I'll leave this open, because I think it could use a better fix.

    Danny

     
  • Aaron W. LaFramboise

    Logged In: YES
    user_id=1040098

    Yes, if you compile with -fno-omit-frame-pointer, theres no
    reason to save and restore it.

     
  • Earnie Boyd

    Earnie Boyd - 2013-02-04
    • labels: mingw runtime (deprecated use WSL) -->
    • status: open --> closed
    • resolution: --> fixed
    • category: --> Aged_issue
    • milestone: --> WSL
     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks