I second Sean's opinion in this matter. We should truly address the issue and not paper over it by ignoring flags designed to suss it out.
Dana > -----Original Message----- > From: Hdf-forum [mailto:hdf-forum-boun...@lists.hdfgroup.org] On Behalf > Of Sean McBride > Sent: Thursday, October 17, 2013 5:02 PM > To: Raymond Lu; HDF Users Discussion List > Subject: Re: [Hdf-forum] HDF5 1.8.12 release candidate (pre1) is available for > testing > > On Thu, 17 Oct 2013 14:47:59 -0500, Raymond Lu said: > > >Several of us including Quincey and Elena looked at the issue. We > >decided that since the algorithm is trying to detect the alignment of > >integers, ideally the flag -fcatch-undefined-behavior and other > >optimization flags shouldn't to be used for H5detect.c. In the future, > >we can separate flags for H5detect.c from the rest of the library. > > > >(For those who don't know what Issue 8147 is: CLANG compiler on mac > >machines with the options -fcatch-undefined-behavior and -ftrapv > >catches some undefined behavior in the alignment algorithm of the macro > >DETECT_I in H5detect.c.) > > Thanks for your followup! > > >Please let us know your thoughts. > > I must respectfully disagree. :) I'll try to convince you... :) > > Perhaps you misunderstand the purpose and meaning of -fcatch-undefined- > behavior: it is not an optimization flag, it is a bug finding tool. It has > found a > bug in HDF5. Ignoring it is not helpful. :) > > For a good read on undefined behaviour (really worth your time!) see: > > "What Every C Programmer Should Know About Undefined Behavior" > <http://blog.llvm.org/2011/05/what-every-c-programmer-should- > know.html> > > C compilers are getting very smart these days, and performing optimizations > based on the (valid!) assumption that undefined behaviour *must not* > occur. For a great example see "A Fun Case Analysis" here: > <http://blog.regehr.org/archives/213> > > HDF's own HISTORY-1.8.txt even discusses problems at higher optimization > levels: > > ---------- > * For gcc v4.3 and v4.4, with production mode, if -O3 is used, H5Tinit.c > would fail to compile. Actually bad H5Tinit.c is produced. If -O (same > as -O1) is used, H5Tinit.c compiled okay but test/dt_arith would fail. > When -O0 (no optimizatio) is used, H5Tinit.c compilete okay and all > tests passed. Therefore, -O0 is imposed for v4.3 and v4.4 of gcc. > AKC - 2009/04/20 > ---------- > > And lo and behold, dt_arith is one of the tests that fails when -fcatch- > undefined-behavior is enabled: > <http://cdash.hdfgroup.uiuc.edu/testDetails.php?test=299376&build=9516> > > I'd wager that's exactly what I'm describing: gcc's optimizer is in fact doing > nothing wrong, rather, HDF5's code invokes undefined behaviour in several > places, and so the compiler can generate whatever garbage it wants to. > > -fcatch-undefined-behavior is a tool to catch (some of) these problems in > debug mode, before the optimizer screws you. > > Consider this obvious example: > > ---------- > int main (void) > { > float big = 1e20; > unsigned char small = big; > > printf ("small is %u \n", small); > > return 0; > } > ---------- > > unsigned char (on most platforms at least) has too little range to hold that > big > number. What do you expect this code to do? It's undefined behaviour, so > the compiler can do whatever it wants! Let's see: > > $ clang -O0 test.c > $ ./a.out > small is 0 > > $ clang -O3 test.c > $ ./a.out > small is 255 > > Now, let's use modern tools: > > $ clang -fsanitize=undefined-trap test.c $ ./a.out > test.c:8:13: runtime error: value 1e+20 is outside the range of representable > values of type 'unsigned char' > small is 0 > > Bug identified! Line number included! > > IIRC, the DETECT_I macro is violating alignment rules, doing something like > this: > > ---------- > int main (void) > { > char big[] = {0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66}; > int* foo = (int*) &(big[1]); > int bar = *foo; > > printf ("bar is %x \n", bar); > > return 0; > } > ---------- > > $ clang -fsanitize=undefined-trap test.c $ ./a.out > test.c:9:13: runtime error: load of misaligned address 0x7fff50ab69a6 for type > 'int', which requires 4 byte alignment > 0x7fff50ab69a6: note: pointer points here ab 50 ff 00 11 22 33 44 55 66 00 > 00 > 00 00 c8 69 ab 50 ff 7f 00 00 e1 47 39 89 ff 7f 00 00 e1 47 > ^ > bar is 44332211 > > You can't do that, see: > > <https://www.securecoding.cert.org/confluence/display/seccode/EXP36- > C.+Do+not+convert+pointers+into+more+strictly+aligned+pointer+types> > > In other words, DETECT_I will *detect the wrong thing* depending how the > compiler decides to react to the invalid code. > > Phew! I hope I've convinced you now... :) > > Cheers, > > -- > __________________________________________________________ > __ > Sean McBride, B. Eng s...@rogue-research.com > Rogue Research www.rogue-research.com > Mac Software Developer Montréal, Québec, Canada > > > > _______________________________________________ > Hdf-forum is for HDF software users discussion. > Hdf-forum@lists.hdfgroup.org > http://mail.lists.hdfgroup.org/mailman/listinfo/hdf-forum_lists.hdfgroup.org _______________________________________________ Hdf-forum is for HDF software users discussion. Hdf-forum@lists.hdfgroup.org http://mail.lists.hdfgroup.org/mailman/listinfo/hdf-forum_lists.hdfgroup.org
