Embarassing bugs in open source C/C++ projects

Viva64 develops PVS-Studio, which is a static code analyzer for C/C++/C++11. But their popularity has come from the fact that they run this software on quite a few major open source projects around the internet and post the results. They hit the mark of 1000 error samples yesterday.

Their bug database is a treasure trove. For one they expose bugs in open source projects that may not be easily seen by the naked eye. And it’s a great learning resource to see common pitfalls.

This bug from the Chromium project is obviously a hapless victim of copy-pasting:

bool AutoFillProfileHasName(const AutoFillProfile& profile) {
  return
   !profile.GetFieldText(AutofillType(NAME_FIRST)).empty() ||
   !profile.GetFieldText(AutofillType(NAME_MIDDLE)).empty() ||
   !profile.GetFieldText(AutofillType(NAME_MIDDLE)).empty();
}

Most probably the programmer intended to put NAME_LAST at the end.

Dereferencing a null pointer in Clang. Ouch.

void MatcherGen::EmitLeafMatchCode(const TreePatternNode *N) {
  ...
  if (DI == 0) {
    errs() << "Unknown leaf kind: " << *DI << "\n";
    abort();
  }
  ...
}

And in this snippet from WinMerge, it might not be clear what the programmer has intended at first.

void CDirView::GetItemFileNames(int sel,
  String& strLeft, String& strRight) const
{
  UINT_PTR diffpos = GetItemKey(sel);
  if (diffpos == (UINT_PTR)SPECIAL_ITEM_POS)
  {
    strLeft.empty();
    strRight.empty();
  }
  else
  {
     ...
  }
}

It turns out that the intention was to clear the string. Actually it should’ve been strLeft.clear(); and strRight.clear();. The static analyzer has caught the issue because the return value of empty() is required to be utilized, which is not done here.

2 responses on “Embarassing bugs in open source C/C++ projects

  1. John David Stone

    “Actually it should have been strLeft.clear; and strRight.clear();.” Actually strLeft.clear should have been called rather than just mentioned.

Leave a Reply