[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
* Melchior Franz -- Friday 02 December 2005 01:10: > Modified Files: > tiny_xdr.cxx > Log Message: > returning addresses of auto vars is *dangerous* [...] But ... we weren't really returning the address of an auto var. Making dummy "static" fixes the problem, but the reason must be another one. gcc 4.0.2 bug? m. ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
* Melchior FRANZ -- Friday 02 December 2005 01:43:
> But ... we weren't really returning the address of an auto var.
Is it a gcc 4.0.2 (SuSE 10.0) compiler bug? tiny_xdr.cxx contains
this function;
float
XDR_decode_float ( const xdr_data_t & f_Val )
{
float* tmp;
xdr_data_t dummy;
dummy = XDR_decode_int32 (f_Val);
tmp = (float*) &dummy;
return (*tmp);
}
And it turned out that when compiled with gcc 4.0.2 the return
value wasn't safe. When called three times in a row with different
values, we would get three times the same result. None of those
correct. This placed MP aircraft somewhere around the middle
of our Earth. For those understanding x86 assembler, here is
the resulting code (why does it not call _Z16XDR_decode_int32RKj?
"Optimized" away?):
non-static "dummy" (-O2) --> doesn't work
(gdb) disass XDR_decode_float
Dump of assembler code for function _Z16XDR_decode_floatRKj:
0x08310816 <_Z16XDR_decode_floatRKj+0>: push %ebp
0x08310817 <_Z16XDR_decode_floatRKj+1>: mov%esp,%ebp
0x08310819 <_Z16XDR_decode_floatRKj+3>: sub$0x10,%esp
0x0831081c <_Z16XDR_decode_floatRKj+6>: flds 0xfffc(%ebp)
0x0831081f <_Z16XDR_decode_floatRKj+9>: leave
0x08310820 <_Z16XDR_decode_floatRKj+10>: ret
End of assembler dump.
The following was compiled without optimization. The resulting code
works.
non-static "dummy" (-O0) --> works
(gdb) disass XDR_decode_float
Dump of assembler code for function _Z16XDR_decode_floatRKj:
0x083be33a <_Z16XDR_decode_floatRKj+0>: push %ebp
0x083be33b <_Z16XDR_decode_floatRKj+1>: mov%esp,%ebp
0x083be33d <_Z16XDR_decode_floatRKj+3>: sub$0x18,%esp
0x083be340 <_Z16XDR_decode_floatRKj+6>: mov0x8(%ebp),%eax
0x083be343 <_Z16XDR_decode_floatRKj+9>: mov%eax,(%esp)
0x083be346 <_Z16XDR_decode_floatRKj+12>: call 0x83be1b4
<_Z16XDR_decode_int32RKj>
0x083be34b <_Z16XDR_decode_floatRKj+17>: mov%eax,0xfff8(%ebp)
0x083be34e <_Z16XDR_decode_floatRKj+20>: lea0xfff8(%ebp),%eax
0x083be351 <_Z16XDR_decode_floatRKj+23>: mov%eax,0xfffc(%ebp)
0x083be354 <_Z16XDR_decode_floatRKj+26>: mov0xfffc(%ebp),%eax
0x083be357 <_Z16XDR_decode_floatRKj+29>: mov(%eax),%eax
0x083be359 <_Z16XDR_decode_floatRKj+31>: mov%eax,0xffec(%ebp)
0x083be35c <_Z16XDR_decode_floatRKj+34>: flds 0xffec(%ebp)
0x083be35f <_Z16XDR_decode_floatRKj+37>: leave
0x083be360 <_Z16XDR_decode_floatRKj+38>: ret
End of assembler dump.
and this hack (committed to cvs) works with and without optimization.
Making "dummy" static shouldn't be necessary, but
static "dummy"--> works
(gdb) disass XDR_decode_float
Dump of assembler code for function _Z16XDR_decode_floatRKj:
0x08310816 <_Z16XDR_decode_floatRKj+0>: push %ebp
0x08310817 <_Z16XDR_decode_floatRKj+1>: mov%esp,%ebp
0x08310819 <_Z16XDR_decode_floatRKj+3>: sub$0x4,%esp
0x0831081c <_Z16XDR_decode_floatRKj+6>: mov0x8(%ebp),%eax
0x0831081f <_Z16XDR_decode_floatRKj+9>: mov%eax,(%esp)
0x08310822 <_Z16XDR_decode_floatRKj+12>: call 0x83107e2
<_Z16XDR_decode_int32RKj>
0x08310827 <_Z16XDR_decode_floatRKj+17>: mov%eax,0x8560e00
0x0831082c <_Z16XDR_decode_floatRKj+22>: flds 0x8560e00
0x08310832 <_Z16XDR_decode_floatRKj+28>: leave
0x08310833 <_Z16XDR_decode_floatRKj+29>: ret
End of assembler dump.
clueless
m.
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
* Curtis L. Olson -- Friday 02 December 2005 02:55: > Here you are just returning the contents of a memory location, not the > pointer itself (aka a value). I know. That's why I had written > > * Melchior FRANZ -- Friday 02 December 2005 01:43: > > > But ... we weren't really returning the address of an auto var. I was just overwhelmed by the joy. And the not so early hour did the rest. 02:00 here in Europe then. Even later now ... -/ > So this should be perfectly safe. My bet is that the value didn't > get decoded properly, but whatever got decoded > incorrectly was returned correctly. XDR_decode_int32() seemed OK, judging by the assembler code. It's a simple byte swapper and I couldn't imagine that the compiler wouldn't get it. But, of course, a compiler bug could also be there. > I'd check the value if (*tmp) before it get's returned to see if > it looks like what you think it should. Yes. I'll do that. It works now, but the fix doesn't make sense. And depending on silly hacks isn't a pleasure. > I'll bail out here before I get to the x86 assembler portion of your > message. :-) I was fluent in 68000 assembler. But I always detested little endianness and couldn't imagine to learn x86 assembler. :-) m. ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
Mathias Fröhlich <[EMAIL PROTECTED]> writes:
> By the way: the right fix would be:
>
> float
> XDR_decode_float ( const xdr_data_t & f_Val )
> {
> union {
> float f;
> xdr_data_t x;
> } tmp;
> tmp.x = XDR_decode_int32 (f_Val);
> return tmp.f;
> }
>
> Please use this one. And I believe, without looking into the code,
> that there are likely more of them ...
well, this is c++ so the appropriate fix would be
float
XDR_decode_float ( const xdr_data_t & f_Val )
{
return (static_cast (XDR_decode_int32 (f_Val)));
}
i.e., we should be using static_cast.
--alex--
--
| I believe the moment is at hand when, by a paranoiac and active |
| advance of the mind, it will be possible (simultaneously with |
| automatism and other passive states) to systematize confusion |
| and thus to help to discredit completely the world of reality. |
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
Mathias Fröhlich <> writes:
> Please use this one. And I believe, without looking into the code,
> that there are likely more of them ...
please apply the attached patch which uses static_cast:
Index: src/MultiPlayer/tiny_xdr.cxx
===
RCS file: /var/cvs/FlightGear-0.9/FlightGear/src/MultiPlayer/tiny_xdr.cxx,v
retrieving revision 1.2
diff -u -r1.2 tiny_xdr.cxx
--- src/MultiPlayer/tiny_xdr.cxx 2 Dec 2005 00:10:25 - 1.2
+++ src/MultiPlayer/tiny_xdr.cxx 2 Dec 2005 07:12:41 -
@@ -122,42 +122,24 @@
xdr_data_t
XDR_encode_float ( const float & f_Val )
{
-xdr_data_t* tmp;
-
-tmp = (xdr_data_t*) &f_Val;
-return (XDR_encode_int32 (*tmp));
+return (XDR_encode_int32 (static_cast(f_Val)));
}
float
XDR_decode_float ( const xdr_data_t & f_Val )
{
-float* tmp;
-static xdr_data_t dummy;
-
-dummy = XDR_decode_int32 (f_Val);
-tmp = (float*) &dummy;
-return (*tmp);
+return (static_cast (XDR_decode_int32 (f_Val)));
}
/* double */
xdr_data2_t
XDR_encode_double ( const double & d_Val )
{
-xdr_data2_t* tmp;
-
-tmp = (xdr_data2_t*) &d_Val;
-return (XDR_encode_int64 (*tmp));
+return (XDR_encode_int64 (static_cast (d_Val)));
}
double
XDR_decode_double ( const xdr_data2_t & d_Val )
{
-double* tmp;
-static xdr_data2_t dummy;
-
-dummy = XDR_decode_int64 (d_Val);
-tmp = (double*) &dummy;
-return (*tmp);
+return (static_cast (XDR_decode_int64 (d_Val)));
}
-
-
--alex--
--
| I believe the moment is at hand when, by a paranoiac and active |
| advance of the mind, it will be possible (simultaneously with |
| automatism and other passive states) to systematize confusion |
| and thus to help to discredit completely the world of reality. |
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
* Alex Romosan -- Friday 02 December 2005 08:16: > Mathias Fröhlich <> writes: > > Please use this one. And I believe, without looking into the code, > > that there are likely more of them ... I'll try all solutions later today. But I don't understand why any of them should be necessary. The code may not be the most elegant, but it looks right to me. Do I really have to tell the optimizer not to translate it wrongly? > please apply the attached patch which uses static_cast: Looks nice. If it works (and Oliver doesn't have other plans) I'll commit that. But even if it works, why does the current code *not* work? m. ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
* Alex Romosan -- Friday 02 December 2005 08:16: > please apply the attached patch which uses static_cast: Haven't yet tested, but it looks good. At least it calls _Z16XDR_decode_int32RKj. :-) (gdb) disass XDR_decode_float Dump of assembler code for function _Z16XDR_decode_floatRKj: 0x0831086e <_Z16XDR_decode_floatRKj+0>: push %ebp 0x0831086f <_Z16XDR_decode_floatRKj+1>: mov%esp,%ebp 0x08310871 <_Z16XDR_decode_floatRKj+3>: sub$0x4,%esp 0x08310874 <_Z16XDR_decode_floatRKj+6>: mov0x8(%ebp),%eax 0x08310877 <_Z16XDR_decode_floatRKj+9>: mov%eax,(%esp) 0x0831087a <_Z16XDR_decode_floatRKj+12>: call 0x831083a <_Z16XDR_decode_int32RKj> 0x0831087f <_Z16XDR_decode_floatRKj+17>: push %eax 0x08310880 <_Z16XDR_decode_floatRKj+18>: fildl (%esp) 0x08310883 <_Z16XDR_decode_floatRKj+21>: add$0x4,%esp 0x08310886 <_Z16XDR_decode_floatRKj+24>: leave 0x08310887 <_Z16XDR_decode_floatRKj+25>: ret Will test and apply later. Thanks! m. ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
* Melchior FRANZ -- Friday 02 December 2005 09:57: > * Alex Romosan -- Friday 02 December 2005 08:16: > > please apply the attached patch which uses static_cast: No, this patch doesn't work. m. ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
* Harald JOHNSEN -- Friday 02 December 2005 11:36:
> Melchior FRANZ wrote:
> > (why does it not call _Z16XDR_decode_int32RKj? "Optimized" away?):
> decode_int32 is a nop on a x86 anyway
Huh? Looks like a nop for big-endian:
int32_t
XDR_decode_int32 ( const xdr_data_t & n_Val )
{
return (static_cast (SWAP32(n_Val)));
}
and
#define SWAP32(arg) sgIsLittleEndian() ? sg_bswap_32(arg) : arg
> Perhaps adding a volatile modifier on the tmp pointer
> could do the trick (of course doing that disables optimisations).
That wouldn't matter.
m.
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
* Mathias Fröhlich -- Friday 02 December 2005 07:35:
> float
> XDR_decode_float ( const xdr_data_t & f_Val )
> {
> union {
> float f;
> xdr_data_t x;
> } tmp;
> tmp.x = XDR_decode_int32 (f_Val);
> return tmp.f;
> }
This works.
Dump of assembler code for function _Z16XDR_decode_floatRKj:
0x08310816 <_Z16XDR_decode_floatRKj+0>: push %ebp
0x08310817 <_Z16XDR_decode_floatRKj+1>: mov%esp,%ebp
0x08310819 <_Z16XDR_decode_floatRKj+3>: sub$0x8,%esp
0x0831081c <_Z16XDR_decode_floatRKj+6>: mov0x8(%ebp),%eax
0x0831081f <_Z16XDR_decode_floatRKj+9>: mov%eax,(%esp)
0x08310822 <_Z16XDR_decode_floatRKj+12>: call 0x83107e2
<_Z16XDR_decode_int32RKj>
0x08310827 <_Z16XDR_decode_floatRKj+17>: mov%eax,0xfffc(%ebp)
0x0831082a <_Z16XDR_decode_floatRKj+20>: flds 0xfffc(%ebp)
0x0831082d <_Z16XDR_decode_floatRKj+23>: leave
0x0831082e <_Z16XDR_decode_floatRKj+24>: ret
m.
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
* Harald JOHNSEN -- Friday 02 December 2005 11:36: > Perhaps adding a volatile modifier on the tmp pointer > could do the trick (of course doing that disables optimisations). It doesn't. Dump of assembler code for function _Z16XDR_decode_floatRKj: 0x08310816 <_Z16XDR_decode_floatRKj+0>: push %ebp 0x08310817 <_Z16XDR_decode_floatRKj+1>: mov%esp,%ebp 0x08310819 <_Z16XDR_decode_floatRKj+3>: sub$0x10,%esp 0x0831081c <_Z16XDR_decode_floatRKj+6>: flds 0xfffc(%ebp) 0x0831081f <_Z16XDR_decode_floatRKj+9>: leave 0x08310820 <_Z16XDR_decode_floatRKj+10>: ret m. ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
* Andy Ross -- Friday 02 December 2005 16:36: > This violates the strict aliasing rules that are the default for > gcc 4.x -- I believe it issues a warning to that effect. There's is no warning (using -Wall), and info & man page claim that strict aliasing is turned off by default, even if the optimize option is used. > If you want to type-pun, you need to use a union: Yes. Mathias told me so already, and even sent me a patch in the morning. I'll commit that (if Oliver doesn't object, or anyone else with authority :-). m. ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
Melchior FRANZ wrote:
* Melchior FRANZ -- Friday 02 December 2005 01:43:
But ... we weren't really returning the address of an auto var.
Is it a gcc 4.0.2 (SuSE 10.0) compiler bug? tiny_xdr.cxx contains
this function;
float
XDR_decode_float ( const xdr_data_t & f_Val )
{
float* tmp;
xdr_data_t dummy;
dummy = XDR_decode_int32 (f_Val);
tmp = (float*) &dummy;
return (*tmp);
}
Here you are just returning the contents of a memory location, not the
pointer itself (aka a value). So this should be perfectly safe. My bet
is that the value didn't get decoded properly, but whatever got decoded
incorrectly was returned correctly. I'd check the value if (*tmp)
before it get's returned to see if it looks like what you think it should.
I'll bail out here before I get to the x86 assembler portion of your
message. :-)
Curt.
--
Curtis Olsonhttp://www.flightgear.org/~curt
HumanFIRST Program http://www.humanfirst.umn.edu/
FlightGear Project http://www.flightgear.org
Unique text:2f585eeea02e2c79d7b1d8c4963bae2d
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
On Freitag 02 Dezember 2005 02:29, Melchior FRANZ wrote:
> Is it a gcc 4.0.2 (SuSE 10.0) compiler bug? tiny_xdr.cxx contains
> this function;
>
> float
> XDR_decode_float ( const xdr_data_t & f_Val )
> {
> float* tmp;
> xdr_data_t dummy;
>
> dummy = XDR_decode_int32 (f_Val);
> tmp = (float*) &dummy;
> return (*tmp);
> }
>
>
> And it turned out that when compiled with gcc 4.0.2 the return
> value wasn't safe. When called three times in a row with different
> values, we would get three times the same result. None of those
> correct. This placed MP aircraft somewhere around the middle
> of our Earth. For those understanding x86 assembler, here is
> the resulting code (why does it not call _Z16XDR_decode_int32RKj?
> "Optimized" away?):
Try with -fno-strict-aliasing
:)
I for myself really like this feature in C/C++ called 'aliasing rules'. It
enables the optimizer to do really good jobs in reodering instructions and
eliminating dead stores which in the end keeps the CPU piplines full.
But be careful with casts like this one ...
Greetings
Mathias
--
Mathias Fröhlich, email: [EMAIL PROTECTED]
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
By the way: the right fix would be:
float
XDR_decode_float ( const xdr_data_t & f_Val )
{
union {
float f;
xdr_data_t x;
} tmp;
tmp.x = XDR_decode_int32 (f_Val);
return tmp.f;
}
Please use this one. And I believe, without looking into the code, that there
are likely more of them ...
Greetings
Mathias
--
Mathias Fröhlich, email: [EMAIL PROTECTED]
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
Melchior FRANZ wrote:
* Melchior FRANZ -- Friday 02 December 2005 01:43:
But ... we weren't really returning the address of an auto var.
Is it a gcc 4.0.2 (SuSE 10.0) compiler bug? tiny_xdr.cxx contains
this function;
float
XDR_decode_float ( const xdr_data_t & f_Val )
{
float* tmp;
xdr_data_t dummy;
dummy = XDR_decode_int32 (f_Val);
tmp = (float*) &dummy;
return (*tmp);
}
And it turned out that when compiled with gcc 4.0.2 the return
value wasn't safe. When called three times in a row with different
values, we would get three times the same result. None of those
correct. This placed MP aircraft somewhere around the middle
of our Earth. For those understanding x86 assembler, here is
the resulting code (why does it not call _Z16XDR_decode_int32RKj?
"Optimized" away?):
decode_int32 is a nop on a x86 anyway
non-static "dummy" (-O2) --> doesn't work
(gdb) disass XDR_decode_float
Dump of assembler code for function _Z16XDR_decode_floatRKj:
0x08310816 <_Z16XDR_decode_floatRKj+0>: push %ebp
0x08310817 <_Z16XDR_decode_floatRKj+1>: mov%esp,%ebp
0x08310819 <_Z16XDR_decode_floatRKj+3>: sub$0x10,%esp
0x0831081c <_Z16XDR_decode_floatRKj+6>: flds 0xfffc(%ebp)
0x0831081f <_Z16XDR_decode_floatRKj+9>: leave
0x08310820 <_Z16XDR_decode_floatRKj+10>: ret
End of assembler dump.
This code does what it is supposed do to ie : return *((float *) arg);
except that & arg == 0xfffc(%esp)
non-static "dummy" (-O0) --> works
(gdb) disass XDR_decode_float
Dump of assembler code for function _Z16XDR_decode_floatRKj:
0x083be33a <_Z16XDR_decode_floatRKj+0>: push %ebp
0x083be33b <_Z16XDR_decode_floatRKj+1>: mov%esp,%ebp
0x083be33d <_Z16XDR_decode_floatRKj+3>: sub$0x18,%esp
0x083be340 <_Z16XDR_decode_floatRKj+6>: mov0x8(%ebp),%eax
0x083be343 <_Z16XDR_decode_floatRKj+9>: mov%eax,(%esp)
0x083be346 <_Z16XDR_decode_floatRKj+12>: call 0x83be1b4
<_Z16XDR_decode_int32RKj>
& arg == 0x8(%esp)
static "dummy"--> works
(gdb) disass XDR_decode_float
Dump of assembler code for function _Z16XDR_decode_floatRKj:
0x08310816 <_Z16XDR_decode_floatRKj+0>: push %ebp
0x08310817 <_Z16XDR_decode_floatRKj+1>: mov%esp,%ebp
0x08310819 <_Z16XDR_decode_floatRKj+3>: sub$0x4,%esp
0x0831081c <_Z16XDR_decode_floatRKj+6>: mov0x8(%ebp),%eax
0x0831081f <_Z16XDR_decode_floatRKj+9>: mov%eax,(%esp)
0x08310822 <_Z16XDR_decode_floatRKj+12>: call 0x83107e2
<_Z16XDR_decode_int32RKj>
& arg == 0x8(%esp)
The code generated with -O2 and without static is wrong. Functions
parameters are
allways at a positive offset from sp. Perhaps adding a volatile modifier
on the tmp pointer
could do the trick (of course doing that disables optimisations).
Harald.
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
Harald JOHNSEN wrote:
> Is it a gcc 4.0.2 (SuSE 10.0) compiler bug? tiny_xdr.cxx contains
> this function;
>
> dummy = XDR_decode_int32 (f_Val);
> tmp = (float*) &dummy;
> return (*tmp);
This violates the strict aliasing rules that are the default for
gcc 4.x -- I believe it issues a warning to that effect. (Although
in this case the compiler should be able to detect that the pointer
can never be incorrectly aliased and optimize the warning away). If you
want to type-pun, you need to use a union:
union { int i; float f; } dummy;
dummy.i = XDR_decode_int32(f_Val);
return dummy.f;
The union trick also tends to be a little more readable, IMHO.
Andy
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: CVS: FlightGear/src/MultiPlayer tiny_xdr.cxx, 1.1, 1.2
On Freitag 02 Dezember 2005 12:48, Melchior FRANZ wrote:
> * Mathias Fröhlich -- Friday 02 December 2005 07:35:
> > float
> > XDR_decode_float ( const xdr_data_t & f_Val )
> > {
> > union {
> > float f;
> > xdr_data_t x;
> > } tmp;
> > tmp.x = XDR_decode_int32 (f_Val);
> > return tmp.f;
> > }
>
> This works.
Aliasing rules roughtly tell that pointers to different types not contained in
each other are assumed to point at different locations.
The original way gcc identifies that int store on the stack as a dead store as
it will never be read again - a float is read that cannot point to the same
address.
That it worked with making that variable static is propably that gcc did not
see that nowhere in the range of visibility of that static variable, this
variable ever read. If it would be smart enough to see that it would also
optimize that store away may be including the static variable itself.
Greetings
Mathias
--
Mathias Fröhlich, email: [EMAIL PROTECTED]
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
