Good evening,
Several of us including Quincey and Elena looked at the issue.
Hmmm… my role was to make sure that developers look at the code But I take full responsibility for this answer to the forum since our decision and how we arrived at it is not explained at all!
Sean,
Thank you for looking into the issue and for holding our feet to the fire! I hope we will be able to resolve the problem very soon.
Elena
···
On Oct 17, 2013, at 4:01 PM, Sean McBride <sean@rogue-research.com> wrote:
On Thu, 17 Oct 2013 14:47:59 -0500, Raymond Lu said:
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 255Now, 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 0Bug 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 44332211You can't do that, see:
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 sean@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