Add SVFBugRecoder and modify source loc format #1067

JoelYYoung · 2023-04-24T05:40:33Z

No description provided.

codecov · 2023-04-24T06:05:17Z

Codecov Report

Merging #1067 (21e8041) into master (53983e0) will decrease coverage by 0.33%.
The diff coverage is 36.00%.

❗ Current head 21e8041 differs from pull request most recent head 5eb7db8. Consider uploading reports for the commit 5eb7db8 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1067      +/-   ##
==========================================
- Coverage   62.72%   62.39%   -0.33%     
==========================================
  Files         218      220       +2     
  Lines       21620    21863     +243     
==========================================
+ Hits        13561    13642      +81     
- Misses       8059     8221     +162

Impacted Files	Coverage Δ
svf/include/SABER/FileChecker.h	`0.00% <ø> (ø)`
svf/include/SABER/LeakChecker.h	`100.00% <ø> (ø)`
svf/include/SABER/ProgSlice.h	`100.00% <ø> (ø)`
svf/include/SABER/SrcSnkDDA.h	`91.66% <ø> (ø)`
svf/lib/SABER/FileChecker.cpp	`0.00% <0.00%> (ø)`
svf/lib/SABER/SrcSnkDDA.cpp	`82.47% <ø> (ø)`
svf/lib/Util/SVFBugReport.cpp	`20.83% <20.83%> (ø)`
svf/include/Util/SVFBugReport.h	`66.00% <66.00%> (ø)`
svf-llvm/lib/LLVMUtil.cpp	`68.68% <100.00%> (+0.07%)`	⬆️
svf/lib/SABER/DoubleFreeChecker.cpp	`68.85% <100.00%> (-0.51%)`	⬇️
... and 2 more

... and 2 files with indirect coverage changes

yuleisui · 2023-04-25T07:02:32Z

svf/include/Util/SVFBugReport.h

+    typedef std::vector<GenericEvent *> EventStack;
+
+public:
+    enum BugType{BOA, NEVERFREE, PARTIALLEAK, DOUBLEFREE, FILENEVERCLOSE, FILEPARTIALCLOSE};


BOA=>BuffOverflow

yuleisui · 2023-04-25T07:02:55Z

svf/include/Util/SVFBugReport.h

+protected:
+    BugType bugType;
+    const SVFInstruction * bugInst;
+    EventStack *bugEventStack = nullptr;


why not assign nullptr in the constructor?

yuleisui · 2023-04-25T07:03:29Z

svf/include/Util/SVFBugReport.h

+    inline BugType getBugType(){ return bugType; }
+    std::string getLoc();
+    std::string getFuncName();
+    inline void setEventStack(EventStack *eventStack) { bugEventStack = eventStack; }


why we need setEventStack ? why not set this in the constructor?

yuleisui · 2023-04-25T07:03:53Z

svf/include/Util/SVFBugReport.h

+    typedef std::vector<GenericEvent *> EventStack;
+    typedef std::vector<GenericEvent *> EventSet;
+    typedef std::vector<GenericBug *> BugVector;
+    typedef std::vector<EventStack *> EventStackVector;


why do we have a set and a vector?

yuleisui · 2023-04-25T07:05:39Z

svf/lib/Util/SVFBugReport.cpp

+    auto bugIt = bugVector.begin();
+    for(;bugIt != bugVector.end(); bugIt ++){
+        delete (*bugIt);
+    }
+    auto eventStackIt = eventStackVector.begin();


For(auto event : eventSet)
delete event

yuleisui · 2023-04-25T07:29:45Z

svf/include/Util/SVFBugReport.h

+    enum BugType{BOA, NEVERFREE, PARTIALLEAK, DOUBLEFREE, FILENEVERCLOSE, FILEPARTIALCLOSE};
+
+protected:
+    BugType bugType;


classof method for each subclass.

yuleisui · 2023-04-25T07:33:29Z

svf/include/Util/SVFBugReport.h

+    virtual ~GenericBug() = default;
+
+    inline BugType getBugType(){ return bugType; }
+    std::string getLoc();


const std::string

yuleisui · 2023-04-25T07:34:31Z

svf/include/Util/SVFBugReport.h

+    EventStack *bugEventStack = nullptr;
+
+public:
+    inline GenericBug(BugType bugType, const SVFInstruction * bugInst): bugType(bugType), bugInst(bugInst){ };


GenericBug(BugType bugType, eventstack)

remove const SVFInstruction * bugInst,

yuleisui · 2023-04-25T07:41:31Z

svf/include/Util/SVFBugReport.h

+    typedef std::vector<EventStack *> EventStackVector;
+
+protected:
+    EventStack eventStack;  // maintain current execution events


BugVector bugVector; and move eventStack to each Bug class

yuleisui · 2023-04-25T09:50:40Z

svf/include/Util/SVFBugReport.h

+
+class GenericEvent{
+public:
+    enum EventType{Branch, Caller, CallSite, Loop};


Add "SourceInst'' event

yuleisui · 2023-04-25T09:51:18Z

svf/include/Util/SVFBugReport.h

+    /// branch statement and branch condition true or false
+protected:
+    const BranchStmt *branchStmt;
+    std::string description;


remove description and make it retrievable in getEventDescription

yuleisui · 2023-04-25T09:52:47Z

svf/include/Util/SVFBugReport.h

+public:
+    SVFBugReport() = default;
+    ~SVFBugReport();
+    typedef std::vector<GenericBug *> BugVector;


make it a set.

yuleisui · 2023-04-25T09:53:26Z

svf/lib/Util/SVFBugReport.cpp

+    SVFUtil::errs() << "\n";
+}
+
+const std::string CallSiteEvent::getFuncName() const


const std::string&

this may be unfeasible because getName() returns const std::string but not const std::string&.

yuleisui · 2023-04-25T09:53:58Z

svf/lib/Util/SVFBugReport.cpp

+    return description;
+}
+
+const std::string BranchEvent::getFuncName() const


const std::string&

yuleisui · 2023-04-25T09:54:04Z

svf/lib/Util/SVFBugReport.cpp

+
+const std::string BranchEvent::getFuncName() const
+{
+    return branchStmt->getInst()->getFunction()->getName();


const std::string&

this may be unfeasible because getName() returns const std::string but not const std::string&.

Ok, pls use const std::string

yuleisui · 2023-04-25T09:54:29Z

svf/lib/Util/SVFBugReport.cpp

+
+SVFBugReport::~SVFBugReport()
+{
+    auto bugIt = bugVector.begin();


for( bug : bugSet)
delete bug;

yuleisui · 2023-04-25T09:55:51Z

svf/lib/Util/SVFBugReport.cpp

+    }
+}
+
+std::string SVFBugReport::toString()


better change the method to "void SVFBugReport::toJson()". Never return large string!!!

chang to void dumpToFile(const std::string filePath).

yuleisui · 2023-04-25T12:31:59Z

svf/include/Util/SVFBugReport.h

+    /// branch statement and branch condition true or false
+protected:
+    const BranchStmt *branchStmt;
+    bool branchCondition;


Do we need this book field or it can be obtained via BranchStmt?

yuleisui · 2023-04-25T12:32:57Z

svf/include/Util/SVFBugReport.h

+    const CallICFGNode *callSite;
+
+public:
+    inline CallSiteEvent(const CallICFGNode *callSite): GenericEvent(GenericEvent::CallSite), callSite(callSite){ }


Remove inline for constructor

yuleisui · 2023-04-25T12:33:16Z

svf/include/Util/SVFBugReport.h

+    const SVFInstruction *sourceSVFInst;
+
+public:
+    inline SourceInstructionEvent(const SVFInstruction *sourceSVFInst):


Same as above and following constructors

yuleisui · 2023-04-25T12:34:03Z

svf/include/Util/SVFBugReport.h

+
+class BufferOverflowBug: public GenericBug{
+protected:
+    long long allocLowerBound, allocUpperBound, accessLowerBound, accessUpperBound;


Use SVF’s type and DON’T use std long long

yuleisui · 2023-04-25T12:34:20Z

svf/include/Util/SVFBugReport.h

+
+public:
+    inline BufferOverflowBug(GenericBug::BugType bugType, EventStack eventStack,
+                             long long allocLowerBound, long long allocUpperBound,


Same as above and followings

yuleisui · 2023-04-25T12:35:21Z

svf/include/Util/SVFBugReport.h

+protected:
+    Set<std::string> conditionalFreePath;
+public:
+    PartialLeakBug(EventStack bugEventStack, Set<std::string> conditionalFreePath):


If we have an event stack, do we need a set of condfree paths? Can we put these into the event stack?

yuleisui · 2023-04-25T12:36:17Z

svf/include/Util/SVFBugReport.h

+
+class NeverFreeBug : public GenericBug{
+public:
+    NeverFreeBug(EventStack bugEventStack):


Use “const EventStack& “ to avoid copy construction

I'm afraid that copy construction is necessary here because the EventStack created by detector is a function local variable, so it had better to be copied and saved.

Or we can use new to dynamically create EventStack in saber's reportBug function and destroy it in the destructor of GenericBug. I am not sure if its better.

I'm afraid that copy construction is necessary here because the EventStack created by detector is a function local variable, so it had better to be copied and saved.

It should be const reference because it will do the copy in GeneralBug’s constructor.

yuleisui · 2023-04-25T12:37:10Z

svf/include/Util/SVFBugReport.h

+protected:
+    Set<std::string> conditionalFreePath;
+public:
+    PartialLeakBug(EventStack bugEventStack, Set<std::string> conditionalFreePath):


Use const EventStack&

same for the following constructors

yuleisui · 2023-04-25T12:37:37Z

svf/include/Util/SVFBugReport.h

+
+class DoubleFreeBug : public GenericBug{
+protected:
+    Set<std::string> doubleFreePath;


Do we need this or put into the event stack?

If you refer to doing so for DoubleFreeBug only, I think it's feasible, but may loss consistency with other bug classes like FilePartialCloseBug, ParitalLeakBug, etc.

What are the strings in this vector? Can we get or extract the from the event stack?

And the reason for not doing so for other bug classes is that eventStack should be used to store events happening on the execution path from entry to the bug site, but not side effects like partial free paths and partial close paths.

yuleisui · 2023-04-25T12:39:00Z

svf/lib/Util/SVFBugReport.cpp

+    SVFUtil::errs() << "\t\t Events : \n";
+
+    auto eventIt = bugEventStack.begin();
+    for(; eventIt != bugEventStack.end(); eventIt ++){


for (event : bugEventStack)
….

yuleisui · 2023-04-25T12:41:33Z

svf/lib/Util/SVFBugReport.cpp

+
+const std::string GenericBug::getLoc() const
+{
+    return bugEventStack.at(bugEventStack.size() -1)->getEventLoc();


Add an assertion, otherwise how can you sure the size-1 is the element you want?

yuleisui · 2023-04-25T12:44:28Z

svf/include/Util/SVFBugReport.h

+    /// note: should be initialized with a bugEventStack
+    inline GenericBug(BugType bugType, EventStack bugEventStack): bugType(bugType), bugEventStack(bugEventStack){ };
+
+    virtual ~GenericBug() = default;


Where is the destructor? How did you release the memory of the events in EventStack?

yuleisui · 2023-04-25T14:51:15Z

svf/include/Util/SVFBugReport.h

+
+public:
+    /// note: should be initialized with a bugEventStack
+    inline GenericBug(BugType bugType, EventStack bugEventStack): bugType(bugType), bugEventStack(bugEventStack){ };


const EventStack&

yuleisui · 2023-04-26T04:28:01Z

svf/lib/Util/SVFBugReport.cpp

+    }
+}
+
+void SVFBugReport::dumpToFile(const std::string& filePath)


dumpToFile => dumpToJsonFile

yuleisui · 2023-04-26T04:29:40Z

svf/include/Util/SVFBugReport.h

+
+public:
+    /// note: should be initialized with a bugEventStack
+    GenericBug(BugType bugType, EventStack bugEventStack):


const EventStack& bugEventStack

yuleisui · 2023-04-26T04:30:55Z

svf/include/Util/SVFBugReport.h

+
+class GenericBug{
+public:
+    typedef std::vector<GenericEvent *> EventStack;


const GenericEvent*

yuleisui · 2023-04-26T04:32:49Z

svf/include/Util/SVFBugReport.h

+public:
+    SVFBugReport() = default;
+    ~SVFBugReport();
+    typedef SVF::Set<GenericBug *> BugSet;


const GenericBug*

yuleisui · 2023-04-26T04:40:18Z

svf/include/Util/SVFBugReport.h

+     * usage: addBug<GenericBug::BOA>(BufferOverflowBug(bugInst, 0, 10, 1, 11))
+     */
+    template <typename T>
+    void addBug(T bug)


void addBug(eventstack, flag){

switch (xx)
case1:
xxxx

default:
assert(false && "no such kind of bug");

}

yuleisui · 2023-04-26T08:31:46Z

svf/include/Util/SVFBugReport.h

    {
-        T *newBug = new T(bug);
-        bugSet.insert(newBug);
+        GenericBug *newBug;


remove this.

GenericBug *newBug = nullptr;

yuleisui · 2023-04-26T08:32:09Z

svf/include/Util/SVFBugReport.h

+        switch(bugType){
+        case GenericBug::NEVERFREE:{
+            newBug = new NeverFreeBug(eventStack);
+            bugSet.insert(newBug);


bugSet.insert(new NeverFreeBug(eventStack));

newBug is use at line 333 newBug->printBugToTerminal()

yuleisui · 2023-04-26T09:13:59Z

svf/include/Util/SVFBugReport.h

+
+    void addAbsExecBug(GenericBug::BugType bugType, const GenericBug::EventStack &eventStack,
+                       s64_t allocLowerBound, s64_t allocUpperBound, s64_t accessLowerBound, s64_t accessUpperBound){
+        GenericBug *newBug;


GenericBug *newBug = nullptr;

yuleisui · 2023-04-26T09:15:40Z

svf/include/Util/SVFBugReport.h

+    }
+};
+
+class SourceInstructionEvent: public GenericEvent{


SourceInstructionEvent => SourceInstEvent

yuleisui · 2023-04-26T10:08:02Z

svf/lib/SABER/ProgSlice.cpp

+        auto stmt = PAG::getPAG()->getValueEdges(tinst);
+        assert(stmt.size() == 1 && "returned SVFStmtSet should be of size 1!");
+        if(pathAllocator->isNegCond(*it))
+            eventStack.push_back(new BranchEvent(*(stmt.begin()), false));


new BranchEvent(tinst, false)

JoelYYoung added 2 commits April 24, 2023 13:31

add json bug recoder

66dae8e

Merge branch 'master' of https://github.com/SVF-tools/SVF

5332504

JoelYYoung added 3 commits April 24, 2023 22:51

add json bug recoder to SABER

a66aca2

refactor names

5a715b4

refactor bug type system

236c6ce

yuleisui reviewed Apr 25, 2023

View reviewed changes

refactor event stacks and vectors

a4cb99f

yuleisui reviewed Apr 25, 2023

View reviewed changes

refactors

c2d62b9

yuleisui reviewed Apr 25, 2023

View reviewed changes

JoelYYoung requested a review from yuleisui April 25, 2023 14:56

bug fixes and refactor

c138efb

yuleisui reviewed Apr 26, 2023

View reviewed changes

JoelYYoung added 2 commits April 26, 2023 16:03

refactor and bug fixs

4d1cfd7

chang to const

a39e7ad

yuleisui reviewed Apr 26, 2023

View reviewed changes

JoelYYoung requested a review from yuleisui April 26, 2023 09:01

yuleisui reviewed Apr 26, 2023

View reviewed changes

refactor and destroy events by eventSet

21e8041

yuleisui reviewed Apr 26, 2023

View reviewed changes

JoelYYoung added 2 commits April 26, 2023 18:29

refactor

1c9a83b

set code coverage threshold to 62.3%

5eb7db8

yuleisui merged commit 5c98c07 into SVF-tools:master Apr 26, 2023

Add SVFBugRecoder and modify source loc format #1067

Add SVFBugRecoder and modify source loc format #1067

Conversation

JoelYYoung commented Apr 24, 2023

codecov bot commented Apr 24, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoelYYoung Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 24, 2023 •

edited

Loading

JoelYYoung Apr 25, 2023 •

edited

Loading