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
