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

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Request our prices
New License
License Renewal
--Select currency--
USD
EUR
GBP
RUB
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
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

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
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

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
I am interested to try it on the platforms:
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
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.

>
>
>
PVS-Studio vs Chromium - Continuation

PVS-Studio vs Chromium - Continuation

Oct 13 2011
Author:

About half a year ago we checked the Chromium project and wrote an article about it. The PVS-Studio analyzer naturally keeps developing, and in the new Chromium version we have found some errors undetected before. Many errors, of course, refer not to the Chromium project itself but to libraries it employs. But in this article I want to show you how the analyzer's capabilities have improved and not tell you about what we have found in this or that part of Chromium. That's why I will give messages together.

So, in this post we will enumerate the new defects found by the analyzer. This is far from all of the things found, we've only glanced through the report and included only what we found at first glance. If you want a more thorough analysis of Chromium or its libraries, we can provide developers with the full version of PVS-Studio for a time so that they perform it themselves. By the way, go by this link and perhaps you will get interested in the opportunity to try the full PVS-Studio version too: https://pvs-studio.com/en/blog/posts/0092/

Fragment N1

std::string TestFileIO::TestParallelReads() {
  ...
  const char* expected_result_1 =
    "__border__abc__border__";
  const char* expected_result_2 =
    "__border__defghijkl__border__";
  if (strncmp(extended_buf_1, expected_result_1,
              sizeof(expected_result_1)) != 0 ||
      strncmp(extended_buf_2, expected_result_2,
              sizeof(expected_result_2)) != 0) {
  ...
}

PVS-Studio's diagnostic messages:

V579 The strncmp function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. ppapi_tests test_file_io.cc 759

V579 The strncmp function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. ppapi_tests test_file_io.cc 761

Calls of the strncmp() function in this code compare only the first characters and not the whole strings. To calcluate strings' lengths the programmer tried to use the sizeof() operator absolutely inappropriate for this purpose. The sizeof() operator will calculate the pointer's size instead of the number of bytes in a string.

Fragment N2

int  AffixMgr::parse_convtable(..., const char * keyword)
{
  ...
  if (strncmp(piece, keyword, sizeof(keyword)) != 0) {
  ...
}

PVS-Studio's diagnostic message:

V579 The strncmp function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. hunspell affixmgr.cxx 3545

Again, the mistake here is that only the first 4 or 8 bytes will be compared depending on the pointer's size.

Fragment N3

#define SEC_ASN1_CHOICE        0x100000

typedef struct sec_ASN1Template_struct {
  unsigned long kind; 
  ...
} SEC_ASN1Template;

PRBool SEC_ASN1IsTemplateSimple(
  const SEC_ASN1Template *theTemplate)
{
  ...
  if (!theTemplate->kind & SEC_ASN1_CHOICE) {
  ...
}

PVS-Studio's diagnostic message:

V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. nss secasn1u.c 121

The error is caused by the issue of operation priorities. The correct code is this one:

if (!(theTemplate->kind & SEC_ASN1_CHOICE)) {

Fragment N4

bool GetPlatformFileInfo(...) {
  ...
  info->is_directory =
    file_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY != 0;
  ...
}

PVS-Studio's diagnostic message:

V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. base platform_file_win.cc 219

The error is caused by the issue of operation priorities. The correct code is this one:

info->is_directory =
  (file_info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0;

Fragment N5

WebRtc_Word32
interpolator::SupportedVideoType(VideoType srcVideoType,
                                 VideoType dstVideoType)
{
  ...
  if ((srcVideoType != kI420) ||
      (srcVideoType != kIYUV) ||
      (srcVideoType != kYV12))
  {
      return -1;
  }
  ...
}

PVS-Studio's diagnostic message:

V547 Expression is always true. Probably the '&&' operator should be used here. webrtc_vplib interpolator.cc 119

The (A != 123 || A!= 321) - like condition is always true. There is obviously a misprint here and the condition must look in a different way.

Fragment N6

static GLenum
get_temp_image_type(GLcontext *ctx, GLenum baseFormat)
{
  ...
  if (ctx->DrawBuffer->Visual.redBits <= 8)
     return GL_UNSIGNED_BYTE;
  else if (ctx->DrawBuffer->Visual.redBits <= 8)
     return GL_UNSIGNED_SHORT;
  else
     return GL_FLOAT;
  ...
}

PVS-Studio's diagnostic message:

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 2520, 2522. osmesa meta.c 2520

One and the same check is performed twice. Perhaps it should look this way:

if (ctx->DrawBuffer->Visual.redBits <= 8)
   return GL_UNSIGNED_BYTE;
else if (ctx->DrawBuffer->Visual.redBits <= 16)
   return GL_UNSIGNED_SHORT;

Fragment N7

WebRtc_Word32 ModuleFileUtility::UpdateWavHeader(OutStream& wav)
{
  ...
  if(STR_CASE_CMP(codec_info_.plname, "L16") == 0)
  {
     res = WriteWavHeader(wav, codec_info_.plfreq, 2,
             channels, kWaveFormatPcm, _bytesWritten);
  } else if(STR_CASE_CMP(codec_info_.plname, "PCMU") == 0) {
     res = WriteWavHeader(wav, 8000, 1, channels,
             kWaveFormatMuLaw, _bytesWritten);
  } else if(STR_CASE_CMP(codec_info_.plname, "PCMU") == 0) {
     res = WriteWavHeader(wav, 8000, 1, channels,
             kWaveFormatALaw, _bytesWritten);
  } else {
  ...
}

PVS-Studio's diagnostic message:

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 1324, 1327. media_file media_file_utility.cc 1324

The 'plname' variable is compared to the "PCMU" string twice in a row. Most likely, another string must be used for the second time.

Fragment N8

enum ContentSettingsType;
struct EntryMapKey {
  ContentSettingsType content_type;
  ...
};

bool OriginIdentifierValueMap::EntryMapKey::operator<(
    const OriginIdentifierValueMap::EntryMapKey& other) const {
  if (content_type < other.content_type)
    return true;
  else if (other.content_type > content_type)
    return false;
  return (resource_identifier < other.resource_identifier);
}

PVS-Studio's diagnostic message:

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 61, 63. browser content_settings_origin_identifier_value_map.cc 61

The condition looks this way for the first time: "A < B"; and this way for the second time: "B > A". Thus, the check is meaningles. There is obviously a misrpint in the code.

Fragment N9

WebRtc_Word32

RTPReceiverVideo::ReceiveH263Codec(...)
{
  ...
  if (IP_PACKET_SIZE < parsedPacket.info.H263.dataLength +
       parsedPacket.info.H263.insert2byteStartCode? 2:0)
  ...
}

PVS-Studio's diagnostic message:

V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '+' operator. rtp_rtcp rtp_receiver_video.cc 480

The '?:' operator has a lower priority than the '+' operator. As a result, the condition works in a way other than the programmer expected. The correct condition must look this way:

if (IP_PACKET_SIZE < parsedPacket.info.H263.dataLength +
    (parsedPacket.info.H263.insert2byteStartCode ? 2:0))

The same error is here:

V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '+' operator. rtp_rtcp rtp_receiver_video.cc 504

Fragment N10

static int 
xmlXPathCompOpEvalFirst(...)
{
  ...
  total += xmlXPathCompOpEvalFirst(...);
  ...
  total =+ xmlXPathCompOpEvalFilterFirst(ctxt, op, first);
  ...
}

PVS-Studio's diagnostic message:

V588 The expression of the 'A =+ B' kind is utilized. Consider reviewing it, as it is possible that 'A += B' was meant. libxml xpath.c 12676

As you may see from the first line, a certain sum is being calculated in the total variable. But then there is a misprint and we have "=+" instead of "+=".

Fragment N11

static VisiblePosition updateAXLineStartForVisiblePosition(...)
{
  ...
  tempPosition = startPosition.previous();
  if (tempPosition.isNull() || tempPosition.isNull())
      break;
  ...
}

PVS-Studio's diagnostic message:

V501 There are identical sub-expressions 'tempPosition.isNull ()' to the left and to the right of the '||' operator. webcore_remaining accessibilityobject.cpp 489

Strange code.

Fragment N12

TEST(SharedMemoryTest, MultipleThreads) {
  ...
  int threadcounts[] = { 1, kNumThreads };
  for (size_t i = 0;
       i < sizeof(threadcounts) / sizeof(threadcounts); i++) {
  ...
}

PVS-Studio's diagnostic message:

V501 There are identical sub-expressions 'sizeof (threadcounts)' to the left and to the right of the '/' operator. base_unittests shared_memory_unittest.cc 231

Because of the error, the loop in the test function performs only one iteration. This is the correct loop:

for (size_t i = 0;
     i < sizeof(threadcounts) / sizeof(*threadcounts); i++) {

Fragment N13

bool
ir_algebraic_visitor::reassociate_constant(...)
{
  if (ir1->operands[0]->type->is_matrix() ||
      ir1->operands[0]->type->is_matrix() ||
      ir2->operands[1]->type->is_matrix() ||
      ir2->operands[1]->type->is_matrix())
    return false;
}

PVS-Studio's diagnostic message:

V501 There are identical sub-expressions 'ir1->operands [0]->type->is_matrix ()' to the left and to the right of the '||' operator. mesa ir_algebraic.cpp 189

The code seems to have been written through the Copy-Paste method, while the programmer fixed the indexes incorrectly. This is how the comparison must look:

if (ir1->operands[0]->type->is_matrix() ||
    ir1->operands[1]->type->is_matrix() ||
    ir2->operands[0]->type->is_matrix() ||
    ir2->operands[1]->type->is_matrix())

Fragment N15

#define FRAMESAMPLES_HALF      240
#define FRAMESAMPLES           480

typedef struct {
  ...
  WebRtc_Word16 realFFT[FRAMESAMPLES_HALF];
  WebRtc_Word16 imagFFT[FRAMESAMPLES_HALF];
} ISACUBSaveEncDataStruct;

int WebRtcIsac_EncodeStoredDataUb12(...)
{
  ...
  for(n = 0; n < FRAMESAMPLES; n++)
  {
    realFFT[n] = (WebRtc_Word16)
      (scale * (float)ISACSavedEnc_obj->realFFT[n] + 0.5f);
    imagFFT[n] = (WebRtc_Word16)
      (scale * (float)ISACSavedEnc_obj->imagFFT[n] + 0.5f);
  }
  ...
}

PVS-Studio's diagnostic messages:

V557 Array overrun is possible. The value of 'n' index could reach 479. iSAC encode.c 1307

V557 Array overrun is possible. The value of 'n' index could reach 479. iSAC encode.c 1308

Array overruns occur in the loop. The loop must search through FRAMESAMPLES_HALF items only.

Fragment N16

static int
coff_helper_gasflags(...)
{
  ...
  case 'd':
      datasect = 1;
      load = 1;
      readonly = 0;
  case 'x':
      code = 1;
      load = 1;
      break;
  ...
}

PVS-Studio's diagnostic message:

V519 The 'load' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1346, 1350. yasm coff-objfmt.c 1350

The 'break;' operator seems to be missing here.

Popular related articles
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…
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 Ultimate Question of Programming, Refactoring, and Everything

Date: Apr 14 2016

Author: Andrey Karpov

Yes, you've guessed correctly - the answer is "42". In this article you will find 42 recommendations about coding in C++ that can help a programmer avoid a lot of errors, save time and effort. The au…
PVS-Studio for Java

Date: Jan 17 2019

Author: Andrey Karpov

In the seventh version of the PVS-Studio static analyzer, we added support of the Java language. It's time for a brief story of how we've started making support of the Java language, how far we've co…
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…
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 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…
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…
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…
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…

Comments (0)

Next comments

This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
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