Geekly Articles each Day
📜 ⬆️ ⬇️

Errors found in C ++ Builder libraries

We checked the header files that are part of the Embarcadero C ++ Builder XE3. In fact, this only means checking a small number of inline functions. Accordingly, quite a few suspicious places were found, but enough for a small note.

Introduction


We regularly check open projects , as well as anything else that can be checked. For example, we checked the libraries that are included with Visual C ++ 2012. The result was a note: " Errors were detected in the Visual C ++ 2012 libraries ."

The Visual C ++ distribution kit includes the source codes of the libraries. The situation with C ++ Builder is worse. There are only header files. Therefore, it was possible to check only some inline functions. However, I managed to find something interesting. Consider the relevant sections of the code.

Work with warnings


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

Diagnostic message issued by PVS-Studio:
')
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 is not good to set the warning mode to the default state. The correct approach is to save and then restore the previous state. To do this, use "#pragma warning (push [, n])" and "#pragma warning (pop)".

Bad 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;
 }

Diagnostic message issued by PVS-Studio:

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 written poorly. Its contents are not framed by curly braces {}. As a result, the condition "if (src)" will apply only to the first line of the macro.

Indefinite 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)
   {
     _Unt _Arr [3 + _Nw];
     ....
     int _Lsh = _BITS_BYTE * sizeof (_Uint);
     ....

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

Diagnostic message issued by PVS-Studio:

V610 Instantiate linear_congruential <unsigned long, 40014, 0, 2147483563>: Undefined behavior. Check the shift operator '<<. The operand is the right operand. random 738

In this function, the variable '_Lsh' will take the value 32. You cannot shift the 32-bit types by more than 31 bits. Excerpt from the standard: the behavior of the left operand.

Also, the DXVABitMask macro is made in a dangerous way:
  #define DXVABitMask (__ n) (~ ((~ 0) << __n))

We quote the corresponding line from the standard: Otherwise, if E1 has a signed type and non-negative value, and E1 * 2 ^ E2 is a representation of the result, then it is the resulting value; otherwise, the behavior is undefined.

Because of this macro, PVS-Studio issues several warnings. Example:

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

More information about the shifts and uncertain behavior can be found in the article: Not knowing the ford, do not climb into the water. Part Three

Unaccounted change in the behavior of the new operator.


Found a lot of places where, after calling the 'new' operator, it is checked that the pointer is not NULL. Now it does not make sense and even harmful. In case of a memory allocation error, the operator 'new' throws an exception std :: bad_alloc.

You can call the operator 'new', which does not generate an exception. C ++ Builder even has a special macro for this:
  #define NEW_NOTHROW (_bytes) new (nothrow) BYTE [_bytes]

However, apparently not everywhere it was possible to restore order with the allocation of memory. I will give a few examples:
  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);
     }
   }
 }

Diagnostic message issued by PVS-Studio:

V668 has been defined as the m_Data. The exception will be generated in the case of memory allocation error. comutil.h 454

Line "_com_issue_error (E_OUTOFMEMORY);" never done. In case of an error, an exception std :: bad_alloc () will be generated.
  static inline BYTE * __ CorHlprNewThrows (size_t bytes)
 {
   BYTE * pbMemory = new BYTE [bytes];
   if (pbMemory == NULL)
     __CorHlprThrowOOM ();
   return pbMemory;
 }

Diagnostic message issued by PVS-Studio:

V668 has been defined 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;
   ....
 }

Diagnostic message issued by PVS-Studio:

The pnewdata has been defined using the 'new' operator. The exception will be generated in the case of memory allocation error. dxtmpl.h 338

The rest of the code snippets are similar, and there is no point in bringing them. I will confine myself only to diagnostic messages:

And this is only in inline functions! Imagine what's going on in * .cpp files. :)

Note


The moment I finished writing this article, Embarcadero C ++ Builder XE4 was released. However, this does not negate the benefits of the analysis done and well demonstrates the capabilities of PVS-Studio. Plus, I still waited a little bit with the publication of this article. Today we have released a new version of PVS-Studio, which already supports C ++ Builder XE4. This is a good reason to publish and suggest trying our code analyzer.

Conclusion


Thank you all for your attention. I hope the C ++ Builder developers will pay attention to us and will want to check the source codes of the compiler and libraries with PVS-Studio. In conclusion, I want to offer a few useful links:
  1. Description of the PVS-Studio tool . You can download a fully functional trial version.
  2. Andrey Karpov. C ++ Builder, build 64-bit applications and renaissance Viva64 .
  3. Our twitter @Code_Analysis . We publish many interesting links on the subject of C / C ++.
  4. About what PVS-Studio can do. Errors discovered in Open Source projects by PVS-Studio developers using static analysis .

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

More articles:


All Articles