{"id":388523,"date":"2024-06-29T07:59:40","date_gmt":"2024-06-29T07:59:40","guid":{"rendered":"http:\/\/savepearlharbor.com\/?p=388523"},"modified":"-0001-11-30T00:00:00","modified_gmt":"2024-06-29T07:59:40","slug":"","status":"publish","type":"post","link":"https:\/\/savepearlharbor.com\/?p=388523","title":{"rendered":"Top 10 bugs found in C++ projects in 2021<\/span>"},"content":{"rendered":"
<\/div>\n
\n
\n
\n
\n

It’s freezing outside, everyone has already decorated the Christmas tree and bought tangerines. New Year is coming! So, it’s time to meet the Top 10 interesting bugs found by the PVS-Studio C++ analyzer in 2021.<\/p>\n

<\/figcaption><\/figure>\n

10th: pretty typo in a loop<\/h3>\n

V533<\/a> [CWE-691] It is likely that a wrong variable is being incremented inside the ‘for’ operator. Consider reviewing ‘i’. gskvulkanimage.c 721<\/p>\n

void gsk_vulkan_image_upload_regions (GskVulkanImage    *self,                                  GskVulkanUploader *uploader,                                  guint              num_regions,                                  GskImageRegion    *regions) {   ....   for (int i = 0; i < num_regions; i++)   {     m = mem + offset;     if (regions[i].stride == regions[i].width * 4)     {       memcpy (m, regions[i].data, regions[i].stride * regions[i].height);     }     else     {       for (gsize r = 0; r < regions[i].height; i++)          \/\/ <=         memcpy (m + r * regions[i].width * 4,                 regions[i].data + r * regions[i].stride, regions[i].width * 4);     }     ....   }   .... } <\/code><\/pre>\n
Note that in a nested loop the i<\/em> variable is incremented instead of the r<\/em> variable. No need to comment. It’s a golden classic!<\/p>\n
My colleague described this error in the «Finding typos in the GTK 4 project by PVS-Studio<\/a>» article.<\/p>\n
9th: suddenly, let’s talk about… HTML!<\/h3>\n
V735<\/a> Possibly an incorrect HTML. The «<\/body>» closing tag was encountered, while the «<\/ul>» tag was expected. qpixeltool.cpp 707<\/p>\n
QString QPixelTool::aboutText() const {   const QList<QScreen *> screens = QGuiApplication::screens();   const QScreen *windowScreen = windowHandle()->screen();    QString result;   QTextStream str(&result);   str << \"<html><\/head><body><h2>Qt Pixeltool<\/h2><p>Qt \" << QT_VERSION_STR     << \"<\/p><p>Copyright (C) 2017 The Qt Company Ltd.<\/p><h3>Screens<\/h3><ul>\";   for (const QScreen *screen : screens)     str << \"<li>\" << (screen == windowScreen ? \"* \" : \"  \")         << screen << \"<\/li>\";   str << \"<ul><\/body><\/html>\";   return result; } <\/code><\/pre>\n
PVS-Studio provides diagnostics that don’t just check code \u2013 they also look for abnormalities in string literals. The code above triggered one of these diagnostics. Such cases are quite rare. That’s why, this one is so intriguing.<\/p>\n
Someone intended to create one list but added two tags that open this list instead of one. This is clearly a typo. The first tag must open the list, and the second one must close it. Here is the correct code:<\/p>\n
str << \"<\/ul><\/body><\/html>\";<\/code><\/pre>\n
This error was described in the article: «Date processing attracts bugs or 77 defects in Qt 6<\/a>«.<\/p>\n
8th: an unsafe macro<\/h3>\n
V634<\/a> The priority of the ‘*’ operation is higher than that of the ‘<<‘ operation. It’s possible that parentheses should be used in the expression. bug34427.c 160<\/p>\n
#define PM_EXP2(A) 1 << A  int process_val(const u_int8_t *data, u_int32_t data_len,                                u_int32_t *retvalue, ....)  {   *retvalue = 0;   ....   \/* Now find the actual value *\/   for (; i < data_len; i++) {     *retvalue += data[i] * PM_EXP2(8 * (data_len - i - 1));   }   return(0); } <\/code><\/pre>\n
The analyzer warns that after the macro expands, it may produce an incorrect expression. The function will first multiply the variable by one, and then conduct the bitwise shift to the expression in parentheses. It was a lucky coincidence that in this line the x * 1 << y<\/em> expression is equal to x * (1  << y)<\/em>.  If to its left or right the macro has \/<\/em>, %<\/em>, +<\/em>, —<\/em>, or other operations with a priority greater than <<,<\/em> or if the macro contains an operation that has a lesser priority than <<<\/em>, the expression will not be evaluated correctly. Always wrap the macro and its arguments in parentheses to avoid problems in the future. The following is correct:<\/p>\n
Define PM_EXP2(A) (1 << (A))<\/code><\/pre>\n
You can find this error in the «PVS-Studio analyzer scans Snort, network traffic scanner<\/a>» article.<\/p>\n
7th: mixed-up divisor and dividend<\/h3>\n
V1064<\/a> The ‘gPEClockFrequencyInfo.bus_clock_rate_hz’ operand of integer division is less than the ‘gPEClockFrequencyInfo.dec_clock_rate_hz’ one. The result will always be zero. pe_identify_machine.c 72<\/p>\n
void pe_identify_machine(__unused boot_args *args) {   ....   \/\/ Start with default values.   gPEClockFrequencyInfo.timebase_frequency_hz = 1000000000;   gPEClockFrequencyInfo.bus_frequency_hz      =  100000000;   ....   gPEClockFrequencyInfo.dec_clock_rate_hz =      gPEClockFrequencyInfo.timebase_frequency_hz;   gPEClockFrequencyInfo.bus_clock_rate_hz =    gPEClockFrequencyInfo.bus_frequency_hz;   ....    gPEClockFrequencyInfo.bus_to_dec_rate_den =     gPEClockFrequencyInfo.bus_clock_rate_hz \/     gPEClockFrequencyInfo.dec_clock_rate_hz; } <\/code><\/pre>\n
All fields used here have integer types:<\/p>\n
extern clock_frequency_info_t gPEClockFrequencyInfo;  struct clock_frequency_info_t {   unsigned long bus_clock_rate_hz;   unsigned long dec_clock_rate_hz;   unsigned long bus_to_dec_rate_den;   unsigned long long bus_frequency_hz;   unsigned long timebase_frequency_hz;   .... }; <\/code><\/pre>\n
Through intermediate assignments, the dividend data member gPEClockFrequencyInfo.bus_clock_rate_hz,<\/em> is assigned the 100000000 value, and the divisor data member gPEClockFrequencyInfo.dec_clock_rate_hz<\/em> is assigned the 1000000000 value. In this case, the divisor is ten times greater than the dividend. Since all the data members here are integers, the gPEClockFrequencyInfo.bus_to_dec_rate_den<\/em> data member is 0.<\/p>\n
Judging by the name of the resulting bus_to_dec_rate_den<\/em> data member, the divisor and the dividend are mixed up.<\/p>\n
<\/figcaption><\/figure>\n
My colleague described this error in the following article: «MacOS Kernel, how good is this apple?<\/a>«.<\/p>\n
6th: error in selecting types<\/h3>\n
V610<\/a> Undefined behavior. Check the shift operator ‘>>=’. The right operand (‘bitpos % 64’ = [0..63]) is greater than or equal to the length in bits of the promoted left operand. master.c 354<\/p>\n
\/\/ bitsperlong.h #ifdef CONFIG_64BIT #define BITS_PER_LONG 64 #else #define BITS_PER_LONG 32 #endif \/* CONFIG_64BIT *\/  \/\/ bits.h \/*  * Create a contiguous bitmask starting at bit position @l and ending at  * position @h. For example  * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.  *\/ #define __GENMASK(h, l) ....  \/\/ master.h #define I2C_MAX_ADDR      GENMASK(6, 0)  \/\/ master.c static enum i3c_addr_slot_status i3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr) {   int status, bitpos = addr * 2;                   \/\/ <=    if (addr > I2C_MAX_ADDR)     return I3C_ADDR_SLOT_RSVD;    status = bus->addrslots[bitpos \/ BITS_PER_LONG];   status >>= bitpos % BITS_PER_LONG;               \/\/ <=    return status & I3C_ADDR_SLOT_STATUS_MASK; } <\/code><\/pre>\n
Note that the BITS_PER_LONG<\/em> macro can be 64-bit.<\/p>\n
The code contains undefined behavior:<\/p>\n