FYI : Static analysis at commit 3ede247ad7 shows some minor issues

[SteveGilham]

I got a chance to do a static analysis run of a recent master; and setting aside the reports that are in tests, in system header files, are clearly false positives, or indicate that something has already gone horribly wrong (such as bad inputs) and all that could be done is emit more diagnostics as we terminate; we are left with the following:

• Access in ProfilerManager.cs to MemoryManger._blocks is not guarded by the lock used to guard all the other accesses (GetBlocks could take a predicate and return a reified list from a lock clause)
• MemoryManager.Initialise() could double-checked lock to consistently guard the initialised fields
• ProfilerManager.StartProcessingThread() could call threadActivated.Dispose() before exiting to release the system handle
• ProfilerManager.RunProcess() could call queueMgmt.Dispose() before exiting to release the system handle
• ProfilerManager.RunProcess() could call environmentKeyRead.Dispose() before exiting to release the system handle
• CCodeCoverage::Initialize() local nCount written but not read
• Method::GetInstructionAtOffset(5 args) last argument is always true (bitwise & with zero constant)
• Method::InsertInstructionsAtOffset() local actualOffset written but not read
• Method::EndOfBranch sets next->m_jump then checks next against NULL

and about half of those are primarily aesthetic.

[sawilde]

thanks @SteveGilham - things definitely worth looking at

[ddur]

Method::EnOfBranch is my sin :)
It should be like this:

Instruction* Method::EndOfBranch(Instruction* toFollow)
    {
        Instruction* next = NULL;
        if (toFollow != NULL && ( toFollow->m_operation == CEE_BR || toFollow->m_operation == CEE_BR_S ))
        {
            Instruction* jump = NULL;
            Instruction* jumpTo = NULL;
            next = toFollow;
            next->m_jump = NULL;
            do {
                _ASSERTE(next->m_isBranch);
                _ASSERTE(next->m_branches.size() == 1);
                _ASSERTE(next->m_branches[0] != NULL);
                jumpTo = next->m_branches[0];
                _ASSERTE(jumpTo != NULL);
                if ( jumpTo == NULL ) break;
                jump = next; // store last BR instruction
                next = jumpTo; // store last BR jump-target instruction found (so far) 
                next->m_jump = jump; // set m_jump to last BR instruction
            }
            while ( next->m_operation == CEE_BR || next->m_operation == CEE_BR_S )
        }
        return next == NULL? toFollow : next;
    }

[ddur]

In fact, Method::EndOfBranch is abandoned/unused and can be removed, including line 720

#define MOVE_TO_ENDOFBRANCH FALSE

EndOfBranch functionality is moved to C#.

[molnargab]

@SteveGilham, how would one reproduce these results? Is it a simple Visual Studio Code Analysis?

[SteveGilham]

No, this is a selection of the results from running Coverity over the code.

On 26 January 2015 at 00:03, molnargab notifications@github.com wrote:

@SteveGilham https://github.com/SteveGilham, how would one reproduce
these results? Is it a simple Visual Studio Code Analysis?

—
Reply to this email directly or view it on GitHub
#242 (comment).

[sawilde]

I've actually registered for coverity and plan on adding the output to the build pipeline (which I was going to do as part of this card)

[sawilde]

Hi can anyone provide steps on coverity for me (saves reinventing the wheel)

I also see that appveyor have recently added coverity to their build servers so we can bootstrap on that as well.

[SteveGilham]

I just used the wizard to wrap a Visual Studio build. Default settings
should suffice for c# code (though I switched up the inspection level to
high, which gives a lot of false positives). The command line I haven't
touched.

On 1 March 2015 at 01:37, Shaun Wilde notifications@github.com wrote:

Hi can anyone provide steps on coverity for me (saves reinventing the
wheel)

I also see that appveyor have recently added coverity to their build
servers so we can bootstrap on that as well.

—
Reply to this email directly or view it on GitHub
#242 (comment).

[sawilde]

@SteveGilham thanks - I'll give it a go - can't get the thing to download at the mo