Skip to content

Commit

Permalink
FIX: do not ignore a return value if array/vector/struct type
Browse files Browse the repository at this point in the history
This should solve issues #53 and #54.
Thanks Shaobo for reporting the problem.
  • Loading branch information
caballa committed Nov 7, 2019
1 parent 63f713d commit dde25dd
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/DsaLocal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1214,8 +1214,12 @@ void IntraBlockBuilder::visitIntToPtrInst(IntToPtrInst &I) {

void IntraBlockBuilder::visitReturnInst(ReturnInst &RI) {
Value *v = RI.getReturnValue();
if (!v || isSkip(*v))

// We don't skip the return value if its type is a

This comment has been minimized.

Copy link
@agurfinkel

agurfinkel Nov 8, 2019

Contributor

should not we check whether the aggregate can contain a pointer? If the type of the aggregate has no pointers, then we can still skip it.

This comment has been minimized.

Copy link
@caballa

caballa Nov 8, 2019

Author Contributor

yes, that's better.

// struct/vector/array because it can contain pointers inside.
if (!v || (isSkip(*v) && !isa<CompositeType>(RI.getReturnValue()->getType()))) {
return;
}

sea_dsa::Cell c = valueCell(*v);
if (c.isNull())
Expand Down
22 changes: 22 additions & 0 deletions tests/c/test-ret-struct.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#include <assert.h>

int* A;

typedef struct {
int* a;
long b;
} S;

S foo() {
S x = {A, 2L};
assert(1);
return x;
}

int main() {
int a = 1;
A = &a;
S y = foo();
assert(*(y.a) == 1);
return 0;
}
38 changes: 38 additions & 0 deletions tests/expected_graphs/test-ret-struct.c.main.mem.dot
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
digraph unnamed {
graph [center=true, ratio=true, bgcolor=lightgray, fontname=Helvetica];
node [fontname=Helvetica, fontsize=11];

Node0x7f9ac0506130 [shape=record,label="{\{0:i32*\}:MR|{<s0>\<0, unknown\>}}"];
Node0x7f9ac0506210 [shape=record,label="{\{void\}:}"];
Node0x7f9ac05062a0 [shape=record,label="{\{void\}:}"];
Node0x7f9ac05063a0 [shape=record,label="{\{void\}:}"];
Node0x7f9ac0506480 [shape=record,label="{\{0:i32\}:SM}"];
Node0x7f9ac05066c0 [shape=record,label="{\{0:i32*,8:i64\}:SMR|{<s0>\<0, unknown\>}}"];
Node0x7f9ac0506880 [shape=record,label="{\{0:i32*,8:i64\}:SMR|{<s0>\<0, unknown\>}}"];
Node0x7f9ac0506a40 [shape=record,label="{\{0:i32\}:SMR}"];
Node0x7f9ac040c638 [shape=plaintext, label =" %4 = call \{ i32*, i64 \} @foo()"];
Node0x7f9ac040c638 -> Node0x7f9ac0506880[arrowtail=tee,label="0",fontsize=8,color=purple];
Node0x7f9ac0500048 [shape=plaintext, label =" %8 = getelementptr inbounds \{ i32*, i64 \}, \{ i32*, i64 \}* %5, i32 0, i32 1"];
Node0x7f9ac0500048 -> Node0x7f9ac05066c0[arrowtail=tee,label="8",fontsize=8,color=purple];
Node0x7f9ac040b088 [shape=plaintext, label =".str.1"];
Node0x7f9ac040b088 -> Node0x7f9ac05063a0[arrowtail=tee,label="0",fontsize=8,color=purple];
Node0x7f9ac040ac38 [shape=plaintext, label ="A"];
Node0x7f9ac040ac38 -> Node0x7f9ac0506130[arrowtail=tee,label="0",fontsize=8,color=purple];
Node0x7f9ac040c7b8 [shape=plaintext, label =" %7 = extractvalue \{ i32*, i64 \} %4, 0"];
Node0x7f9ac040c7b8 -> Node0x7f9ac0506a40[arrowtail=tee,label="0",fontsize=8,color=purple];
Node0x7f9ac040c3c8 [shape=plaintext, label =" %1 = alloca i32, align 4"];
Node0x7f9ac040c3c8 -> Node0x7f9ac0506480[arrowtail=tee,label="0",fontsize=8,color=purple];
Node0x7f9ac040c428 [shape=plaintext, label =" %2 = alloca i32, align 4"];
Node0x7f9ac040c428 -> Node0x7f9ac0506a40[arrowtail=tee,label="0",fontsize=8,color=purple];
Node0x7f9ac040c488 [shape=plaintext, label =" %3 = alloca %struct.S, align 8"];
Node0x7f9ac040c488 -> Node0x7f9ac05066c0[arrowtail=tee,label="0",fontsize=8,color=purple];
Node0x7f9ac05002a8 [shape=plaintext, label =" %11 = load i32*, i32** %10, align 8"];
Node0x7f9ac05002a8 -> Node0x7f9ac0506a40[arrowtail=tee,label="0",fontsize=8,color=purple];
Node0x7f9ac040afa8 [shape=plaintext, label =".str"];
Node0x7f9ac040afa8 -> Node0x7f9ac05062a0[arrowtail=tee,label="0",fontsize=8,color=purple];
Node0x7f9ac040aeb8 [shape=plaintext, label ="__func__.main"];
Node0x7f9ac040aeb8 -> Node0x7f9ac0506210[arrowtail=tee,label="0",fontsize=8,color=purple];
Node0x7f9ac0506130:s0 -> Node0x7f9ac0506a40[arrowtail=tee,label="0",fontsize=8];
Node0x7f9ac05066c0:s0 -> Node0x7f9ac0506a40[arrowtail=tee,label="0",fontsize=8];
Node0x7f9ac0506880:s0 -> Node0x7f9ac0506a40[arrowtail=tee,label="0",fontsize=8];
}
90 changes: 90 additions & 0 deletions tests/test-ret-struct.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
; RUN: %seadsa %butd_dsa --sea-dsa-dot %s --sea-dsa-dot-outdir=%T/test-ret-struct.ll
; RUN: %cmp-graphs %tests/test-ret-struct.c.main.mem.dot %T/test-ret-struct.ll/main.mem.dot | OutputCheck %s -d --comment=";"
; CHECK: ^OK$


; ModuleID = 'test-shaobo.1.c'
source_filename = "test-shaobo.1.c"
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.14.0"

%struct.S = type { i32*, i64 }

@A = common global i32* null, align 8
@__func__.main = private unnamed_addr constant [5 x i8] c"main\00", align 1
@.str = private unnamed_addr constant [16 x i8] c"test-shaobo.1.c\00", align 1
@.str.1 = private unnamed_addr constant [12 x i8] c"*(y.a) == 1\00", align 1

; Function Attrs: noinline nounwind optnone ssp uwtable
define { i32*, i64 } @foo() #0 {
%1 = alloca %struct.S, align 8
%2 = alloca %struct.S, align 8
%3 = getelementptr inbounds %struct.S, %struct.S* %2, i32 0, i32 0
%4 = load i32*, i32** @A, align 8
store i32* %4, i32** %3, align 8
%5 = getelementptr inbounds %struct.S, %struct.S* %2, i32 0, i32 1
store i64 2, i64* %5, align 8
%6 = bitcast %struct.S* %1 to i8*
%7 = bitcast %struct.S* %2 to i8*
call void @llvm.memcpy.p0i8.p0i8.i64(i8* %6, i8* %7, i64 16, i32 8, i1 false)
%8 = bitcast %struct.S* %1 to { i32*, i64 }*
%9 = load { i32*, i64 }, { i32*, i64 }* %8, align 8
ret { i32*, i64 } %9
}

; Function Attrs: argmemonly nounwind
declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32, i1) #1

; Function Attrs: noinline nounwind optnone ssp uwtable
define i32 @main() #0 {
%1 = alloca i32, align 4
%2 = alloca i32, align 4
%3 = alloca %struct.S, align 8
store i32 0, i32* %1, align 4
store i32 1, i32* %2, align 4
store i32* %2, i32** @A, align 8
%4 = call { i32*, i64 } @foo()
%5 = bitcast %struct.S* %3 to { i32*, i64 }*
%6 = getelementptr inbounds { i32*, i64 }, { i32*, i64 }* %5, i32 0, i32 0
%7 = extractvalue { i32*, i64 } %4, 0
store i32* %7, i32** %6, align 8
%8 = getelementptr inbounds { i32*, i64 }, { i32*, i64 }* %5, i32 0, i32 1
%9 = extractvalue { i32*, i64 } %4, 1
store i64 %9, i64* %8, align 8
%10 = getelementptr inbounds %struct.S, %struct.S* %3, i32 0, i32 0
%11 = load i32*, i32** %10, align 8
%12 = load i32, i32* %11, align 4
%13 = icmp eq i32 %12, 1
%14 = xor i1 %13, true
%15 = zext i1 %14 to i32
%16 = sext i32 %15 to i64
%17 = icmp ne i64 %16, 0
br i1 %17, label %18, label %20

; <label>:18: ; preds = %0
call void @__assert_rtn(i8* getelementptr inbounds ([5 x i8], [5 x i8]* @__func__.main, i32 0, i32 0), i8* getelementptr inbounds ([16 x i8], [16 x i8]* @.str, i32 0, i32 0), i32 20, i8* getelementptr inbounds ([12 x i8], [12 x i8]* @.str.1, i32 0, i32 0)) #3
unreachable
; No predecessors!
br label %21

; <label>:20: ; preds = %0
br label %21

; <label>:21: ; preds = %20, %19
ret i32 0
}

; Function Attrs: noreturn
declare void @__assert_rtn(i8*, i8*, i32, i8*) #2

attributes #0 = { noinline nounwind optnone ssp uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="penryn" "target-features"="+cx16,+fxsr,+mmx,+sse,+sse2,+sse3,+sse4.1,+ssse3,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
attributes #1 = { argmemonly nounwind }
attributes #2 = { noreturn "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="true" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="penryn" "target-features"="+cx16,+fxsr,+mmx,+sse,+sse2,+sse3,+sse4.1,+ssse3,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
attributes #3 = { noreturn }

!llvm.module.flags = !{!0, !1}
!llvm.ident = !{!2}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 7, !"PIC Level", i32 2}
!2 = !{!"clang version 5.0.0 (tags/RELEASE_500/final)"}

0 comments on commit dde25dd

Please sign in to comment.