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.
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.

>
>
>
Errors detected in C++Builder

Errors detected in C++Builder

May 14, 2013
Author:

We have checked the header files from the Embarcadero C++Builder XE3 project. In fact, it means that we have checked just a small number of inline-functions. Accordingly, quite few issues were found, but they are enough to write a small post.

Introduction

We regularly check open-source projects and many other things that can be checked. For instance, we once checked the libraries included into Visual C++ 2012, which resulted in publishing the post "Errors detected in the Visual C++ 2012 libraries".

The Visual C++ distribution kit includes the libraries' source codes. But the things are worse with C++Builder: there are only header files available, so we managed to analyze only some of the inline-functions. However, we did find some interesting issues. Let's see what these are.

Handling warnings

#pragma warning(disable : 4115)
#include <objbase.h>
#pragma warning(default : 4115)

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: 16, 18. iaguid.h 18

It's no good setting the warning output mode to the default state. A good practice is to save and then restore the previous state. Use "#pragma warning(push[ ,n ])" and "#pragma warning(pop)" to do that.

A poorly written macro

#define SET_VTYPE_AND_VARREF(type, val) \
  this->vt = VT_ ## type | VT_BYREF; \
  V_ ## type ## REF (this) = val;

TVariantT& operator=(System::Currency* src)
{
  Clear();
  if(src)
    SET_VTYPE_AND_VARREF(CY,
      reinterpret_cast<tagCY*>(&(src->Val)));
  return* this;
}

PVS-Studio's diagnostic message:

V640 The code's operational logic does not correspond with its formatting. The second statement will always be executed. It is possible that curly brackets are missing. utilcls.h 1781

The SET_VTYPE_AND_VARREF macro is bad. Its contents are not framed with curly brackets { }. This results in the "if (src)" condition referring only to the first line of the macro.

Undefined behavior

#define _BITS_BYTE    8
template<class _Uint,
    _Uint _Ax,
    _Uint _Cx,
    _Uint _Mx>
    class linear_congruential
{
  static _CONST_DATA int _Nw =
    (_BITS_BYTE * sizeof (_Uint) + 31) / 32;

  void seed(seed_seq& _Seq)
  {
    _Uint _Arr[3 + _Nw];
    ....
    int _Lsh = _BITS_BYTE * sizeof (_Uint);
    ....

    for (int _Idx = _Nw; 0 < --_Idx; )
      _Arr[3 + _Idx - 1] |=
        _Arr[3 + _Idx] << _Lsh;
    ....
  }
}

PVS-Studio's diagnostic message:

V610 Instantiate linear_congruential < unsigned long, 40014, 0, 2147483563 >: Undefined behavior. Check the shift operator '<<. The right operand '_Lsh' is greater than or equal to the length in bits of the promoted left operand. random 738

The '_Lsh' variable takes value 32 in this function. You can't shift 32-bit types more than by 31 bits. Here's a quote from the standard specification: The behavior is undefined if the right operand is negative, or greater than or equal to the length in bits of the promoted left operand.

The DXVABitMask macro is implemented in a dangerous way too:

#define DXVABitMask(__n) (~((~0) << __n))

Here is another quote from the standard specification on this: Otherwise, if E1 has a signed type and non-negative value, and E1*2^E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined.

Because of this macro PVS-Studio generates several warnings. For example:

V610 Undefined behavior. Check the shift operator '<<. The left operand '(~0)' is negative. dxva.h 1080

To learn more about shifts and undefined behavior see the post: Wade not in unknown waters. Part three.

The new operator's behavior change unaccounted for

The code appeared to contain a lot of fragments where, after calling the 'new' operator, pointers are checked for not being NULL pointers. It is pointless now and even harmful: if a memory allocation error occurs, the 'new' operator throws the exception std::bad_alloc.

We can call the 'new' operator so that it won't throw exceptions. C++ Builder even has a special macro for that purpose:

#define NEW_NOTHROW(_bytes) new (nothrow) BYTE[_bytes]

But there are some fragments where the memory allocation issue wasn't solved. For example:

inline void _bstr_t::Assign(BSTR s) throw(_com_error)
{
  if (m_Data != NULL) {
    m_Data->Assign(s); 
  } 
  else {
    m_Data = new Data_t(s, TRUE);
    if (m_Data == NULL) {
      _com_issue_error(E_OUTOFMEMORY);
    }
  }
}

PVS-Studio's diagnostic message:

V668 There is no sense in testing the 'm_Data' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. comutil.h 454

The line "_com_issue_error(E_OUTOFMEMORY);" is never executed. If an error occurs, the exception std::bad_alloc() will be thrown.

static inline BYTE *__CorHlprNewThrows(size_t bytes)
{
  BYTE *pbMemory = new BYTE[bytes];
  if (pbMemory == NULL)
    __CorHlprThrowOOM();
  return pbMemory;
}

PVS-Studio's diagnostic message:

V668 There is no sense in testing the 'pbMemory' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. corhlpr.h 56

template<class TYPE, class ARG_TYPE>
void CDXArray<TYPE, ARG_TYPE>::SetSize(int nNewSize, int nGrowBy)
{
  ....
  TYPE* pNewData = (TYPE*) new BYTE[nNewMax * sizeof(TYPE)];

  // oh well, it's better than crashing
  if (pNewData == NULL)
    return;
  ....
}

PVS-Studio's diagnostic message:

V668 There is no sense in testing the 'pNewData' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. dxtmpl.h 338

All the rest code fragments are much alike, and there is no point citing them. Let me only give you the list of diagnostic messages:

  • V668 There is no sense in testing the 'p' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. d3dx10math.inl 1008
  • V668 There is no sense in testing the 'p' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. dxtmpl.h 123
  • V668 There is no sense in testing the 'pNewData' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. dxtmpl.h 395
  • V668 There is no sense in testing the 'm_pHashTable' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. dxtmpl.h 1126
  • V668 There is no sense in testing the 'newBrush' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbrush.h 44
  • V668 There is no sense in testing the 'retimage' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbrush.h 374
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbrush.h 615
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbrush.h 645
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdipluspath.h 1196
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdipluspath.h 1231
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdipluspath.h 1372
  • V668 There is no sense in testing the 'argbs' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdipluspath.h 1405
  • V668 There is no sense in testing the 'newLineCap' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdipluslinecaps.h 153
  • V668 There is no sense in testing the 'nativeRegions' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusgraphics.h 1415
  • V668 There is no sense in testing the 'newRegion' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusregion.h 89
  • V668 There is no sense in testing the 'nativeFamilyList' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusfontcollection.h 57
  • V668 There is no sense in testing the 'newImage' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbitmap.h 334
  • V668 There is no sense in testing the 'bitmap' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbitmap.h 819
  • V668 There is no sense in testing the 'bitmap' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. gdiplusbitmap.h 862
  • V668 There is no sense in testing the 'm_pData' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. spcollec.h 266
  • V668 There is no sense in testing the 'pNewData' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. spcollec.h 325

All these bugs were found only in inline-functions! I can imagine what horrible stuff can be found in the *.cpp files. :)

Note

At the moment when I finished writing this article, Embarcadero C++Builder XE4 was released. Nevertheless, this fact doesn't diminish the value of the analysis we have performed, as it has demonstrated PVS-Studio's capabilities very well.

Conclusion

Thank you all for your attention. I hope that the C++Builder developers will take notice of our post and get interested in checking the compiler and libraries' source files. In conclusion, I want to share a few useful links with you:

Popular related articles
Technologies used in the PVS-Studio code analyzer for finding bugs and potential vulnerabilities

Date: 11.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 Ultimate Question of Programming, Refactoring, and Everything

Date: 04.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 ROI

Date: 01.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…
The Evil within the Comparison Functions

Date: 05.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…
PVS-Studio for Java

Date: 01.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…
How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers

Date: 10.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…
The way static analyzers fight against false positives, and why they do it

Date: 03.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…
Static analysis as part of the development process in Unreal Engine

Date: 06.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…
Appreciate Static Code Analysis!

Date: 10.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 Last Line Effect

Date: 05.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…

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