New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 19535 - UBSan: Floating point division by zero is not undefined
Summary: UBSan: Floating point division by zero is not undefined
Status: RESOLVED FIXED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: trunk
Hardware: PC Windows NT
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-23 16:17 PDT by Nicholas Chapman
Modified: 2019-07-09 19:06 PDT (History)
10 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicholas Chapman 2014-04-23 16:17:08 PDT
float x = 1.f;
float y = x / 0.f;

Gives the UBSan error:

'runtime error: division by zero'.

It's not really an error though, the result is of course +INF and is well defined AFAIK.
Comment 1 Richard Smith 2014-04-23 16:22:00 PDT
C++11 5.6/4: "If the second operand of / or % is zero the behavior is undefined."

Turn this UBSan error off (with -fno-sanitize=float-divide-by-zero) if you don't want to check this, and you should get the +/-inf answer you're expecting (I'm reasonably confident we don't optimize on the basis of this being undefined behavior).
Comment 2 Matt Arsenault 2014-04-23 16:25:47 PDT
(In reply to comment #1)
> C++11 5.6/4: "If the second operand of / or % is zero the behavior is
> undefined."
> 
> Turn this UBSan error off (with -fno-sanitize=float-divide-by-zero) if you
> don't want to check this, and you should get the +/-inf answer you're
> expecting (I'm reasonably confident we don't optimize on the basis of this
> being undefined behavior).

This is more of QoI issue. Nobody cares that C/C++ say it's undefined, they care about getting IEEE-754 behavior. -fno-sanitize=float-divide-by-zero should be the default
Comment 3 Richard Smith 2014-04-23 16:34:00 PDT
(In reply to comment #2)
> This is more of QoI issue. Nobody cares that C/C++ say it's undefined, they
> care about getting IEEE-754 behavior. -fno-sanitize=float-divide-by-zero
> should be the default

It is, but you said -fsanitize=undefined, so we sanitize undefined behavior. If you want some other set of sanitizers, we have command-line arguments for them.

Also, note that Clang+LLVM does *not* guarantee IEEE-754 behavior. This division might fault or otherwise misbehave in some circumstances on some platforms.
Comment 4 Rich Felker 2016-12-09 00:08:35 PST
Comment 1 is wrong; the quoted text only applies to implementations which do not attempt to conform to Annex F, whereas clang/llvm does. The directly relevant text is F.3 ¶1: "The +, -, *, and / operators provide the IEC 60559 add, subtract, multiply, and divide operations."
Comment 5 Paul Robinson 2016-12-09 02:08:05 PST
(In reply to comment #4)
> Comment 1 is wrong; the quoted text only applies to implementations which do
> not attempt to conform to Annex F, whereas clang/llvm does. The directly
> relevant text is F.3 ¶1: "The +, -, *, and / operators provide the IEC 60559
> add, subtract, multiply, and divide operations."

Comment 1 cites the C++ standard, while comment 4 cites Annex F from the
C standard.  Neither is "wrong" but they make different assumptions.
So is the original source C or C++?  Comment 0 does not specify.
Apparently it's UB in C++ but not C-with-Annex-F.
Comment 6 Richard Smith 2016-12-09 02:11:40 PST
Also, Clang does not claim to support Annex F.
Comment 7 Rich Felker 2016-12-09 10:08:05 PST
Do you have a citation for clang not supporting Annex F? I find it plausible that the project is aware of some minor conformance bugs, but not that there's no intent to implement it at all. C without annex F is essentially a language without floating point.
Comment 8 Paul Robinson 2016-12-09 10:17:36 PST
(In reply to comment #7)
> Do you have a citation for clang not supporting Annex F?

For one thing, Clang does not predefine __STDC_IEC_559__.

For another, I'd say Richard Smith would be the definitive word
on what conformance Clang does or does not claim.  The degree
of C conformance is not spelled out very explicitly, I admit,
certainly not to the degree of detail that C++ conformance is
(see http://clang.llvm.org/cxx_status.html).
Comment 9 Rich Felker 2016-12-09 10:23:35 PST
GCC doesn't either because there are also library requirements for conformance; thus, it's left up to stdc-predef.h. GCC does define __GCC_IEC_559, though, to assist the library-level headers.

In any case, if it's the official position of clang/llvm that Annex F is not supported (not just incomplete or possibly buggy, but completely unsupported) then I think I have to mark it as a potentially unsuitable compiler for use with musl libc, since we assume IEEE floating point and aim to provide it (modulo fenv on softfloat targets) to applications.
Comment 10 Richard Smith 2016-12-09 12:00:14 PST
It would certainly be desirable for clang to offer (the compiler side of) complete Annex F support. What we need is someone who cares about that support to figure out which pieces we're missing (in both clang and llvm) and provide patches for the necessary changes.

Nonetheless, it is correct that the floating point division by zero sanitizer diagnoses floating point division by zero. Whether this should be part of the 'undefined' sanitizer is really a question of what you think it's for -- if we're only sanitizing things that clang/llvm will optimize, then it may not make sense to include this as part of 'undefined', at least not on targets that support floating point divide by zero. But if we're looking for portability issues, then it does. Our current approach is more the latter than the former.
Comment 11 Rich Felker 2016-12-09 13:24:47 PST
My point is that floating point division by zero is not a "portability issue". All LLVM targets have IEEE float (or at least an only-slightly-buggy version), and without Annex F/IEEE float, use of any floating point code at all is non-portable. The C standard literally allows 2.0+2.0==5.0 (or 2.0+2.0==-42.0 if you prefer) in implementations that do not aim to support Annex F. Non-annex-F implementations are a historical curiosity on the same level as non-twos-complement implementations.
Comment 12 Nikita Popov 2019-06-20 03:13:32 PDT
Bump. Working around this is pretty annoying, as there's no easy way to implement correct IEEE-754 division by zero behavior in the presence of this sanitizer. Doing so requires detecting negative zero, and unfortunately the standard way of doing that without library support brings us back to division by zero.
Comment 13 Fangrui Song 2019-07-09 19:06:40 PDT
float-divide-by-zero was removed from the -fsanitize=undefined by https://reviews.llvm.org/D63793

% cat a.c
int main() { float x = 1.f; float y = x/0.f; y = x/0.f; }
% clang -fsanitize=undefined a.c -o a && ./a
% clang++ -xc++ -fsanitize=undefined a.c -o a && ./a
% gcc -fsanitize=undefined a.c -o a && ./a
% gcc -xc++ -fsanitize=undefined a.c -o a && ./a
# no complaint

The fallout should be fixed by D64317.

% clang -fsanitize=float-divide-by-zero a.c -o a && ./a
a.c:1:40: runtime error: division by zero
a.c:1:51: runtime error: division by zero
% gcc -fsanitize=float-divide-by-zero a.c -o a && ./a
a.c:1:40: runtime error: division by zero
a.c:1:51: runtime error: division by zero

The changes shall be included by clang 9.