2021年C++项目中的十大Bug:乍一看都正确的代码,实则暗藏玄机

Posted CSDN资讯

tags:

篇首语:本文由小常识网(cha138.com)小编为大家整理,主要介绍了2021年C++项目中的十大Bug:乍一看都正确的代码,实则暗藏玄机相关的知识,希望对你有一定的参考价值。

作者 | Vladislav Stolyarov 译者 | 弯月
出品 | CSDN(ID:CSDNnews)

程序员的新年祝福中,大家或多或少会来一句,新年编码无Bug、Bug越写越少……对程序员来说,无论何时何地,绕不去的话题里总有一个是关于Bug的。这不,本文作者便总结了2021年C++代码中的TOP 10 bug,或许与你有些许共通之处。

第10名:循环中的输入错误

V533 [CWE-691](https://pvs-studio.com/en/docs/warnings/v533/) for循环内的递增变量错误,请检查i。


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

注意内层循环中递增的是变量i,而不是r。无需多说,这个错误太经典了。

第9名:突然变成了html

V735(https://pvs-studio.com/en/docs/warnings/v735/) HTML错误。"“之前缺少”"标签。


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;
 

我们提供的检查不仅会检查代码,还会检查字符串中的异常。上述代码就触发了这个检查。这种情况很罕见。

作者想要创建一个列表,但添加了两个

  • 标签,导致了这个错误。这显然是输入错误。第一个标签开始列表,而第二个标签应该结束列表。下面是正确的代码:


str << "</ul></body></html>";

第8名:不安全的宏

V634(https://pvs-studio.com/en/docs/warnings/v634/)中运算符 ‘*‘的优先级高于’<<’。建议在此表达式中使用括号。


#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);
 

分析器警告说,在展开两个宏之后,可能会产生不正确的表达式。函数首先将一个函数乘以1,然后执行移位操作。幸运的是在这一行中 x * 1 << y 表达式与 x * (1 << y) 相等。如果左侧或右侧的宏中有 /、%、+、- 或其他优先级高于 << 的运算符,或者宏包含优先级低于 << 的运算符,表达式就无法正确求值了。宏本身和宏的参数必须放在括号中,来避免可能出现的问题。下面是正确的写法:


Define PM_EXP2(A) (1 << (A))

第7名:被除数与除数相混淆

V1064(https://pvs-studio.com/en/docs/warnings/v1064/)整数除法的操作数’gPEClockFrequencyInfo.bus_clock_rate_hz’小于’gPEClockFrequencyInfo.dec_clock_rate_hz’。结果始终为0。


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;
 

所有字段都是整型:

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

在中间的赋值中,被除数 gPEClockFrequencyInfo.bus_clock_rate_hz 赋值为 100000000,而除数 gPEClockFrequencyInfo.dec_clock_rate_hz 赋值为 1000000000。本例中,除数是被除数的十倍。因为所有值都是整型,得到的结果 gPEClockFrequencyInfo.bus_to_dec_rate_den 就是0。

从名字来推测,除数和被除数写反了。

第6名:选择类型时的错误

V610(https://pvs-studio.com/en/docs/warnings/v610/)未定义的行为。请检查移位操作符’>>=’。右边的操作数 (‘bitpos % 64’ = [0…63])大于或等于左边的操作数的位长。


// 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;
 

注意BITS_PER_LONG宏可以是64位的。

这段代码包含以下未定义的行为:

  • 在检查后,addr变量的取值范围为[0…127]

  • 如果参数 addr >= 16,那么status变量的右移位数将超过int类型包含的比特数

也许作者想要减少代码行数,所以bitpos变量的定义紧跟status。但是,作者并没有考虑到int与long不同,它在64位平台上只有32位。

要改正这个错误,应该将status变量定义成long。

第5名:模块间的分析和丢失的memset

今年,我们添加了一个重要功能:对C++项目进行模块间分析。这个功能找到了codelite项目中的以下警告。

V597(https://pvs-studio.com/en/docs/warnings/v597/)编译器可能会删掉’memset’函数调用,该函数调用会给’current’赋值。擦除私有数据应该使用memset_s()。)


// args.c
extern void eFree (void *const ptr);
 
extern void argDelete (Arguments* const current)

  Assert (current != NULL);
  if (current->type ==  ARG_STRING  &&  current->item != NULL)
    eFree (current->item);
  memset (current, 0, sizeof (Arguments));  // <=
  eFree (current);                          // <=

 
// routines.c
extern void eFree (void *const ptr)

  Assert (ptr != NULL);
  free (ptr);

LTO(连接时间分析)使用了memset调用。编译器通过 as-if 规则发现,eFree并没有计算出任何有用的指针相关的数据。eFree只是调用了free函数来释放内存。

没有LTO,eFree调用看上去像一个未知的外部函数,所以memset会保留。

第4名:不合理的检查和Unreal引擎

最近,我们加强了对于Unreal引擎的检查。我们来看看分析器发现了什么问题。

V547(https://pvs-studio.com/en/docs/warnings/v547/)表达式’m_trail == 0’始终未false。


std::size_t m_trail;
....
inline int context::execute(const char* data, std::size_t len,
 std::size_t& off)

  ....
  case MSGPACK_CS_EXT_8: 
                uint8_t tmp;
                load<uint8_t>(tmp, n);
                m_trail = tmp + 1;
                if(m_trail == 0) 
                    unpack_ext(m_user, n, m_trail, obj);
                    int ret = push_proc(obj, off);
                    if (ret != 0) return ret;
                
                else 
                    m_cs = MSGPACK_ACS_EXT_VALUE;
                    fixed_trail_again = true;
                
             break;
  ....

我们来看看这段代码的问题。

tmp变量是uint8_t类型,它的值为8比特,即[0, 255]。作者认为tmp可以为255。在m_trail = tmp +1赋值之后,作者检查有没有发生整型溢出,因为无符号整数运算可能会导致值回到起点。因此,tmp + 1操作可能为0。

但是,分析器指出 m_trail == 0永远为false。我们来看看为什么。

首先,回忆一下std::common_type。

该行代码包含加法运算。对于值和其他类型之间的二元操作,编译器使用通常的算数转换,对tmp变量实行整数提升。因此,该表达式中的扩展为1的类型,即int。这样,即使tmp的值是255,加法运算也会产生256。int类型能够存储该结果。因此m_trail==0是无意义的。

第3名:日期处理函数中的解释错误

在本例中,我们的分析器给出了多条警告:

  • V547 [CWE-571] 表达式’month’始终为true。

  • V560 [CWE-570](https://pvs-studio.com/en/docs/warnings/v560/) 部分条件表达式始终为false。

  • V547表达式’month’始终为true。

  • V560部分条件表达式始终为false。

首先来看看这段代码,它接收一个月份的缩写,返回整数值。


static const char qt_shortMonthNames[][4] = 
    "Jan", "Feb", "Mar", "Apr", "May", "Jun",
    "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"
;
 
static int fromShortMonthName(QStringView monthName)

  for (unsigned int i = 0;
       i < sizeof(qt_shortMonthNames) / sizeof(qt_shortMonthNames[0]); ++i)
  
    if (monthName == QLatin1String(qt_shortMonthNames[i], 3))
      return i + 1;
  
  return -1;
 

如果操作成功,函数会返回月份的整数(1到12的整数值)。如果月份名称不正确,则返回-1。注意该函数不可能返回0。

但是,开发人员在调用该函数时,认为它在错误情况下会返回0。下面是错误使用该函数的代码:


QDateTime QDateTime::fromString(QStringView string, Qt::DateFormat format)

  ....
  month = fromShortMonthName(parts.at(1));
  if (month)
    day = parts.at(2).toInt(&ok);
 
  // If failed, try day then month
  if (!ok || !month || !day) 
    month = fromShortMonthName(parts.at(2));
    if (month) 
      QStringView dayPart = parts.at(1);
      if (dayPart.endsWith(u'.'))
        day = dayPart.chopped(1).toInt(&ok);
    
  
  ....
 

这段程序永远不可能运行月份值为0的情况,只会使用错误的月份值继续运行。

第2名:不注意导致的错误

V726(https://pvs-studio.com/en/docs/warnings/v726/)试图使用’free’函数释放包含’wbuf’数组的内存,但由于’wbuf’建立在堆栈中,因此是不正确的。


template<typename T>
static ALWAYS_INLINE void FormatLogMessageAndPrintW(....)

  ....
  wchar_t wbuf[512];
  wchar_t* wmessage_buf = wbuf;
  ....
  if (wmessage_buf != wbuf)
  
    std::free(wbuf);
  
  if (message_buf != buf)
  
    std::free(message_buf);
  
  ....
 

这段代码触发了警告。它试图删除一个栈上分配的数组,从而引发了错误。数组的内存不是从堆上分配的, 所以没有必要调用std::free。当对象摧毁时,内存也会释放。

我认为,这个警告的原因比警告本身更危险。

第1名:不注意导致的更大的错误

我们一直在警告用户的错误。但今年,我们自己也犯了错误。

V645 'strncat’函数调用会导致’a.consoleText’缓存溢出。边界不应该包含缓存的大小,而应该包含字符数。


struct A

  char consoleText[512];
;
 
void foo(A a)

  char inputBuffer[1024];
  ....
  strncat(a.consoleText, inputBuffer, sizeof(a.consoleText)strlen(a.consoleText) - 5);
  ....
 

乍一看这段代码似乎是正确的,没有未定义错误。但仔细看一下这段代码:

sizeof(a.consoleText)strlen(a.consoleText)5

这个表达式可能会产生负数!例如,当strlen(a.consoleText) = 508的时候,就会产生无符号整数溢出。表达式的结果就是结果类型size_t的最大值。

  • 参考链接:https://pvs-studio.com/en/blog/posts/cpp/0901/

以上是关于2021年C++项目中的十大Bug:乍一看都正确的代码,实则暗藏玄机的主要内容,如果未能解决你的问题,请参考以下文章

2021年游戏项目的十大编程语言:C++JavaC#均上榜

2021 年 GitHub 上十大最火 Python 项目,看完之后我裂开了

C++开源项目:十行代码15个BUG,你入坑了吗?

2021年企业机器学习十大趋势:83%企业增加预算和人手,部署模型所需时间增长64%

SQL Server @@ERROR的小误区大Bug

将主题中的 CSS 和 JS 文件合并到 Angular8 项目中的完美正确方法是啥