C++/CLI Now Supported in PVS-Studio

04 Mai 2014

Author: Andrey Karpov

C++/CLI Support
SlimDX
Analysis results
Conclusion

Supporting C++/CLI projects has never been a first-priority target in PVS-Studio. Such projects are pretty few, but we still happen on them from time to time. The Microsoft company is not going to stop supporting the C++/CLI language for now, so we decided we should add support for this language specification too.

C++/CLI Support

Wikipedia: C++/CLI (Common Language Infrastructure) is a language specification created by Microsoft and intended to supersede Managed Extensions for C++. It is a complete revision that aims to simplify the older Managed C++ syntax, which is now deprecated.

The C++/CLI support we have implemented in PVS-Studio is at the level sufficient to check most projects. However, we didn't have enough projects at hand for proper testing, so some language constructs may be handled incorrectly or trigger obviously false positives. It's difficult to keep track of everything at once. So if you encounter any troubles when checking your projects, please let us know.

We could have finished here, but the article would be too boring then. That's why we decided to check a small project SlimDX, and here is the report.

SlimDX

Wikipedia: SlimDX is an open-source API to DirectX programming under .NET. SlimDX can be used from any language under the .NET runtime (due to the CLR). SlimDX can be used to develop multimedia and interactive applications (e.g. games). Enabling high performance graphical representation and enabling the programmer to make use of modern graphical hardware while working inside the .NET framework.

Website: https://github.com/SlimDX/slimdx

Wrapping was check with the PVS-Studio static code analyzer. Since the project is small and is in fact a wrapper around another library, very few suspicious fragments have been found - yet quite enough for an article.

Analysis results

Below are code fragments that I found incorrect.

Fragment No.1

ContainmentType BoundingBox::Contains(
  BoundingBox box, BoundingSphere sphere )
{
  ....
  if( box.Minimum.X + radius <= sphere.Center.X &&
      sphere.Center.X <= box.Maximum.X - radius && 
      box.Maximum.X - box.Minimum.X > radius    &&  <<<===
      box.Minimum.Y + radius <= sphere.Center.Y &&
      sphere.Center.Y <= box.Maximum.Y - radius &&
      box.Maximum.Y - box.Minimum.Y > radius    && 
      box.Minimum.Z + radius <= sphere.Center.Z &&
      sphere.Center.Z <= box.Maximum.Z - radius &&
      box.Maximum.X - box.Minimum.X > radius)       <<<===
    return ContainmentType::Contains;
  ....
}

PVS-Studio's diagnostic message: V501 There are identical sub-expressions 'box.Maximum.X - box.Minimum.X > radius' to the left and to the right of the '&&' operator. boundingbox.cpp 94

This code must have been written through the Copy-Paste method, the programmer forgetting to edit the last line. The following line should have been written at the end of the expression: "box.Maximum.Z - box.Minimum.Z > radius".

Fragment No.2

typedef struct DIJOYSTATE2 {
  ....
  LONG  rglSlider[2];
  ....
  LONG  rglVSlider[2];
  ....
  LONG  rglASlider[2];
  ....
  LONG  rglFSlider[2];
  ....
} DIJOYSTATE2, *LPDIJOYSTATE2;

void JoystickState::AssignState(const DIJOYSTATE2 &joystate)
{
  ....
  for( int i = 0; i < 2; i++ )
  {
    sliders[i] = joystate.rglSlider[i];
    asliders[i] = joystate.rglASlider[i];
    vsliders[i] = joystate.rglVSlider[i];
    fsliders[i] = joystate.rglVSlider[i];
  }
  ....
}

PVS-Studio's diagnostic message: V525 The code containing the collection of similar blocks. Check items 'rglSlider', 'rglASlider', 'rglVSlider', 'rglVSlider' in lines 93, 94, 95, 96. joystickstate.cpp 93

I suspect there is a typo in this code; the rglFSlider array was most likely meant to be used in the last line:

sliders[i] = joystate.rglSlider[i];
asliders[i] = joystate.rglASlider[i];
vsliders[i] = joystate.rglVSlider[i];
fsliders[i] = joystate.rglFSlider[i];

Fragment No.3

array<SoundEffectResult>^ SecondarySoundBuffer::SetEffects(
  array<Guid>^ effects )
{
  DWORD count = effects->Length;
  ....
  if( effects != nullptr && count > 0 )
  ....
}

PVS-Studio's diagnostic message: V595 The 'effects' pointer was utilized before it was verified against nullptr. Check lines: 66, 73. secondarysoundbuffer.cpp 66

The 'effects' pointer is first dereferenced and then, a bit further in the code, checked for being null.

Fragment No.4

There is the class 'TVariable' that contains virtual functions:

template<typename IBaseInterface>
struct TVariable : public IBaseInterface
{
  virtual BOOL IsValid() { .... }
  ....
};

This class is base for the SMember class. Notice that the ZeroMemory() function is used to fill this class' fields with zeroes.

struct SMember :
  public TVariable<TMember<ID3DX11EffectVariable> >
{
};

CEffectVectorOwner<SMember> m_pMemberInterfaces;
ZeroMemory
HRESULT CEffect::CopyMemberInterfaces(....)
{
  ....
  ZeroMemory( &m_pMemberInterfaces[i],
              sizeof(SMember) * ( Members - i ) );
  ....
}

PVS-Studio's diagnostic message: V598 The 'memset' function is used to nullify the fields of 'SMember' class. Virtual method table will be damaged by this. effectnonruntime.cpp 1739

Since there are virtual functions used in the code, the SMember class contains a pointer to a virtual method table. This pointer will get spoiled when calling the ZeroMemory() function.

Other similar defects:

effectload.cpp 1106
effectload.cpp 1107

Fragment No.5

#pragma warning(disable: 4369)
public enum class WaveFormatTag : System::Int32
{
  Pcm = WAVE_FORMAT_PCM,
  AdPcm = WAVE_FORMAT_ADPCM,
  IeeeFloat = WAVE_FORMAT_IEEE_FLOAT,
  MpegLayer3 = WAVE_FORMAT_MPEGLAYER3,
  DolbyAC3Spdif = WAVE_FORMAT_DOLBY_AC3_SPDIF,
  WMAudio2 = WAVE_FORMAT_WMAUDIO2,
  WMAudio3 = WAVE_FORMAT_WMAUDIO3,
  WmaSpdif = WAVE_FORMAT_WMASPDIF,
  Extensible = WAVE_FORMAT_EXTENSIBLE,
};
#pragma warning(default: 4369)

PVS-Studio's diagnostic message: V665 Possibly, the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 1089, 1102. enums.h 1102

Compiler warnings are suppressed incorrectly here: the default state is restored at the end of the fragment. A more correct way is to save the settings at first and restore them later. It is hardly a serious error, but it is very important that libraries don't spoil the warning level settings in user projects. To find out how to suppress warnings correctly, see the description of the V665 warning.

There are a few other similar defects in the header file "enums.h": lines 224, 267, 346.

Conclusion

I admit that this first article about a check of a C++/CLI project has turned out to be brief and boring. But the first try is always a flop. I hope we'll get ourselves a larger and more interesting project to check next time.

I invite you all to download and check your C++/CLI projects with PVS-Studio. We'll appreciate any comments and suggestions from you.

#Cpp