* [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined @ 2016-02-02 20:03 John Snow 2016-02-02 21:47 ` Laszlo Ersek 2016-02-02 21:59 ` Eric Blake 0 siblings, 2 replies; 8+ messages in thread From: John Snow @ 2016-02-02 20:03 UTC (permalink / raw) To: Qemu-block Cc: Kevin Wolf, Paolo Bonzini, Laszlo Ersek, qemu-devel, Peter Maydell Recently, qemu iotest 013 has started to fail for me: Fedora release 22 (Twenty Two) 3.5.0-9.fc22 clang version 3.5.0 (tags/RELEASE_350/final) Target: x86_64-redhat-linux-gnu Thread model: posix +4 KiB/home/jsnow/src/qemu/qemu-io-cmds.c:230:18: runtime error: division by zero The problem is that in the print report for read_f, t2 and t1 can actually be the same exact timestamp, and tdiv will try to divide by 0.0. Normally this is not a problem as this is defined to be INFINITY in C99 Annex F. Clang, however, has once again decided to take the pedantic road and state that Annex F is optional, and therefore division by 0.0 is actually undefined when using -fsanitize=undefined. Groan. Two workarounds: (1) Modify the tdiv() function to just return INFINITY manually if the timestamp provided is 0 (2) Modify tester scripts to also use -fno-sanitize=float-divide-by-zero I prepared a patch to do the first workaround [1] so I could test patches with clang in peace as I need to test my pull requests under clang to make sure I don't break OSX, but it seems so absurd to have to do this, so I have copied our resident language lawyers (and language pragmatists) so that they can have a say. Relevant upstream BZ: https://llvm.org/bugs/show_bug.cgi?id=17000 --js [1] https://github.com/jnsnow/qemu/commit/af93977dd2bc7ea936b8064c41c5a0f9d25ae2d1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined 2016-02-02 20:03 [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined John Snow @ 2016-02-02 21:47 ` Laszlo Ersek 2016-02-02 22:00 ` John Snow 2016-02-03 7:19 ` Markus Armbruster 2016-02-02 21:59 ` Eric Blake 1 sibling, 2 replies; 8+ messages in thread From: Laszlo Ersek @ 2016-02-02 21:47 UTC (permalink / raw) To: John Snow Cc: Kevin Wolf, Paolo Bonzini, qemu-devel, Qemu-block, Peter Maydell On 02/02/16 21:03, John Snow wrote: > Recently, qemu iotest 013 has started to fail for me: > > Fedora release 22 (Twenty Two) > > 3.5.0-9.fc22 > clang version 3.5.0 (tags/RELEASE_350/final) > Target: x86_64-redhat-linux-gnu > Thread model: posix > > > +4 KiB/home/jsnow/src/qemu/qemu-io-cmds.c:230:18: runtime error: > division by zero > > > The problem is that in the print report for read_f, t2 and t1 can > actually be the same exact timestamp, and tdiv will try to divide by 0.0. > > Normally this is not a problem as this is defined to be INFINITY in C99 > Annex F. > > Clang, however, has once again decided to take the pedantic road and > state that Annex F is optional, and therefore division by 0.0 is > actually undefined when using -fsanitize=undefined. > > Groan. > > Two workarounds: > > (1) Modify the tdiv() function to just return INFINITY manually if the > timestamp provided is 0 > > (2) Modify tester scripts to also use -fno-sanitize=float-divide-by-zero > > > I prepared a patch to do the first workaround [1] so I could test > patches with clang in peace as I need to test my pull requests under > clang to make sure I don't break OSX, but it seems so absurd to have to > do this, so I have copied our resident language lawyers (and language > pragmatists) so that they can have a say. > > Relevant upstream BZ: https://llvm.org/bugs/show_bug.cgi?id=17000 > > --js > > [1] > https://github.com/jnsnow/qemu/commit/af93977dd2bc7ea936b8064c41c5a0f9d25ae2d1 > Apologies in advance for the knee-jerk reaction: I don't use double, ever. The last time I did anything resembling numerical analysis was in college (now gracefully veiled by time). If I need decimals after the point, I opt for fixed point math, done with integers. Surely uint64_t suffices for the purposes of "qemu-io-cmds.c"; it just forces the programmer to think about those issues explicitly that "double" promises, but fails, to solve. I doubt microsecond resolution is necessary here, but even if it is, I'd assume that approx. 584,942 years sufficed as an upper limit on time differences. To frobnicate the saying about regular expressions, "when people want to print decimals, they reach for floating point -- now they have two problems". Thanks and sorry :( Laszlo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined 2016-02-02 21:47 ` Laszlo Ersek @ 2016-02-02 22:00 ` John Snow 2016-02-02 22:13 ` Laszlo Ersek 2016-02-03 7:19 ` Markus Armbruster 1 sibling, 1 reply; 8+ messages in thread From: John Snow @ 2016-02-02 22:00 UTC (permalink / raw) To: Laszlo Ersek Cc: Kevin Wolf, Paolo Bonzini, qemu-devel, Qemu-block, Peter Maydell On 02/02/2016 04:47 PM, Laszlo Ersek wrote: > On 02/02/16 21:03, John Snow wrote: >> Recently, qemu iotest 013 has started to fail for me: >> >> Fedora release 22 (Twenty Two) >> >> 3.5.0-9.fc22 >> clang version 3.5.0 (tags/RELEASE_350/final) >> Target: x86_64-redhat-linux-gnu >> Thread model: posix >> >> >> +4 KiB/home/jsnow/src/qemu/qemu-io-cmds.c:230:18: runtime error: >> division by zero >> >> >> The problem is that in the print report for read_f, t2 and t1 can >> actually be the same exact timestamp, and tdiv will try to divide by 0.0. >> >> Normally this is not a problem as this is defined to be INFINITY in C99 >> Annex F. >> >> Clang, however, has once again decided to take the pedantic road and >> state that Annex F is optional, and therefore division by 0.0 is >> actually undefined when using -fsanitize=undefined. >> >> Groan. >> >> Two workarounds: >> >> (1) Modify the tdiv() function to just return INFINITY manually if the >> timestamp provided is 0 >> >> (2) Modify tester scripts to also use -fno-sanitize=float-divide-by-zero >> >> >> I prepared a patch to do the first workaround [1] so I could test >> patches with clang in peace as I need to test my pull requests under >> clang to make sure I don't break OSX, but it seems so absurd to have to >> do this, so I have copied our resident language lawyers (and language >> pragmatists) so that they can have a say. >> >> Relevant upstream BZ: https://llvm.org/bugs/show_bug.cgi?id=17000 >> >> --js >> >> [1] >> https://github.com/jnsnow/qemu/commit/af93977dd2bc7ea936b8064c41c5a0f9d25ae2d1 >> > > Apologies in advance for the knee-jerk reaction: > > I don't use double, ever. The last time I did anything resembling > numerical analysis was in college (now gracefully veiled by time). > > If I need decimals after the point, I opt for fixed point math, done > with integers. Surely uint64_t suffices for the purposes of > "qemu-io-cmds.c"; it just forces the programmer to think about those > issues explicitly that "double" promises, but fails, to solve. > > I doubt microsecond resolution is necessary here, but even if it is, I'd > assume that approx. 584,942 years sufficed as an upper limit on time > differences. > Microsecond precision appears to not be good /enough/, where two subsequent reads return the same microsecond value. > To frobnicate the saying about regular expressions, "when people want to > print decimals, they reach for floating point -- now they have two > problems". > Now I've got a third problem: no real input on if clang is correct to whine or not. > Thanks and sorry :( > Laszlo > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined 2016-02-02 22:00 ` John Snow @ 2016-02-02 22:13 ` Laszlo Ersek 0 siblings, 0 replies; 8+ messages in thread From: Laszlo Ersek @ 2016-02-02 22:13 UTC (permalink / raw) To: John Snow Cc: Kevin Wolf, Paolo Bonzini, qemu-devel, Qemu-block, Peter Maydell On 02/02/16 23:00, John Snow wrote: > > > On 02/02/2016 04:47 PM, Laszlo Ersek wrote: >> On 02/02/16 21:03, John Snow wrote: >>> Recently, qemu iotest 013 has started to fail for me: >>> >>> Fedora release 22 (Twenty Two) >>> >>> 3.5.0-9.fc22 >>> clang version 3.5.0 (tags/RELEASE_350/final) >>> Target: x86_64-redhat-linux-gnu >>> Thread model: posix >>> >>> >>> +4 KiB/home/jsnow/src/qemu/qemu-io-cmds.c:230:18: runtime error: >>> division by zero >>> >>> >>> The problem is that in the print report for read_f, t2 and t1 can >>> actually be the same exact timestamp, and tdiv will try to divide by 0.0. >>> >>> Normally this is not a problem as this is defined to be INFINITY in C99 >>> Annex F. >>> >>> Clang, however, has once again decided to take the pedantic road and >>> state that Annex F is optional, and therefore division by 0.0 is >>> actually undefined when using -fsanitize=undefined. >>> >>> Groan. >>> >>> Two workarounds: >>> >>> (1) Modify the tdiv() function to just return INFINITY manually if the >>> timestamp provided is 0 >>> >>> (2) Modify tester scripts to also use -fno-sanitize=float-divide-by-zero >>> >>> >>> I prepared a patch to do the first workaround [1] so I could test >>> patches with clang in peace as I need to test my pull requests under >>> clang to make sure I don't break OSX, but it seems so absurd to have to >>> do this, so I have copied our resident language lawyers (and language >>> pragmatists) so that they can have a say. >>> >>> Relevant upstream BZ: https://llvm.org/bugs/show_bug.cgi?id=17000 >>> >>> --js >>> >>> [1] >>> https://github.com/jnsnow/qemu/commit/af93977dd2bc7ea936b8064c41c5a0f9d25ae2d1 >>> >> >> Apologies in advance for the knee-jerk reaction: >> >> I don't use double, ever. The last time I did anything resembling >> numerical analysis was in college (now gracefully veiled by time). >> >> If I need decimals after the point, I opt for fixed point math, done >> with integers. Surely uint64_t suffices for the purposes of >> "qemu-io-cmds.c"; it just forces the programmer to think about those >> issues explicitly that "double" promises, but fails, to solve. >> >> I doubt microsecond resolution is necessary here, but even if it is, I'd >> assume that approx. 584,942 years sufficed as an upper limit on time >> differences. >> > > Microsecond precision appears to not be good /enough/, where two > subsequent reads return the same microsecond value. Right, but "same timestamp with microsecond precision" is a problem independent of how you represent the difference of zero between them. With double, you are tempted to go ahead and divide with the difference. Maybe the quotient (in C) is infinity, maybe undefined behavior. Either way, we need to read the floating point stuff in the spec (which we otherwise never do), and argue with the compiler writers (which we sometimes cannot avoid, but we don't like it). With uint64_t, you cannot avoid thinking about "division by zero", so your code will be explicit about it. > >> To frobnicate the saying about regular expressions, "when people want to >> print decimals, they reach for floating point -- now they have two >> problems". >> > > Now I've got a third problem: no real input on if clang is correct to > whine or not. Indeed. That's my fault, but in this case I'm not ashamed of it. I can choose between (a) reading the standardese on floating point *plus* internalizing all of: What Every Computer Scientist Should Know About Floating-Point Arithmetic http://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html and (b) not using float. I'm an honest & practical person :), I openly go for (b). That doesn't mean someone who mastered (a) shouldn't respond! Thanks Laszlo > >> Thanks and sorry :( >> Laszlo >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined 2016-02-02 21:47 ` Laszlo Ersek 2016-02-02 22:00 ` John Snow @ 2016-02-03 7:19 ` Markus Armbruster 1 sibling, 0 replies; 8+ messages in thread From: Markus Armbruster @ 2016-02-03 7:19 UTC (permalink / raw) To: Laszlo Ersek Cc: Kevin Wolf, Peter Maydell, Qemu-block, qemu-devel, Paolo Bonzini, John Snow Laszlo Ersek <lersek@redhat.com> writes: > On 02/02/16 21:03, John Snow wrote: >> Recently, qemu iotest 013 has started to fail for me: >> >> Fedora release 22 (Twenty Two) >> >> 3.5.0-9.fc22 >> clang version 3.5.0 (tags/RELEASE_350/final) >> Target: x86_64-redhat-linux-gnu >> Thread model: posix >> >> >> +4 KiB/home/jsnow/src/qemu/qemu-io-cmds.c:230:18: runtime error: >> division by zero >> >> >> The problem is that in the print report for read_f, t2 and t1 can >> actually be the same exact timestamp, and tdiv will try to divide by 0.0. >> >> Normally this is not a problem as this is defined to be INFINITY in C99 >> Annex F. >> >> Clang, however, has once again decided to take the pedantic road and >> state that Annex F is optional, and therefore division by 0.0 is >> actually undefined when using -fsanitize=undefined. >> >> Groan. >> >> Two workarounds: >> >> (1) Modify the tdiv() function to just return INFINITY manually if the >> timestamp provided is 0 >> >> (2) Modify tester scripts to also use -fno-sanitize=float-divide-by-zero >> >> >> I prepared a patch to do the first workaround [1] so I could test >> patches with clang in peace as I need to test my pull requests under >> clang to make sure I don't break OSX, but it seems so absurd to have to >> do this, so I have copied our resident language lawyers (and language >> pragmatists) so that they can have a say. >> >> Relevant upstream BZ: https://llvm.org/bugs/show_bug.cgi?id=17000 >> >> --js >> >> [1] >> https://github.com/jnsnow/qemu/commit/af93977dd2bc7ea936b8064c41c5a0f9d25ae2d1 >> > > Apologies in advance for the knee-jerk reaction: > > I don't use double, ever. The last time I did anything resembling > numerical analysis was in college (now gracefully veiled by time). > > If I need decimals after the point, I opt for fixed point math, done > with integers. Surely uint64_t suffices for the purposes of > "qemu-io-cmds.c"; it just forces the programmer to think about those > issues explicitly that "double" promises, but fails, to solve. I doubt integer types can "force" someone who uses double unthinkingly not to use uint64_t equally unthinkingly. People who manage to misuse floating-point for trivial computations like "given an amount and a time, compute rate per second" are likely to misuse integers as well. With integers, they get a crash when computing a rate for a time that got flushed to zero, which has a chance to do some forcing after the fact (whatever good that may do). With double, they get an infinity, which has a real chance to just work (it does in qemu-io). > I doubt microsecond resolution is necessary here, but even if it is, I'd > assume that approx. 584,942 years sufficed as an upper limit on time > differences. > > To frobnicate the saying about regular expressions, "when people want to > print decimals, they reach for floating point -- now they have two > problems". Fixed point has its own set of problems. Sometimes they're preferrable to floating-point problems, sometimes they aren't. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined 2016-02-02 20:03 [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined John Snow 2016-02-02 21:47 ` Laszlo Ersek @ 2016-02-02 21:59 ` Eric Blake 2016-02-02 22:16 ` Paolo Bonzini 1 sibling, 1 reply; 8+ messages in thread From: Eric Blake @ 2016-02-02 21:59 UTC (permalink / raw) To: John Snow, Qemu-block Cc: Kevin Wolf, Paolo Bonzini, Laszlo Ersek, qemu-devel, Peter Maydell [-- Attachment #1: Type: text/plain, Size: 1114 bytes --] On 02/02/2016 01:03 PM, John Snow wrote: > The problem is that in the print report for read_f, t2 and t1 can > actually be the same exact timestamp, and tdiv will try to divide by 0.0. > > Normally this is not a problem as this is defined to be INFINITY in C99 > Annex F. > > Clang, however, has once again decided to take the pedantic road and > state that Annex F is optional, and therefore division by 0.0 is > actually undefined when using -fsanitize=undefined. > > Groan. No kidding. :( A compiler ceases to be useful when it spews out noise "just because it can". > > Two workarounds: > > (1) Modify the tdiv() function to just return INFINITY manually if the > timestamp provided is 0 Seems reasonable to me. > https://github.com/jnsnow/qemu/commit/af93977dd2bc7ea936b8064c41c5a0f9d25ae2d1 I'd be okay with this patch, if you want to make it a formal submission, and if no one else chimes in with any opinion other than disgust at clang's shenanigans. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined 2016-02-02 21:59 ` Eric Blake @ 2016-02-02 22:16 ` Paolo Bonzini 2016-02-03 7:21 ` Markus Armbruster 0 siblings, 1 reply; 8+ messages in thread From: Paolo Bonzini @ 2016-02-02 22:16 UTC (permalink / raw) To: Eric Blake, John Snow, Qemu-block Cc: Kevin Wolf, Peter Maydell, Laszlo Ersek, qemu-devel On 02/02/2016 22:59, Eric Blake wrote: > I'd be okay with this patch, if you want to make it a formal > submission, and if no one else chimes in with any opinion other > than disgust at clang's shenanigans. I'm not okay with the patch. What's wrong with -fno-sanitize=float-divide-by-zero? GCC accepts it, the only difference is in having a more useful default. Paolo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined 2016-02-02 22:16 ` Paolo Bonzini @ 2016-02-03 7:21 ` Markus Armbruster 0 siblings, 0 replies; 8+ messages in thread From: Markus Armbruster @ 2016-02-03 7:21 UTC (permalink / raw) To: Paolo Bonzini Cc: Kevin Wolf, Peter Maydell, Qemu-block, qemu-devel, Laszlo Ersek, John Snow Paolo Bonzini <pbonzini@redhat.com> writes: > On 02/02/2016 22:59, Eric Blake wrote: >> I'd be okay with this patch, if you want to make it a formal >> submission, and if no one else chimes in with any opinion other >> than disgust at clang's shenanigans. > > I'm not okay with the patch. What's wrong with > -fno-sanitize=float-divide-by-zero? GCC accepts it, the only > difference is in having a more useful default. Agreed. Do not complicate the code to placate some tool's silliness when you can tell the tool to stop being silly instead. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-02-03 7:22 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-02-02 20:03 [Qemu-devel] iotest 013 failure under clang -fsanitize=undefined John Snow 2016-02-02 21:47 ` Laszlo Ersek 2016-02-02 22:00 ` John Snow 2016-02-02 22:13 ` Laszlo Ersek 2016-02-03 7:19 ` Markus Armbruster 2016-02-02 21:59 ` Eric Blake 2016-02-02 22:16 ` Paolo Bonzini 2016-02-03 7:21 ` Markus Armbruster
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.