llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.05k stars 11.58k forks source link

Clang stop detecting UBs after a divide by zero #45469

Open haoxintu opened 4 years ago

haoxintu commented 4 years ago
Bugzilla Link 46124
Version trunk
OS All
CC @dwblaikie,@DougGregor,@efriedma-quic,@zygoloid

Extended Description

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)
    #&#8203;0 0x431817  (/app/output.s+0x431817)
    #&#8203;1 0x7f56e7e59b96  (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #&#8203;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)
    #&#8203;0 0x4317fd  (/app/output.s+0x4317fd)
    #&#8203;1 0x7fe566be1b96  (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #&#8203;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

dwblaikie commented 4 years ago

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

test4.cc

include

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.

haoxintu commented 4 years ago

just print "inf" to "0/0"

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

haoxintu commented 4 years ago

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.

haoxintu commented 4 years ago

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.

dwblaikie commented 4 years ago

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

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?)

efriedma-quic commented 4 years ago

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.

haoxintu commented 4 years ago

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.

efriedma-quic commented 4 years ago

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?