-
Notifications
You must be signed in to change notification settings - Fork 482
refactor AE, use worklist algorithm(naive) #1786
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| // This maps cycle head nodes to their corresponding WTO cycles for efficient lookup | ||
| for (auto& [func, wto] : funcToWTO) | ||
| { | ||
| // Recursive lambda to collect cycle heads from nested WTO components |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove lambda
|
Needs to fix this CI and move the precision-loss test case to a separate folder and address it after optimizations are enabled in later implementation. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1786 +/- ##
==========================================
- Coverage 64.15% 64.14% -0.01%
==========================================
Files 243 243
Lines 24569 24624 +55
Branches 4627 4657 +30
==========================================
+ Hits 15762 15796 +34
- Misses 8807 8828 +21
🚀 New features to boost your workflow:
|
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
| nonRecursiveCallSites.end(); | ||
| } | ||
|
|
||
| /// Recursion Handling Decision Methods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add examples for three cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
describe what are set to top, where is widened and narrowed.
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
| } | ||
|
|
||
| bool AbstractInterpretation::shouldApplyNarrowingInRecursion(const FunObjVar* fun) | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add an assertion if this is not recursion function.
| /// All recursion mode (TOP/WIDEN_ONLY/WIDEN_NARROW) logic is centralized here | ||
|
|
||
| bool AbstractInterpretation::skipRecursiveCall( | ||
| const CallICFGNode* callNode, const FunObjVar* callee) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
callNode is enough as the argument.
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
| } | ||
|
|
||
| /// handle wto cycle (loop) using worklist-compatible widening/narrowing iteration | ||
| /// Handle WTO cycle (loop or recursive function) using widening/narrowing iteration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name is ICFG cycle now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe can rename handleICFGCycleOrRecursion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HandleLoopOrRecursion. It would also be good to note that ICFGNode cycles refers to loops only, not interprocedural ICFGNode cycles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HandleLoopOrRecursion. It would also be good to note that ICFGNode cycles refers to loops only, not interprocedural ICFGNode cycles
Sure
| // Skip recursive call if applicable (returns true if skipped) | ||
| if (skipRecursiveCall(callNode, funObjVar)) | ||
| return; | ||
| const FunObjVar* callee = getCallee(callNode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we using the andersen's points-to result here?
I think handling indirectcall is the same as handling direct call if the calleenode is known? Can the code below be shared with the direct call handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we using the andersen's points-to result here?
I think handling indirectcall is the same as handling direct call if the calleenode is known? Can the code below be shared with the direct call handling.
we use Address Domain here.
Yes, I am trying to merge them.
I think you could continue to narrow down the usage of OPtions::HandleREcur(). I mean maybe you could put the option check inside the function.
For example,
"// Check if this recursive call should be skipped
if (shouldSkipRecursiveCall(callNode, funObjVar))
{
// In TOP mode, set return value and stores to TOP
// In WIDEN\_ONLY/WIDEN\_NARROW, just skip (WTO handles it)
if (Options::HandleRecur() == TOP)
handleSkippedRecursiveCall(callNode);
return;
}
"
maybe you should move the if check inside the function. You could try this way to reduce the Options::handleRecur as low as possible.
| */ | ||
| virtual void handleSVFStatement(const SVFStmt* stmt); | ||
|
|
||
| virtual void setRecursiveCallStoresToTop(const CallICFGNode* callnode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setTopToObjInRecursion.
| virtual void directCallFunPass(const CallICFGNode* callNode); | ||
| virtual bool isIndirectCall(const CallICFGNode* callNode); | ||
| virtual void indirectCallFunPass(const CallICFGNode* callNode); | ||
| virtual void callFunPass(const CallICFGNode* callNode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HandleFunCall
你方便把 setRecursiveCallStoresToTop改名setTopToObjInRecursion 然后把callFunPass 改名HandleFunCall吗? 改名就行
| if (isExtCall(callNode)) | ||
| { | ||
| indirectCallFunPass(callNode); | ||
| extCallPass(callNode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleExtCall
| assert(false && "implement this part"); | ||
| { | ||
| // Handle both direct and indirect calls uniformly | ||
| HandleFunCall(callNode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleFunCall
你方便把 setRecursiveCallStoresToTop改名setTopToObjInRecursion 然后把callFunPass 改名HandleFunCall吗? 改名就行
你方便把 setRecursiveCallStoresToTop改名setTopToObjInRecursion 然后把callFunPass 改名HandleFunCall吗? 改名就行
Pre-Requisite: Should Merge this Test Suite PR
Introduction: This PR replace wto with worklist algotihm, naive version.
May Need To Discuss: At svf/lib/AE/Svfexe/AbstractInterpretation.cpp line 145, I use a lambda code to build cycleHeadToCycle map. Maybe I can change it to a member function.
Next PR: Use-Def Pre-Analysis Map.