Мы используем куки, чтобы пользоваться сайтом
было удобно.
Как-то так странно получилось, что у нас уже почти год как существует и поддерживается плагин PVS-Studio для Qt Creator. И при этом мы до сих пор не выпустили хорошей статьи с проверкой самой IDE. Исправляем сие недоразумение и предлагаем вам посмотреть, чем живёт недавно переродившаяся среда для разработки.
Хоть Qt Creator и довольно известная IDE, но не настолько, чтобы пропустить пару слов о ней. Qt Creator — это кроссплатформенная среда разработки для создания приложений на языке C++ с использованием фреймворка Qt. Она предоставляет удобный интерфейс для написания, отладки и развёртывания приложений, а также инструменты для работы с редактором кода, системой контроля версий и прочими функциями. Qt Creator поддерживает различные операционные системы, включая Windows, macOS и Linux. Особый интерес для проверки представляет тот факт, что не так давно Qt приобрела компанию-разработчика статического анализатора и уже успела встроить его поддержку в свою IDE. Будет интересно оценить качество внутреннего анализа. :)
Почему я указал на некое "перерождение" в аннотации к статье? Всё просто — не так давно, начиная с версии 5, разработчики явно решили кардинально перелопатить IDE для соответствия современным стандартам. Мне, как разработчику, эти изменения нравятся: во многих сценариях Qt Creator теперь показывает себя не хуже, а местами даже лучше Visual Studio и VS Code (по моим субъективным ощущениям. Желающие обсудить приглашаются в комментарии). Есть, конечно, и за что поругать разработчиков (со стороны поддержки API для сторонних разработчиков), но, пожалуй, не в этой статье.
Я же уже упоминал, что существует расширение PVS-Studio для Qt Creator, да? Именно благодаря ему в заголовке Qt Creator со звёздочкой, да. Итак, сегодня мы проверим Qt Creator из Qt Creator с помощью плагина PVS-Studio для Qt Creator. Что же нам для этого понадобится? Рецепт довольно прост:
Если следовать рецепту, то в итоге у вас должен получиться следующий результат:
Начать предлагаю с наиболее интересного с точки зрения анализа пункта, а именно утечек памяти. Для тех, кто не в теме, кратко поясню суть проблемы. В Qt реализована нетривиальная система владения объектами, и, чтобы понять, что память не утекла, нужно учитывать множество способов как создания объектов, так и передачи владения (привет, setParent и layout'ы). Не буду лукавить, ложных срабатываний выдалось достаточно. Но среди предупреждений нашлось и несколько интересных экземпляров. Предлагаю посмотреть на них, пока мы занимаемся доработками для уменьшения ложных срабатываний. :)
Начнём с небольшой утечки памяти в функции получения пути до отчётов о падении IDE.
Файл main.cpp:1813
QString crashReportsPath()
{
std::unique_ptr<Utils::QtcSettings> settings(createUserSettings());
QFileInfo(settings->fileName()).path() + "/crashpad_reports";
if (Utils::HostOsInfo::isMacHost())
return QFileInfo(createUserSettings()->fileName()).path() +
"/crashpad_reports";
else
return QCoreApplication::applicationDirPath() + '/' +
RELATIVE_LIBEXEC_PATH + "crashpad_reports";
}Если вас тоже смутило повторное использование функции createUserSettings, то у вас явно намётан глаз на поиск ошибок. Вы на верном пути. В первый раз указатель, полученный из этой функции, сохраняется в std::unique_ptr и корректно удаляется после выхода из области видимости. А вот во второй раз — в ветке для macOS — просто забывается после получения пути до файла, о чём и сообщает анализатор:
V773 [CWE-401, CERT-MEM31-C, CERT-MEM51-CPP] The return value of the 'createUserSettings' function is not saved. A memory leak is possible. main.cpp 422
"А точно ли есть беда?" — спросите вы. Точно. Реализация функции createUserSettings выглядит вот так:
Файл main.cpp:274
static Utils::QtcSettings *createUserSettings()
{
return new Utils::QtcSettings(
QSettings::IniFormat,
QSettings::UserScope,
QLatin1String(Core::Constants::IDE_SETTINGSVARIANT_STR),
QLatin1String(Core::Constants::IDE_CASED_ID));
}Также нашлось два случая, когда объектам не назначали parent'ов — объектов, отвечающих за их удаление, — а дальше просто забыли об их удалении. Первый случай достаточно тривиален —создали объект и конкретно здесь забыли утилизировать, хотя в других местах всё нормально:
void DebuggerPlugin::attachToProcess(....)
{
....
auto kitChooser = new KitChooser;
kitChooser->setShowIcons(true);
kitChooser->populate();
Kit *kit = kitChooser->currentKit();
dd->attachToRunningProcess(kit, processInfo, false);
}Во втором случае в конструкторе класса создаётся модель без указания родителя. Хотелось бы, чтобы модель удалялась в деструкторе, но, увы, его реализация пуста. Возможно, здесь хотели применить какой-нибудь умный указатель, но забыли...
CurveEditorView::CurveEditorView(
ExternalDependenciesInterface &externalDepoendencies
)
: AbstractView{externalDepoendencies}
, m_block(false)
, m_model(new CurveEditorModel())
, m_editor(new CurveEditor(m_model))
{
....
}
CurveEditorView::~CurveEditorView() {}Соответствующие предупреждения:
"Дальше предлагаю посмотреть на классику жанра", — сказал бы я и предложил посмотреть на разыменование непроверенных указателей, если бы не одно НО. Проект у нас активно использует С++17 и std::optional вместе с ним, что похвально. Жалко только, ошибки на то и называются классическими, что не зависят от используемых инструментов. Попробуете найти сами? ;-)
Файл aspects.cpp:2189
void IntegerAspect::addToLayout(Layouting::LayoutItem &parent)
{
QTC_CHECK(!d->m_spinBox);
d->m_spinBox = createSubWidget<QSpinBox>();
d->m_spinBox->setDisplayIntegerBase(d->m_displayIntegerBase);
d->m_spinBox->setPrefix(d->m_prefix);
d->m_spinBox->setSuffix(d->m_suffix);
d->m_spinBox->setSingleStep(d->m_singleStep);
d->m_spinBox->setSpecialValueText(d->m_specialValueText);
if (d->m_maximumValue && d->m_maximumValue)
d->m_spinBox->setRange(
int(d->m_minimumValue.value() / d->m_displayScaleFactor),
int(d->m_maximumValue.value() / d->m_displayScaleFactor)
);
// Must happen after setRange()
d->m_spinBox->setValue(int(value() / d->m_displayScaleFactor));
addLabeledItem(parent, d->m_spinBox);
connect(d->m_spinBox.data(), &QSpinBox::valueChanged,
this, &IntegerAspect::handleGuiChanged);
}
Если вы нашли повторное использование m_maximumValue вместо m_minimumValue, то могу вас поздравить. Не так-то просто, да? Думаю, разработчики Qt Creator тоже согласятся, ведь тот же самый код был найден чуть дальше в том же файле:
Файл aspects.cpp:2291
void DoubleAspect::addToLayout(LayoutItem &builder)
{
QTC_CHECK(!d->m_spinBox);
d->m_spinBox = createSubWidget<QDoubleSpinBox>();
d->m_spinBox->setPrefix(d->m_prefix);
d->m_spinBox->setSuffix(d->m_suffix);
d->m_spinBox->setSingleStep(d->m_singleStep);
d->m_spinBox->setSpecialValueText(d->m_specialValueText);
if (d->m_maximumValue && d->m_maximumValue)
d->m_spinBox->setRange(d->m_minimumValue.value(),
d->m_maximumValue.value());
bufferToGui(); // Must happen after setRange()!
addLabeledItem(builder, d->m_spinBox);
connect(d->m_spinBox.data(), &QDoubleSpinBox::valueChanged,
this, &DoubleAspect::handleGuiChanged);
}К слову, а часто ли вы обращаете внимание на то, что именно вам предлагает использовать IntelliSense (или его аналог)? Что-то мне подсказывает, что именно он посодействовал появлению этой ошибки. Раньше частенько ловил себя на том, что написал часть названия переменной, по запарке тыкнул Enter и пошёл дальше — мысль-то не остановить. Пишите в комментарии, посмотрим сколько нас. :)
Ах да, предупреждения:
Предлагаю ещё немного размять глазки и поискать ошибку в функции фильтрации флагов компиляции. Ну и куда же без подсказок: "яиненварс иицкнуф в ытакилбуд иртомС". Да, не всё так просто.
Файл gcctoolchain.cpp:474
// For querying operations such as -dM
static QStringList filteredFlags(const QStringList &allFlags,
bool considerSysroot)
{
QStringList filtered;
for (int i = 0; i < allFlags.size(); ++i)
{
const QString &a = allFlags.at(i);
if (a.startsWith("--gcc-toolchain="))
{
filtered << a;
}
else if (a == "-arch")
{
if (++i < allFlags.length() && !filtered.contains(a))
filtered << a << allFlags.at(i);
}
else if (a == "-Xclang")
{
filtered << a;
}
else if ( (considerSysroot && (a == "--sysroot" || a == "-isysroot"))
|| a == "-D" || a == "-U"
|| a == "-gcc-toolchain"
|| a == "-target"
|| a == "-mllvm"
|| a == "-isystem")
{
if (++i < allFlags.length())
filtered << a << allFlags.at(i);
}
else if ( a.startsWith("-m") || a.startsWith("-f") || a.startsWith("-O")
|| a.startsWith("-std=") || a.startsWith("-stdlib=")
|| a.startsWith("-specs=") || a == "-ansi" || a == "-undef"
|| a.startsWith("-D") || a.startsWith("-U")
|| a.startsWith("-stdlib=") || a.startsWith("-B")
|| a.startsWith("--target=")
|| (a.startsWith("-isystem") && a.length() > 8)
|| a == "-Wno-deprecated"
|| a == "-nostdinc" || a == "-nostdinc++")
{
filtered << a;
}
}
return filtered;
}Если у вас не получилось с наскока найти лишнюю проверку подстроки "-stdlib", то не огорчайтесь — как раз для нас и создаются статические анализаторы. Остальных могу только поздравить с прокачанной внимательностью.
Увы, подарки лежат только в конце статьи, поэтому я попросил нейросеть сгенерировать медаль для настоящего сыщика-программиста:
"Программисткости" не хватает, согласен, но в целом получилось неплохо. А, и чуть не забыл про предупреждение анализатора:
V501 [CWE-570] There are identical sub-expressions 'a.startsWith("-stdlib=")' to the left and to the right of the '||' operator. gcctoolchain.cpp 491.
Честно, очень хотелось вставить сюда полный код этой функции — почти 400 строчек, — чтобы было интереснее искать ошибку. Но поберегу ваше колёсико мыши и оставлю только самую интересную часть.
Файл symbolgroupvalue.cpp:1196
static KnownType knownClassTypeHelper(const std::string &type,
std::string::size_type pos,
std::string::size_type endPos)
{
....
// Remaining non-template types
switch (endPos - qPos)
{
....
case 30:
if (!type.compare(qPos, 30, "QPatternist::SequenceType::Ptr"))
return KT_QPatternist_SequenceType_Ptr;
if (!type.compare(qPos, 30, "QXmlStreamNamespaceDeclaration"))
return KT_QXmlStreamNamespaceDeclaration;
break;
case 32:
break; // <=
if (!type.compare(qPos, 32, "QPatternist::Item::Iterator::Ptr"))
return KT_QPatternist_Item_Iterator_Ptr;
case 34:
break; // <=
if (!type.compare(qPos, 34, "QPatternist::ItemSequenceCacheCell"))
return KT_QPatternist_ItemSequenceCacheCell;
case 37:
break; // <=
if (!type.compare(qPos, 37, "QNetworkHeadersPrivate::RawHeaderPair"))
return KT_QNetworkHeadersPrivate_RawHeaderPair;
if (!type.compare(qPos, 37, "QPatternist::AccelTree::BasicNodeData"))
return KT_QPatternist_AccelTree_BasicNodeData;
break;
}
return KT_Unknown;
}Прошу обратить внимание на ветки 32, 34 и 37 показанной выше инструкции switch. Честно, я не придумал какого-либо обоснования для использования break; прямо перед исполняемым кодом.
Были мысли насчёт того, что это просто временные заглушки, но нет, они появились одновременно с окружающим кодом, да и прожили там, судя по истории, уже около 14 лет. Если кто-нибудь из читателей может подсказать что именно здесь происходит, то добро пожаловать в комментарии.
Анализатор предлагает обратить внимание на этот фрагмент кода при помощи предупреждения:
V779 [CWE-561, CERT-MSC12-C] Unreachable code detected. It is possible that an error is present. symbolgroupvalue.cpp 1565
Одна из самых заковыристых вещей для новичков в Qt — написание корректных моделей данных. И неспроста — вариантов налажать тут хоть отбавляй. Настолько, что даже разработчики Qt Creator смогли попасться. :)
Файл cppquickfixes:4676
QVariant data(int column, int role) const override
{
if (role == Qt::DisplayRole && column == NameColumn)
return m_memberInfo->data.memberVariableName;
if ( role == Qt::CheckStateRole && column > 0
&& column <= static_cast<int>(std::size(ColumnFlag)))
{
return m_memberInfo->requestedFlags & ColumnFlag[column] ? Qt::Checked :
Qt::Unchecked;
}
return {};
}С виду код вполне корректный... Но что, если заглянуть в определение перечисления ColumnFlag (добавил комментарии, чтобы вам не пришлось считать самим)?
Файл cppquickfixes:4661
using Flag = GenerateGetterSetterOp::GenerateFlag;
constexpr static Flag ColumnFlag[] = {
Flag::Invalid, // 0
Flag::GenerateGetter, // 1
Flag::GenerateSetter, // 2
Flag::GenerateSignal, // 3
Flag::GenerateReset, // 4
Flag::GenerateProperty, // 5
Flag::GenerateConstantProperty, // 6
};Думаю, с такой подсказкой уже не сложно понять, в чём проблема: можно получить выход за границы массива ColumnFlag, если в функцию будет передан column равный 7. А всё из-за притаившегося <= в проверке максимального значения. Корректный код выглядит так:
if ( role == Qt::CheckStateRole
&& column > 0
&& column < static_cast<int>(std::size(ColumnFlag)))
{
....
}Я же говорил, что вариантов налажать немало? В той же самой модели, но теперь ещё и несколько раз скопирована проверка следующего вида:
Файл cppquickfixes:4693
bool setData(int column, const QVariant &data, int role) override
{
if (column < 1 || column > static_cast<int>(std::size(ColumnFlag)))
return false;
if (role != Qt::CheckStateRole)
return false;
if (!(m_memberInfo->possibleFlags & ColumnFlag[column]))
return false;
....
}Ошибка та же самая, только теперь в другую сторону — с оператором >. Всё это добро было найдено благодаря следующим предупреждениям:
Если вы всё ещё думаете, что это единичные проблемы, то в качестве последнего примера оставлю этот небольшой сниппет, и будем двигаться дальше.
Файл defaultannotations.cpp:27
QVariant DefaultAnnotationsModel::data(const QModelIndex &index, int role) const
{
const auto row = static_cast<size_t>(index.row());
if (!index.isValid() || m_defaults.size() < row)
return {};
auto &item = m_defaults[row];
switch (role)
{
....
}
return {};
}V557. [CWE-119, CERT-ARR30-C] Array overrun is possible. The 'row' index is pointing beyond array bound. defaultannotations.cpp 33
Почти у каждого из нас есть любимый вид ошибок. В моём случае это так называемое "выбор без выбора" (я же не один его так называю, да?). Суть в том, что ты можешь написать сколь угодно сложное условие и при этом иметь всегда один и тот же результат из-за дублирования кода в различных ветках. Qt Creator порадовал парочкой таких проблем.
QString NodeMetaInfo::componentFileName() const
{
if constexpr (!useProjectStorage())
{
if (isValid())
{
return m_privateData->componentFileName();
}
}
else
{
if (isValid())
{
return m_privateData->componentFileName();
}
}
return {};
}Честно говоря, не знаю, в чём был изначальный замысел кода, однако мне нравится, что ненужная ветка будет отброшена компилятором при сборке проекта. Вот так, ошибки вроде и нет, но предупреждение всё равно посмотреть стоит:
V523 [CWE-691] The 'then' statement is equivalent to the 'else' statement. nodemetainfo.cpp 1840
Больше всего в таких случаях меня напрягает, что нельзя точно сказать, ошибка это или нет. Даже соседние комментарии обычно не помогают. Как, например, здесь:
Файл debuggerkitaspect.cpp:358
// We have an executable path.
FilePath fileName = FilePath::fromUserInput(binary);
if (item.command() == fileName)
{
// And it's is the path of this item.
level = std::min(item.matchTarget(tcAbi), DebuggerItem::MatchesSomewhat);
}
else
{
// This item does not match by filename, and is an unlikely candidate.
// However, consider using it as fallback if the tool chain fits.
level = std::min(item.matchTarget(tcAbi), DebuggerItem::MatchesSomewhat);
}V523 [CWE-691] The 'then' statement is equivalent to the 'else' statement. debuggerkitaspect.cpp 362
В этом случае ошибки совсем незаметные, а потому могут жить в коде очень и очень долго. Чаще всего причиной их появления является банальная копи-паста или "местный" рефакторинг.
К сожалению, защититься от таких ошибок нелегко. Они могут без проблем проскочить на код-ревью даже у опытных программистов. Не верите? А зря. Мой коллега как раз недавно писал заметку о мелкой ошибке, которую в упор не видели сразу несколько программистов. Лучше уж доверить поиск таких проблем анализатору — у него хоть взгляд не замыливается. :)
Кстати, довольно любопытно было бы узнать, а есть ли у вас любимые или, наоборот, ненавидимые паттерны ошибок? Сущий пустяк или баг, на отлов которого потратили целые часы? Делитесь ими в комментариях, будет интересно почитать и обсудить. :)
Не суперкрутая ошибка, но мне понравился контекст, в котором она обнаружена. Смотрите сами: в коде проверки порядка (чего-то) допущена ошибка в порядке (операций). Даже готовое сообщение об ошибке имеется. Всю работу за меня сделали. :)
if (int firstError = checkOrder() != -1)
{
setInvalid(tr("Invalid order at %1").arg(firstError), firstError);
return;
}Ну ладно, не всю. Нужно же ещё рассказать, что здесь не так — с виду-то ничего плохого.
Беда поджидает нас в условии: выражение 'A = B == C' на самом деле вычисляется как 'A = (B == C)'. Конкретно в этом случае вычисляется значение функции checkOrder, которая возвращает позицию, где произошла ошибка. Оно сравнивается с -1, и уже этот результат (0 или 1) записывается в переменную. Нужно ли говорить, что в таком случае в сообщении об ошибке вместо нормальной позиции будет записана бесполезная единица? Смотрим предупреждение:
V593 [CWE-783, CERT-EXP00-C] Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. connectionmodel.cpp 2032
Раз уж у нас тут проект на С++17, то почему бы не воспользоваться "if statement with initializer" и не переписать код следующим образом:
if (int firstError = checkOrder(); firstError != -1)
{
setInvalid(tr("Invalid order at %1").arg(firstError), firstError);
return;
}Теперь уж точно не получится перепутать приоритет операций.
Предлагаю немного передохнуть и посмотреть на совсем простенькую ошибку. Хотя как сказать —ошибка хоть и простая, но живёт в коде проекта уже больше четырёх лет. Суть в том, что потеряли инструкцию return или присвоение переменной boundingRect.
Файл quickitemnodeinstance.cpp:576
QRectF QuickItemNodeInstance::boundingRectWithStepChilds(
QQuickItem *parentItem) const
{
QRectF boundingRect = parentItem->boundingRect();
boundingRect = boundingRect.united(QRectF(QPointF(0, 0), size()));
for (QQuickItem *childItem : parentItem->childItems())
{
....
}
if (boundingRect.isEmpty())
QRectF{0, 0, 640, 480}; // <=
return boundingRect;
}V607 Ownerless expression 'QRectF { 0, 0, 640, 480 }'. quickitemnodeinstance.cpp 592
Начнём сразу с кода, так как иначе может быть непонятно, о чём идёт разговор:
Файл common.h:15
#ifdef __clang__
# pragma clang diagnostic push
# pragma clang diagnostic ignored "-Wc++11-narrowing"
# include <wdbgexts.h>
# pragma clang diagnostic pop
#else
# pragma warning( disable : 4838 )
# include <wdbgexts.h>
# pragma warning( default : 4838 )
#endif // __clang__Здесь разработчики пытаются подавить предупреждение компилятора о сужающих преобразованиях в стороннем заголовочном файле. В целом ничего плохого в этом нет, только вот реализация подавления вызывает вопросы. Проблема здесь в том, что использование #pragma warning(default : X) не восстанавливает состояние предупреждения, выключенного ранее с помощью #pragma warning(disable: X). Вместо этого она возвращает его в состояние по умолчанию, что не всегда корректно.
Анализатор справедливо предостерегает от проблем:
V665 [CERT-MSC00-C] Possibly, the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 21, 23. common.h 23
Кстати, в документации к диагностике V665 есть очень хороший пример как наступить на грабли. Позволю себе процитировать его:
Предположим, что файл компилируется с ключом /Wall. В этом случае должно выдаваться предупреждение C4061. Если написать "#pragma warning(default : 4061)", то это предупреждение перестанет выдаваться, так как по умолчанию оно является отключённым.
Самое интересное, что в случае с компилятором Clang разработчики как раз сделали всё правильно — использовали связку push/pop. Внесём небольшое исправление в код, чтобы и с другими компиляторами не было проблем при сборке:
#ifdef __clang__
# pragma clang diagnostic push
# pragma clang diagnostic ignored "-Wc++11-narrowing"
# include <wdbgexts.h>
# pragma clang diagnostic pop
#else
# pragma warning( push )
# pragma warning( disable : 4838 )
# include <wdbgexts.h>
# pragma warning( pop )
#endif // __clang__Одна из наиболее частых ошибок, которую периодически совершают программисты на С++. Даже с учётом того, что вы прочитали заголовок пункта, предлагаю вам посмотреть конструктор класса Editor и подумать, что же здесь не так...
Файл compilerexplorereditor.cpp:773
Editor::Editor(TextEditorActionHandler &actionHandler)
: m_document(new JsonSettingsDocument(&m_undoStack))
{
setContext(Core::Context(Constants::CE_EDITOR_ID));
setWidget(new EditorWidget(m_document, &m_undoStack, actionHandler));
connect(&m_undoStack, &QUndoStack::canUndoChanged, this,
[&actionHandler] { actionHandler.updateActions(); });
connect(&m_undoStack, &QUndoStack::canRedoChanged, this,
[&actionHandler] { actionHandler.updateActions(); });
}Ну и чтобы всё было совсем честно, добавлю декларацию класса:
Файл compilerexplorereditor.h:237
class Editor : public Core::IEditor
{
public:
Editor(TextEditor::TextEditorActionHandler &actionHandler);
~Editor();
Core::IDocument *document() const override { return m_document.data(); }
QWidget *toolBar() override;
QSharedPointer<JsonSettingsDocument> m_document;
QUndoStack m_undoStack;
std::unique_ptr<QToolBar> m_toolBar;
};Удалось найти? Может быть, тогда и квиз от Сергей Кушниренко стоит пройти? Говорят, там ещё более заковыристые вопросы... Но это чисто между нами. :)
Если же вопрос оказался не по зубам, то сейчас тоже объясню. Согласно стандарту языка С++, порядок инициализации членов класса в конструкторе происходит в порядке их объявления в классе. Получается, что поле m_undoStack будет инициализировано уже после того, как поучаствует в инициализации объекта m_document, что может без проблем запутать стороннего разработчика.
Беглый взгляд по коду показал, что сейчас неинициализованный объект не используется раньше времени. Но давайте представим, что новому разработчику понадобится ограничить размер истории m_undoStack. Почему бы не сделать это, например, в конструкторе JsonSettingsDocument?
Файл compilerexplorereditor.cpp:118
JsonSettingsDocument::JsonSettingsDocument(QUndoStack *undoStack)
: m_undoStack(undoStack)
{
m_undoStack->setUndoLimit(50); // Ops... Added for example
....
}В целом не самая плохая идея. Жалко только, что такой код ведёт к неопределённому поведению. Не спасёт даже проверка указателя перед разыменованием — он вполне валидный. Просто объект, на который он указывает, ещё не проинициализирован. Поэтому код лучше поправить. Тем более что это совсем не сложно. Есть несколько вариантов, и самый простой из них — поменять местами порядок полей в классе.
class Editor : public Core::IEditor
{
public:
Editor(TextEditor::TextEditorActionHandler &actionHandler);
~Editor();
Core::IDocument *document() const override { return m_document.data(); }
QWidget *toolBar() override;
QUndoStack m_undoStack; // <=
QSharedPointer<JsonSettingsDocument> m_document; // <=
std::unique_ptr<QToolBar> m_toolBar;
};Соответствующее предупреждение:
V670 [CWE-457, CERT-EXP53-CPP] The uninitialized class member 'm_undoStack' is used to initialize the 'm_document' member. Remember that members are initialized in the order of their declarations inside a class. compilerexplorereditor.cpp 774
Анализатор обнаружил исключение, брошенное по указателю. Чаще всего принято бросать исключения по значению, а перехватывать по ссылке. Бросание указателя может привести к тому, что исключение не будет поймано. А ещё это вынуждает перехватывающую сторону вызвать оператор delete для уничтожения созданного объекта-исключения.
Файл celliterator.cpp:53
CellIterator &CellIterator::operator-=(int n)
{
....
if (m_pos - n < 0)
throw new std::runtime_error("-= n too big!");
}Бегло пробежался по коду проекта и не нашёл мест, где бы перехватывались исключения по указателю. С большой долей вероятности это ошибка, которая рано или поздно приведёт к падению программы. Увы.
Само по себе бросание исключения по указателю не является ошибкой, однако этого лучше избегать. Особенно в переиспользуемом коде (название класса намекает). А анализатор в этом ещё и поможет:
V1022 [CWE-755] An exception was thrown by pointer. Consider throwing it by value instead. celliterator.cpp 59
В конце предлагаю посмотреть интересный экземпляр — формально это ошибка, но конкретно здесь она не "стреляет". Хороший пример, как можно на ровном месте завезти себе в программу немного неопределённого поведения (UB).
Файл containers.cpp:18
static void *readPointerArray(ULONG64 address, unsigned count,
const SymbolGroupValueContext &ctx)
{
const unsigned pointerSize = SymbolGroupValue::pointerSize();
const ULONG allocSize = pointerSize * count;
ULONG bytesRead = 0;
void *data = new unsigned char[allocSize];
const HRESULT hr = ctx.dataspaces->ReadVirtual(address, data, allocSize,
&bytesRead);
if (FAILED(hr) || bytesRead != allocSize)
{
delete [] data; // <=
return 0;
}
return data;
}"Так, ну и где ошибка?" — спросите вы. Выделили же через new, удалили через delete[], всё как в статье "Почему в С++ массивы нужно удалять через delete[]". Что опять-то не так?
Проблема в указателе на void (потере настоящего типа). Дело в том, что, с точки зрения стандарта С++, такое удаление массива является неопределённым поведением. Цитата $8.3.5/3 стандарта C++20:
In the first alternative (delete object), if the static type of the object to be deleted is different from its dynamic type, the static type shall be a base class of the dynamic type of the object to be deleted and the static type shall have a virtual destructor or the behavior is undefined. In the second alternative (delete array) if the dynamic type of the object to be deleted differs from its static type, the behavior is undefined.
Конкретно здесь везёт, что память выделяется под тривиальный тип и, похоже, это не приводит к серьёзным проблемам. Вот был бы это класс, да ещё и с нетривиальным деструктором... Хотя о чём это я, UB на то и UB — никто не знает, что произойдёт на самом деле.
Анализатор оберегает и от таких проблем:
Что ж, на этом, думаю, можно и остановиться. Мы увидели достаточное количество ошибок, но при этом я бы хотел отметить, что проект достаточно хорошо написан: современный код, документация, неплохое API (хоть и не без грехов). Похвально, особенно для проекта с такой долгой историей и собственным статическим анализатором за спиной.
Кстати, я же упоминал подарок... А вот и он: специально для этой статьи, мы подготовили промокод QTCREATOR, чтобы вы могли попробовать анализатор PVS-Studio c плагином для Qt Creator на своём коде. Для этого достаточно просто скачать анализатор по ссылке.
0
0
0
28
0
37
0
120
0
Попробуйте PVS-Studio бесплатно
Мы ответим вам на
Если вы так и не получили ответ, пожалуйста, проверьте, отфильтровано ли письмо в одну из следующих стандартных папок:
0