[cfe-dev] Bad IR involving the use of bzero

[John McCall via cfe-dev]

> On Nov 7, 2017, at 12:15 AM, Bharathi Seshadri via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> I am seeing an issue that seems to be specific to the use of bzero as below.
> 
> bash-4.1$ cat test.c
> 
> typedef __SIZE_TYPE__ size_t;
> void bzero(void*, size_t);
> void foo(void);
> 
> void test_bzero() {
>  char dst[20];
>  int _sz = 20, len = 20;
>  return (_sz
>          ? ((_sz >= len)
>             ? bzero(dst, len)
>             : foo())
>          : bzero(dst, len));
> }
> 
> Compiling this test case results in the following error:
> ---------
> Instruction does not dominate all uses!
>  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
> fatal error: error in backend: Broken function found, compilation aborted!
> ---------
> 
> Inspecting the IR (see below), the generation of Phi node appears to
> be incorrect. It seems to me
> that the phi node (for merging dst) is rather unnecessary since bzero
> returns void.
> In EmitBuiltinExpr(), bzero is replaced by a call to memset, which
> returns a pointer to the destination.
> Here is a patch that fixes the issue. (The llvm test suite and spec
> runs are succcessful with the patch).
> 
> Is this patch reasonable? If not, could some one advice where the
> issue is likely to be?

The patch is reasonable.  "Returning" a value here is triggering code that is supposed
to be suppressed when the return value is void, but it only ends up mattering inside a
conditional operator.

If you wouldn't mind formalizing your test case above into a real lit test case, we can
commit your fix.

John.

> 
> Thanks,
> Bharathi
> 
> -------------------------------------------------------------------------------
> 
> Index: CGBuiltin.cpp
> ===================================================================
> --- CGBuiltin.cpp       (revision 316884)
> +++ CGBuiltin.cpp       (working copy)
> @@ -1431,7 +1431,7 @@
>     EmitNonNullArgCheck(RValue::get(Dest.getPointer()),
> E->getArg(0)->getType(),
>                         E->getArg(0)->getExprLoc(), FD, 0);
>     Builder.CreateMemSet(Dest, Builder.getInt8(0), SizeVal, false);
> -    return RValue::get(Dest.getPointer());
> +    return RValue::get(nullptr);
>   }
>   case Builtin::BImemcpy:
>   case Builtin::BI__builtin_memcpy: {
> 
> The bad IR for the test case is :
> 
> define void @test_bzero() #0 {
> entry:
>  %dst = alloca [20 x i8], align 16
>  %_sz = alloca i32, align 4
>  %len = alloca i32, align 4
>  store i32 20, i32* %_sz, align 4
>  store i32 20, i32* %len, align 4
>  %0 = load i32, i32* %_sz, align 4
>  %tobool = icmp ne i32 %0, 0
>  br i1 %tobool, label %cond.true, label %cond.false2
> 
> cond.true:                                        ; preds = %entry
>  %1 = load i32, i32* %_sz, align 4
>  %2 = load i32, i32* %len, align 4
>  %cmp = icmp sge i32 %1, %2
>  br i1 %cmp, label %cond.true1, label %cond.false
> 
> cond.true1:                                       ; preds = %cond.true
>  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>  %3 = load i32, i32* %len, align 4
>  %conv = sext i32 %3 to i64
>  call void @llvm.memset.p0i8.i64(i8* %arraydecay, i8 0, i64 %conv,
> i32 16, i1 false)
>  br label %cond.end
> 
> cond.false:                                       ; preds = %cond.true
>  call void @foo()
>  br label %cond.end
> 
> cond.end:                                         ; preds =
> %cond.false, %cond.true1
>  br label %cond.end5
> 
> cond.false2:                                      ; preds = %entry
>  %arraydecay3 = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>  %4 = load i32, i32* %len, align 4
>  %conv4 = sext i32 %4 to i64
>  call void @llvm.memset.p0i8.i64(i8* %arraydecay3, i8 0, i64 %conv4,
> i32 16, i1 false)
>  br label %cond.end5
> 
> cond.end5:                                        ; preds =
> %cond.false2, %cond.end
>  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
>  ret void
> }
> Instruction does not dominate all uses!
>  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
> fatal error: error in backend: Broken function found, compilation aborted!
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev