LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 14893 - -simplifycfg -instcombine miscompiles bool, aka i8 !range[0,2)
Summary: -simplifycfg -instcombine miscompiles bool, aka i8 !range[0,2)
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: Scalar Optimizations (show other bugs)
Version: trunk
Hardware: All All
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on: 14945
Blocks:
  Show dependency tree
 
Reported: 2013-01-10 07:52 PST by NAKAMURA Takumi
Modified: 2014-01-28 10:58 PST (History)
4 users (show)

See Also:
Fixed By Commit(s):


Attachments
comment (1.47 KB, text/plain)
2013-05-27 11:55 PDT, Rafael Ávila de Espíndola
Details

Note You need to log in before you can comment on or make changes to this bug.
Description NAKAMURA Takumi 2013-01-10 07:52:53 PST
target datalayout = "e-p:64:64:64-S128-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f16:16:16-f32:32:32-f64:64:64-f128:128:128-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
target triple = "x86_64-redhat-linux"

define zeroext i8 @_Z3BARv() unnamed_addr nounwind uwtable {
entry:
  %r = alloca i8, align 1
  %tmp = call zeroext i8 @_Z3FOORb(i8* %r) nounwind
  %tmp9 = and i8 %tmp, 1
  %tmp1 = icmp eq i8 %tmp9, 0
  br i1 %tmp1, label %"5", label %"3"

"3":                                              ; preds = %entry
  %tmp3 = load i8* %r, align 1, !range !0
  %tmp4 = icmp eq i8 %tmp3, 0
  br i1 %tmp4, label %"5", label %"7"

"5":                                              ; preds = %"3", %entry
  br label %"7"

"7":                                              ; preds = %"3", %"5"
  %tmp6 = phi i8 [ 0, %"5" ], [ 1, %"3" ]
  call void @llvm.lifetime.end(i64 1, i8* %r)
  ret i8 %tmp6
}

declare zeroext i8 @_Z3FOORb(i8*)

declare void @llvm.lifetime.end(i64, i8* nocapture) nounwind

!0 = metadata !{i8 0, i8 2}

;========

With -simplifycfg -instcombine,

entry:
  %r = alloca i8, align 1
  %tmp = call zeroext i8 @_Z3FOORb(i8* %r) nounwind
  %tmp9 = and i8 %tmp, 1
  %tmp3 = load i8* %r, align 1, !range !0
  %0 = xor i8 %tmp9, 1
  %1 = xor i8 %tmp3, 1
  %2 = or i8 %0, %1
  %tmp6 = xor i8 %2, 1
  call void @llvm.lifetime.end(i64 1, i8* %r)
  ret i8 %tmp6

;====

0) "load i8 %r" is undef if %tmp is false.
1) "load i8 %r" should not be executed when %tmp is false.
2) "load i8 %r" affects the result even when "load i8 %r" might be out of [0,2).
3) It would not be miscompiled when !range is removed.


FYI, this issue was found on gcc47 with dragonegg.

// r is not updated (undef) when return value is false.
// Simplified from clang/lib/Sema/SemaExpr.cpp EvaluatesAsTrue()
extern bool FOO(bool& r);

bool BAR() {
  bool r;
  return FOO(r) && r;
}
Comment 1 NAKAMURA Takumi 2013-01-13 19:42:02 PST
As I talked to Duncan, it can be a bug in simplifycfg.
I filed bug 14945 .

InstCombine does the right job as far as !range is valid.
Comment 2 Duncan Sands 2013-05-27 10:00:41 PDT
This causes dragonegg to miscompile DiagnoseLogicalAndInLogicalOrLHS in clang, causing the test clang/test/Sema/parentheses.c to fail.
Comment 3 Rafael Ávila de Espíndola 2013-05-27 11:50:56 PDT
test if bugzilla is working
Comment 4 Rafael Ávila de Espíndola 2013-05-27 11:55:33 PDT
Created attachment 10586 [details]
comment

bugzilla doesn't like me putting this comment inline.
Comment 5 Duncan Sands 2013-05-27 11:57:33 PDT
I agree that simplifycfg must drop the metadata.
Comment 6 Rafael Ávila de Espíndola 2014-01-28 10:58:01 PST
Fixed in r200322.