Telegram开放网络（TON）是开发Telegram Messenger的同一团队的平台。 除了区块链，TON还提供大量服务。 开发人员最近使该平台的代码（用C ++编写）公开可用，并将其上载到GitHub。 我们决定在项目正式发布之前对其进行检查。
引言
电报开放网络是一组各种服务。 除其他外，它提供了基于Gram加密货币的自己的支付系统，以及一个执行TON的虚拟机TON VM。 它还提供消息传递服务TON Messages。 整个项目被视为对互联网审查的对策。
该项目是使用CMake构建的，因此我对构建和检查它没有任何困难。 源代码是用C ++ 14编写的，运行到21万个LOC：
由于该项目是一个小型且高质量的项目，因此其中没有很多错误，但仍应加以解决。
返回码
static int process_workchain_shard_hashes(....) { .... if (f == 1) { if ((shard.shard & 1) || cs.size_ext() != 0x20000) { return false;
PVS-Studio诊断消息：
V601'false '值隐式转换为整数类型。 mc-config.cpp 884
看起来函数在这里返回了错误的错误状态类型。 该函数显然应该为失败返回负值，而不是true / false。 至少这是它在代码中进一步做的事情，返回-1。
将变量与其自身进行比较
class LastBlock : public td::actor::Actor { .... ton::ZeroStateIdExt zero_state_id_; .... }; void LastBlock::update_zero_state(ton::ZeroStateIdExt zero_state_id) { .... if (zero_state_id_ == zero_state_id_) { return; } LOG(FATAL) << ....; }
PVS-Studio诊断消息：
V501 '=='运算符左右两侧有相同的子表达式：zero_state_id_ == zero_state_id_ LastBlock.cpp 66
TON遵循编码标准，该标准规定类成员的名称应以下划线结尾。 但是，在这种情况下，这种表示法可能会导致错误，因为您可能会忽略下划线。 传递给该函数的参数名称与类成员的名称相似，这使得将它们混合起来变得容易。 这个论点很可能意味着要参与比较。
不安全的宏
namespace td { namespace detail { [[noreturn]] void process_check_error(const char *message, const char *file, int line); }
PVS-Studio诊断消息：
V581彼此并排的'if'语句的条件表达式相同。 检查行：80，84。blockdb.cpp 84
CHECK宏中的条件将永远不会执行，因为前面的
if语句已经检查过了。
这里还存在另一个错误：
CHECK宏是不安全的，因为它的内部条件没有包装在
do {...} while（0）构造中。 需要这种包装以避免与
else分支中的其他条件冲突。 换句话说，以下代码无法正常工作：
if (X) CHECK(condition) else foo();
检查有符号变量
class Slice { .... char operator[](size_t i) const; .... }; td::Result<int> CellSerializationInfo::get_bits(td::Slice cell) const { .... int last = cell[data_offset + data_len - 1]; if (!last || last == 0x80) {
PVS-Studio诊断消息：
V560条件表达式的一部分始终为false：last == 0x80。 boc.cpp 78
条件的第二部分将永远不会执行，因为在这种情况下，类型为
char是签名的。 当将值赋给
int类型的变量时，将发生符号扩展，因此其值仍将位于[-128，127]范围内，而不是[0，256]。
应当指出的是，
char并不总是被签名的：其行为取决于平台和编译器。 因此，从理论上讲，当在其他平台上构建时，所讨论的条件仍然可以执行。
负数按位移位
template <class Tr> bool AnyIntView<Tr>::export_bits_any(....) const { .... int mask = (-0x100 >> offs) & 0xff; .... }
PVS-Studio诊断消息：
V610未指定的行为。 检查移位运算符“ >>”。 左操作数'-0x100'为负。 bigint.hpp 1925
对负数执行按位右移运算是未指定的行为：不可能事先知道符号是扩展还是填充零。
新后空检查
CellBuilder* CellBuilder::make_copy() const { CellBuilder* c = new CellBuilder(); if (!c) {
PVS-Studio诊断消息：
V668没有任何意义将'c'指针测试为null，因为使用'new'运算符分配了内存。 如果内存分配错误，将生成异常。 CellBuilder.cpp 531
消息说明了一切：如果内存分配失败，程序将引发异常，而不是返回空指针。 这意味着检查毫无意义。
冗余检查
int main(int argc, char* const argv[]) { .... if (!no_env) { const char* path = std::getenv("FIFTPATH"); if (path) { parse_include_path_set(path ? path : "/usr/lib/fift", source_include_path); } } .... }
PVS-Studio诊断消息：
V547表达式“路径”始终为真。 第136章
此摘录摘自项目的内部实用程序之一。 在这种情况下，三元运算符是多余的：它检查的条件已经由前面的
if语句检查了。 当开发人员决定放弃使用标准路径时，看起来好像忘记了删除该三元运算符（帮助消息中至少没有提及那些路径）。
未使用的变量
bool Op::set_var_info_except(const VarDescrList& new_var_info, const std::vector<var_idx_t>& var_list) { if (!var_list.size()) { return set_var_info(new_var_info); } VarDescrList tmp_info{new_var_info}; tmp_info -= var_list; return set_var_info(new_var_info);
PVS-Studio诊断消息：
V1001分配了'tmp_info'变量，但未在功能结束时使用。 分析器.cpp 140
开发人员显然将在此函数的最后一行中使用一个名为
tmp_info的变量。 这是相同功能的代码，但带有其他参数说明符：
bool Op::set_var_info_except(VarDescrList&& new_var_info, const std::vector<var_idx_t>& var_list) { if (var_list.size()) { new_var_info -= var_list;
大于或小于？
int compute_compare(const VarDescr& x, const VarDescr& y, int mode) { switch (mode) { case 1:
PVS-Studio诊断消息：
V1037两个或更多案例分支执行相同的操作。 检查行：639，645 Builtins.cpp 639
如果仔细阅读，您会发现该代码缺少<=操作。 确实，案例6应该处理的就是这个操作。 我们可以通过观察两个点来推断出这一点。 第一个是初始化代码：
AsmOp compile_cmp_int(std::vector<VarDescr>& res, std::vector<VarDescr>& args, int mode) { .... if (x.is_int_const() && y.is_int_const()) { r.set_const(compute_compare(x.int_const, y.int_const, mode)); x.unused(); y.unused(); return push_const(r.int_const); } int v = compute_compare(x, y, mode); .... } void define_builtins() { .... define_builtin_func("_==_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 2)); define_builtin_func("_!=_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 5)); define_builtin_func("_<_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 4)); define_builtin_func("_>_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 1)); define_builtin_func("_<=_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 6)); define_builtin_func("_>=_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 3)); define_builtin_func("_<=>_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 7)); .... }
如您所见，
define_builtins函数包含对
<=运算符的调用
compile_cmp_int ，其mode参数设置为6。
第二点是
compile_cmp_int函数本身，其中列出了操作名称：
AsmOp compile_cmp_int(std::vector<VarDescr>& res, std::vector<VarDescr>& args, int mode) { .... static const char* cmp_names[] = {"", "GREATER", "EQUAL", "GEQ", "LESS", "NEQ", "LEQ", "CMP"}; .... return exec_op(cmp_names[mode], 2); }
索引6对应于
LEQ词，表示“小于或等于”。
这是
比较函数中发现的另一类不错的错误。
杂项
#define VM_LOG_IMPL(st, mask) \ LOG_IMPL_FULL(get_log_interface(st), ...., VERBOSITY_NAME(DEBUG), \ (get_log_mask(st) & mask) != 0, "")
PVS-Studio诊断消息：
V1003宏“ VM_LOG_IMPL”是危险的表达。 参数“掩码”必须用括号括起来。 log.h 23
VM_LOG_IMPL宏不安全。 它的第二个参数未括在括号中，如果将复杂的表达式传递给该条件，则有可能引起不良的副作用。 但是，如果
mask只是一个常量，那么此代码将完全没有问题地运行。 也就是说，没有什么可以阻止您将其他任何内容传递给宏。
结论
TON的体积很小，因此在该处几乎找不到bug，Telegram开发人员团队当然应该对此予以称赞。 但是每个人都时不时地犯错误，即使是这些人。 代码分析器是功能强大的工具，即使在最优质的代码库中，也可以在早期开发阶段检测源代码中的危险点，因此请不要忽略它们。 静态分析并不是要不时地运行，而应该成为开发过程的一部分：“
在过程中引入静态分析，而不仅仅是使用它来搜索错误 ”。