Щороку ми спостерігаємо одну й ту саму драму: помилки руйнують наш код, ніби прагнучи продемонструвати свою владу. Але сьогодні ситуація змінюється — час для суду. Давайте заглибимося в найцікавіші помилки, які ми виявили цього року.
Всі встаньте!
Протягом останніх дванадцяти місяців наша команда ретельно перевірила безліч помилок з відкритих C і C++ проектів. Жваві суперечки та обговорення розгорнулися в коментарях, щоб захистити невинність кожної помилки. Тепер настав час розплати!
Ми розглянемо 10 найбільш відомих помилок з цих проектів, які ми описували у статтях. Для запальних фанатів C і C++ ми вибрали найбільш популярні статті цього року:
· Як не перевіряти розмір масиву в C++
· Що не так з Telegram: десятки помилок виявлено
· Комар сидить у вашому компіляторі і не хоче йти вже 13 років
· PPSSPP чи PSP? Виявляємо помилки з майбутнього
Повний список можна знайти за посиланням.
Обвинувачені помилки
Обвинувачений N10
Щоб зберегти інтригу і не розкрити самого небезпечного злочинця одразу, почнемо знизу списку. Виводимо десятого порушника! О, звісно — це infamous пам'яті витік. Давайте покажемо, де він ховається:
int sceNetAdhocMatchingSetHelloOpt(int matchingId,
int optLenAddr, u32 optDataAddr)
{
....
if (optLenAddr > context->hellolen)
{
hello = realloc(hello, optLenAddr);
}
....
}
На перший погляд, код виглядає правильним, і програма працює правильно 99% часу. Поки що realloc не може виконати запит програміста. Давайте звернемося до документації функції realloc:
Якщо не вистачає пам'яті, стара пам'ять не звільняється, а повертається нульовий вказівник.
Отже, якщо під час переназначення пам'яті не вистачає ресурсів, ми запишемо нульовий вказівник у вказівник hello, і буде витік пам'яті. Ось що повідомляє аналізатор:
Попередження PVS-Studio: V701 realloc() можливий витік: коли realloc() не вдається виділити пам'ять, оригінальний вказівник ‘hello’ втрачається. Рекомендується призначити realloc() тимчасовому вказівнику. sceNetAdhoc.cpp 5407
Ми знайшли цю помилку в проекті PPSSPP. Повний опис статті можна знайти за посиланням.
Обвинувачений N9
Гаразд, ми впоралися з цим порушником — тепер пора для наступного. Зачекайте, це не один? Два? Скільки їх? Хто справжні злочинці тут?
Тепер давайте зіграємо в міні-гру: знайдіть помилки в коді проекту OpenVINO. Ті, хто знайшли їх, заслуговують на похвалу! Ті, хто не знайшли — також заслуговують на похвалу! Однак сподіваємося, що це очевидно для всіх.
Ось чудовий приклад того, чому розробникам потрібен статичний аналізатор.
template
using caseless_unordered_map = std::unordered_map, CaselessEq>;
using TypeToNameMap = ov::intel_cpu::
caseless_unordered_map;
static const TypeToNameMap& get_type_to_name_tbl() {
static const TypeToNameMap type_to_name_tbl = {
{"Constant", Type::Input},
{"Parameter", Type::Input},
{"Result", Type::Output},
{"Eye", Type::Eye},
{"Convolution", Type::Convolution},
{"GroupConvolution", Type::Convolution},
{"MatMul", Type::MatMul},
{"FullyConnected", Type::FullyConnected},
{"MaxPool", Type::Pooling},
{"AvgPool", Type::Pooling},
{"AdaptiveMaxPool", Type::AdaptivePooling},
{"AdaptiveAvgPool", Type::AdaptivePooling},
{"Add", Type::Eltwise},
{"IsFinite", Type::Eltwise},
{"IsInf", Type::Eltwise},
{"IsNaN", Type::Eltwise},
{"Subtract", Type::Eltwise},
{"Multiply", Type::Eltwise},
{"Divide", Type::Eltwise},
{"SquaredDifference", Type::Eltwise},
{"Maximum", Type::Eltwise},
{"Minimum", Type::Eltwise},
{"Mod", Type::Eltwise},
{"FloorMod", Type::Eltwise},
{"Power", Type::Eltwise},
{"PowerStatic", Type::Eltwise},
{"Equal", Type::Eltwise},
{"NotEqual", Type::Eltwise},
{"Greater", Type::Eltwise},
{"GreaterEqual", Type::Eltwise},
{"Less", Type::Eltwise},
{"LessEqual", Type::Eltwise},
{"LogicalAnd", Type::Eltwise},
{"LogicalOr", Type::Eltwise},
{"LogicalXor", Type::Eltwise},
{"LogicalNot", Type::Eltwise},
{"Relu", Type::Eltwise},
{"LeakyRelu", Type::Eltwise},
{"Gelu", Type::Eltwise},
{"Elu", Type::Eltwise},
{"Tanh", Type::Eltwise},
{"Sigmoid", Type::Eltwise},
{"Abs", Type::Eltwise},
{"Sqrt", Type::Eltwise},
{"Clamp", Type::Eltwise},
{"Exp", Type::Eltwise},
{"SwishCPU", Type::Eltwise},
{"HSwish", Type::Eltwise},
{"Mish", Type::Eltwise},
{"HSigmoid", Type::Eltwise},
{"Round", Type::Eltwise},
{"PRelu", Type::Eltwise},
{"Erf", Type::Eltwise},
{"SoftPlus", Type::Eltwise},
{"SoftSign", Type::Eltwise},
{"Select", Type::Eltwise},
{"Log", Type::Eltwise},
{"BitwiseAnd", Type::Eltwise},
{"BitwiseNot", Type::Eltwise},
{"BitwiseOr", Type::Eltwise},
{"BitwiseXor", Type::Eltwise},
{"Reshape", Type::Reshape},
{"Squeeze", Type::Reshape},
{"Unsqueeze", Type::Reshape},
{"ShapeOf", Type::ShapeOf},
{"NonZero", Type::NonZero},
{"Softmax", Type::Softmax},
{"Reorder", Type::Reorder},
{"BatchToSpace", Type::BatchToSpace},
{"SpaceToBatch", Type::SpaceToBatch},
{"DepthToSpace", Type::DepthToSpace},
{"SpaceToDepth", Type::SpaceToDepth},
{"Roll", Type::Roll},
{"LRN", Type::Lrn},
{"Split", Type::Split},
{"VariadicSplit", Type::Split},
{"Concat", Type::Concatenation},
{"ConvolutionBackpropData", Type::Deconvolution},
{"GroupConvolutionBackpropData", Type::Deconvolution},
{"StridedSlice", Type::StridedSlice},
{"Slice", Type::StridedSlice},
{"Tile", Type::Tile},
{"ROIAlign", Type::ROIAlign},
{"ROIPooling", Type::ROIPooling},
{"PSROIPooling", Type::PSROIPooling},
{"DeformablePSROIPooling", Type::PSROIPooling},
{"Pad", Type::Pad},
{"Transpose", Type::Transpose},
{"LSTMCell", Type::RNNCell},
{"GRUCell", Type::RNNCell},
{"AUGRUCell", Type::RNNCell},
{"RNNCell", Type::RNNCell},
{"LSTMSequence", Type::RNNSeq},
{"GRUSequence", Type::RNNSeq},
{"AUGRUSequence", Type::RNNSeq},
{"RNNSequence", Type::RNNSeq},
{"FakeQuantize", Type::FakeQuantize},
{"BinaryConvolution", Type::BinaryConvolution},
{"DeformableConvolution", Type::DeformableConvolution},
{"TensorIterator", Type::TensorIterator},
{"Loop", Type::TensorIterator},
{"ReadValue", Type::MemoryInput}, // for construction from name
// ctor, arbitrary name is used
{"Assign", Type::MemoryOutput}, // for construction from layer ctor
{
"Convert", Type::Convert},
{"NV12toRGB", Type::ColorConvert},
{"NV12toBGR", Type::ColorConvert},
{"I420toRGB", Type::ColorConvert},
{"I420toBGR", Type::ColorConvert},
{"MVN", Type::MVN},
{"NormalizeL2", Type::NormalizeL2},
{"ScatterUpdate", Type::ScatterUpdate},
{"ScatterElementsUpdate", Type::ScatterElementsUpdate},
{"ScatterNDUpdate", Type::ScatterNDUpdate},
{"Interpolate", Type::Interpolate},
{"RandomUniform", Type::RandomUniform},
{"ReduceL1", Type::Reduce},
{"ReduceL2", Type::Reduce},
{"ReduceLogicalAnd", Type::Reduce},
{"ReduceLogicalOr", Type::Reduce},
{"ReduceMax", Type::Reduce},
{"ReduceMean", Type::Reduce},
{"ReduceMin", Type::Reduce},
{"ReduceProd", Type::Reduce},
{"ReduceSum", Type::Reduce},
{"ReduceLogSum", Type::Reduce},
{"ReduceLogSumExp", Type::Reduce},
{"ReduceSumSquare", Type::Reduce},
{"Broadcast", Type::Broadcast},
{"EmbeddingSegmentsSum", Type::EmbeddingSegmentsSum},
{"EmbeddingBagPackedSum", Type::EmbeddingBagPackedSum},
{"EmbeddingBagOffsetsSum", Type::EmbeddingBagOffsetsSum},
{"Gather", Type::Gather},
{"GatherElements", Type::GatherElements},
{"GatherND", Type::GatherND},
{"GridSample", Type::GridSample},
{"OneHot", Type::OneHot},
{"RegionYolo", Type::RegionYolo},
{"ShuffleChannels", Type::ShuffleChannels},
{"DFT", Type::DFT},
{"IDFT", Type::DFT},
{"RDFT", Type::RDFT},
{"IRDFT", Type::RDFT},
{"Abs", Type::Math},
{"Acos", Type::Math},
{"Acosh", Type::Math},
{"Asin", Type::Math},
{"Asinh", Type::Math},
{"Atan", Type::Math},
{"Atanh", Type::Math},
{"Ceil", Type::Math},
{"Ceiling", Type::Math},
{"Cos", Type::Math},
{"Cosh", Type::Math},
{"Floor", Type::Math},
{"HardSigmoid", Type::Math},
{"If", Type::If},
{"Neg", Type::Math},
{"Reciprocal", Type::Math},
{"Selu", Type::Math},
{"Sign", Type::Math},
{"Sin", Type::Math},
{"Sinh", Type::Math},
{"SoftPlus", Type::Math},
{"Softsign", Type::Math},
{"Tan", Type::Math},
{"CTCLoss", Type::CTCLoss},
{"Bucketize", Type::Bucketize},
{"CTCGreedyDecoder", Type::CTCGreedyDecoder},
{"CTCGreedyDecoderSeqLen", Type::CTCGreedyDecoderSeqLen},
{"CumSum", Type::CumSum},
{"DetectionOutput", Type::DetectionOutput},
{"ExperimentalDetectronDetectionOutput",
Type::ExperimentalDetectronDetectionOutput},
{"LogSoftmax", Type::LogSoftmax},
{"TopK", Type::TopK},
{"GatherTree", Type::GatherTree},
{"GRN", Type::GRN},
{"Range", Type::Range},
{"Proposal", Type::Proposal},
{"ReorgYolo", Type::ReorgYolo},
{"ReverseSequence", Type::ReverseSequence},
{"ExperimentalDetectronTopKROIs",
Type::ExperimentalDetectronTopKROIs},
{"ExperimentalDetectronROIFeatureExtractor",
Type::ExperimentalDetectronROIFeatureExtractor},
{"ExperimentalDetectronPriorGridGenerator",
Type::ExperimentalDetectronPriorGridGenerator},
{"ExperimentalDetectronGenerateProposalsSingleImage",
Type::ExperimentalDetectronGenerateProposalsSingleImage},
{"ExtractImagePatches", Type::ExtractImagePatches},
{"GenerateProposals", Type::GenerateProposals},
{"Inverse", Type::Inverse},
{"NonMaxSuppression", Type::NonMaxSuppression},
{"NonMaxSuppressionIEInternal", Type::NonMaxSuppression},
{"NMSRotated", Type::NonMaxSuppression},
{"MatrixNms", Type::MatrixNms},
{"MulticlassNms", Type::MulticlassNms},
{"MulticlassNmsIEInternal", Type::MulticlassNms},
{"Multinomial", Type::Multinomial},
{"Reference", Type::Reference},
{"Subgraph", Type::Subgraph},
{"PriorBox", Type::PriorBox},
{"PriorBoxClustered", Type::PriorBoxClustered},
{"Interaction", Type::Interaction},
{"MHA", Type::MHA},
{"Unique", Type::Unique},
{"Ngram", Type::Ngram},
{"ScaledDotProductAttention", Type::ScaledDotProductAttention},
{"ScaledDotProductAttentionWithKVCache",
Type::ScaledDotProductAttention},
{
"PagedAttentionExtension", Type::ScaledDotProductAttention},
{"RoPE", Type::RoPE},
{"GatherCompressed", Type::Gather},
{"CausalMaskPreprocess", Type::CausalMaskPreprocess},
};
return type_to_name_tbl;
}
Цікаво, що багато розробників вважають безглуздим шукати помилки в такому коді.
Отже, вони просто прокручують вниз, щоб побачити відповідь одразу.
Є ще іронія в тому, як, незважаючи на оцінку неефективності та витрат на працю, розробники (developers) все одно монотонно шукають помилки, не використовуючи статичні аналізатори (static analyzers).
Це так просто: візьміть аналізатор (analyzer), запустіть аналіз, подивіться на список помилок. Профіт. Не треба витрачати час на пошук багів — вони вже підсвічені. Просто виправте їх і все.
Попередження PVS-Studio:
-
V766 Елемент з таким самим ключем ‘“Abs”’ вже був доданий. cpu_types.cpp 178
-
V766 Елемент з таким самим ключем ‘“SoftPlus”’ вже був доданий. cpu_types.cpp 198
Ось деякі помилки:
static const TypeToNameMap& get_type_to_name_tbl() {
static const TypeToNameMap type_to_name_tbl = {
....,
{"Abs", Type::Eltwise}, // <=
....,
{"SoftPlus", Type::Eltwise}, // <=
....,
{"Abs", Type::Math}, // <=
....,
{"SoftPlus", Type::Math}, // <=
....,
};
return type_to_name_tbl;
}
Очевидно, не повинно бути однакових значень, і дублікати ніколи не використовуються.
Ми знайшли цю помилку в проекті OpenVINO. Повний текст статті за посиланням.
Обвинувачений N8
Це було боляче дивитись, так? Добре, давайте перейдемо до наступного... Почекайте, що це!? Святі Боже... Кожна помилка жахлива по-своєму. Тепер подивіться на цю нахабну.
Оскільки цей випадок складний і дуже заплутаний, подивіться на попередження:
V762 Можливо, віртуальну функцію було перевизначено неправильно. Дивіться перший аргумент функції ‘preferreddomain’ у похідному класі ‘HandlePositionFieldInput’ та базовому класі ‘CurvesFieldInput’. nodegeoinputcurve_handles.cc 95
Отже, ми бачимо, що проблема полягає в неправильному перевизначенні віртуальної функції в випадкових класах. Що саме не так? Давайте дізнаємось.
Почнемо з базового класу:
typedef struct CurvesGeometry { .... };
namespace bke
{
....
class CurvesGeometry : public ::CurvesGeometry { .... };
class CurvesFieldInput : public fn::FieldInput
{
....
virtual std::optional preferred_domain(
const CurvesGeometry &curves) const;
};
....
}
Віртуальна функція preferred_domain приймає параметр типу bke::CurvesGeometry. Запам’ятаємо це.
Тепер подивіться на нащадка:
namespace blender::nodes::node_geo_input_curve_handles_cc
{
class HandlePositionFieldInput final : public bke::CurvesFieldInput
{
....
std::optional preferred_domain(
const CurvesGeometry & /*curves*/) const;
};
}
Знайшли проблему? Якщо ні, не переживайте, ми спочатку теж не зрозуміли 🙂
У базовому класі віртуальна функція приймає параметр з незвукованим ім’ям, CurvesGeometry. Коли компілятор шукає цей тип, він починає з області видимості класу CurvesFieldInput і переглядає всі обмежені області, поки не знайде тип. Як результат, знаходиться тип bke::CurvesGeometry.
Тепер подивіться на похідні класи. Вони визначені в іншому просторі імен, ніж той, у якому знаходиться базовий клас. Компілятор також починає шукати потрібне ім’я CurvesGeometry, не знаходить його в обмежених областях і доходить до глобального простору імен.
Однак, глобальний простір імен також містить CurvesGeometry, тільки це не той, який нам потрібен для перевизначення функції 🙂
Ми можемо виправити це, вказавши кваліфіковане ім’я типу — просто використаємо можливості C++11 (override) і захистимо майбутні покоління від помилок:
namespace blender::nodes::node_geo_input_curve_handles_cc
{
class HandlePositionFieldInput final : public bke::CurvesFieldInput
{
....
std::optional preferred_domain(
const bke::CurvesGeometry & /*curves*/) const override;
};
}
Ми знайшли цю помилку в проекті Blender. Повний текст статті за посиланням.
Обвинувачений N7
Сьомий обвинувачений, що ти ховаєш за спиною? Покажи швидше! Ах! Маленький злодій! Вирішив припритулити смачний шматок коду, а? Без шансів, ми виведемо це на світло.
Чесно кажучи, ми справді хотіли вставити повний код функції сюди — вона містить майже 400 рядків коду — щоб зробити пошук помилок більш цікавим. Однак ми не хочемо зловживати вашою мишкою, тому включили тільки найцікавішу частину.
Файл: [symbolgroupvalue.cpp:1196](https://github.com/qt-creator/qt-creator/blob/27055e4c3904b09385c96a2ac336aef87f3a4f8e/src/libs/qtcreatorcdbext/symbolgroupvalue.cpp#L1196)
static KnownType knownClassTypeHelper(const std::string &type,
std::string::size_type pos,
std::string::size_type endPos)
{
....
// Залишкові не-шаблонні типи
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 років. Якщо хтось з читачів може пояснити, що саме тут відбувається — ласкаво просимо до коментарів.
Аналізатор (Analyzer) вказує на цей фрагмент коду з попередженням:
V779 [CWE-561, CERT-MSC12-C] Виявлено недосяжний код. Можливо, є помилка. symbolgroupvalue.cpp 1565
Ми знайшли цю помилку в проекті Qt Creator. Повний текст статті за посиланням.
Щоб легко переглядати результати аналізу, можна використовувати розширення (plugin) PVS-Studio для Qt Creator. Для отримання додаткової інформації про його встановлення та використання, звертайтесь до документації: Using the PVS-Studio extension for Qt Creator.
Обвинувачений N6
Ха-ха-ха-ха! Помилки копіювання-вставлення, ми вас очікували! Ці баги ховаються майже в кожному коді, і їх не завжди легко помітити. Ось один із них на нашому суді.
Крок вперед і покажи себе!
void WebViewInstance::show(const QString &url, uint64 queryId)
{
....
const auto allowClipboardRead = v::is(_source)
|| v::is(_source)
|| (attached != end(bots)
&& (attached->inAttachMenu || attached->inMainMenu));
....
}
Попередження від аналізатора:
V501 Ідентичні підвирази ‘v::is (_source)’ зліва і справа від оператора ‘||’. botattachweb_view.cpp 1129
Один і той самий вираз v::is(_source) перевіряється в умові. Розробникам слід замінити один з них на щось інше — але на що?
Подивіться на останній рядок умови і подивіться, як перевіряються значення наступних виразів:
(attached->inAttachMenu || attached->inMainMenu)
Розглянемо імена змінних та ідентичні вирази, на які вказує аналізатор. Можливо, розробникам варто виправити умову наступним чином:
const auto allowClipboardRead =
v::is(_source)
|| v::is(_source)
....
Ми знайшли цю помилку в проекті Telegram. Повний текст статті за посиланням.
Обвинувачений N5
Середина шляху, і ми вже побачили багато антен у підсудних. В повітрі відчувається небезпека. Так, всі винні баги повинні бути знищені в майбутніх випусках. Особливо ті, що ми розглянемо далі.
Ось і п’ятий обвинувачений: здавалося б, дрібна помилка, але який шкоди вона може завдати! Ця точно заслуговує на місце за ґратами.
const qdGameObjectStateWalk* qdGameObjectMoving::current_walk_state() const
{
const qdGameObjectState* st = get_cur_state();
if(!st || st -> state_type() != qdGameObjectState::STATE_WALK){
#ifndef _QUEST_EDITOR
st = last_walk_state_;
if(!st || st -> state_type() != qdGameObjectState::STATE_WALK)
st = get_default_state();
else
st = get_default_state();
if(!st) st = get_state(0);
#endif
}
....
}
Попередження PVS-Studio: V523 Оператор ‘then’ еквівалентний оператору ‘else’. qdgameobject_moving.cpp 2781
Аналізатор вказує на те, що одна й та сама дія виконується незалежно від умови:
if(!st || st -> state_type() != qdGameObjectState::STATE_WALK)
st = get_default_state();
else
st = get_default_state();
Зверніть увагу, що код відформатовано дивно. Ключове слово else знаходиться на початку рядка.
Якщо подивитись на код, можна побачити, що розробники хотіли написати директиву препроцесора #else, а не оператор else. Однак вони зробили помилку в написанні і забули написати символ хешу (#).
Виправлений код:
const qdGameObjectState* st = get_cur_state();
if(!st || st -> state_type() != qdGameObjectState::STATE_WALK){
#ifndef _QUEST_EDITOR
st = last_walk_state_;
if(!st || st -> state_type() != qdGameObjectState::STATE_WALK)
st = get_default_state();
#else
st = get_default_state();
if(!st) st = get_state(0);
#endif
Ми знайшли цю помилку в проекті qdEngine. Повний текст статті за посиланням.
Обвинувачений N4
Гаразд, де четвертий, хтось його бачить? Куди поділася та помилка? Гаразд, покажіть місце злочину — ми знайдемо цього лиходія.
static const char *script_list[][2] = {
....
{ "Myanmar / Burmese", "Mymr" },
{ "Nag Mundari", "Nagm" },
{ "Nandinagari", "Nand" },
....
}
Читач може запитати: "Що тут не так?" Ми б не здогадались, якби не правило діагностики [V1076](https://pvs-studio.com/en/docs/warnings/v1076/). Цікаво, що це перше попередження, яке ми описали. Це правило діагностики перевіряє код програми на наявність невидимих символів. Такі символи — це немов "задні двері", які програміст може не побачити через налаштування відображення тексту в середовищі розробки, але компілятор бачить їх і коректно обробляє.
Попередження аналізатора:
[V1076](https://pvs-studio.com/en/docs/warnings/v1076/) Код містить невидимі символи, які можуть змінити його логіку. Розгляньте можливість увімкнення відображення невидимих символів у редакторі коду. [locales.h](https://github.com/godotengine/godot/blob/15073afe3856abd2aa1622492fe50026c7d63dc1/core/string/locales.h#L1114) 1114
Давайте уважніше подивимось на наступний рядок:
{ "Nag Mundari", "Nagm" },
```
Він містить задні двері з невидимим символом. Якщо скористатися шістнадцятковим редактором, ми побачимо наступне:
Між подвійними лапками та символом N знаходяться 3 байти: E2, 80 та 8B. Вони відповідають ZERO WIDTH SPACE (U+200B) юнікодовому символу.
На щастя, наявність цього символу в рядку літерала не впливає на логіку програми.
Рядки з масиву script_list, що містять "заражений" рядок літерала потрапляють до хеш-таблиці TranslationServer::script_map. Ключ хеш-таблиці — це другий рядок літерала пари, а значення — перший. Отже, заражений рядок літерала потрапляє до хеш-таблиці як значення, і пошук за хешем не порушується.
Далі, ми можемо подивитись, куди саме це значення з хеш-таблиці може потрапити. Ми знайшли кілька місць:
-
Значення може потрапити в рядок, що повертається функцією
[TranslationServer::get_locale_name](https://github.com/godotengine/godot/blob/4.2.2-stable/core/string/translation.cpp#L470). Аналізуючи викликаючі функції, ми можемо побачити, що цей рядок потрапляє в графічний інтерфейс ([1], [2], [3], [4]) різними способами. -
Значення повертається з функції
[TranslationServer::get_script_name](https://github.com/godotengine/godot/blob/4.2.2-stable/core/string/translation.cpp#L503). Аналізуючи викликаючі функції, ми також можемо побачити, що цей рядок потрапляє в графічний інтерфейс ([1], [2]).
Швидше за все, задні двері були додані випадково при копіюванні назви з якогось сайту. Ми можемо просто видалити цей символ з рядка літерала.
Ми знайшли цю помилку в проекті Godot Engine. Повний текст статті за посиланням.
Обвинувачений N3
Три злісні підсудні на лаві. Який з них найнебезпечніший злочинець?
Третій баг, ти намагаєшся бути хитрим? Обчислюючи складні рівняння, від яких залежать важливі змінні? Напишіть повне визнання! Ви поламали це.
void StfsContainerDevice::BlockToOffsetSVOD(size_t block, ....)
{
....
}
Попередження PVS-Studio:
[V1064](https://pvs-studio.com/en/docs/warnings/v1064/) Операнд цілочисельного ділення ‘level0_table_count’ менший за ‘HASHES_PER_L1_HASH’. Результат завжди буде нулем. [stfs_container_device.cc 500](https://github.com/xenia-project/xenia/blob/3d30b2eec3ab1f83140b09745bee881fb5d5dde2/src/xenia/vfs/devices/stfs_container_device.cc#L500)
Аналізатор попереджає, що значення `level1_table_count` завжди дорівнює 0, оскільки лівий операнд ділення для `level0_table_count` менший за правий операнд `HASHES_PER_L1_HASH` під час операції цілочисельного ділення. Значення `HASHES_PER_L1_HASH` становить `41412`, тому, щоб визначити значення `level0_table_count`, погляньмо на код вище.
Змінна `file_block` обчислюється шляхом ділення `true_block` на `BLOCKS_PER_FILE`, тому її значення знаходиться в межах `[0 .. 82823]`.
Змінна `BLOCKS_PER_L0_HASH` ділить значення на `408`, і до результату додається `1`. Коли `file_block` досягне свого максимального значення, ми отримаємо `202`, тому значення змінної `level0_table_count` знаходиться в межах `[1 .. 203]`.
Отже, змінна `level1_table_count` обчислюється як `203/41412+1`, і дорівнює `1` для будь-яких значень `true_block`.
Чи можемо ми десь помилитись? Здається [ні](https://godbolt.org/z/79ereYfnj), це не тільки наш аналізатор, який попереджає про це.
Це цікаве випадок, де ми навіть не можемо помітити помилку з першого погляду. Рецензент легко може її пропустити, адже для цього потрібно вручну виконати складні обчислення.
Є [коментар](https://github.com/xenia-project/xenia/blob/3d30b2eec3ab1f83140b09745bee881fb5d5dde2/src/xenia/vfs/devices/stfs_container_device.cc#L462-L473) у коді, який може пролити світло на цю загадку. Можливо, хтось уже має якісь думки щодо цього?
Ми знайшли цю помилку в проекті Xenia. Повний текст статті за [посиланням](https://pvs-studio.com/en/blog/posts/cpp/1177/).
**Обвинувачений N2**
Що це? Розіменування нульового вказівника? Це одна й та ж історія кожного року… Жодне огляд коду не обходиться без цього поганця. Ми можемо побачити його маленькі сліди в майже кожному кодовому базі. Очікуйте суворі вироки. Тепер давайте поглянемо на місце злочину!
Подивіться на наступну структуру:
struct ForceCodegenLinking {
ForceCodegenLinking() {
// Ми повинні посилатись на передавання таким чином, щоб компілятори не
// видаляли це як мертвий код, навіть з оптимізацією всього програми,
// але при цьому це ефективно буде бездіяльною операцією. Оскільки компілятор не настільки розумний,
// щоб знати, що getenv() ніколи не повертає -1, це вирішить проблему.
// Це необхідно для того, щоб глобальні змінні в трансляційних одиницях, де ці функції
// визначені, обов'язково ініціалізувались, заповнюючи різні
// реєстри.
У своєму конструкторі викликається кілька функцій для створення різних сутностей. Ми розглянемо лише функції, до яких передаються аргументи (їх шість). Ось, наприклад, деякі з них:
ScheduleDAGSDNodes *llvm::createBURRListDAGScheduler(SelectionDAGISel *IS,
CodeGenOptLevel OptLevel)
{
const TargetSubtargetInfo &STI = IS->MF->getSubtarget();
....
}
Або
ScheduleDAGSDNodes* createDefaultScheduler(SelectionDAGISel *IS,
CodeGenOpt::Level OptLevel)
{
const TargetLowering *TLI = IS->TLI;
const TargetSubtargetInfo &ST = IS->MF->getSubtarget();
....
}
Якщо поглянути на виклик функції в конструкторі, можна побачити, що як перший аргумент передається nullptr. Це саме той параметр IS, який розіменовується на першій лінії функції.
Це дивний код. Можливо, розробники LLVM знають щось і можуть фактично контролювати UB (Undefined Behavior).
Кожна функція з першим нульовим аргументом має його розіменування.
Отже, ось попередження аналізатора:
- V522 Може відбутись розіменування нульового вказівника. Нульовий вказівник передається в функцію ‘createBURRListDAGScheduler’. Перевірте перший аргумент. Перегляньте рядки: ‘ScheduleDAGRRList.cpp:3147’, ‘LinkAllCodegenComponents.h:40’.
- V522 Може відбутись розіменування нульового вказівника. Нульовий вказівник передається в функцію ‘createSourceListDAGScheduler’. Перевірте перший аргумент. Перегляньте рядки: ‘ScheduleDAGRRList.cpp:3161’, ‘LinkAllCodegenComponents.h:42’.
- V522 Може відбутись розіменування нульового вказівника. Нульовий вказівник передається в функцію ‘createHybridListDAGScheduler’. Перевірте перший аргумент. Перегляньте рядки: ‘ScheduleDAGRRList.cpp:3175’, ‘LinkAllCodegenComponents.h:44’.
- … (інші три подібні)
Ми знайшли цю помилку в проекті LLVM. Повний текст статті за посиланням.
Обвинувачений N1
Ви чекали цього. Ось воно! Будьте обережні! Ось воно — лідер банди помилок! Це день розплати! Ми чекали на це цілий рік.
Крок вперед і розкрий себе!
Розглянемо список іменованих констант:
enum dbg_status {
DBG_STATUS_OK,
DBG_STATUS_APP_VERSION_NOT_SET,
DBG_STATUS_UNSUPPORTED_APP_VERSION,
DBG_STATUS_DBG_BLOCK_NOT_RESET,
DBG_STATUS_INVALID_ARGS,
DBG_STATUS_OUTPUT_ALREADY_SET,
DBG_STATUS_INVALID_PCI_BUF_SIZE,
DBG_STATUS_PCI_BUF_ALLOC_FAILED,
DBG_STATUS_PCI_BUF_NOT_ALLOCATED,
DBG_STATUS_INVALID_FILTER_TRIGGER_DWORDS,
DBG_STATUS_NO_MATCHING_FRAMING_MODE,
DBG_STATUS_VFC_READ_ERROR,
DBG_STATUS_STORM_ALREADY_ENABLED,
DBG_STATUS_STORM_NOT_ENABLED,
DBG_STATUS_BLOCK_ALREADY_ENABLED,
DBG_STATUS_BLOCK_NOT_ENABLED,
DBG_STATUS_NO_INPUT_ENABLED,
DBG_STATUS_NO_FILTER_TRIGGER_256B,
DBG_STATUS_FILTER_ALREADY_ENABLED,
DBG_STATUS_TRIGGER_ALREADY_ENABLED,
DBG_STATUS_TRIGGER_NOT_ENABLED,
DBG_STATUS_CANT_ADD_CONSTRAINT,
DBG_STATUS_TOO_MANY_TRIGGER_STATES,
DBG_STATUS_TOO_MANY_CONSTRAINTS,
DBG_STATUS_RECORDING_NOT_STARTED,
DBG_STATUS_DATA_DIDNT_TRIGGER,
DBG_STATUS_NO_DATA_RECORDED,
DBG_STATUS_DUMP_BUF_TOO_SMALL,
DBG_STATUS_DUMP_NOT_CHUNK_ALIGNED,
DBG_STATUS_UNKNOWN_CHIP,
DBG_STATUS_VIRT_MEM_ALLOC_FAILED,
DBG_STATUS_BLOCK_IN_RESET,
DBG_STATUS_INVALID_TRACE_SIGNATURE,
DBG_STATUS_INVALID_NVRAM_BUNDLE,
DBG_STATUS_NVRAM_GET_IMAGE_FAILED,
DBG_STATUS_NON_ALIGNED_NVRAM_IMAGE,
DBG_STATUS_NVRAM_READ_FAILED,
DBG_STATUS_IDLE_CHK_PARSE_FAILED,
DBG_STATUS_MCP_TRACE_BAD_DATA,
DBG_STATUS_MCP_TRACE_NO_META,
DBG_STATUS_MCP_COULD_NOT_HALT,
DBG_STATUS_MCP_COULD_NOT_RESUME,
DBG_STATUS_RESERVED0,
DBG_STATUS_SEMI_FIFO_NOT_EMPTY,
DBG_STATUS_IGU_FIFO_BAD_DATA,
DBG_STATUS_MCP_COULD_NOT_MASK_PRTY,
DBG_STATUS_FW_ASSERTS_PARSE_FAILED,
DBG_STATUS_REG_FIFO_BAD_DATA,
DBG_STATUS_PROTECTION_OVERRIDE_BAD_DATA,
DBG_STATUS_DBG_ARRAY_NOT_SET,
DBG_STATUS_RESERVED1,
DBG_STATUS_NON_MATCHING_LINES,
DBG_STATUS_INSUFFICIENT_HW_IDS,
DBG_STATUS_DBG_BUS_IN_USE,
DBG_STATUS_INVALID_STORM_DBG_MODE,
DBG_STATUS_OTHER_ENGINE_BB_ONLY,
DBG_STATUS_FILTER_SINGLE_HW_ID,
DBG_STATUS_TRIGGER_SINGLE_HW_ID,
DBG_STATUS_MISSING_TRIGGER_STATE_STORM,
MAX_DBG_STATUS
};
Тепер звернімо увагу на масив рядків:
static const char * const s_status_str[] = {
/* DBG_STATUS_OK */
"Операція завершена успішно",
/* DBG_STATUS_APP_VERSION_NOT_SET */
"Версія додатка для налагодження не була задана",
/* DBG_STATUS_UNSUPPORTED_APP_VERSION */
"Невідповідна версія додатка для налагодження",
/* DBG_STATUS_DBG_BLOCK_NOT_RESET */
"Налагоджувальний блок не був скинутий після останнього запису",
/* DBG_STATUS_INVALID_ARGS */
"Невірні аргументи",
/* DBG_STATUS_OUTPUT_ALREADY_SET */
"Налагоджувальний вивід уже було задано",
/* DBG_STATUS_INVALID_PCI_BUF_SIZE */
"Невірний розмір PCI буфера",
/* DBG_STATUS_PCI_BUF_ALLOC_FAILED */
"Не вдалося виділити PCI буфер",
/* DBG_STATUS_PCI_BUF_NOT_ALLOCATED */
"PCI буфер не було виділено",
/* DBG_STATUS_INVALID_FILTER_TRIGGER_DWORDS */
"Офсети dword для фільтру/тригера не увімкнено для запису",
/* DBG_STATUS_VFC_READ_ERROR */
"Помилка читання з VFC",
/* DBG_STATUS_STORM_ALREADY_ENABLED */
"Шторм уже був увімкнений",
/* DBG_STATUS_STORM_NOT_ENABLED */
"Вказаний шторм не був увімкнений",
/* DBG_STATUS_BLOCK_ALREADY_ENABLED */
"Блок уже був увімкнений",
/* DBG_STATUS_BLOCK_NOT_ENABLED */
"Вказаний блок не був увімкнений",
/* DBG_STATUS_NO_INPUT_ENABLED */
"Для запису не було увімкнено жодного входу",
/* DBG_STATUS_NO_FILTER_TRIGGER_256B */
"Фільтри та тригери не дозволяються в E4 256-бітному режимі",
/* DBG_STATUS_FILTER_ALREADY_ENABLED */
"Фільтр уже був увімкнений",
/* DBG_STATUS_TRIGGER_ALREADY_ENABLED */
"Тригер уже був увімкнений",
/* DBG_STATUS_TRIGGER_NOT_ENABLED */
"Тригер не був увімкнений",
/* DBG_STATUS_CANT_ADD_CONSTRAINT */
"Обмеження можна додавати лише після того, як фільтр був "
"увімкнений або був доданий стан тригера",
/* DBG_STATUS_TOO_MANY_TRIGGER_STATES */
"Не можна додавати більше ніж 3 стани тригера",
/* DBG_STATUS_TOO_MANY_CONSTRAINTS */
"Не можна додавати більше ніж 4 обмеження для одного фільтра або стану тригера",
/* DBG_STATUS_RECORDING_NOT_STARTED */
"Запис не був розпочатий",
/* DBG_STATUS_DATA_DID_NOT_TRIGGER */
"Тригер був налаштований, але не спрацював",
/* DBG_STATUS_NO_DATA_RECORDED */
"Дані не були записані",
/* DBG_STATUS_DUMP_BUF_TOO_SMALL */
"Буфер вивантаження занадто малий",
/* DBG_STATUS_DUMP_NOT_CHUNK_ALIGNED */
"Дані вивантаження не вирівняні по шматках",
/* DBG_STATUS_UNKNOWN_CHIP */
"Невідомий чіп",
/* DBG_STATUS_VIRT_MEM_ALLOC_FAILED */
"Не вдалося виділити віртуальну пам'ять",
/* DBG_STATUS_BLOCK_IN_RESET */
"Вхідний блок знаходиться в процесі скидання",
/* DBG_STATUS_INVALID_TRACE_SIGNATURE */
"Знайдена невірна підпис MCP трасування в NVRAM",
/* DBG_STATUS_INVALID_NVRAM_BUNDLE */
"Знайдено невірний ідентифікатор пакету в NVRAM",
/* DBG_STATUS_NVRAM_GET_IMAGE_FAILED */
"Не вдалося отримати зображення NVRAM",
/* DBG_STATUS_NON_ALIGNED_NVRAM_IMAGE */
"Зображення NVRAM не вирівняне по dword",
/* DBG_STATUS_NVRAM_READ_FAILED */
"Не вдалося прочитати з NVRAM",
/* DBG_STATUS_IDLE_CHK_PARSE_FAILED */
"Не вдалося розібрати перевірку простою",
/* DBG_STATUS_MCP_TRACE_BAD_DATA */
"Дані MCP Trace пошкоджені",
/* DBG_STATUS_MCP_TRACE_NO_META */
"Вивантаження не містить метаданих - вони повинні бути надані в зображенні файлу",
/* DBG_STATUS_MCP_COULD_NOT_HALT */
"Не вдалося зупинити MCP",
/* DBG_STATUS_MCP_COULD_NOT_RESUME */
"Не вдалося відновити MCP після зупинки",
/* DBG_STATUS_RESERVED0 */
"",
/* DBG_STATUS_SEMI_FIFO_NOT_EMPTY */
"Не вдалося очистити FIFO SEMI синхронізації",
/* DBG_STATUS_IGU_FIFO_BAD_DATA */
"Дані FIFO IGU пошкоджені",
/* DBG_STATUS_MCP_COULD_NOT_MASK_PRTY */
"MCP не вдалося приховати паритети",
/* DBG_STATUS_FW_ASSERTS_PARSE_FAILED */
"Не вдалося розібрати FW Asserts",
/* DBG_STATUS_REG_FIFO_BAD_DATA */
"Дані FIFO GRC пошкоджені",
/* DBG_STATUS_PROTECTION_OVERRIDE_BAD_DATA */
"Дані перевизначення захисту пошкоджені",
/* DBG_STATUS_DBG_ARRAY_NOT_SET */
"Масиви для налагодження не були задані "
"(при використанні бінарних файлів, потрібно викликати dbg_set_bin_ptr)",
/* DBG_STATUS_RESERVED1 */
"",
/* DBG_STATUS_NON_MATCHING_LINES */
"Невідповідні лінії для налагодження - в E4 всі лінії повинні бути одного типу "
"(або 128b, або 256b)",
/* DBG_STATUS_INSUFFICIENT_HW_IDS */
"Недостатньо HW ідентифікаторів",
Спробуйте записувати менше Storms/блоків",
/* DBG_STATUS_DBG_BUS_IN_USE */
"Шина налагодження використовується",
/* DBG_STATUS_INVALID_STORM_DBG_MODE */
"Режим налагодження Storm не підтримується в поточному чіпі",
/* DBG_STATUS_OTHER_ENGINE_BB_ONLY */
"Інший двигун підтримується лише в BB",
/* DBG_STATUS_FILTER_SINGLE_HW_ID */
"Налаштований фільтр вимагає один вхід для Storm/блоку",
/* DBG_STATUS_TRIGGER_SINGLE_HW_ID */
"Налаштований фільтр вимагає, щоб усі обмеження одного "
"стану тригера були визначені для одного входу Storm/блоку",
/* DBG_STATUS_MISSING_TRIGGER_STATE_STORM */
"При тригеруванні за даними Storm, необхідно вказати Storm, за яким буде тригер",
};
Тепер подивіться на код, який перетворює названу константу на рядок:
const char *qed_dbg_get_status_str(enum dbg_status status)
{
return (status < MAX_DBG_STATUS) ?
s_status_str[status] : "Invalid debug status";
}
Де помилка?
Знайшли?
Є підказка, до якої теоретично можна додуматись. В масиві один порожній рядок відокремлює всі елементи, за винятком одного місця. Давайте почнемо з попередження аналізатора PVS-Studio:
V557 Можливий переповнення масиву. Значення індексу 'status' може досягти 58. qede_debug.c 7149
Воно вказує на фрагмент коду, де індекс масиву повертає вказівник на рядок:
return (status < MAX_DBG_STATUS) ?
s_status_str[status] : "Invalid debug status";
Спочатку ми неправильно діагностували проблему, і це сталося непомітно. Ми помилково подумали, що це типова помилка "off-by-one" (помилка на один елемент) — ми стикалися з таким у багатьох проектах.
Ось суть класичного антишаблону. Є enum (перелічення), останній елемент якого визначає кількість елементів.
enum Efoo {
A,
B,
C,
COUNT
};
Константи в enum отримують значення, починаючи з 0. Тому допоміжна константа COUNT буде дорівнювати 3, що відповідає кількості основних названих констант.
Є масив, в якому кожній названій константі відповідає щось:
char Cfoo[] = { 'A', 'B', 'C' };
Це типова помилка, коли розробники перевіряють, щоб індекс не виходив за межі масиву:
char Convert(unsigned id)
{
return (id > COUNT) ? 0 : Cfoo[id];
}
Перевірка працює неправильно: якщо id дорівнює COUNT, масив вийде за межі. PVS-Studio дає схоже попередження про подібні помилки.
Правильний варіант виглядатиме так:
return (id >= COUNT) ? 0 : Cfoo[id]; // OK
return (id < COUNT) ? Cfoo[id] : 0; // OK
Тут нічого особливого. Продовжимо. Ми можемо написати попередження про переповнення масиву в статті, але не будемо аналізувати помилку в статті про перевірку DPDK.
Але в останню мить — СТОП!
Перевірка насправді правильна!
return (status < MAX_DBG_STATUS) ?
s_status_str[status] : "Invalid debug status";
Тепер все стає менш зрозумілим і більш інтригуючим! Чому аналізатор видав попередження? Хибнопозитивне? Думаю, що ні. Не може бути, щоб він заплутався.
Підкочуючи рукави, я починаю вивчати код. І ось він!
Відсутній рядок для константи DBG_STATUS_NO_MATCHING_FRAMING_MODE.
В enum:
DBG_STATUS_INVALID_FILTER_TRIGGER_DWORDS,
DBG_STATUS_NO_MATCHING_FRAMING_MODE,
DBG_STATUS_VFC_READ_ERROR,
В масиві:
/* DBG_STATUS_INVALID_FILTER_TRIGGER_DWORDS */
"Відступи dword фільтру/тригеру не "
"увімкнено для запису",
/* DBG_STATUS_VFC_READ_ERROR */
"Помилка читання з VFC",
Подивіться на роздільник з двома порожніми рядками. Щось пішло не так.
Можливо, масив був заповнений неправильно, або рядок випадково було видалено, чи сталася помилка при злитті гілок, або є інша причина.
Однак, коли ми вже знайшли помилку, два порожні рядки виглядають підозріло. Ну, розробники навряд чи звернуть на це увагу під час огляду коду. Навіть якщо вони це помітять, вони просто видалять один з них для краси. Ніхто не скаже: "Ми могли щось пропустити. Давайте перевіримо це" 🙂
Результати помилки:
· можливе переповнення масиву;
· функція повертає неправильні рядки для всіх констант, що починаються з DBG_STATUS_NO_MATCHING_FRAMING_MODE.
Ми знайшли цю помилку в проекті DPDK. Повну статтю можна прочитати за посиланням.
Вирок
Суд мовчить. Навіть суддя не має слів. Найнебезпечніші злочинці року були затримані. Залишилося лише знищити їх, але це вже інша історія… Однак ми не змогли б знайти ці помилки без допомоги нашого вірного детектива — аналізатора PVS-Studio.
У 2024 році ми розв’язали чимало помилок, але ще є безліч проектів, які варто перевірити. Наш детектив готовий допомогти вам відшукати небезпечні фрагменти коду, які можуть ховатися у вашому проекті:
Принесіть свою ліцензію:
· для відкритих проектів
· для освітніх цілей
А якщо вам цікаво прочитати про всі найцікавіші помилки цього року, запрошую вас ознайомитися з топ-10 помилок у проектах на Java та C#:
· Топ-10 найцікавіших помилок Java у 2024 році
· Топ-10 помилок у проектах на C# у 2024 році.
Перекладено з: Court is in session: Top 10 most notorious C and C++ errors in 2024