To get a trial key
fill out the form below
Team License (a basic version)
Enterprise License (an extended version)
* By clicking this button you agree to our Privacy Policy statement

Request our prices
New License
License Renewal
--Select currency--
USD
EUR
GBP
RUB
* By clicking this button you agree to our Privacy Policy statement

Free PVS-Studio license for Microsoft MVP specialists
* By clicking this button you agree to our Privacy Policy statement

To get the licence for your open-source project, please fill out this form
* By clicking this button you agree to our Privacy Policy statement

I am interested to try it on the platforms:
* By clicking this button you agree to our Privacy Policy statement

Message submitted.

Your message has been sent. We will email you at


If you haven't received our response, please do the following:
check your Spam/Junk folder and click the "Not Spam" button for our message.
This way, you won't miss messages from our team in the future.

>
>
>
Analyzing the Dolphin-emu project

Analyzing the Dolphin-emu project

Feb 24 2012
Author:

We are regularly asked to check various open-source projects with the PVS-Studio analyzer. If you want to offer some project for us to analyze too, please follow this link. Another project we have checked is Dolphin-emu.

Introduction

Dolphin-emu is a Gamecube and Wii emulator. Since this is an open-source project, anyone can introduce modifications into it. The code can be found on Github.

We have found quite few errors in the project. First of all, this is because of its small size: it is about 260 000 code lines. The rest of the project (1340 000 code lines) is comprised by third-party libraries which are not so much interesting to test.

Though there are few errors, certain code fragments are worth being told about in the article. What the other unsafe code fragments is concerned, the developers can examine them themselves using the trial version of PVS-Studio.

Misprints

Misprints are insidious and they can be found in any application. Programmers are sure that they make only complicated mistakes and that analyzers should look for memory leaks and synchronization errors first of all. Unfortunately, reality is much more trivial: the most widespread errors are misprints and copy-paste mistakes. For example, there is this function in Dolphin-emu:

bool IRBuilder::maskedValueIsZero(
  InstLoc Op1, InstLoc Op2) const
{
  return (~ComputeKnownZeroBits(Op1) &
          ~ComputeKnownZeroBits(Op1)) == 0;
}

PVS-Studio's diagnostic message:

V501 There are identical sub-expressions '~ComputeKnownZeroBits(Op1)' to the left and to the right of the '&' operator. Core ir.cpp 1215

The misprint in this code causes the 'Op1' variable to be used twice, while the 'Op2' variable is not used at all. Here is another sample where a closing parenthesis ')' is in a wrong place.

static const char iplverPAL[0x100] = "(C) 1999-2001 ....";
static const char iplverNTSC[0x100]= "(C) 1999-2001 ....";
CEXIIPL::CEXIIPL() : ....
{
  ...
  memcpy(m_pIPL, m_bNTSC ? iplverNTSC : iplverPAL,
         sizeof(m_bNTSC ? iplverNTSC : iplverPAL));
  ...
}

PVS-Studio's diagnostic message:

V568 It's odd that the argument of sizeof() operator is the 'm_bNTSC ? iplverNTSC : iplverPAL' expression. Core exi_deviceipl.cpp 112

The expression inside the sizeof() operator is not calculated. This code works only because the types of the 'iplverNTSC' and 'iplverPAL' arrays coincide. It appears that sizeof() always returns 0x100. This is an interesting thing: if the sizes of the 'iplverNTSC' and 'iplverPAL ' arrays were different, the code would work quite differently. Let's examine the test sample to make it clear:

const char A[10] = "123";
const char B[10] = "123";
const char C[20] = "123";
cout << sizeof(true ? A : B) << ", "
     << sizeof(true ? A : C) << endl;

This is the result of program execution: 10, 4.

In the first case the array's size is printed, and in the second the size of pointer.

Low-level memory handling errors

Besides misprints there are many errors of handling such functions as memset() and memcpy().

u32 Flatten(..., BlockStats *st, ...)
{
  ...
  memset(st, 0, sizeof(st));
  ...
}

PVS-Studio's diagnostic message:

V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. Core ppcanalyst.cpp 302

It is the size of the pointer to an object which is accidentally calculated instead of the size of the BlockStats object itself. The correct code is: sizeof(*st).

Here is another similar situation:

void drawShadedTexSubQuad(...,
  const MathUtil::Rectangle<float>* rDest, ...)
{
  ...
  if (stsq_observer ||
      memcmp(rDest,
        &tex_sub_quad_data.rdest, sizeof(rDest)) != 0 ||
      tex_sub_quad_data.u1 != u1 ||
      tex_sub_quad_data.v1 != v1 ||
      tex_sub_quad_data.u2 != u2 ||
      tex_sub_quad_data.v2 != v2 ||
      tex_sub_quad_data.G != G)
  ...
}

Only a part of the structure is participating in comparison. The correct code is this: sizeof(*rDest).

Handling float

In some fragments of the Dolphin-emu project, handling variables of the float types looks too optimistic. Operators == and != are used to compare float-variables. Such comparisons are admissible only in certain cases. To know more about comparison of float-variables see the documentation on the V550 diagnostic rule. Consider the following sample:

float Slope::GetValue(float dx, float dy)
{
  return f0 + (dfdx * dx) + (dfdy * dy);
}

void BuildBlock(s32 blockX, s32 blockY)
{
  ...
  float invW = 1.0f / WSlope.GetValue(dx, dy);
  ...
  float q = TexSlopes[i][2].GetValue(dx, dy) * invW;
  if (q != 0.0f)
    projection = invW / q;
  ...
}

PVS-Studio's diagnostic message:

V550 An odd precise comparison: q != 0.0f. It's probably better to use a comparison with defined precision: fabs(A - B) > Epsilon. VideoSoftware rasterizer.cpp 264

Note the "if (q != 0.0f)" comparison. As you can see, the 'q' variable is calculated in a rather complicated way. As a consequence, it is almost improbable that it is CERTAINLY equal to zero. The variable will most likely get some value like 0.00000002, for instance. It is not 0, but division by such a small number might cause an overflow. A special check for such cases is needed.

String violence

void CMemoryWindow::onSearch(wxCommandEvent& event)
{
  ...
  //sprintf(tmpstr, "%s%s", tmpstr, rawData.c_str());
  //strcpy(&tmpstr[1], rawData.ToAscii());
  //memcpy(&tmpstr[1], &rawData.c_str()[0], rawData.size());
  sprintf(tmpstr, "%s%s", tmpstr, (const char *)rawData.mb_str());
  ...
}

You can see from the commented code that this is a weak point. This is already a fourth attempt to form a string. Unfortunately, it is far from being ideal, too. The PVS-Studio analyzer warns us:

V541 It is dangerous to print the string 'tmpstr' into itself. Dolphin memorywindow.cpp 344

What is dangerous about it is that the "tmpstr" string is printed into itself. This code can work correctly but you'd better not do it that way. Depending on how the sprintf() function is implemented, you may unexpectedly get an incorrect result. Consider using the strcat() function instead.

There are other code fragments that work well but are potentially dangerous:

V541 It is dangerous to print the string 'pathData_bin' into itself. Dolphin wiisavecrypted.cpp 513

V541 It is dangerous to print the string 'regs' into itself. Core interpreter.cpp 84

V541 It is dangerous to print the string 'fregs' into itself. Core interpreter.cpp 89

Conclusion

You can review all these and other errors yourselves by downloading PVS-Studio. The new trial mode allows you to see all the detected issues.

Popular related articles
The Evil within the Comparison Functions

Date: May 19 2017

Author: Andrey Karpov

Perhaps, readers remember my article titled "Last line effect". It describes a pattern I've once noticed: in most cases programmers make an error in the last line of similar text blocks. Now I want t…
Technologies used in the PVS-Studio code analyzer for finding bugs and potential vulnerabilities

Date: Nov 21 2018

Author: Andrey Karpov

A brief description of technologies used in the PVS-Studio tool, which let us effectively detect a large number of error patterns and potential vulnerabilities. The article describes the implementati…
Free PVS-Studio for those who develops open source projects

Date: Dec 22 2018

Author: Andrey Karpov

On the New 2019 year's eve, a PVS-Studio team decided to make a nice gift for all contributors of open-source projects hosted on GitHub, GitLab or Bitbucket. They are given free usage of PVS-Studio s…
How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers

Date: Oct 22 2018

Author: Andrey Karpov

Just like other static analyzers, PVS-Studio often produces false positives. What you are about to read is a short story where I'll tell you how PVS-Studio proved, just one more time, to be more atte…
Characteristics of PVS-Studio Analyzer by the Example of EFL Core Libraries, 10-15% of False Positives

Date: Jul 31 2017

Author: Andrey Karpov

After I wrote quite a big article about the analysis of the Tizen OS code, I received a large number of questions concerning the percentage of false positives and the density of errors (how many erro…
The Last Line Effect

Date: May 31 2014

Author: Andrey Karpov

I have studied many errors caused by the use of the Copy-Paste method, and can assure you that programmers most often tend to make mistakes in the last fragment of a homogeneous code block. I have ne…
Static analysis as part of the development process in Unreal Engine

Date: Jun 27 2017

Author: Andrey Karpov

Unreal Engine continues to develop as new code is added and previously written code is changed. What is the inevitable consequence of ongoing development in a project? The emergence of new bugs in th…
PVS-Studio ROI

Date: Jan 30 2019

Author: Andrey Karpov

Occasionally, we're asked a question, what monetary value the company will receive from using PVS-Studio. We decided to draw up a response in the form of an article and provide tables, which will sho…
Appreciate Static Code Analysis!

Date: Oct 16 2017

Author: Andrey Karpov

I am really astonished by the capabilities of static code analysis even though I am one of the developers of PVS-Studio analyzer myself. The tool surprised me the other day as it turned out to be sma…
The way static analyzers fight against false positives, and why they do it

Date: Mar 20 2017

Author: Andrey Karpov

In my previous article I wrote that I don't like the approach of evaluating the efficiency of static analyzers with the help of synthetic tests. In that article, I give the example of a code fragment…

Comments (0)

Next comments
This website uses cookies and other technology to provide you a more personalized experience. By continuing the view of our web-pages you accept the terms of using these files. If you don't want your personal data to be processed, please, leave this site.
Learn More →
Accept