Hi there, I second Sean's opinion too. Actually it is the same point I was trying to make in http://article.gmane.org/gmane.comp.programming.hdf/4805 and relative thread. No doubt Sean did a much better job explaining!
Andrea On 18 October 2013 08:44, Dana Robinson <[email protected]> wrote: > 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:[email protected]] 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 [email protected] > > Rogue Research www.rogue-research.com > > Mac Software Developer Montréal, Québec, Canada > > > > > > > > _______________________________________________ > > Hdf-forum is for HDF software users discussion. > > [email protected] > > > http://mail.lists.hdfgroup.org/mailman/listinfo/hdf-forum_lists.hdfgroup.org > > _______________________________________________ > Hdf-forum is for HDF software users discussion. > [email protected] > > http://mail.lists.hdfgroup.org/mailman/listinfo/hdf-forum_lists.hdfgroup.org > -- Andrea Bedini <[email protected]>
_______________________________________________ Hdf-forum is for HDF software users discussion. [email protected] http://mail.lists.hdfgroup.org/mailman/listinfo/hdf-forum_lists.hdfgroup.org
