Get best of the week

使用Linux的PVS-Studio C＃进行单行代码或Nethermind验证

图片1

本文致力于启动针对Linux的PVS-Studio C＃beta测试以及Rider插件。出于如此奇妙的原因，使用这些工具，我们检查了Nethermind产品的源代码，在本文中，我们将介绍一些有趣且有时很有趣的错误。

Nethermind是Linux，Windows，MacOs的.NET Core以太坊的快速客户端。设置以太坊或dApps专用网络时，可以在项目中使用它。Nethermind是开放源代码位于GitHub上。该项目成立于2017年，并且在不断发展。

介绍


你喜欢体力劳动吗？例如，例如在程序代码中查找错误。同意，阅读和分析您编写的网站或整个项目以寻找一些棘手的bug相当繁琐。好吧，如果项目很小，那么假设有5,000条线，但是如果它的规模已经超过了十万或一百万条线呢？而且，它是由不同的程序员编写的，有时不是很容易理解的形式。在这种情况下该怎么办？您是否真的必须睡在某个地方，一个被低估的地方以及100％的时间才能意识到所有这些无尽的线索，以便了解这个令人讨厌的错误在哪里？我怀疑您想这样做。那么该怎么办，也许有现代化的方法可以使它自动化？

图3

然后像静态代码分析器这样的工具开始起作用。静态分析器是用于检测程序源代码中的缺陷的工具。与手动验证相比，此工具的优势如下：

  • 几乎不花时间寻找错误的位置（至少肯定比用眼睛寻找失败的复制粘贴要快）；
  • 不累，不像一个人经过一段时间的搜索后需要休息；
  • 知道很多人甚至可能不知道的错误模式；
  • 使用诸如数据流分析，符号执行，模式匹配等技术；
  • 使您能够定期和随时进行分析；
  • 等等

当然，使用静态代码分析器不会替换或取消代码检查。但是代码审查变得越来越有效率和有用。您可以专注于发现高级错误，转移知识，而不是费力地阅读代码以查找错别字。

如果您有兴趣阅读更多有关它的内容，那么建议您阅读以下文章，以及有关PVS-Studio中使用的技术的文章。

适用于Linux / macOS的PVS-Studio C＃


目前，我们正在将C＃分析器移植到.NET Core，并且我们也在积极开发IDE Rider的插件。

如果您有兴趣，可以通过填写此页面上的表格来注册Beta测试。安装说明将发送到您的邮件中（不用担心，这很简单）以及使用分析仪的许可。

这是带有PVS-Studio插件的Rider的外观：

图片4

有点愤慨


我想说的是，在某些地方很难阅读Nethermind的源代码，因为其中300-500个字符的行很正常。是的，所有代码都没有一行格式化。例如，这些行包含多个三元运算符和逻辑运算符，并且那里什么也没有。通常，享受从王座游戏的最后一个赛季开始。

我将解释一些以了解范围。我有一台长约82厘米的UltraWide显示器。如果您以全屏方式在其上打开IDE，则它将适合340个字符。也就是说，我所谈论的界限甚至不合适。如果您想看一下它的外观，我在GitHub上留下了指向文件的链接：

示例1

private void LogBlockAuthorNicely(Block block, ISyncPeer syncPeer)
{
    string authorString = (block.Author == null ? null : "sealed by " +
(KnownAddresses.GoerliValidators.ContainsKey(block.Author) ?
KnownAddresses.GoerliValidators[block.Author] : block.Author?.ToString())) ??
(block.Beneficiary == null ? string.Empty : "mined by " +
(KnownAddresses.KnownMiners.ContainsKey(block.Beneficiary) ?
KnownAddresses.KnownMiners[block.Beneficiary] : block.Beneficiary?.ToString()));
    if (_logger.IsInfo)
    {
        if (_logger.IsInfo) _logger.Info($"Discovered a new block
{string.Empty.PadLeft(9 - block.Number.ToString().Length, '
')}{block.ToString(Block.Format.HashNumberAndTx)} {authorString}, sent by
{syncPeer:s}");
    }
}

→ 文件链接

示例2

private void BuildTransitions()
{
    ...
    releaseSpec.IsEip1283Enabled = (_chainSpec.Parameters.Eip1283Transition ??
long.MaxValue) <= releaseStartBlock &&
((_chainSpec.Parameters.Eip1283DisableTransition ?? long.MaxValue) 
> releaseStartBlock || (_chainSpec.Parameters.Eip1283ReenableTransition ??
long.MaxValue) <= releaseStartBlock);           
    ...
}

→ 链接到文件

public void 
Will_not_reject_block_with_bad_total_diff_but_will_reset_diff_to_null()
{
    ...
    _syncServer = new SyncServer(new StateDb(), new StateDb(), localBlockTree,
NullReceiptStorage.Instance, new BlockValidator(Always.Valid, new
HeaderValidator(localBlockTree, Always.Valid, MainnetSpecProvider.Instance,
LimboLogs.Instance), Always.Valid, MainnetSpecProvider.Instance, 
LimboLogs.Instance), Always.Valid, _peerPool, StaticSelector.Full, 
new SyncConfig(), LimboLogs.Instance);
    ...     
}

→ 链接到文件

现在想象在这样的部分中发生错误，寻找它会令人愉悦吗？我确定不是这样，每个人都知道不可能这样写。顺便说一句，这个项目中有一个类似的地方出错。

验证结果


不喜欢0的条件


条件1

public ReceiptsMessage Deserialize(byte[] bytes)
{
    if (bytes.Length == 0 && bytes[0] == Rlp.OfEmptySequence[0])
        return new ReceiptsMessage(null);
    ...
}

PVS-Studio警告：V3106可能索引超出范围。索引“ 0”指向“字节”界限之外。Nethermind.Network ReceiptsMessageSerializer.cs 50

要注意错误，请考虑数组中元素的数量为0的情况。然后，bytes.Length == 0条件为true，并且访问数组element时将引发类型为0的indexOutOfRangeException。

他们想在数组为空或0元素等于某个值时立即退出此方法，但似乎他们不小心混淆了“ ||” 与“ &&”。我建议按以下方式解决此问题：

public ReceiptsMessage Deserialize(byte[] bytes)
{
    if (bytes.Length == 0 || bytes[0] == Rlp.OfEmptySequence[0])
        return new ReceiptsMessage(null);
    ...
}

条件2

public void DiscoverAll()
{
    ...
    Type? GetStepType(Type[] typesInGroup)
    {
        Type? GetStepTypeRecursive(Type? contextType)
        {
            ...
        }
        ...
        return typesInGroup.Length == 0 ? typesInGroup[0] :
               GetStepTypeRecursive(_context.GetType());
    }
    ...
}

PVS-Studio警告：V3106可能索引超出范围。索引“ 0”指向“ typesInGroup”界限之外。Nethermind.Runner EthereumStepsManager.cs 70

发生类似于上述情况。如果typesInGroup中的元素数等于0，则在访问0元素时，将抛出IndexOutOfRangeException类型的异常。

仅在这种情况下，我才不了解开发人员的需求。最有可能的是，您需要编写null，而不是typesInGroup [0]。

错误或未完成的优化？


private void DeleteBlocks(Keccak deletePointer)
{
   ...
   if (currentLevel.BlockInfos.Length == 1)
   {
      shouldRemoveLevel = true;
   }
   else
   {
      for (int i = 0; i < currentLevel.BlockInfos.Length; i++)
      {
         if (currentLevel.BlockInfos[0].BlockHash == currentHash) // <=
         {
            currentLevel.BlockInfos = currentLevel.BlockInfos
                                      .Where(bi => bi.BlockHash != currentHash)
                                      .ToArray();
            break;
         }
      }
   }
   ...
}

PVS-Studio警告：V3102通过循环内的常量索引可疑访问'currentLevel.BlockInfos'对象的元素。 Nethermind.Blockchain BlockTree.cs 895

乍看之下，该错误是显而易见的-循环旨在遍历currentLevel.BlockInfos元素，但是在访问currentLevel.BlockInfos [i]时，他们编写了currentLevel.BlockInfos [0]。我们在i上固定0，任务完成。但是不要着急，让我们弄清楚。

现在，我们通过Length访问null元素的BlockHash。如果等于currentHash，那么我们从currentLevel.BlockInfos中获取所有不相等的元素currentHash，对其进行写入并退出循环。事实证明，该循环是多余的。

我认为以前有一种算法，他们决定使用linq进行更改/优化，但是出了点问题。现在，如果条件为假，我们将得到无意义的迭代。

顺便说一句，如果编写此文档的开发人员将使用增量分析模式，那么他立即意识到有些问题是错误的，并且可以快速修复所有问题。此刻，我将像这样重写代码：

private void DeleteBlocks(Keccak deletePointer)
{
    ...
    if (currentLevel.BlockInfos.Length == 1)
    {
        shouldRemoveLevel = true;
    }
    else
    {
        currentLevel.BlockInfos = currentLevel.BlockInfos
                                  .Where(bi => bi.BlockHash != currentHash)
                                  .ToArray();
    }
    ...
}

取消引用空引用


解除引用1

public void Sign(Transaction tx, int chainId)
{
    if (_logger.IsDebug)
        _logger?.Debug($"Signing transaction: {tx.Value} to {tx.To}");
    IBasicWallet.Sign(this, tx, chainId);
}

PVS-Studio警告：V3095在验证是否为null之前，已使用'_logger'对象。检查行：118，118。Nethermind.Wallet DevKeyStoreWallet.cs 118

错误的顺序错误。首先是要上诉_logger.IsDebug只有经过是_logger检查为空。因此，在_logger为null的情况下，我们将获得NullReferenceException类型的异常。

取消引用2

private void BuildNodeInfo()
{
    _nodeInfo = new NodeInfo();
    _nodeInfo.Name = ClientVersion.Description;
    _nodeInfo.Enode = _enode.Info;                           // <=
    byte[] publicKeyBytes = _enode?.PublicKey?.Bytes;        // <=
    _nodeInfo.Id = (publicKeyBytes == null ? Keccak.Zero :
                   Keccak.Compute(publicKeyBytes)).ToString(false);
    _nodeInfo.Ip = _enode?.HostIp?.ToString();
    _nodeInfo.ListenAddress = $"{_enode.HostIp}:{_enode.Port}";
    _nodeInfo.Ports.Discovery = _networkConfig.DiscoveryPort;
    _nodeInfo.Ports.Listener = _networkConfig.P2PPort;
    UpdateEthProtocolInfo();
}

PVS-Studio警告：V3095在验证为空之前使用了'_enode'对象。检查行：55、56。Nethermind.JsonRpc AdminModule.cs 55该

错误与上述完全相同，仅这次是罪魁祸首是_enode。

我要补充一点，如果您忘记检查是否为null，那么仅在程序崩溃时记住这一点。分析仪会提醒您这一点，一切都会好起来的。

我们最喜欢的复制粘贴


情况1

public static bool Equals(ref UInt256 a, ref UInt256 b)
{
    return a.s0 == b.s0 && a.s1 == b.s1 && a.s2 == b.s2 && a.s2 == b.s2;
}

PVS-Studio警告：V3001 '&&'运算符的左侧和右侧有相同的子表达式'a.s2 == b.s2'。Nethermind.Dirichlet.Numerics UInt256.cs 1154

在此相同条件被检查2次：

a.s2 == b.s2

由于参数a和b具有字段s3，因此我假设在复制时我们只是忘记将s2更改为s3。

事实证明，参数相等的频率比代码作者期望的更频繁。同时，一些开发人员认为他们不这样做，而是开始在一个完全不同的地方寻找错误，这花费了很多精力和神经。

顺便说一下，比较函数中的错误通常是经典的。显然，程序员认为这样的功能很简单，所以他们的写作非常随意和漫不经心。证明。知道了这一点，现在要小心:)！

情况二

public async Task<ApiResponse> 
PublishBlockAsync(SignedBeaconBlock signedBlock,
                  CancellationToken cancellationToken)
{
    bool acceptedLocally = false;
    ...
    if (acceptedLocally)
    {
        return new ApiResponse(StatusCode.Success);
    }
    else
    {
        return new ApiResponse(StatusCode.Success);
    }
    ...
}

PVS-Studio警告：V3004'then '语句等效于'else'语句。 Nethermind.BeaconNode BeaconNodeFacade.cs 177

对于acceptedLocally变量的任何值，该方法将返回相同的值。很难说这是否是一个错误。假设程序员复制了一行并忘记将StatusCode.Success更改为其他内容，那么这是一个真正的错误。此外，StatusCode具有InternalError和InvalidRequest。但是代码重构很可能是罪魁祸首，无论如何我们都不在乎acceptedLocally的值，在这种情况下，条件变成让您坐下来思考是否错误的地方。因此，无论如何，这种情况是非常不愉快的。

情况3

public void TearDown()
{
    ...
    foreach (var testResult in _results)
    {
         string message = $"{testResult.Order}. {testResult.Name} has " 
               + $"{(testResult.Passed ? "passed [+]" : "failed [-]")}";
         if (testResult.Passed)
         {
               TestContext.WriteLine(message);
         }
         else
         {
               TestContext.WriteLine(message);
         }
     }
}

PVS-Studio警告：V3004'then '语句等效于'else'语句。Nethermind.Overseer.Test TestBuilder.cs 46

同样，我们也不关心验证，因为最终我们得到的是相同的东西。我们再次坐下来苦苦思索，但是开发人员想在这里写些什么。通过使用静态分析并立即修复此类含糊的代码，可以避免浪费时间。

案例4

public void Setup()
{
    if (_decoderBuffer.ReadableBytes > 0)
    {
        throw new Exception("decoder buffer");
    }

    if (_decoderBuffer.ReadableBytes > 0)
    {
        throw new Exception("decoder buffer");
    }
    ...
}

PVS-Studio警告：V3021有两个带有相同条件表达式的'if'语句。第一个'if'语句包含方法return。这意味着第二个“ if”语句是毫无意义的Nethermind.Network.Benchmark InFlowBenchmarks.cs 55

只需再次按Ctrl + V即可。我们删除了多余的支票，一切都很好。我敢肯定，如果其他一些条件在这里很重要，那么每个人都会在一个写，如果通过逻辑运算符，一

箱5

private void LogBlockAuthorNicely(Block block, ISyncPeer syncPeer)
{
    if (_logger.IsInfo)
    {
        if (_logger.IsInfo)
        {
            ...
        }
    }
}

PVS-Studio警告：V3030定期检查。'_logger.IsInfo'条件已在第242行中得到验证。Nethermind.Synchronization SyncServer.cs 244

与第四种情况一样，将执行额外的检查。但是，区别在于_logger不仅具有一个属性，而且还具有例如' bool IsError {get; } '。因此，代码可能看起来像这样：

private void LogBlockAuthorNicely(Block block, ISyncPeer syncPeer)
{
    if (_logger.IsInfo)
    {
        if (!_logger.IsError) // <=
        {
            ...
        }
    }
}

好吧，或者代码重构的所有错误，不再需要检查。

案例6

if (missingParamsCount != 0)
{
    bool incorrectParametersCount = missingParamsCount != 0; // <=
    if (missingParamsCount > 0)
    {
        ...
    }
    ...
}

PVS-Studio 警告：V3022表达式'missingParamsCount！= 0'始终为true。Nethermind.JsonRpc JsonRpcService.cs 127

检查条件（missingParamsCount！= 0），如果为true，则再次进行计算并将结果分配给变量。同意这是一种相当真实的写作方式。

令人困惑的支票


public async Task<long> 
DownloadHeaders(PeerInfo bestPeer, 
                BlocksRequest blocksRequest, 
                CancellationToken cancellation)
{
  ...
  for (int i = 1; i < headers.Length; i++)
  {
    ...
    BlockHeader currentHeader = headers[i];
    ...
    bool isValid = i > 1 ? 
        _blockValidator.ValidateHeader(currentHeader, headers[i - 1], false):
        _blockValidator.ValidateHeader(currentHeader, false);
    ...
    if (HandleAddResult(bestPeer, 
                        currentHeader, 
                        i == 0,                              // <=
                        _blockTree.Insert(currentHeader))) 
    {
       headersSynced++;
    }

    ...
  }
  ...
}

PVS-Studio 警告：V3022表达式'i == 0'始终为false。Nethermind.Synchronization BlockDownloader.cs 192

让我们按顺序开始。初始化时，变量i被赋值为1。接下来，变量仅递增；因此，该值将始终传递给函数false。

现在让我们看一下HandleAddResult：

private bool HandleAddResult(PeerInfo peerInfo, 
                             BlockHeader block,
                             bool isFirstInBatch, 
                             AddBlockResult addResult)
{
    ...
    if (isFirstInBatch)
    {
        ...
    }
    else
    {
        ...
    }
    ...
}

在这里，我们对isFirstInBatch感兴趣。根据此参数的名称判断，它负责确定某方是否是第一个。嗯首先 让我们在上面再次查看，发现使用i有2个调用：

BlockHeader currentHeader = headers[i];
_blockValidator.ValidateHeader(currentHeader, headers[i - 1], false)

不要忘了，在这种情况下，倒计时从1变为事实证明，我们有2个选择：要么“第一”的意思指数1下一个元素，或在索引为0。但在任何情况下，这将等于我。

事实证明，函数调用应如下所示：

HandleAddResult(bestPeer, currentHeader, 
                i == 1, _blockTree.Insert(currentHeader))

或像这样：

HandleAddResult(bestPeer, currentHeader, 
                i - 1 == 0, _blockTree.Insert(currentHeader))

同样，如果开发人员经常使用静态分析器，那么他将编写此代码并看到警告，将迅速修复它并享受生活。

优先权 ？？


情况1

public int MemorySize
{
  get
  {
    int unaligned = (Keccak == null ? MemorySizes.RefSize : 
        MemorySizes.RefSize + Keccak.MemorySize) 
        + (MemorySizes.RefSize + FullRlp?.Length 
                                 ?? MemorySizes.ArrayOverhead)   // <=
        + (MemorySizes.RefSize + _rlpStream?.MemorySize 
                                 ?? MemorySizes.RefSize)         // <=
        + MemorySizes.RefSize + (MemorySizes.ArrayOverhead + _data?.Length 
        * MemorySizes.RefSize ?? MemorySizes.ArrayOverhead) 
        + MemorySizes.SmallObjectOverhead + (Key?.MemorySize ?? 0);
    return MemorySizes.Align(unaligned);
  }
}

PVS-Studio警告：

  • V3123也许是“ ??” 操作员的工作方式与预期不同。它的优先级低于其左侧的其他运营商的优先级。Nethermind.Trie TrieNode.cs 43
  • V3123也许是“ ??” 操作员的工作方式与预期不同。它的优先级低于其左侧的其他运营商的优先级。Nethermind.Trie TrieNode.cs 44

分析器建议您检查我们如何使用“ ??”运算符，并了解问题所在，因此建议考虑以下情况。我们在这里看这行：

(MemorySizes.RefSize + FullRlp?.Length ?? MemorySizes.ArrayOverhead)

MemorySizes.RefSize和MemorySizes.ArrayOverhead是常量：

public static class MemorySizes
{
    ...
    public const int RefSize = 8;
    public const int ArrayOverhead = 20;
    ...
}

因此，为清楚起见，我建议重写字符串，替换其值：

(8 + FullRlp?.Length ?? 20)

现在假设FullRlp为空。然后（8 + null）将为null 。接下来，我们得到了表达（空?? 20），这将返回20.

事实证明，只要FullRlp是空，从价值MemorySizes.ArrayOverhead总是会回来，不管是什么存储在MemorySizes.RefSize。下一行的片段类似。

但是问题是，开发人员是否想要这种行为？让我们看一下下面的行：

MemorySizes.RefSize + (MemorySizes.ArrayOverhead 
    + _data?.Length * MemorySizes.RefSize ?? MemorySizes.ArrayOverhead)

在这里，与上面讨论的部分一样，将MemorySizes.RefSize添加到表达式中，但是

请注意，在第一个“ +”运算符之后有一个括号。事实证明，我们应该向MemorySizes.RefSize添加一些表达式，如果它为null，那么我们将添加另一个表达式。因此，代码应如下所示：

public int MemorySize
{
  get
  {
    int unaligned = (Keccak == null ? MemorySizes.RefSize : 
       MemorySizes.RefSize + Keccak.MemorySize) 
       + (MemorySizes.RefSize + (FullRlp?.Length 
                                 ?? MemorySizes.ArrayOverhead))    // <=
       + (MemorySizes.RefSize + (_rlpStream?.MemorySize 
                                 ?? MemorySizes.RefSize))          // <=
       + MemorySizes.RefSize + (MemorySizes.ArrayOverhead + _data?.Length 
       * MemorySizes.RefSize ?? MemorySizes.ArrayOverhead) 
       + MemorySizes.SmallObjectOverhead + (Key?.MemorySize ?? 0);
    return MemorySizes.Align(unaligned);
  }
}

同样，这只是一个假设，但是，如果开发人员想要其他行为，则应该明确指出：

((MemorySizes.RefSize + FullRlp?.Length) ?? MemorySizes.ArrayOverhead)

然后，阅读此代码的人就不必长时间研究这里发生的事情以及程序员想要的东西。

情况二

private async Task<JsonRpcResponse> 
ExecuteAsync(JsonRpcRequest request, 
             string methodName,
             (MethodInfo Info, bool ReadOnly) method)
{
    var expectedParameters = method.Info.GetParameters();
    var providedParameters = request.Params;
    ...
    int missingParamsCount = expectedParameters.Length 
            - (providedParameters?.Length ?? 0) 
            + providedParameters?.Count(string.IsNullOrWhiteSpace) ?? 0; // <=
    if (missingParamsCount != 0)
    {
        ...
    }
    ...
}

PVS-Studio警告：V3123也许是“ ??” 操作员的工作方式与预期不同。它的优先级低于其左侧的其他运营商的优先级。Nethermind.JsonRpc JsonRpcService.cs 123

同样，该操作的优先级是“ ??”，因此，正如上一次一样，我们考虑这种情况。我们在这里看这行：

expectedParameters.Length 
            - (providedParameters?.Length ?? 0) 
            + providedParameters?.Count(string.IsNullOrWhiteSpace) ?? 0;

假设providedParameters为空，然后为清楚起见，请立即更换相关的一切providedParameters与空，并替换一个随机值，而不是expectedParameters.Length：

100 - (null ?? 0) + null ?? 0;

现在，我们立即注意到有两项类似的检查，只有一种情况下有括号，而另一种情况下有括号。让我们运行这个例子。首先我们得到（null ?? 0）将返回0，然后从100中减去0并得到100：

100 + null ?? 0;

现在，我们不再完全执行“ null ?? 0 ”，最后得到（100 + 0），而不是。

首先它将执行（100 + null），然后得到null。然后，它检查（null ?? 0），这导致missingParamsCount变量的值为0。

由于下一个条件是missingParamsCount等于零，因此我们可以假定这是开发人员所寻求的行为。我会说，为什么不放在方括号中并明确表达我的想法呢？通常，此检查可能是由于误解了有时返回0的原因，而这仅仅是拐杖。

再一次，尽管我们可能没有这样做，但我们还是在浪费时间；在编写代码时使用增量分析模式。

结论


最后，我希望我能向您传达的是，静态分析仪是您的朋友，而不是邪恶的监督者，他只是在等您犯错。

还应该注意的是，通过一次使用或很少使用分析仪，您当然会发现错误，并且其中一些错误甚至会很快得到解决，但也有一些错误需要您的注意。因此，您需要不断使用静态分析仪。然后，您将发现更多错误，并在编写程序代码时纠正了这些错误，并且您完全了解要执行的操作。

一个简单的事实是，每个人都会犯错，这很正常。我们都从错误中学习，但只能从那些能够注意到和理解的错误中学习。因此，请使用现代工具搜索这些错误，例如-PVS-Studio。谢谢您的关注。



如果您想与讲英语的读者分享这篇文章，请使用以下链接：Nikolay Mironov。使用Linux的PVS-Studio C＃进行单行代码或Nethermind的检查。

More articles:


All Articles