[xorg-bugzilla-noise] [Bug 347] New: fonttosfnt: fix for crash on x86_64

[bugzilla-daemon at pdx.freedesktop.org]

http://pdx.freedesktop.org/cgi-bin/bugzilla/show_bug.cgi?id=347

           Summary: fonttosfnt: fix for crash on x86_64
           Product: xorg
           Version: unspecified
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: xlib
        AssignedTo: xorg-bugzilla-noise at freedesktop.org
        ReportedBy: mfabian at suse.de


I tried to use fonttosfnt from  the X.org tree from:

cvs -d :pserver:anoncvs at pdx.freedesktop.org:/cvs/xorg co -P -rXORG-RELEASE-1 xc

It crashes on x86_64.

The problem appears to be that vsnprintf is called more than once on
the same argument list. According to the man-page of the va_functions:

man va_start>    va_copy
man va_start>        An obvious implementation would have a va_list  a  pointer
man va_start>        to  the  stack  frame of the variadic function.  In such a
man va_start>        setup (by far the most common) there seems nothing against
man va_start>        an assignment
man va_start>                    va_list aq = ap;
man va_start>        Unfortunately,  there  are  also  systems  that make it an
man va_start>        array of pointers (of length 1), and there one needs
man va_start>                    va_list aq;
man va_start>                    *aq = *ap;
man va_start>        Finally, on systems where parameters are passed in  regis-
man va_start>        ters, it may be necessary for va_start to allocate memory,
man va_start>        store the parameters there,  and  also  an  indication  of
man va_start>        which  parameter  is next, so that va_arg can step through
man va_start>        the list. Now va_end can free the allocated memory  again.
man va_start>        To  accommodate  this situation, C99 adds a macro va_copy,
man va_start>        so that the above assignment can be replaced by
man va_start>                    va_list aq;
man va_start>                    va_copy(aq, ap);
man va_start>                    ...
man va_start>                    va_end(aq);
man va_start>        Each invocation of va_copy must be  matched  by  a  corre-
man va_start>        sponding  invocation of va_end in the same function.  Some
man va_start>        systems that do not supply va_copy have __va_copy instead,
man va_start>        since that was the name used in the draft proposal.

I.e. this won't work on some platforms. Indeed it crashes on x86_64
(AMD 64 bit system).

I tried to fix it with the help of va_copy. Unfortunately as written
in the man-page quoted above, va_copy is C99 therefore it might not be
available everywhere. Therefore my patch uses __va_copy if va_copy
is not defined. But

   - this is ugly
   - probably it still doesn't work everywhere.

How to fix this correctly?

My patch also makes vsprintf_reliable a static function, because it is
apparently only used in util.c.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.