WIP: Baseline which can run big programs#1789
WIP: Baseline which can run big programs#1789bjjwwang wants to merge 33 commits intoSVF-tools:masterfrom
Conversation
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.
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.
你方便把 setRecursiveCallStoresToTop改名setTopToObjInRecursion 然后把callFunPass 改名HandleFunCall吗? 改名就行
你方便把 setRecursiveCallStoresToTop改名setTopToObjInRecursion 然后把callFunPass 改名HandleFunCall吗? 改名就行
你方便把 setRecursiveCallStoresToTop改名setTopToObjInRecursion 然后把callFunPass 改名HandleFunCall吗? 改名就行
- Add collectEntryFunctions() to find functions without callers - Add analyseFromAllEntries() for analyzing from all entry points - Implement flow-sensitive join for same function called multiple times - Add ICFG and function coverage statistics in AEStat - Add allAnalyzedNodes set to track analyzed nodes across entry points - Fix bottom interval assertion errors in AbsExtAPI (handleMemcpy/handleMemset) When no main function exists, the analysis automatically starts from all entry points (functions with no callers). Each entry point is analyzed independently with fresh state. Coverage statistics now correctly track all analyzed nodes across multiple entry points. Co-Authored-By: Claude <noreply@anthropic.com>
- Add new command-line option -ae-multientry (default: false) - When false (default): analyze from main() only, preserving original behavior for Test-Suite test cases - When true: analyze from all entry points (functions without callers), useful for library code without main function - If no main function exists and -ae-multientry is not set, automatically falls back to multi-entry analysis Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Revert the flow-sensitive join logic in handleICFGNode that was incorrectly breaking the original function entry state initialization. The previous change introduced unnecessary complexity that caused incorrect state propagation when functions are called. The multi-entry analysis feature still works correctly because each entry point is analyzed independently with clearAbstractTrace() called before each entry, so flow-sensitive join at function entries is not needed. This fixes 7 out of 8 failing test cases: - BASIC_ptr_call2-0.c.bc - LOOP_for_call-0.c.bc - CWE121_Stack_Based_Buffer_Overflow__CWE129_fgets_01.c.bc - CWE121_Stack_Based_Buffer_Overflow__CWE129_fgets_01.c.bc-widen-narrow - CWE126_Buffer_Overread__CWE129_fgets_01.c.bc - CWE126_Buffer_Overread__CWE129_fgets_01.c.bc-widen-narrow - demo.c.bc-widen-narrow The remaining failure (INTERVAL_test_10-0.c.bc) is a pre-existing bug unrelated to the multi-entry changes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1. Fix BufOverflowDetector assertion (AEDetector.cpp:482)
- When a variable is not an address type in multi-entry analysis,
conservatively return true (assume safe) instead of asserting
2. Fix undefined compare predicate assertion (AbstractInterpretation.cpp)
- Add support for FCMP_ORD and FCMP_UNO floating-point comparisons
- These predicates check for NaN conditions, conservatively return [0,1]
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1789 +/- ##
==========================================
+ Coverage 64.14% 64.19% +0.04%
==========================================
Files 243 243
Lines 24626 24668 +42
Branches 4658 4663 +5
==========================================
+ Hits 15797 15836 +39
- Misses 8829 8832 +3
🚀 New features to boost your workflow:
|
| void analyse(); | ||
|
|
||
| /// Analyze all entry points (functions without callers) | ||
| void analyseFromAllEntries(); |
There was a problem hiding this comment.
analyzeFromAllProgEntries
| void analyseFromAllEntries(); | ||
|
|
||
| /// Get all entry point functions (functions without callers) | ||
| std::vector<const FunObjVar*> collectEntryFunctions(); |
There was a problem hiding this comment.
done, the name is collectProgEntryFuns now
|
|
||
| /// Program entry | ||
| /// Collect all entry point functions (functions without callers) | ||
| std::vector<const FunObjVar*> AbstractInterpretation::collectEntryFunctions() |
There was a problem hiding this comment.
Better to use a deque if you can both push from back and push to front (for main). Then no std::find_if is needed later.
There was a problem hiding this comment.
after last meeting, keep this version
| icfg->getGlobalICFGNode())[PAG::getPAG()->getBlkPtr()] = IntervalValue::top(); | ||
|
|
||
| // If -ae-multientry is set, always use multi-entry analysis | ||
| if (Options::AEMultiEntry()) |
There was a problem hiding this comment.
This should be by default.
There was a problem hiding this comment.
done
This should be by default.
done, remove it
| getAbsStateFromTrace( | ||
| icfg->getGlobalICFGNode())[PAG::getPAG()->getBlkPtr()] = IntervalValue::top(); |
There was a problem hiding this comment.
should this be moved to handleGlobalNode? Please also make a note to explain why this assignment is done here.
svf/lib/AE/Svfexe/AEDetector.cpp
Outdated
| NodeID value_id = value->getId(); | ||
|
|
||
| assert(as[value_id].isAddr()); | ||
| // In multi-entry analysis, some variables may not be initialized as addresses |
There was a problem hiding this comment.
Could you add an example here?
Shall we also intitalize them?
SVF-tools#1789 Could you read comments in this PR?
SVF-tools#1789 Could you read comments in this PR?
|
|
||
| /// Program entry | ||
| /// Parse comma-separated function names from the -ae-entry-funcs option | ||
| static Set<std::string> parseEntryFuncNames() |
There was a problem hiding this comment.
move this to Options.cpp
There was a problem hiding this comment.
after meeting, I remove it.
SVF-tools#1789 Could you read comments in this PR?
SVF-tools#1789 Could you read comments in this PR?
svf/lib/AE/Svfexe/AbsExtAPI.cpp
Outdated
| const SVFVar* arg1Val = call->getArgument(1); | ||
| IntervalValue strLen = getStrlen(as, arg1Val); | ||
| // no need to -1, since it has \0 as the last byte | ||
| // Skip if strLen is bottom or unbounded |
There was a problem hiding this comment.
could we share as much code as possible for string handling functions you have
| // Use worklist-based function handling instead of recursive WTO component handling | ||
| const ICFGNode* mainEntry = icfg->getFunEntryICFGNode(cgn->getFunction()); | ||
| handleFunction(mainEntry); | ||
| SVFUtil::errs() << "Warning: No entry functions found for analysis\n"; |
There was a problem hiding this comment.
We should always have at least one entry function (no caller function). May be an assert is better.
There was a problem hiding this comment.
Could you make an assert here?
There was a problem hiding this comment.
Could you make an assert here?
Sure Sure. I am double checking all comments.
| } | ||
|
|
||
| // Analyze from each entry point independently (Scenario 2: different entries -> fresh start) | ||
| for (const FunObjVar* entryFun : entryFunctions) |
There was a problem hiding this comment.
I think handle global should be done before entry function?
Also it would be good to add each entry icfgnode of entry function into the worklist for later abstract interpretation?
There was a problem hiding this comment.
yes, now the handleGlobalNode has been moved to the location before for-loop.
| // Analyze from each entry point independently (Scenario 2: different entries -> fresh start) | ||
| for (const FunObjVar* entryFun : entryFunctions) | ||
| { | ||
| // Clear abstract trace for fresh analysis from this entry |
There was a problem hiding this comment.
handle global node can be done outside this for loop?
It is unclear why we need to clear abstract trace here? The abstract states that for an ICFGNode A should be merged if A has two callers (if both callers are entry functions)?
| std::deque<const FunObjVar*> AbstractInterpretation::collectProgEntryFuns() | ||
| { | ||
| std::deque<const FunObjVar*> entryFunctions; | ||
| const CallGraph* callGraph = svfir->getCallGraph(); |
There was a problem hiding this comment.
need to use andersen's call graph if we have.
There was a problem hiding this comment.
yes. now we use andersen's call graph at handle call site (especially for indirect call).
There was a problem hiding this comment.
Did you update the code?
|
|
||
| // Analyze from this entry function | ||
| const ICFGNode* funEntry = icfg->getFunEntryICFGNode(entryFun); | ||
| handleFunction(funEntry); |
There was a problem hiding this comment.
need to double-check handleCallsite whether andersen's call graph shall be used?
There was a problem hiding this comment.
yes. andersen's call graph is in effect now.
…programs 3)share states btw multiple entries
svf/lib/AE/Svfexe/AEDetector.cpp
Outdated
| if (!as[value_id].isAddr()) | ||
| { | ||
| NodeID blkPtrId = svfir->getBlkPtr(); | ||
| as[value_id] = AddressValue(AbstractState::getVirtualMemAddress(blkPtrId)); |
There was a problem hiding this comment.
Should use black hole object
There was a problem hiding this comment.
And I haven't finished this commit, and I still check these comments and wait for the large programs' result.
| @@ -98,7 +100,7 @@ void AbstractInterpretation::initWTO() | |||
There was a problem hiding this comment.
Shall the Andersen analysis and callgraph initialisation be done in the constructor of AbstractInterpretation so that callgraph will not be assigned a nullptr
There was a problem hiding this comment.
Shall the Andersen analysis and callgraph initialisation be done in the constructor of AbstractInterpretation so that callgraph will not be assigned a nullptr
yes, that makes more sense.
There was a problem hiding this comment.
Could you move andersen's analysis and callgraph to AbstractInterpretation's constructor?
There was a problem hiding this comment.
Could you move andersen's analysis and callgraph to AbstractInterpretation's constructor?
ok it has been in AbstractInterpretation's constructor. And I also add CallGraphSCC in class AbstractInterpretation in order to remove all callgraph construction stuff in initWTO().
|
The CI failed. |
| CallGraphSCC* callGraphScc; | ||
| AEStat* stat; | ||
|
|
||
| std::vector<const CallICFGNode*> callSiteStack; |
There was a problem hiding this comment.
Do you need this callSiteStack? Where this has been used apart from push/pop?
| // while being conservatively sound. | ||
| if (!as[value_id].isAddr()) | ||
| { | ||
| as[value_id] = AddressValue(InvalidMemAddr); |
There was a problem hiding this comment.
Would be good to rename InvalidMemAddr to be BlackHoleObjAddr.
No caller functions as entries
AE traverses all callgraph node and pick the node that has no in edges, which means they are no caller functions. No caller functions can serve as entries.
Current Result (every functions as entry)
LIMIT: 7200 seconds or 64GB Memory
Result (No Caller function as entry)