Re: Difference between -O2 and -g

[Rich Felker <dalias@aerifal.cx>]

On Fri, Oct 26, 2012 at 02:44:36PM +0800, Brian Wang wrote:
> >> The failed call is within dix/colormap.c:CreateColormap().
> >>
> >> Thanks for helping.  I really appreciate it.
> >
> > I would check out Xext/security.c SecurityResource() and see if you
> > can figure out what it's doing. That appears to be the callback that's
> > getting called. You might want to check and see if there are any
> > others that could be registered; this could be done by grepping for
> > XaceRegister.*RESOURCE, which I didn't do because I'm browsing the
> > source online and the web interface seems to lack grep.
> >
> 
> With a bit of digging, I found the source code of this oldish xserver
> may trigger an undefined behaviour.
> When calling XaceHook(int hook, ...), the switch case looks like this:
> ---------
> switch (hook)
>     {
> 	case XACE_RESOURCE_ACCESS: {
> 	    XaceResourceAccessRec rec = {
> 		va_arg(ap, ClientPtr),
> 		va_arg(ap, XID),
> 		va_arg(ap, RESTYPE),
> 		va_arg(ap, pointer),
> 		va_arg(ap, RESTYPE),
> 		va_arg(ap, pointer),
> 		va_arg(ap, Mask),
> 		Success /* default allow */
> 	    };
> 	    calldata = &rec;
> 	    prv = &rec.status;
> 	    break;
> 	}
> ---------
> I think gcc-4.7.2 looks at 'rec' and thinks it is local to the switch
> case and optimizes it away.  However, 'prv' will be accessed
> down below outside the case.
> I do not know whose fault this is (most likely this piece of code),
> but certainly not musl's. :-)

Nice catch. This is definitely a case of invoking undefined behavior.
Actually, gcc should have issued a big warning about this (letting a
pointer to an automatic object escape its lifetime); I'm surprised it
didn't. Unfortunately I was looking at the current version of the code
which does not have this bug.

Rich