A scrupulous code review - 15 bugs in C++ code

A close look at 15 problems one can find when reviewing C++ code.
Offers code examples.
Covers indexed loops, tainted data, copy and paste errors, problems with comparisons, exceptions, etc.
You can use static code analysis tools to make code review easier. Code analyzers find errors and potential vulnerabilities in code, while saving the developers' time and the companies' money.
Manual code review is expensive - a group of programmers get together regularly to review the code.
One can run static analysis tools regularly to find mistakes and vulnerabilities early
Here you can learn how to find the most interesting and significant bugs in your project: https://pvs-studio.com/en/docs/manual/6532/

Transcript

1. A scrupulous code review – 15 bugs in C++ code Phillip Khandeliants khandeliants@viva64.com
2. About Me ▪ Over 5 years with the PVS-Studio team ▪ Team lead at the C++ analyzer development team ▪ Microsoft Certified Professional, C# ▪ Talk on modern C++ ▪ Live by the C++ ISO standard 2
3. Intro: the code review 3 ▪ We all do code reviews ▪ Who doesn't admit this – does it twice as often
4. 1. 'Auto'matic coding 4
5. void foo(const std::vector<....> &vec) { .... for (auto i = 0; i < vec.size(); ++i) { // do some magic with vec[i] .... } .... } 5
6. void foo(const std::vector<....> &vec) { .... for (int i = 0; i < vec.size(); ++i) // 64-bit problems :) { // do some magic with vec[i] .... } .... } 6
7. void foo(const std::vector<....> &vec) { .... for (size_t i = 0; i < vec.size(); ++i) // ok { // do some magic with vec[i] .... } .... } 7
8. void foo(const std::vector<....> &vec) { .... for (std::vector<....>::size_type i = 0; i < vec.size(); ++i) { // do some magic with vec[i] .... } .... } 8
9. void foo(const std::vector<....> &vec) { .... for (auto i = 0uLL; i < vec.size(); ++i) // don't do that on // 128-bit processors { // do some magic with vec[i] .... } .... } 9
10. void foo(const std::vector<....> &vec) { .... for (auto i = 0uLLL; i < vec.size(); ++i) // ok since C++7d { // do some magic with vec[i] .... } .... } 10
11. void foo(const std::vector<....> &vec) { .... for (auto i = 0uLLL; i < vec.size(); ++i) // ok since C++7d { // do some magic with vec[i] .... } .... } 11
12. 2. Reference! I said reference! Perfection! 12
13. 13 template <typename T> int ColumnDecimal<T>::compareAt(size_t n, size_t m, const IColumn & rhs_, int) const { auto other = static_cast<const Self &>(rhs_); const T &a = data[n]; const T &b = other.data[m]; return decimalLess<T>(b, a, other.scale, scale) ? 1 : (decimalLess<T>(a, b, scale, other.scale) ? -1 : 0); }
14. 14 template <typename T> int ColumnDecimal<T>::compareAt(size_t n, size_t m, const IColumn & rhs_, int) const { auto other = static_cast<const Self &>(rhs_); const T &a = data[n]; const T &b = other.data[m]; return decimalLess<T>(b, a, other.scale, scale) ? 1 : (decimalLess<T>(a, b, scale, other.scale) ? -1 : 0); }
15. 15 template <typename T> int ColumnDecimal<T>::compareAt(size_t n, size_t m, const IColumn & rhs_, int) const { auto &other = static_cast<const Self &>(rhs_); const T &a = data[n]; const T &b = other.data[m]; return decimalLess<T>(b, a, other.scale, scale) ? 1 : (decimalLess<T>(a, b, scale, other.scale) ? -1 : 0); }
16. 16 template <typename T> int ColumnDecimal<T>::compareAt(size_t n, size_t m, const IColumn & rhs_, int) const { decltype(auto) other = static_cast<const Self &>(rhs_); const T &a = data[n]; const T &b = other.data[m]; return decimalLess<T>(b, a, other.scale, scale) ? 1 : (decimalLess<T>(a, b, scale, other.scale) ? -1 : 0); }
17. 3. Vector-vector, *pChar++ 17
18. void vector32_inc(std::vector<uint32_t> &v) { for (size_t i = 0; i < v.size(); i++) { v[i]++; } } 18
19. void vector32_inc(std::vector<uint32_t> &v) { for (size_t i = 0; i < v.size(); i++) { v[i]++; } } void vector8_inc(std::vector<uint8_t> &v) { for (size_t i = 0; i < v.size(); i++) { v[i]++; } } 19
20. Let's benchmark :) 20 Compiler Element -O1 -O2 -O3 gcc 8 uint8_t 2.0 2.0 2.0 gcc 8 uint32_t 2.3 1.3 0.2 clang 8 uint8_t 9.2 2.0 2.0 clang 8 uint32_t 9.2 0.2 0.2
21. 21 vector8_inc(std::vector<uint8_t> &): mov rdx, QWORD PTR [rdi] ; it = begin() cmp rdx, QWORD PTR [rdi+8] ; if (it == end()) je .L1 ; return xor eax, eax ; i = 0 .L3: ; do { add BYTE PTR [rdx+rax], 1 ; ++(*(it + i)) mov rdx, QWORD PTR [rdi] ; it = begin() add rax, 1 ; ++i mov rcx, QWORD PTR [rdi+8] ; end = end() sub rcx, rdx ; end = end - it cmp rax, rcx jb .L3 ; } while (i < end) .L1: ret vector32_inc(std::vector<uint32_t> &): mov rax, QWORD PTR [rdi] ; it = begin() mov rdx, QWORD PTR [rdi+8] ; end = end() sub rdx, rax ; end = end - it mov rcx, rdx ; size in bytes shr rcx, 2 ; size in elements je .L1 ; if (size == 0) ; return add rdx, rax ; end = end + it .L3: ; do { add DWORD PTR [rax], 1 ; ++(*it) add rax, 4 ; ++it cmp rax, rdx jne .L3 ; } while (it != end) .L1: ret
22. 22 vector8_inc(std::vector<uint8_t> &): mov rdx, QWORD PTR [rdi] ; it = begin() cmp rdx, QWORD PTR [rdi+8] ; if (it == end()) je .L1 ; return xor eax, eax ; i = 0 .L3: ; do { add BYTE PTR [rdx+rax], 1 ; ++(*(it + i)) mov rdx, QWORD PTR [rdi] ; it = begin() add rax, 1 ; ++i mov rcx, QWORD PTR [rdi+8] ; end = end() sub rcx, rdx ; end = end - it cmp rax, rcx jb .L3 ; } while (i < end) .L1: ret vector32_inc(std::vector<uint32_t> &): mov rax, QWORD PTR [rdi] ; it = begin() mov rdx, QWORD PTR [rdi+8] ; end = end() sub rdx, rax ; end = end - it mov rcx, rdx ; size in bytes shr rcx, 2 ; size in elements je .L1 ; if (size == 0) ; return add rdx, rax ; end = end + it .L3: ; do { add DWORD PTR [rax], 1 ; ++(*it) add rax, 4 ; ++it cmp rax, rdx jne .L3 ; } while (it != end) .L1: ret
23. void vector8_inc(std::vector<uint8_t> &v) { for (size_t i = 0; i < v.size(); i++) { v[i]++; } } 23
24. void vector8_inc(std::vector<uint8_t> &v) { for (size_t i = 0; i < v.size(); i++) { v[i]++; } } void vector8_inc(std::vector<uint8_t> &v) { auto it = v.begin(); const auto end = v.end(); for (; it != end; ++it) { ++(*it); } } 24
25. 25 vector8_inc(std::vector<uint8_t> &): mov rax, QWORD PTR [rdi] ; it = begin() mov rdx, QWORD PTR [rdi+8] ; end = end() cmp rax, rdx ; if (it == end) je .L1 ; return .L3: ; do { add BYTE PTR [rax], 1 ; ++(*it) add rax, 1 ; ++it cmp rax, rdx ; } while (it != end) .L1: ret
26. 26 vector8_inc(std::vector<uint8_t> &): mov rax, QWORD PTR [rdi] ; it = begin() mov rdx, QWORD PTR [rdi+8] ; end = end() cmp rax, rdx ; if (it == end) je .L1 ; return .L3: ; do { add BYTE PTR [rax], 1 ; ++(*it) add rax, 1 ; ++it cmp rax, rdx ; } while (it != end) .L1: ret
27. Let's make benchmarks great again! Compiler uint8_t -O1 -O2 -O3 gcc 8 (before) 2.0 2.0 2.0 gcc 8 (after) 1.3 1.3 0.06 gcc speedup 1.5x 1.5x 33.4x Compiler uint8_t -O1 -O2 -O3 clang 8 (before) 9.2 2.0 2.0 clang 8 (after) 20.3 0.06 0.06 clang speedup 0.45x 33.4x 33.4x
28. void vector8_inc(std::vector<uint8_t> &v) { auto it = v.begin(); const auto end = v.end(); for (; it != end; ++it) { ++(*it); } } 28
29. void vector8_inc(std::vector<uint8_t> &v) { auto it = v.begin(); const auto end = v.end(); for (; it != end; ++it) { ++(*it); } } void vector8_inc(std::vector<uint8_t> &v) { for (auto &elem : v) { ++elem; } } 29
31. 4. Security? Security! 31
32. #include <cstring> #include <memory> void InputPassword(char *pswd); void ProcessPassword(const char *pswd); #define MAX_PASSWORD_LEN .... void Foo() { char password[MAX_PASSWORD_LEN]; InputPassword(password); ProcessPassword(password); memset(password, 0, sizeof(password)); } 32
33. #include <cstring> #include <memory> void InputPassword(char *pswd); void ProcessPassword(const char *pswd); #define MAX_PASSWORD_LEN .... void Foo() { char password[MAX_PASSWORD_LEN]; InputPassword(password); ProcessPassword(password); memset(password, 0, sizeof(password)); } 33
35. #include <cstring> #include <memory> void InputPassword(char *pswd); void ProcessPassword(const char *pswd); #define MAX_PASSWORD_LEN .... void Foo() { char *password = new char[MAX_PASSWORD_LEN]; InputPassword(password); ProcessPassword(password); memset(password, 0, MAX_PASSWORD_LEN); delete[] password; } 35
37. void tds_answer_challenge(....) { #define MAX_PW_SZ 14 .... if (ntlm_v == 1) { .... /* with security is best be pedantic */ memset(hash, 0, sizeof(hash)); memset(passwd_buf, 0, sizeof(passwd_buf)); memset(ntlm2_challenge, 0, sizeof(ntlm2_challenge)); } else { .... } } 37
38. typedef struct tds_answer { unsigned char lm_resp[24]; unsigned char nt_resp[24]; } TDSANSWER; static TDSRET tds7_send_auth(....) { size_t current_pos; TDSANSWER answer; .... /* for security reason clear structure */ memset(&answer, 0, sizeof(TDSANSWER)); return tds_flush_packet(tds); } 38
39. char* crypt_md5(const char* pw, const char* salt) { unsigned char final[MD5_SIZE]; .... /* Don't leave anything around in vm they could use. */ memset(final, 0, sizeof final); return passwd; } 39
40. void MD4Engine::transform (UInt32 state[4], const unsigned char block[64]) { UInt32 a = state[0], b = state[1], c = state[2], d = state[3], x[16]; decode(x, block, 64); .... /* Zeroize sensitive information. */ std::memset(x, 0, sizeof(x)); } 40
41. char* px_crypt_md5(const char *pw, const char *salt, char *passwd, unsigned dstlen) { .... unsigned char final[MD5_SIZE]; .... /* Don't leave anything around in vm they could use. */ memset(final, 0, sizeof final); .... } 41
43. ▪ Custom safe_memset + disabled LTO/WPO ▪ Access a non-volatile object through a volatile pointer ▪ Call memset through a volatile function pointer ▪ Volatile assembly code ▪ Memset + memory barrier ▪ Disable compiler optimizations (-fno-builtin-memset) ▪ C11: memset_s ▪ Windows: RtlSecureZeroMemory ▪ FreeBSD & OpenBSD: explicit_bzero ▪ Linux Kernel: memzero_explicit Ways to fix 43
44. 5. Dirty thoughts data 44
45. static const char* basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if ( fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } /* remove endline */ command_buf[strlen(command_buf)-1] = '\0'; break; } .... } 45
46. static const char* basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if ( fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } /* remove endline */ command_buf[strlen(command_buf)-1] = '\0'; break; } .... } 46
47. int main (int argc, char *argv[]) { .... else if (fgets(readbuf, BUFSIZ, stdin) == NULL) { if (feof (stdin)) break; error (EXIT_FAILURE, errno, _("input error")); } if (readbuf[strlen(readbuf) - 1] == '\n') readbuf[strlen(readbuf) - 1] = '\0'; .... } 47
48. int main (int argc, char *argv[]) { .... else if (fgets(readbuf, BUFSIZ, stdin) == NULL) { if (feof (stdin)) break; error (EXIT_FAILURE, errno, _("input error")); } if (readbuf[strlen(readbuf) - 1] == '\n') readbuf[strlen(readbuf) - 1] = '\0'; .... } 48 CVE-2015-8948
49. int main (int argc, char *argv[]) { .... else if (getline(&line, &linelen, stdin) == -1) { if (feof (stdin)) break; error (EXIT_FAILURE, errno, _("input error")); } if (line[strlen(line) - 1] == '\n') line[strlen(line) - 1] = '\0'; .... } 49
50. int main (int argc, char *argv[]) { .... else if (getline(&line, &linelen, stdin) == -1) { if (feof (stdin)) break; error (EXIT_FAILURE, errno, _("input error")); } if (line[strlen(line) - 1] == '\n') line[strlen(line) - 1] = '\0'; .... } 50 CVE-2016-6262
51. 6. Last effort 51
52. inline void Init( float ix=0, float iy=0, float iz=0, float iw = 0 ) { SetX( ix ); SetY( iy ); SetZ( iz ); SetZ( iw ); } 52
53. inline void Init( float ix=0, float iy=0, float iz=0, float iw = 0 ) { SetX( ix ); SetY( iy ); SetZ( iz ); SetZ( iw ); } 53
54. 54 inline void Init( float ix=0, float iy=0, float iz=0, float iw = 0 ) { SetX( ix ); SetY( iy ); SetZ( iz ); SetW( iw ); }
55. if (access & FILE_WRITE_ATTRIBUTES) output.append(ASCIIToUTF16("\tFILE_WRITE_ATTRIBUTES\n")); if (access & FILE_WRITE_DATA) output.append(ASCIIToUTF16("\tFILE_WRITE_DATA\n")); if (access & FILE_WRITE_EA) output.append(ASCIIToUTF16("\tFILE_WRITE_EA\n")); if (access & FILE_WRITE_EA) output.append(ASCIIToUTF16("\tFILE_WRITE_EA\n")); break; 55
56. if (access & FILE_WRITE_ATTRIBUTES) output.append(ASCIIToUTF16("\tFILE_WRITE_ATTRIBUTES\n")); if (access & FILE_WRITE_DATA) output.append(ASCIIToUTF16("\tFILE_WRITE_DATA\n")); if (access & FILE_WRITE_EA) output.append(ASCIIToUTF16("\tFILE_WRITE_EA\n")); if (access & FILE_WRITE_EA) output.append(ASCIIToUTF16("\tFILE_WRITE_EA\n")); break; 56
57. if (protocol.EqualsIgnoreCase("http") || protocol.EqualsIgnoreCase("https") || protocol.EqualsIgnoreCase("news") || protocol.EqualsIgnoreCase("ftp") || protocol.EqualsIgnoreCase("file") || protocol.EqualsIgnoreCase("javascript") || protocol.EqualsIgnoreCase("ftp")) { 57
58. if (protocol.EqualsIgnoreCase("http") || protocol.EqualsIgnoreCase("https") || protocol.EqualsIgnoreCase("news") || protocol.EqualsIgnoreCase("ftp") || protocol.EqualsIgnoreCase("file") || protocol.EqualsIgnoreCase("javascript") || protocol.EqualsIgnoreCase("ftp")) { 58
59. 7. Zero, one, two, Freddy's coming for you! 59
60. Sequence< OUString > FirebirdDriver:: getSupportedServiceNames_Static() throw (RuntimeException) { Sequence< OUString > aSNS( 2 ); aSNS[0] = "com.sun.star.sdbc.Driver"; aSNS[0] = "com.sun.star.sdbcx.Driver"; return aSNS; } 60
61. Sequence< OUString > FirebirdDriver:: getSupportedServiceNames_Static() throw (RuntimeException) { Sequence< OUString > aSNS( 2 ); aSNS[0] = "com.sun.star.sdbc.Driver"; aSNS[0] = "com.sun.star.sdbcx.Driver"; return aSNS; } 61
62. struct short2 { short values[2]; short2(short s1, short s2) { values[0] = s1; values[2] = s2; } .... }; 62
63. struct short2 { short values[2]; short2(short s1, short s2) { values[0] = s1; values[2] = s2; } .... }; 63
64. 8. Evil within comparisons! 64
65. string _server; .... bool operator<( const ServerAndQuery& other ) const { if ( ! _orderObject.isEmpty() ) return _orderObject.woCompare( other._orderObject ) < 0; if ( _server < other._server ) return true; if ( other._server > _server ) return false; return _extra.woCompare( other._extra ) < 0; } Pattern: A < B, B > A 65
66. string _server; .... bool operator<( const ServerAndQuery& other ) const { if ( ! _orderObject.isEmpty() ) return _orderObject.woCompare( other._orderObject ) < 0; if ( _server < other._server ) return true; if ( other._server > _server ) return false; return _extra.woCompare( other._extra ) < 0; } Pattern: A < B, B > A 66
67. bool operator==(const SComputePipelineStateDescription &other) const { return 0 == memcmp(this, &other, sizeof(this)); } Pattern: evaluating the size of a pointer instead of the size of the structure/class 67
68. bool operator==(const SComputePipelineStateDescription &other) const { return 0 == memcmp(this, &other, sizeof(this)); } Pattern: evaluating the size of a pointer instead of the size of the structure/class 68
69. SSHORT TextType::compare(ULONG len1, const UCHAR* str1, ULONG len2, const UCHAR* str2) { .... SSHORT cmp = memcmp(str1, str2, MIN(len1, len2)); if (cmp == 0) cmp = (len1 < len2 ? -1 : (len1 > len2 ? 1 : 0)); return cmp; } Pattern: incorrect use of the memcmp result 69
70. SSHORT TextType::compare(ULONG len1, const UCHAR* str1, ULONG len2, const UCHAR* str2) { .... SSHORT cmp = memcmp(str1, str2, MIN(len1, len2)); if (cmp == 0) cmp = (len1 < len2 ? -1 : (len1 > len2 ? 1 : 0)); return cmp; } Pattern: incorrect use of the memcmp result 70
71. if (cmp == 0) cmp = (len1 < len2 ? -1 : (len1 > len2 ? 1 : 0)); Pattern: incorrect use of the memcmp result 71
72. bool Peptide::operator==(Peptide& p) { .... for (i = 0, j = 0; i < this->stripped.length(), j < p.stripped.length(); i++, j++) { .... } Pattern: incorrect loops 72
73. bool Peptide::operator==(Peptide& p) { .... for (i = 0, j = 0; i < this->stripped.length(), j < p.stripped.length(); i++, j++) { .... } Pattern: incorrect loops 73
74. bool equals( class1* val1, class2* val2 ) const { ... size_t size = val1->size(); ... while ( --size >= 0 ) { if ( !comp(*itr1,*itr2) ) return false; itr1++; itr2++; } ... } Pattern: incorrect loops 74
75. bool equals( class1* val1, class2* val2 ) const { ... size_t size = val1->size(); ... while ( --size >= 0 ) { if ( !comp(*itr1,*itr2) ) return false; itr1++; itr2++; } ... } Pattern: Incorrect Loops 75
76. 9. Use <=>, Luke! 76
77. Base equality comparison 77 struct Foo { int a, b; };
78. Base equality comparison 78 struct Foo { int a, b; }; bool operator==(Foo lhs, Foo rhs) { return lhs.a == rhs.a && lhs.b == rhs.b; }
79. Base equality comparison 79 struct Foo { int a, b; }; bool operator==(Foo lhs, Foo rhs) { return lhs.a == rhs.a && lhs.b == rhs.b; } bool operator!=(Foo lhs, Foo rhs) { return !(lhs == rhs); }
80. Base 'less' comparison 80 struct Foo { int a, b; }; bool operator<(Foo lhs, Foo rhs) { return lhs.a < rhs.a && lhs.b < rhs.b; }
81. Base 'less' comparison 81 struct Foo { int a, b; }; bool operator<(Foo lhs, Foo rhs) { return lhs.a < rhs.a && lhs.b < rhs.b; } Foo { 1, 2 } < Foo { 2, 1 }; // <= false Foo { 2, 1 } < Foo { 1, 2 }; // <= false
82. Base 'less' comparison 82 struct Foo { int a, b; }; bool operator<(Foo lhs, Foo rhs) { if (lhs.a < rhs.a) return true; if (rhs.a < lhs.a) return false; return lhs.b < rhs.b; } Foo { 1, 2 } < Foo { 2, 1 }; // <= true Foo { 2, 1 } < Foo { 1, 2 }; // <= false
83. Base 'less' comparison 83 struct Foo { double a; }; bool operator<(Foo lhs, Foo rhs) { return lhs.a < rhs.a; }
84. Base 'less' comparison 84 struct Foo { double a; }; bool operator<(Foo lhs, Foo rhs) { return lhs.a < rhs.a; } bool operator>=(Foo lhs, Foo rhs) { return !(lhs < rhs); }
85. Base 'less' comparison 85 struct Foo { double a; }; bool operator<(Foo lhs, Foo rhs) { return lhs.a < rhs.a; } bool operator>=(Foo lhs, Foo rhs) { return !(lhs < rhs); } Foo { 1.0 } < Foo { 2.0 }; // <= true Foo { 1.0 } < Foo { NaN }; // <= false Foo { 1.0 } >= Foo { NaN }; // <= true
86. Comparisons in C++20 86 #include <compare> struct Foo { double a; auto operator<=>(const Foo &rhs) const = default; }; Foo { 1.0 } < Foo { 2.0 }; // <= true Foo { 1.0 } < Foo { NaN }; // <= false Foo { 1.0 } >= Foo { NaN }; // <= false
87. 10. Payne, I can't feel my legs pointer 87
88. void Item_Paint(itemDef_t *item) { vec4_t red; menuDef_t *parent = (menuDef_t*)item->parent; red[0] = red[3] = 1; red[1] = red[2] = 0; if (item == NULL) { return; } .... } 88
89. void Item_Paint(itemDef_t *item) { vec4_t red; menuDef_t *parent = (menuDef_t*)item->parent; red[0] = red[3] = 1; red[1] = red[2] = 0; if (item == NULL) { return; } .... } 89
90. void Item_Paint(std::unique_ptr<itemDef_t> &item) { vec4_t red; menuDef_t *parent = (menuDef_t*)item->parent; red[0] = red[3] = 1; red[1] = red[2] = 0; if (item == NULL) { return; } .... } 90
91. void Item_Paint(std::shared_ptr<itemDef_t> &item) { vec4_t red; menuDef_t *parent = (menuDef_t*)item->parent; red[0] = red[3] = 1; red[1] = red[2] = 0; if (item == NULL) { return; } .... } 91
92. void Item_Paint(std::optional<itemDef_t> &item) { vec4_t red; menuDef_t *parent = (menuDef_t*)item->parent; red[0] = red[3] = 1; red[1] = red[2] = 0; if (!item) { return; } .... } 92
93. static struct DerivedMesh *dynamicPaint_Modifier_apply(....) { .... for (; surface; surface = surface->next) { PaintSurfaceData *sData = surface->data; if (surface && surface->format != MOD_DPAINT_SURFACE_F_IMAGESEQ && sData) { .... } 93
94. static struct DerivedMesh *dynamicPaint_Modifier_apply(....) { .... for (; surface; surface = surface->next) { PaintSurfaceData *sData = surface->data; if (surface && surface->format != MOD_DPAINT_SURFACE_F_IMAGESEQ && sData) { .... } 94
95. 11. Push me and then emplace me! 95
96. struct G584_Info { G584_Info(const Ptree *p, bool add, bool mul, bool sub, bool div) { .... } .... const Ptree *m_p; bool m_add; bool m_mul; bool m_sub; bool m_div; }; 96
97. static void G584_CollectExprInfo(const Ptree *p, vector<G584_Info> &infs) { .... auto what = p->What(); if (what == ntParenExpr) { // Remember the expression in parentheses and continue infs.push_back(G584_Info(p, true, true, false, false)); p = SafeSkipParentesis(p); } .... } 97
98. static void G584_CollectExprInfo(const Ptree *p, vector<G584_Info> &infs) { .... auto what = p->What(); if (what == ntParenExpr) { // Remember the expression in parentheses and continue infs.push_back(G584_Info(p, true, true, false, false)); p = SafeSkipParentesis(p); } .... } 98
99. static void G584_CollectExprInfo(const Ptree *p, vector<G584_Info> &infs) { .... auto what = p->What(); if (what == ntParenExpr) { // Remember the expression in parentheses and continue infs.emplace_back(p, true, true, false, false); p = SafeSkipParentesis(p); } .... } 99
100. struct G584_Info { G584_Info(const Ptree *p, bool add, bool mul, bool sub, bool div) { .... } .... const Ptree *m_p; bool m_add; bool m_mul; bool m_sub; bool m_div; }; 100
101. struct G584_Info { G584_Info(const Ptree *p, bool add, bool mul, bool sub, bool div) { .... } .... const Ptree *m_p; bool m_add; bool m_mul; bool m_sub; bool m_div; }; 101
102. static void G584_CollectExprInfo(const Ptree *p, vector<G584_Info> &infs) { .... auto what = p->What(); if (what == ntParenExpr) { // Remember the expression in parentheses and continue infs.emplace_back(p, true, true, false, false); // ok since C++20 p = SafeSkipParentesis(p); } .... } 102
104. 12. I will find you and insert you! 104
105. void AddFunctionDangerousInfo(const vstring &strFunctionInfo, const FunctionDangerousInfo &info) { FunctionDangerousInfoMap &infoMap = GetFunctionDangerousInfoMap(); DangerousInfoIterator it = infoMap.find(strFunctionInfo); if (it == infoMap.end()) { infoMap.insert(make_pair(strFunctionInfo, dangerousInfo)); } else { FunctionDangerousInfo &a = it->second; // some works with 'a' } } 105
106. void AddFunctionDangerousInfo(const vstring &strFunctionInfo, const FunctionDangerousInfo &info) { FunctionDangerousInfoMap &infoMap = GetFunctionDangerousInfoMap(); DangerousInfoIterator it = infoMap.find(strFunctionInfo); if (it == infoMap.end()) { infoMap.insert(make_pair(strFunctionInfo, dangerousInfo)); } else { FunctionDangerousInfo &a = it->second; // some works with 'a' } } 106
107. void AddFunctionDangerousInfo(....) { auto &infoMap = GetFunctionDangerousInfoMap(); if (auto it = infoMap.find(strFunctionInfo); it == infoMap.end()) { infoMap.emplace(strFunctionInfo, dangerousInfo); } else { FunctionDangerousInfo &a = it->second; // some works with 'a' } } 107
108. void AddFunctionDangerousInfo(....) { auto &infoMap = GetFunctionDangerousInfoMap(); if (auto it = infoMap.find(strFunctionInfo); it == infoMap.end()) { infoMap.emplace(strFunctionInfo, dangerousInfo); } else { FunctionDangerousInfo &a = it->second; // some works with 'a' } } 108
109. void AddFunctionDangerousInfo(....) { auto &infoMap = GetFunctionDangerousInfoMap(); if (auto it = infoMap.lower_bound(strFunctionInfo); it != infoMap.end()) { auto &a = it->second; // some works with 'a' } else { infoMap.emplace_hint(it, strFunctionInfo, dangerousInfo); } } 109
110. void AddFunctionDangerousInfo(....) { auto &infoMap = GetFunctionDangerousInfoMap(); if (auto it = infoMap.lower_bound(strFunctionInfo); it != infoMap.end() && it->first == strFunctionInfo) { auto &a = it->second; // some works with 'a' } else { infoMap.emplace_hint(it, strFunctionInfo, dangerousInfo); } } 110
111. 13. Is it alive? 111
112. struct Foo { int i; double d; }; Foo* bar() { Foo *ptr = (Foo *) malloc(sizeof(Foo)); if (ptr == NULL) return NULL; ptr->i = 0; ptr->d = 0.0; return ptr; } 112
113. struct Foo { int i; double d; }; Foo* bar() { Foo *ptr = (Foo *) malloc(sizeof(Foo)); if (ptr == NULL) return NULL; ptr->i = 0; // ok in C, UB in C++ ptr->d = 0.0; // ok in C, UB in C++ return ptr; } 113
114. struct Foo { int i; double d; }; Foo* bar() { Foo *ptr = (Foo *) malloc(sizeof(Foo)); if (ptr == NULL) return NULL; new (ptr) Foo; ptr->i = 0; // ok in C++ ptr->d = 0.0; // ok in C++ return ptr; } 114
115. struct Foo { int i; double d; }; Foo* bar() { Foo *ptr = (Foo *) malloc(sizeof(Foo)); if (ptr == NULL) return NULL; ptr->i = 0; // ok in C, UB in C++ until C++20 ptr->d = 0.0; // ok in C, UB in C++ until C++20 return ptr; } 115
116. 14. Mind the sign 116
117. int foo(const char *s) { int r = 0; while (*s) { r += ((r * 20891 + *s * 200) | *s ^ 4 | *s ^ 3 ) ^ (r >> 1); s++; } return r & 0x7fffffff; } 117
118. int foo(const char *s) { int r = 0; while (*s) { r += ((r * 20891 + *s * 200) | *s ^ 4 | *s ^ 3 ) ^ (r >> 1); s++; } return r & 0x7fffffff; } gcc-8 -O2 -std=c++17 -funsigned-char source.cpp 118
119. int foo(const char *s) { int r = 0; while (*s) { r += ((r * 20891 + *s * 200) | *s ^ 4 | *s ^ 3 ) ^ (r >> 1); s++; } return r & 0x7fffffff; } gcc-8 -O2 -std=c++17 -funsigned-char source.cpp 119
122. 15. No exceptions, except... 122
123. BOOL WINAPI DllMain(HANDLE hModule, ULONG nReason, LPVOID /*pSituation*/) { BOOL br = TRUE; if (nReason == DLL_PROCESS_ATTACH) { // Remember our hModule. g_hModule = hModule; g_hInstance = (HINSTANCE)g_hModule; // Make sure we're properly registered. if (FAILED(DllRegisterServer())) br = FALSE; } else if (nReason == DLL_PROCESS_DETACH) { // Nothing to do. } return br; } 123
124. BOOL WINAPI DllMain(HANDLE hModule, ULONG nReason, LPVOID /*pSituation*/) { BOOL br = TRUE; if (nReason == DLL_PROCESS_ATTACH) { // Remember our hModule. g_hModule = hModule; g_hInstance = (HINSTANCE)g_hModule; // Make sure we're properly registered. if (FAILED(DllRegisterServer())) br = FALSE; } else if (nReason == DLL_PROCESS_DETACH) { // Nothing to do. } return br; } 124
125. HRESULT WINAPI DllRegisterServer(VOID) { .... hr = ::RegOpenKeyEx(HKEY_CURRENT_USER, "Software\\geO\\geOShell\\Plugins", NULL, KEY_WRITE, &hk); if ( SUCCEEDED(hr = HRESULT_FROM_WIN32(hr)) ) { strcpy(szValue, "geOCommandTime.dll,0"); DllGetObjectInfo(0, GI_Name, szName, MAX_PATH); hr = ::RegSetValueEx(hk, szName, 0, REG_SZ, (LPBYTE)szValue, strlen(szValue)); hr = HRESULT_FROM_WIN32(hr); RegCloseKey(hk); } .... return hr; } 125
126. HRESULT WINAPI DllGetObjectInfo(DWORD dwPluginId, DWORD dwInfo, VOID *pBuffer, DWORD dwBuffer) { .... // Get the specified plugin: hr = DllGetObject(dwPluginId, &pPlugin); if (SUCCEEDED(hr)) { // We got it, so get some info: hr = pPlugin->GetInfo(dwInfo, pBuffer, dwBuffer); // And delete the plugin: delete pPlugin; } .... return hr; } 126
127. HRESULT WINAPI DllGetObjectInfo(DWORD dwPluginId, DWORD dwInfo, VOID *pBuffer, DWORD dwBuffer) { .... // Get the specified plugin: hr = DllGetObject(dwPluginId, &pPlugin); if (SUCCEEDED(hr)) { // We got it, so get some info: hr = pPlugin->GetInfo(dwInfo, pBuffer, dwBuffer); // And delete the plugin: delete pPlugin; } .... return hr; } 127
128. HRESULT WINAPI DllGetObject(DWORD dwPluginId, IShellPlugin **ppPlugin) { .... { // We need to create a new command plugin: switch (dwPluginId) { case 0: *ppPlugin = new CCommandPlugin; break; default: *ppPlugin = NULL; }; .... } .... return hr; } 128
129. HRESULT WINAPI DllGetObject(DWORD dwPluginId, IShellPlugin **ppPlugin) { .... { // We need to create a new command plugin: switch (dwPluginId) { case 0: *ppPlugin = new CCommandPlugin; break; default: *ppPlugin = NULL; }; .... } .... return hr; } 129
130. 10 commandments 130
131. ▪ Thou shalt not auto, unless thy faith is strong and pure ▪ Thou shalt not write indexed loops for they are abominations before the Code ▪ Thou shalt wash thy data thoroughly before releasing it ▪ Thou shalt not accept data from strangers for they might be sinful ▪ Thou shalt not copy-paste thy code blocks 131
132. ▪ Thy comparison routines shall be correct or else the Wrath of Code will get thee ▪ Thou shalt check thy nullables for they are sinful ▪ Thou shalt not push that which can be emplaced ▪ Thou shalt not cook signed values with overflow semantics ▪ He who is without noexcept shall throw, and none other 132
133. END Q&A 133 Brought to you by: pvs-studio.com