Patches item #2989552, was opened at 2010-04-19 22:01 Message generated for change (Comment added) made by zmoelnig You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=478072&aid=2989552...

Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: puredata Group: None Status: Open Resolution: None Priority: 7 Private: No Submitted By: ClaudiusMaximus (claudiusmaximus) Assigned to: Miller Puckette (millerpuckette) Summary: type-punned pointers break strict aliasing rules

Initial Comment: the current development version of Pd (well, assuming the gui-rewrite 0.43 branch is it) stiill suffers from a lot type-punned pointers that break strict aliasing rules (and would better be replaced by unions).

See here for some rationale http://lists.puredata.info/pipermail/pd-dev/2007-10/009677.html

Some command line hackery to print out some stats: ----8<---- claude@zebimus:~/code/pure-data_branches_pd-gui-rewrite_0-43$ make CFLAGS="-fstrict-aliasing -Wall" 2>&1 | grep warning.*strict | sort | sed "s|: warning.*||" | uniq | sed "s|:| |" | sort -V | ( oldsrc="none" ; while read src lineno ; do if [ $src = $oldsrc ] ; then echo $lineno ; else echo $src $lineno ; fi ; oldsrc=$src ; done ) | sed "s|(._.*)|;\1|" | tr "\n;" " \n" | tail -n+2 && echo d_array.c 61 727 d_ctl.c 110 145 327 d_delay.c 88 d_filter.c 72 174 305 307 390 577 810 812 d_math.c 87 101 109 d_osc.c 437 439 d_soundfile.c 252 253 254 285 287 290 298 299 321 337 339 346 355 356 371 987 998 1095 1106 ----8<---- (That's a table of files with line numbers that exhibit the problem)

While replacing (*(inttype *)&floatvalue) with a (union {inttype i; floattype f; } u;) is almost trivial, it's slightly less trivial to do it the right way.

The right way involves taking into account that Pd can be now be compiled to use double instead of float, so you need a different sized inttype for each case

The later 32bit bit-twiddling will probably break in that case anyway, so maybe better to have a compile-time check if it will work, and if not then do it the naive way...).

----------------------------------------------------------------------

Comment By: IOhannes m zmlnig (zmoelnig)

Date: 2011-05-24 19:22

Message: all three patches apply to the current (as of today) master branch in git. might be that 0002 and 0003 should be applied in the right order (as both modify d_soundfiler.c)

----------------------------------------------------------------------

Comment By: IOhannes m zmlnig (zmoelnig) Date: 2011-05-24 19:18

Message: i just stumbled upon the type-punning problem, as [tabwrite~] would not write the data i expected.

so i tried to fix the problem, with the following patches: 0001-implement-PD_BIGORSMALL-with-unions.patch re-implemented PD_BIGORSMALL and PD_BADFLOAT using "static inline" functions that use unions rather than typecasts; the performance is slightly worse (about 16%), but then i prefer slow code to broken code

0002-replace-float-int-casts-with-unions.patch replaces the remaining typecasts in d_math.c and d_soundfiler.c with unions

0003-cleanup-soundfile-parsers-reduce-number-of-casts.patch claude's script also showed up some warnings in the soundfile parsers where it complained about things like "((t_wavechunk *)buf)->wc_id[0]" which i have now replaced by a single cast from char[] to (e.g.) t_wavechunk; i believe that this makes the code more readable (no need to do the typecast in my head), and shuts up compiler warnings which are usually indicators of problematic code anyhow.

it would be great if these could get included in 0.43.1; i will definitely include them in the debian packages, as those are currently broken due to the type-punning problem.

----------------------------------------------------------------------

Comment By: ClaudiusMaximus (claudiusmaximus) Date: 2010-04-19 22:30

Message: It seems you have to be careful even with unions (type-punning via unions is allowed only if you do it the simplest way..)

http://gcc.gnu.org/onlinedocs/gcc-4.5.0/gcc/Optimize-Options.html#index-fstr...

----------------------------------------------------------------------

You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=478072&aid=2989552...