How are we trying to sell PVS-Studio to Google or the next mistakes in Chromium?

PVS-Studio integrates into Ninja to test Chomium.

When we write articles about checking any projects with PVS-Studio, then, as a rule, we add clients. It's all fair. Programmers do not like advertising, but willingly respond to interesting materials that are easy to check. Therefore, we do not advertise our instrument, but simply show that it can. However, although we have already checked the Chromium code three times and found errors in it three times, there are still no orders with mail in google.com in my mail. Since I am wondering what I am doing wrong and why Google does not use PVS-Studio yet, I decided to write another article.

This article consists of two parts. The first one describes the Chromium project infrastructure and the integration nuances, the second one contains the next errors found .
')
By the way, this article was also published in English. If you want to send it to English-speaking colleagues - please give them a link to this .

Want to find out why developing Chromium is difficult and not every tool for programmers can be used in the Chromium project? Then read ...

Chromium developers do not like Visual Studio, they don’t use a Makefile, but they also have fantastic quality code. How so?

Developing projects like Chromium is extremely difficult. In fact, I even find it a bit embarrassing to write “projects like Chromium”, since I don’t know other projects of a similar level. No, it is clear that there is a Linux kernel, there is a Visual Studio environment and many more large and large projects. But personally, I had a chance to “stand side by side” only with Chromium. And what I saw is very interesting for any programmer, since there is something to learn.

For example, when developing Chromium it is not very active, it turns out that they use Visual Studio. The reason is simple - Chromium contains a huge number of projects, and, frankly speaking, the Microsoft IDE does not “pull” such a number. “Aha!” Said the harsh Linux users ... “Still !!!” But when developing Chromium, they don’t use Linux Makefiles either. Exactly for the same reason - standard GNU make does not “pull” so many projects and it takes too long.

What are the developers using Chromium? This is the GYP (Generate Your Projects) assembly system. With it, you can generate either .vcxproj files (MSBuild / Visual C ++) or Ninja system files (this is such a greatly simplified and quick makefile). Therefore, in order to regularly use any static analysis, it is necessary to integrate it into this assembly system. What we did in order to sell PVS-Studio at Google.

The Chromium project is notable for the fact that the size of its source code in C / C ++, including third-party libraries, exceeds 500MB, and each code change is checked by tens of thousands of automated tests in parallel on hundreds of test computers of various architectures and configurations. It is possible to note the high speed of development in this project: in 2012, the Chromium repository created more than 48 thousand revisions by more than 900 unique authors, which corresponds to an average of one revision every 11 minutes and one revision per week from each active project participant.

A project of this size and speed of development imposes particularly stringent requirements on the versatility, accuracy and efficiency of error detectors, as well as on the entire testing system. Many of the errors, inaccuracies and inefficiencies of the detectors were first discovered during the testing of Chromium. In particular, the use of commercial detectors without open source code proved to be unjustifiably difficult - they often incorrectly processed even the basic primitives of the project, and correcting these shortcomings required a long wait for the new version of the detector to be released.

Although PVS-Studio is not an open source project, we cannot refuse to be flexible. Therefore, we wanted to show, using the example of Chromium, that we can integrate into its assembly system and test such a large project without problems.

How to integrate PVS-Studio into the Chromium assembly system for regular checks?

General information about the principles of the work of PVS-Studio

There are 2 main components in the PVS-Studio distribution: the command-line analyzer PVS-Studio.exe and the IDE plugin that integrates this command-line analyzer into one of the supported development environments (Microsoft Visual Studio and Embarcadero RAD Studio).

The command-line analyzer is very similar to the compiler in terms of its operation — it is called for each of the files to be checked with parameters including, among other things, the original compilation parameters of this file. Then, the analyzer calls the external preprocessor it needs (in accordance with the compiler used when building the file being checked) and makes a direct analysis of the received preprocessed temporary file (i-file), i.e. a file in which all include and define directives are expanded.

But using the PVS-Studio.exe analyzer is not limited to IDE plug-ins. As mentioned above, the command-line analyzer is very close in its use directly to the compiler. Accordingly, his challenge can be embedded, if necessary, directly into the build system, along with the compiler itself. For example, if your project is going to the Eclipse IDE using gcc, you can embed the PVS-Studio test into your makefiles.

To directly integrate static analysis into the build process, you need to embed the PVS-Studio.exe call into the build script next to the C / C ++ compiler itself, and pass the same parameters to the analyzer that are passed to the compiler (as well as several additional parameters controlling the output of the analysis). ). This requirement is due to the fact that the static analyzer will need to be run for each scanned file, with specific parameters corresponding to this file. And this is most conveniently done just at the place where the automatic crawling of all project source files takes place.

Checking the Chromium project with the PVS-Studio.exe static analyzer

As written above, Chromium is developed using the GYP (Generate Your Projects) build system, which allows you to get native project files for various operating systems and compilers. Since At the moment, the PVS-Studio analyzer only supports Windows, consider possible options for building Chromium using the Visual C ++ 10 compiler. This compiler (cl.exe) is provided as part of the Visual Studio integrated development environment and can also be installed separately from the free Windows package. Sdk

Using MSBuild project files

The GYP system used in Chromium allows you to get the MSBuild project files (vcxproj) for building Chromium using the Visual C ++ compiler (cl.exe). The MSBuild assembly system is part of the .NET Framework package, which is one of the standard components of the Windows operating system family.

The easiest way to check the Chromium project with the PVS-Studio analyzer is to use the IDE plug-in of the Visual Studio environment that is native to it. MSBuild project files can be opened for inspection in the Visual Studio development environment. At the same time, the IDE PVS-Studio plugin will automatically collect all the necessary information about each of the project files and run the PVS-Studio.exe static analyzer for them. Note that the free version of the development environment of Visual Studio Express Edition does not support working with IDE plugins.

You can also build and check vcxproj project files directly, without using Visual Studio, using the MSBuild build system (or rather, using the command-line utility MSBuild.exe). To check projects with a static analyzer in this mode, you must directly integrate the command-line of the PVS-Studio.exe analyzer into each project file (you can also import into all project files the common props file containing a similar analyzer call).

It is worth noting that although MSBuild allows you to directly call executable files from your build scripts (which, among other things, vcxproj project files), invoke build tools such as the compiler and linker in standard projects using special plug-in wrappers (referred to as build tasks in MSBuild terminology). The PVS-Studio distribution kit contains a similar assembly task module for MSBuild scripts, as well as a Props (property sheet) file using it, which can be directly imported into standard vcxproj projects for integrating static analysis into them.

Using Ninja project files

Building Chromium under Windows using the cl.exe compiler is also possible with the help of the Ninja build system scripts, which can also be obtained using the GYP project generator.

As described above, in order to directly integrate the PVS-Studio static analysis into the build process, you need to embed the PVS-Studio.exe call in the place where the source files bypass the source system when they are compiled.

In the case of the Ninja system files, this integration method is made difficult by the fact that the collected files are hard-coded in the auto-generated * .ninja files as dependencies for the obj files. In this regard, to integrate the analysis at this build step, it will be necessary to modify the build rules corresponding to this step (described in the general file build.ninja): these are cc and cxx, since It is these rules that are used when crawling all source files.

At the moment, we were unable to directly add the PVS-Studio.exe call to the cc and cxx rules. Ninja system build rules allow using only one command variable to define a command to be executed. At the same time, in accordance with the documentation, the command variable may contain several commands separated by && characters. However, if we add to an existing rule, like for example:

command = ninja -t msvc -e $arch -- $cc /nologo /showIncludes /FC 
@$out.rsp /c $in /Fo$out /Fd$pdbname

PVS-Studio.exe:

PVS = "PVS-Studio.exe"
...
command = ninja -t msvc -e $arch -- $cc /nologo /showIncludes /FC 
@$out.rsp /c $in /Fo$out /Fd$pdbname && $PVS –cfg "c:\test.cfg"

, ninja, , , –t msvs cl.exe ($cc). , $PVS , && PVS-Studio.exe.

, -, ninja, PVS-Studio.exe, command (cc cxx). , Chromium PVS-Studio .

Command-line PVS-Studio.exe .

, PVS-Studio.exe (.. IDE ) — . PVS-Studio.exe cl-params, «» . PVS-Studio.exe (, cl.exe), , ( /P cl.exe).

, - , C/C++ . , , include , , precompiled . (, pch ), «cannot open include file».

IDE PVS-Studio precompiled , , include , pch ( ). PVS-Studio.exe , . , –I (/I) .

Chromium , .. , precompiled , Includes , h , . PVS-Studio (.. ) .

. , precompiled headers Chromium PVS-Studio .

?

«» (raw) . PVS-Studio Standalone ( ). .

PVS-Studio Chromium

, PVS-Studio Chromium?

precompiled headers.
Ninja.
Ninja PVS-Studio Wrapper ( PVS-Studio), PVS-Studio.
– – PVS-Studio Standalone .

– .

Chromium , . PVS-Studio . . , - , . , Chromium. , . Chromium .

. , . , Chromium , , , , PVS-Studio.

, , Chromium:

( 2011);
( 2011);
( 2013);

, . -, . -, Chromium. , , , . , ( ) , . , Chromium .

(, :)

static SECStatus
ssl3_SendEncryptedExtensions(sslSocket *ss)
{
  static const unsigned char P256_SPKI_PREFIX[] = {
    0x30, 0x59, 0x30, 0x13, 0x06, 0x07, 0x2a, 0x86,
    0x48, 0xce, 0x3d, 0x02, 0x01, 0x06, 0x08, 0x2a,
    0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, 0x03,
    0x42, 0x00, 0x04
  };
  ....
  if (.... ||
      memcmp(spki->data, P256_SPKI_PREFIX,
             sizeof(P256_SPKI_PREFIX) != 0))
  {
    PORT_SetError(SSL_ERROR_INVALID_CHANNEL_ID_KEY);
    rv = SECFailure;
    goto loser;
  }
  ....
}

PVS-Studio ( Network Security Services): V526 The 'memcmp' function returns 0 if corresponding buffers are equal. Consider examining the condition for mistakes. ssl3con.c 10533

- memcmp() .

«sizeof(P256_SPKI_PREFIX) != 0» . - 1.

:

if (.... ||
    memcmp(spki->data, P256_SPKI_PREFIX,
           sizeof(P256_SPKI_PREFIX)) != 0)

'i' 1.

void SkCanvasStack::pushCanvas(....) {
  ....
  for (int i = fList.count() - 1; i > 0; --i) {
    SkIRect localBounds = canvasBounds;
    localBounds.offset(origin - fCanvasData[i-1].origin);

    fCanvasData[i-1].requiredClip.op(localBounds,
                                     SkRegion::kDifference_Op);
    fList[i-i]->clipRegion(fCanvasData[i-1].requiredClip);
  }
  ....
}

? :) .

PVS-Studio ( Skia Graphics Engine): V501 There are identical sub-expressions to the left and to the right of the '-' operator: i — i SkCanvasStack.cpp 38

[i — 1]. , [i-i]. . , .

«»

Code* Code::FindFirstCode() {
  ASSERT(is_inline_cache_stub());
  DisallowHeapAllocation no_allocation;
  int mask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET);
  for (RelocIterator it(this, mask); !it.done(); it.next())
  {
    RelocInfo* info = it.rinfo();
    return
      Code::GetCodeFromTargetAddress(info->target_address());
  }
  return NULL;
}

PVS-Studio (Chromium): V612 An unconditional 'return' within a loop. objects.cc 10326

. , . , . .

:

int SymbolTable::Symbolize() {
  ....
  if (socketpair(AF_UNIX, SOCK_STREAM,
                 0, child_fds[i]) == -1)
  {
    for (int j = 0; j < i; j++) {
      close(child_fds[j][0]);
      close(child_fds[j][1]);
      PrintError("Cannot create a socket pair");
      return 0;
    }
  }
  ....
}

PVS-Studio ( tcmalloc): V612 An unconditional 'return' within a loop. symbolize.cc 154

, , . , :

if (socketpair(AF_UNIX, SOCK_STREAM,
               0, child_fds[i]) == -1)
{
  for (int j = 0; j < i; j++) {
    close(child_fds[j][0]);
    close(child_fds[j][1]);
  }
  PrintError("Cannot create a socket pair");
  return 0;
}

—

class CONTENT_EXPORT EventPacket {
  ....
  InputEvents::const_iterator begin() const
    { return events_.end(); }
  InputEvents::const_iterator end() const
    { return events_.end(); }
  ....
protected:
  InputEvents events_;
  ....
};

PVS-Studio (Chromium): V524 It is odd that the body of 'end' function is fully equivalent to the body of 'begin' function. event_packet.h 36

beign() end() . , begin() :

InputEvents::const_iterator begin() const
  { return events_.begin(); }

rdtsc()

__inline__ unsigned long long int rdtsc()
{
#ifdef __x86_64__
  unsigned int a, d;
  __asm__ volatile ("rdtsc" : "=a" (a), "=d" (d));
  return (unsigned long)a | ((unsigned long)d << 32);
#elif defined(__i386__)
  unsigned long long int x;
  __asm__ volatile ("rdtsc" : "=A" (x));
  return x;
#else
#define NO_CYCLE_COUNTER
  return 0;
#endif
}

PVS-Studio ( SMHasher): V629 Consider inspecting the '(unsigned long) d << 32' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. Platform.h 78

. , , long 32-. "(unsigned long)d << 32" . , :

return (unsigned long long)a |
       ((unsigned long long)d << 32);

break

'break' . . .

:

static v8::Handle<v8::Value>
toV8Object(....)
{
  switch (extension->name()) {
    ....
    case WebGLExtension::WebGLCompressedTextureATCName:
      extensionObject = toV8(....);
      referenceName = "webGLCompressedTextureATCName";
    case WebGLExtension::WebGLCompressedTexturePVRTCName:
      extensionObject = toV8(....);
      referenceName = "webGLCompressedTexturePVRTCName";
      break;
  }
  ....
}

PVS-Studio ( WebKit):

V519 The 'extensionObject' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 222, 225. V8WebGLRenderingContextCustom.cpp 225
V519 The 'referenceName' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 223, 226. V8WebGLRenderingContextCustom.cpp 226

. 'break'. .

:

bool ScriptDebugServer::executeSkipPauseRequest(....)
{
  const char* v8MethodName;
  switch (request)
  {
    case ScriptDebugListener::NoSkip:
      return false;
    case ScriptDebugListener::Continue:
      return true;
    case ScriptDebugListener::StepInto:
      v8MethodName = stepIntoV8MethodName;
    case ScriptDebugListener::StepOut:
      v8MethodName = stepOutV8MethodName;
  }
  ....
}

PVS-Studio ( WebKit): V519 The 'v8MethodName' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 412, 414. ScriptDebugServer.cpp 414

. , .

:

int linux_get_device_address (....,
  uint8_t *busnum, uint8_t *devaddr,
  ....)
{
  ....
  *busnum = __read_sysfs_attr(ctx, sys_name, "busnum");
  if (0 > *busnum)
    return *busnum;
  ....
}

PVS-Studio ( LibUSB): V547 Expression '0 > * busnum' is always false. Unsigned type value is never < 0. linux_usbfs.c 620

'busnum' , uint8_t. , (0 > *busnum) .

. . .

, Chromium

, PVS-Studio Chromium. , PVS-Studio . , - . – Chromium. PVS-Studio !

, . – , .

P.S. NDA , .

Source: https://habr.com/ru/post/204342/

All Articles