Clang stop detecting UBs after a divide by zero

[Quuxplusone]

Bugzilla Link	PR46124
Status	NEW
Importance	P enhancement
Reported by	Haoxin Tu (haoxintu@gmail.com)
Reported on	2020-05-28 08:56:13 -0700
Last modified on	2020-05-29 09:37:22 -0700
Version	trunk
Hardware	PC All
CC	blitzrakete@gmail.com, dblaikie@gmail.com, dgregor@apple.com, efriedma@quicinc.com, erik.pilkington@gmail.com, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

This code test1.cc

#include<iostream>
int main () {
    int a = 1;
    for (int i = 0; i < 10; ++i) {
        a /= i; // Error: division by zero on the first iteration
    }

    int b = 0;
    int bb = 0 / b;
    0 / 0;
    std::cout << "ok" << std::endl;
    return 0;
}

$clang++-trunk -w -fsanitize=integer-divide-by-zero test1.cc ; ./a.out
test1.cc:5:11: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior example.cpp:5:11 in
UndefinedBehaviorSanitizer:DEADLYSIGNAL
==1==ERROR: UndefinedBehaviorSanitizer: FPE on unknown address 0x000000431817
(pc 0x000000431817 bp 0x7fff88ddde50 sp 0x7fff88ddde20 T1)
    #0 0x431817  (/app/output.s+0x431817)
    #1 0x7f56e7e59b96  (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #2 0x4031e9  (/app/output.s+0x4031e9)
UndefinedBehaviorSanitizer can not provide additional info.
SUMMARY: UndefinedBehaviorSanitizer: FPE (/app/output.s+0x431817)
==1==ABORTING

Clang only detects one runtime error in for statement but leaves out detecting
the statements following ones.

In test2.cc

#include<iostream>
int main () {
    /*
    int a = 1;
    for (int i = 0; i < 10; ++i) {
        a /= i; // Error: division by zero on the first iteration
    }
    */

    int b = 0;
    int bb = 0 / b;
    0 / 0;
    std::cout << "ok" << std::endl;
    return 0;
}
$clang++-trunk -fsanitize=integer-divide-by-zero test2.cc ; ./a.out
test2.cc:11:16: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior example.cpp:11:16 in
UndefinedBehaviorSanitizer:DEADLYSIGNAL
==1==ERROR: UndefinedBehaviorSanitizer: FPE on unknown address 0x0000004317fd
(pc 0x0000004317fd bp 0x7ffd6c2922f0 sp 0x7ffd6c2922d0 T1)
    #0 0x4317fd  (/app/output.s+0x4317fd)
    #1 0x7fe566be1b96  (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #2 0x4031e9  (/app/output.s+0x4031e9)
UndefinedBehaviorSanitizer can not provide additional info.
SUMMARY: UndefinedBehaviorSanitizer: FPE (/app/output.s+0x4317fd)
==1==ABORTING

In gcc-trunk
$./g++ -w -fsanitize=integer-divide-by-zero test2.cc ; ./a.out
test.cc:11:16: runtime error: division by zero
test.cc:12:7: runtime error: division by zero
ok

Should the main function return from "return 0" rather than exit directly from
the for statement?

I also file a bug report in GCC, they confirmed this limitation immediately.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95385

[Quuxplusone]

UndefinedBehaviorSanitizer: FPE on unknown address 0x000000431817

This means your program crashed... which is what naturally happens on x86 when a "div" instruction that divides by zero. Are you suggesting we should return some arbitrary value instead? What value would you suggest?

[Quuxplusone]

(In reply to Eli Friedman from comment #1)
> > UndefinedBehaviorSanitizer: FPE on unknown address 0x000000431817
>
> This means your program crashed... which is what naturally happens on x86
> when a "div" instruction that divides by zero.  Are you suggesting we should
> return some arbitrary value instead?  What value would you suggest?

Hi, Eli, what value should be returned is not my concern.

look this case test3.cc
#include<iostream>
int main () {
    0 / 0;
    0 / 0;
    std::cout << "ok" << std::endl;
    return 0;
}

in clang-trunk
$clang++-trunk -fsanitize=integer-divide-by-zero test3.cc ; ./a.out
test3.cc:3:7: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior example.cpp:3:7 in
test3.cc:4:7: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior example.cpp:4:7 in
ok

I mean test1.cc and test2.cc should be reproduced like test3.cc, detecting all
the UBs and has a normal return rather than abort.

[Quuxplusone]

(In reply to Haoxin Tu from comment #2)
> Hi, Eli, what value should be returned is not my concern.

Do you mean it doesn't matter to the program?  Or that you personally don't
care what value the compiler chooses?

If you run test1 without any -fsanitize flag, you get exactly the same FPE
that's reported by ubsan.

[Quuxplusone]

(In reply to Haoxin Tu from comment #2)
> (In reply to Eli Friedman from comment #1)
> > > UndefinedBehaviorSanitizer: FPE on unknown address 0x000000431817
> >
> > This means your program crashed... which is what naturally happens on x86
> > when a "div" instruction that divides by zero.  Are you suggesting we should
> > return some arbitrary value instead?  What value would you suggest?
>
> Hi, Eli, what value should be returned is not my concern.
>
> look this case test3.cc
> #include<iostream>
> int main () {
>     0 / 0;
>     0 / 0;
>     std::cout << "ok" << std::endl;
>     return 0;
> }
>
> in clang-trunk
> $clang++-trunk -fsanitize=integer-divide-by-zero test3.cc ; ./a.out
> test3.cc:3:7: runtime error: division by zero
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior example.cpp:3:7 in
> test3.cc:4:7: runtime error: division by zero
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior example.cpp:4:7 in
> ok
>
> I mean test1.cc and test2.cc should be reproduced like test3.cc, detecting
> all the UBs and has a normal return rather than abort.

In this case the dead code (to perform the division) was never emitted - so the
program printed out the diagnostic, but continued.

In your first case, the code isn't dead - the code is used and actually
performs the division, and produces a floating point signal/exception. That's
just what the program does (that's the behavior of the undefined behavior the
program invokes).

All UBSan did was transform the way that failure was printed. If you run the
program without UBSan, you get this:

$ clang++ -w div.cpp && ./a.out
Floating point exception
$

UBSan can't make the program continue beyond that point - there's no value that
could be produced by the division that would allow the program to continue in
any defined way. (that's what Eli was getting at - what value should 0/0
produce to put into the variable to let the program continue to use?)

[Quuxplusone]

(In reply to Eli Friedman from comment #3)

> Do you mean it doesn't matter to the program?  Or that you personally don't
> care what value the compiler chooses?

I think I get your point.

The reason is that GCC treats "0/0" produce 0 but "1/0" as an exception, and
Clang treats all of them as an exception. I am so sorry I have no idea about
what "0/0" should be produced, but I hold the view that Clang and GCC should
produce consistent results.

[Quuxplusone]

Take a look at this case in float-divide-by-zero

test4.cc
#include<iostream>
int main () {
    float a = 1/0.0;
    std::cout << a << std::endl;
    return 0;
}

$clang++-trunk -w -fsanitize=float-divide-by-zero test4.cc ; ./a.out
test4.cc:3:16: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior example.cpp:3:16 in
inf

I think the results in integer-divide-by-zero might be treated similarly to
float-divide-by-zero, just print "inf" to "0/0" rather than trigger a dead
signal to stop following statements.

[Quuxplusone]

(In reply to Haoxin Tu from comment #6)
>  just print "inf" to "0/0"

Sorry, by a mistake, print "nan" to "0/0" and "inf" to "1/0".

[Quuxplusone]

(In reply to Haoxin Tu from comment #6)
> Take a look at this case in float-divide-by-zero
>
> test4.cc
> #include<iostream>
> int main () {
>     float a = 1/0.0;
>     std::cout << a << std::endl;
>     return 0;
> }
>
> $clang++-trunk -w -fsanitize=float-divide-by-zero test4.cc ; ./a.out
> test4.cc:3:16: runtime error: division by zero
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior example.cpp:3:16 in
> inf
>
> I think the results in integer-divide-by-zero might be treated similarly to
> float-divide-by-zero, just print "inf" to "0/0" rather than trigger a dead
> signal to stop following statements.

Nature of undefined behavior - it can vary between compilers. "inf" can't be
printed, because "inf" doesn't fit in an integer. Floating point values have
representations of infinity, integers do not.