hyeongyukim added a comment. In D105169#3116810 <https://reviews.llvm.org/D105169#3116810>, @nathanchance wrote:
> Prior to the latest revert (fd9b099906c61e46574d1ea2d99b973321fe1d21 > <https://reviews.llvm.org/rGfd9b099906c61e46574d1ea2d99b973321fe1d21>), the > Linux kernel's binary verifier (`objtool`) points out an issue when building > with ThinLTO and `-fsanitize=integer-divide-by-zero`. I have no idea if this > is an issue with the tool or this series. A simplified reproducer: > > $ cat ravb_main.i > int ravb_set_gti_ndev_rate; > unsigned int ravb_set_gti_ndev_inc; > void ravb_set_gti_ndev() { > ravb_set_gti_ndev_inc = 1000000000; > ravb_set_gti_ndev_inc = ravb_set_gti_ndev_inc / ravb_set_gti_ndev_rate; > if (ravb_set_gti_ndev_inc) > _dev_err(ravb_set_gti_ndev_inc); > } > > $ clang -std=gnu89 -O2 -flto=thin -fsanitize=integer-divide-by-zero -c -o > ravb_main.o ravb_main.i > > $ llvm-ar cDPrsT ravb.o ravb_main.o > > $ ld.lld -m elf_x86_64 -r -o ravb.lto.o --whole-archive ravb.o > > $ ./objtool orc generate --no-fp --no-unreachable --retpoline --uaccess > --mcount --module ravb.lto.o > ravb.lto.o: warning: objtool: .text.ravb_set_gti_ndev: unexpected end of > section > > With LLVM 13.0.0, there is no warning with those commands. The original and > reduced `.i` file, interestingness test, and static `objtool` binary are > available here > <https://github.com/nathanchance/creduce-files/tree/2010d2dc4fd217948a734d8bfb6be5460222b6b4/D105169>. I checked the reason why Objtool makes a warning. cont.thread: ; preds = %entry tail call void @__ubsan_handle_divrem_overflow(i8* bitcast ({ { [12 x i8]*, i32, i32 }, { i16, i16, [15 x i8] }* }* @1 to i8*), i64 1000000000, i64 0) #3, !nosanitize !8 br label %if.then cont: ; preds = %entry %div = udiv i32 1000000000, %0 store i32 %div, i32* @ravb_set_gti_ndev_inc, align 4, !tbaa !4 %tobool.not = icmp ugt i32 %0, 1000000000 br i1 %tobool.not, label %if.end, label %if.then if.then: ; preds = %cont.thread, %cont %div3 = phi i32 [ poison, %cont.thread ], [ %div, %cont ] %call = tail call i32 (i32, ...) bitcast (i32 (...)* @_dev_err to i32 (i32, ...)*)(i32 noundef %div3) #3 br label %if.end This IR code is an IR that has not passed the optimization pass completely. This code calculates the division only if `ravb_set_gti_ndev_rate` is non-zero and it calls `ubsan_handle_divrem_overflow` function to handle UB if `ravb_set_gti_ndev_rate` is zero. So far, there is no warning. But a warning occurs when this code passes the SimpleCFG optimization. cont.thread: ; preds = %entry tail call void @__ubsan_handle_divrem_overflow(i8* bitcast ({ { [12 x i8]*, i32, i32 }, { i16, i16, [15 x i8] }* }* @1 to i8*), i64 1000000000, i64 0) #3, !nosanitize !8 unreachable cont: ; preds = %entry %div = udiv i32 1000000000, %0 store i32 %div, i32* @ravb_set_gti_ndev_inc, align 4, !tbaa !4 %tobool.not = icmp ugt i32 %0, 1000000000 br i1 %tobool.not, label %if.end, label %if.then if.then: ; preds = %cont %call = tail call i32 (i32, ...) bitcast (i32 (...)* @_dev_err to i32 (i32, ...)*)(i32 noundef %div) #3 br label %if.end if.end: ; preds = %if.then, %cont ret void } After it passes the SimplyCFG, the `br` instruction was changed to the `unreachable` instruction in `cont.thread` block. This patch added noundef to the parameter of the `_dev_err` function, making the `%div3` unable to be Poison. It is impossible to jump from the `cont.thread` block to `if.then` block, so `br` instruction was changed to `unreachable` instruction. It would be nice to remove the unreachable block, but the above IR is not wrong because it is UB when `ravb_set_gti_ndev_rate` is zero. There seems to be no existing problem in clang, and I think we can bypass this warning by adding a code that checks whether the `gravb_set_gti_ndev_rate` is zero or not as follows. int ravb_set_gti_ndev_rate; unsigned int ravb_set_gti_ndev_inc; void ravb_set_gti_ndev() { ravb_set_gti_ndev_inc = 1000000000; ravb_set_gti_ndev_inc = ravb_set_gti_ndev_inc / ravb_set_gti_ndev_rate; if (ravb_set_gti_ndev_rate != 0) if (ravb_set_gti_ndev_inc) _dev_err(ravb_set_gti_ndev_inc); } @nathanchance How about changing the existing test code as above? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105169/new/ https://reviews.llvm.org/D105169 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits