fix getbasevalueforextarg #1652
Conversation
|
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1652 +/- ##
==========================================
+ Coverage 63.98% 64.01% +0.03%
==========================================
Files 247 247
Lines 25952 25977 +25
Branches 4509 4524 +15
==========================================
+ Hits 16605 16629 +24
- Misses 9347 9348 +1
|
| @@ -1398,7 +1398,29 @@ const Value* SVFIRBuilder::getBaseValueForExtArg(const Value* V) | |||
| } | |||
| if(totalidx == 0 && !SVFUtil::isa<StructType>(value->getType())) | |||
| value = gep->getPointerOperand(); | |||
| } else if (const LoadInst* load = SVFUtil::dyn_cast<LoadInst>(value)) { | |||
There was a problem hiding this comment.
Could you put examples as the comments in this function?
/*
- Example 1: LLVM IR
- Example 2: LLVM IR
/
const Value SVFIRBuilder::getBaseValueForExtArg(const Value* V)
There was a problem hiding this comment.
Better to make two examples one for the if branch when not dealing with global and the other for the case in the issue. Would be good to also point out which is the value and which is the gep and base corresponding llvm IR to the implementation code
|
For my small handwritten test it is fine, however, this patch introduces some regression. Without it: With it: Unfortunatelly, I have no small reproducer, only a big one. |
svf-llvm/lib/SVFIRBuilder.cpp
Outdated
| const Value * pointer_operand = gep->getPointerOperand(); | ||
| if (auto *glob = SVFUtil::dyn_cast<GlobalVariable>(pointer_operand)) { | ||
| if (auto *initializer = llvm::dyn_cast< | ||
| ConstantStruct>(glob->getInitializer())) { |
There was a problem hiding this comment.
better to use the existing method to get the object (that SVF has collected) rather than identifying the global object and its initializer.
|
Now it is fine, but the issue is not resolved completely. My case uses global variables for ponter exchange and here is the code: struct file_operations {
void (*f1)();
void (*f2)();
};
struct miscdevice {
int minor;
const char *name;
const struct file_operations *fops;
};
void f1();
void f2();
static const struct file_operations fops = {
.f1 = f1,
.f2 = f2
};
static struct miscdevice mdev = {
.minor=0,
.name="dummy",
.fops=&fops,
};
extern struct miscdevice mdev_stub;
extern struct file_operations fops_stub;
int misc_register(struct miscdevice *misc) {
mdev_stub = *misc;
return 0;
}
void move_and_call() {
fops_stub = *mdev_stub.fops;
fops_stub.f1();
fops_stub.f2();
}
void init() {
misc_register(misc: &mdev);
}In this case I have this IR: %struct.miscdevice = type { i32, i8*, %struct.file_operations* }
%struct.file_operations = type { void (...)*, void (...)* }
@mdev_stub = external global %struct.miscdevice, align 8
@fops_stub = external global %struct.file_operations, align 8
@mdev = internal global %struct.miscdevice { i32 0, i8* getelementptr inbounds ([6 x i8], [6 x i8]* @.str, i32 0, i32 0), %struct.file_ operations* @fops }, align 8
@.str = private unnamed_addr constant [6 x i8] c"dummy\00", align 1
@fops = internal constant %struct.file_operations { void (...)* @f1, void (...)* @f2 }, align 8
; Function Attrs: noinline nounwind optnone uwtable
define dso_local i32 @misc_register(%struct.miscdevice* noundef %0) #0 {
%2 = alloca %struct.miscdevice*, align 8
store %struct.miscdevice* %0, %struct.miscdevice** %2, align 8
%3 = load %struct.miscdevice*, %struct.miscdevice** %2, align 8
%4 = bitcast %struct.miscdevice* %3 to i8*
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 bitcast (%struct.miscdevice* @mdev_stub to i8*), i8* align 8 %4, i64 24, i1 false)
ret i32 0
}
; Function Attrs: argmemonly nofree nounwind willreturn
declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg) #1
; Function Attrs: noinline nounwind optnone uwtable
define dso_local void @move_and_call() #0 {
%1 = load %struct.file_operations*, %struct.file_operations** getelementptr inbounds (%struct.miscdevice, %struct.miscdevice* @mdev_s tub, i32 0, i32 2), align 8
%2 = bitcast %struct.file_operations* %1 to i8*
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 bitcast (%struct.file_operations* @fops_stub to i8*), i8* align 8 %2, i64 16, i1 fal se)
%3 = load void (...)*, void (...)** getelementptr inbounds (%struct.file_operations, %struct.file_operations* @fops_stub, i32 0, i32 0), align 8
call void (...) %3()
%4 = load void (...)*, void (...)** getelementptr inbounds (%struct.file_operations, %struct.file_operations* @fops_stub, i32 0, i32 1), align 8
call void (...) %4()
ret void
}With -ff-eq-base flag |
Thanks for reporting it. Would you be able to tweak my previous case to reproduce this issue? |
|
@yuleisui it seems that one indirection level breaks it: when I first store struct interesting {
int dummy;
void (*f1)();
void (*f2)();
};
struct nested_ptr {
int dummy;
struct interesting* ptr;
};
void f1();
void f2();
struct interesting i1 = {
.f1 = f1,
.f2 = f2
};
struct nested_ptr n1 = {
.ptr = &i1
};
void test_ptr() {
struct nested_ptr tmp = n1;
struct interesting interesting_stub = *tmp.ptr;
interesting_stub.f1();
interesting_stub.f2();
}LLVM IR looks this way: ; Function Attrs: noinline nounwind optnone uwtable
define dso_local void @test_ptr() #1 {
%1 = alloca %struct.nested_ptr, align 8
%2 = alloca %struct.interesting, align 8
%3 = bitcast %struct.nested_ptr* %1 to i8*
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %3, i8* align 8 bitcast (%struct.nested_ptr* @n1 to i8*), i64 16, i1 false)
%4 = getelementptr inbounds %struct.nested_ptr, %struct.nested_ptr* %1, i32 0, i32 1
%5 = load %struct.interesting*, %struct.interesting** %4, align 8
%6 = bitcast %struct.interesting* %2 to i8*
%7 = bitcast %struct.interesting* %5 to i8*
call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %6, i8* align 8 %7, i64 24, i1 false)
%8 = getelementptr inbounds %struct.interesting, %struct.interesting* %2, i32 0, i32 1
%9 = load void (...)*, void (...)** %8, align 8
call void (...) %9()
%10 = getelementptr inbounds %struct.interesting, %struct.interesting* %2, i32 0, i32 2
%11 = load void (...)*, void (...)** %10, align 8
call void (...) %11()
ret void
}SVFIR, full LLVM IR and source file are in the archive |
svf-llvm/lib/SVFIRBuilder.cpp
Outdated
| * q = g->g_n (g is a global struct, g_n is the n-th field) | ||
| * p = *q | ||
| * extapi(p) | ||
| * The base value for p is g_n. Load -> GEP (collect the GEP index) based on g (a global struct) -> g_n. |
There was a problem hiding this comment.
g_n = getBaseValueForExtArg(p)
svf-llvm/lib/ObjTypeInference.cpp
Outdated
| const Value* dst = cs->getArgOperand(0); | ||
| const Value* src = cs->getArgOperand(1); | ||
| const auto name = cs->getName(); | ||
| assert(name == name); |
| if (const CallBase* cs = SVFUtil::dyn_cast<CallBase>(use)) { | ||
| if (const Function* calledFun = cs->getCalledFunction()) | ||
| if (LLVMUtil::isMemcpyExtFun(LLVMModuleSet::getLLVMModuleSet()->getSVFFunction(calledFun))) { | ||
| const Value* dst = cs->getArgOperand(0); |
There was a problem hiding this comment.
put an assert to check the argument number >=2 before this line
| @@ -135,6 +135,31 @@ LLVMContext &ObjTypeInference::getLLVMCtx() | |||
| */ | |||
| const Type *ObjTypeInference::inferObjType(const Value *var) | |||
| { | |||
| const Type* res = inferSingleObjType(var); | |||
| // infer type by leveraging the type alignment of src and dst in memcpy | |||
There was a problem hiding this comment.
Best to put an example here to mention the inference based on the use at a memcpy callsite
| @@ -135,6 +135,31 @@ LLVMContext &ObjTypeInference::getLLVMCtx() | |||
| */ | |||
| const Type *ObjTypeInference::inferObjType(const Value *var) | |||
There was a problem hiding this comment.
Rename the below methods.
inferSingleObjType => typeInference
There was a problem hiding this comment.
inferSingleObjType => inferPointsToType
|
@Frankenween could you check again whether this pull request works for your latest case? |
svf-llvm/lib/SVFIRBuilder.cpp
Outdated
| %0 = getelementptr inbounds %struct.outer, %struct.inner %base, i32 0, i32 0 | ||
| call void @llvm.memcpy(ptr %inner, ptr %0, i64 24, i1 false) | ||
| The base value for %0 is %base. | ||
| Note: We only handle the field index 0 for now. |
There was a problem hiding this comment.
Note: the %base is recognized as the base value if the offset (field index) is 0
| Note: We only handle the field index 0 for now. | ||
|
|
||
| * Example 2: | ||
| * https://github.com/SVF-tools/SVF/issues/1650 |
There was a problem hiding this comment.
add one more link: #1652
|
@Frankenween could you confirm again whether your test can pass now using this updated patch? I will need to merge it soon. |
|
--- log/nginx.log 2025-02-19 17:55:02.325367811 +1100 PTACallGraph Stats (Andersen analysis)****** PTACallGraph Stats (Andersen analysis)****** Persistent Points-To Cache Statistics: Andersen's analysis bitvector Memory SSA Statistics****** SVFG Statistics****** PTACallGraph Stats (Flow-sensitive analysis)****** Persistent Points-To Cache Statistics: flow-sensitive analysis bitvector |
fix issue #1650
--- log/nginx.log 2025-02-11 10:27:54.119860076 +1100
+++ log/nginx-xiao.log 2025-02-11 10:28:55.531230995 +1100
@@ -39,9 +39,9 @@
VarArrayObj 153
VarStructObj 630
----------------Time and memory stats--------------------
-LLVMIRTime 1.236
-SVFIRTime 1.473
-SymbolTableTime 0.17
+LLVMIRTime 1.208
+SVFIRTime 1.429
+SymbolTableTime 0.169
#######################################################
PTACallGraph Stats (Andersen analysis)******
@@ -68,11 +68,11 @@
CollapseTime 0
CopyGepTime 0
LoadStoreTime 0
-MemoryUsageVmrss 2.3221e+06
+MemoryUsageVmrss 2.32208e+06
MemoryUsageVmsize 2.3222e+06
SCCDetectTime 0
SCCMergeTime 0
-TotalTime 126.089
+TotalTime 127.024
UpdateCGTime 0
----------------Numbers stats----------------------------
AddrProcessed 5909
@@ -144,11 +144,11 @@
CollapseTime 0
CopyGepTime 0
LoadStoreTime 0
-MemoryUsageVmrss 2.32711e+06
-MemoryUsageVmsize 2.32703e+06
+MemoryUsageVmrss 2.32708e+06
+MemoryUsageVmsize 2.32704e+06
SCCDetectTime 0
SCCMergeTime 0
-TotalTime 140.533
+TotalTime 141.498
UpdateCGTime 0
----------------Numbers stats----------------------------
AddrProcessed 5909
@@ -220,11 +220,11 @@
################ (program : nginx.bc)###############
----------------Time and memory stats--------------------
AverageRegSize 39.1348
-GenMUCHITime 1.135
-GenRegionTime 86.259
-InsertPHITime 0.388
-SSARenameTime 0.035
-TotalMSSATime 87.818
+GenMUCHITime 1.255
+GenRegionTime 86.796
+InsertPHITime 0.423
+SSARenameTime 0.033
+TotalMSSATime 88.509
----------------Numbers stats----------------------------
BBHasMSSAPhi 4056
CSChiNode 22213
@@ -247,13 +247,13 @@
SVFG Statistics******
################ (program : nginx.bc)###############
----------------Time and memory stats--------------------
-ATNodeTime 0.208
+ATNodeTime 0.202
AvgWeight 198.378
ConnDirEdgeTime 0
-ConnIndEdgeTime 1.323
+ConnIndEdgeTime 1.307
OptTime 0
TLNodeTime 0
-TotalTime 1.531
+TotalTime 1.509
----------------Numbers stats----------------------------
ActualIn 30142
ActualOut 22213
@@ -315,20 +315,20 @@
GepTime 0
IndirectPropaTime 0
LoadTime 0
-MemoryUsageVmrss 4.79128e+06
-MemoryUsageVmsize 4.84208e+06
+MemoryUsageVmrss 4.79166e+06
+MemoryUsageVmsize 4.84191e+06
PhiTime 0
-PrelabelingTime 0.156
+PrelabelingTime 0.167
ProcessTime 0
PropagationTime 0
SCCTime 0
-SolveTime 825.305
+SolveTime 846.687
StoreTime 0
Strong/WeakUpdTime 0
-TotalTime 895.608
+TotalTime 915.415
UpdateCGTime 0
VersionPropTime 0
-meldLabelingTime 67.586
+meldLabelingTime 65.894
----------------Numbers stats----------------------------
CopysNum 80
DummyFieldPtrs 1510
@@ -348,11 +348,11 @@
ProcessedAddr 23636
ProcessedCopy 324
ProcessedFRet 0
-ProcessedGep 1031986
-ProcessedLoad 1293940
+ProcessedGep 1031985
+ProcessedLoad 1294061
ProcessedMSSANode 344692
-ProcessedPhi 86796
-ProcessedStore 687765
+ProcessedPhi 86794
+ProcessedStore 687872
SolveIterations 4
StoresNum 14929
StrongUpdates 282
@@ -368,11 +368,11 @@
Persistent Points-To Cache Statistics: flow-sensitive analysis bitvector
################ (program : nginx.bc)###############
UniquePointsToSets 30238
-TotalUnions 1937098543
-PropertyUnions 1237912403
-UniqueUnions 108827
-LookupUnions 698939016
-PreemptiveUnions 138297
+TotalUnions 1937482060
+PropertyUnions 1238078939
+UniqueUnions 108830
+LookupUnions 699155991
+PreemptiveUnions 138300
TotalComplements 0
PropertyComplements 0
UniqueComplements 0