[PD-dev] [ pure-data-Patches-2989552 ] type-punned pointers break strict aliasing rules

[SourceForge.net]

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&group_id=55736

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 at 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-fstrict_002daliasing-787

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

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