奇怪的代码如何隐藏错误?TensorFlow.NET分析

TensorFlow.NET和PVS-Studio

静态分析对于任何开发人员都是非常有用的工具,因为它不仅可以及时发现错误,而且还可以及时发现可疑和奇怪的代码片段,这些代码可能会使将来不得不与他合作的程序员感到困惑。将通过对TensorFlow.NET开放C#项目进行分析来证明这一想法,该项目正在开发中,可与流行的TensorFlow机器学习库一起使用。

我叫Nikita Lipilin。前一段时间,我加入了PVS-Studio的C#程序员部门。传统上,团队的所有新手都会撰写文章,讨论使用PVS-Studio静态分析器检查各种打开的项目的结果。这些文章可帮助新员工更好地了解产品,同时在推广静态分析方法方面提供更多好处。我建议您使自己熟悉有关开放项目分析的第一篇工作。

介绍


程序代码中各种可能的错误是惊人的。在对所创建的应用程序进行表面检查后,其中一些会立即显示出来,即使在由经验丰富的开发人员团队进行代码审查时,也可能很难注意到其他人。但是,由于疏忽大意或其他原因,程序员有时还会编写简单而又不合逻辑的代码,尽管如此,这些代码(似乎)已成功完成其功能。但只有更进一步,当返回到所写内容时,或当其他人学习该代码时,就会出现无法回答的问题。

重构旧代码可能会导致问题,尤其是在程序的其他部分依赖于旧代码的情况下,因此,即使您发现某些坦率的弯曲结构,也可以解决问题。别碰它! ”。随后,这使得难以研究源,因此,变得难以扩展可用功能。代码库被阻塞,有可能无法及时解决一些小而看不见但潜在的非常不愉快的内部问题。

在某个时候,会感觉到该错误的后果,但是它的搜索将花费很多时间,因为开发人员的怀疑将取决于大量无法一次重构的奇怪代码片段。由此得出结论,特定片段中的各种问题和奇怪之处应在写入后立即予以纠正。如果有合理的理由将所有内容保留为原样(例如,如果代码在以后的“''中写为某种空白“”),则此片段应附有解释性注释。

还要注意的是,不管开发人员的资格如何,某些有问题的,仅是不成功的时刻都可能从他的视线中溜走,有时可以在某个地方应用“临时解决方案”,该解决方案很快就会被人们遗忘并成为“永久性”。随后,对此类代码的分析(很可能将由其他开发人员参与)将花费大量的精力。

在这种情况下,代码审查会有所帮助。但是,如果任务很严重,则此选项将需要大量时间。此外,当存在许多小错误或不足时,检查人员可能很可能不会注意到高级错误。检查代码成为繁琐的例程,并且审查的有效性逐渐降低。

显然,例行任务将更好地从人转移到计算机。这种方法被用于许多现代领域。各种过程的自动化是繁荣的关键。在本主题中,自动化是什么?

静态分析是解决编写合理且稳定的工作代码的可靠助手。每次在将活动结果发送给评论之前,程序员都将能够进行自动检查,而不会给其他开发人员(和他本人)带来不必要的工作负担。仅在考虑了所有分析仪警告之后,才将代码发送给验证:错误已修复,奇怪的时刻已被重写,或至少由注释解释。

当然,对代码审阅的需求并没有消失,但是静态分析补充并大大简化了其实现。借助分析仪,可以修复大部分错误,并且绝对不会忘记并标记相应的奇怪时刻。因此,通过代码审查,将有可能集中精力验证复杂逻辑交互的实现的正确性,并发现不幸的是,到目前为止,分析器无法检测到潜在的问题。

TensorFlow.NET




本文的灵感来自TensorFlow.NET项目。它表示通过C#代码使用流行的TensorFlow机器学习库的功能的能力的实现(顺便说一下,我们也对其进行了测试)。这个想法似乎很有趣,因为在编写本文时,仅在Python,Java和Go中可以使用库。GitHub上

可用的源代码正在不断更新,现在其数量略超过十万行。经过初步研究后,人们希望使用静态分析进行验证。 PVS-Studio被用作特定工具,已在许多不同项目中证明了其有效性


对于TensorFlow.NET,分析器显示许多警报:39个高级别,227个中级别和154个低级别(您可以在“警告级别和诊断规则集”小节中了解有关警告级别的信息)。对它们每个人的详细分析都会使这篇文章巨大,因此下面仅描述对我来说最有趣的内容。还值得注意的是,在项目中几次遇到了一些发现的问题,对每个这样的代码段的分析不在本文的讨论范围之内。

该项目为自己设定了一个相当严肃的任务,不幸的是,不可避免地会出现各种奇怪的代码片段。在本文中,我将尝试表明静态分析的使用可以大大简化程序员的工作,并指出可能引起问题的领域。警告并不总是表示错误,但是很多时候,它表示会导致人为问题的代码。因此,显着增加了重新设计或在任何情况下进行相应注释的奇怪代码的可能性。

研究分析仪报告时引起关注的片段


实际上,针对该项目的大量分析器警告可以称为不是很多错误,而可以称为奇怪的代码。当查看发出警告的行时,至少会产生困惑。当然,下面的一些示例可能是“临时解决方案”,但它们未对此问题发表任何评论,这意味着将来将使用此代码的人会提出疑问,并寻找答案。将花费额外的时间。


同时,一些警告指向的代码显然不仅奇怪,而且是错误的。这是“奇怪代码”的主要危险-如果您到处都看到奇怪的解决方案并逐渐习惯于代码似乎是错误的事实,那么很难注意到真正的错误。

精致的收藏之旅


private static void _RemoveDefaultAttrs(....)
{
  var producer_op_dict = new Dictionary<string, OpDef>();
  producer_op_list.Op.Select(op =>
  {
    producer_op_dict[op.Name] = op;
    return op;
  }).ToArray();           
  ....
}

分析器警告V3010需要使用函数“ ToArray”的返回值。 importer.cs 218

分析器认为此时对Toray的调用可疑,因为此函数返回的值未分配给变量。但是,这样的代码不是错误。该结构用于使用与producer_op_list.Op列表的元素相对应的值来填充producer_op_dict字典。必须调用ToArray,以便为集合中的所有元素调用作为参数传递给Select方法的函数

我认为该代码看起来并不是最好的。填写字典有些不明显,有些开发人员可能希望删除ToArray的“不必要”调用在这里使用foreach循环会更容易理解

var producer_op_dict = new Dictionary<string, OpDef>();

foreach (var op in producer_op_list.Op)
{
  producer_op_dict[op.Name] = op;
}

在这种情况下,代码看起来尽可能简单。

另一个类似的时刻看起来像这样:

public GraphDef convert_variables_to_constants(....)
{
  ....
  inference_graph.Node.Select(x => map_name_to_node[x.Name] = x).ToArray();
  ....
}

分析器警告V3010需要使用函数“ ToArray”的返回值。graph_util_impl.cs 48

唯一的区别是,这样的记录看起来更加简洁,但是只有删除ToArray调用的诱惑并没有消失,并且看起来仍然不明显。

临时解决方案


public GraphDef convert_variables_to_constants(....)
{
  ....
  var source_op_name = get_input_name(node);
  while(map_name_to_node[source_op_name].Op == "Identity")
  {
    throw new NotImplementedException);
    ....
  }
  ....
}

分析器警告V3020循环内无条件的“抛出”。 graph_util_impl.cs 73

在所考虑的项目中,通常使用以下方法:如果以后要实现某些行为,在适当的位置抛出NotImplementedException。很明显,为什么分析器会在这篇文章中警告可能的错误:使用while代替if看起来并不太合理。

这不是由于使用临时解决方案而出现的唯一警告。例如,有一个方法:

public static Tensor[] _SoftmaxCrossEntropyWithLogitsGrad(
  Operation op, Tensor[] grads
)
{
  var grad_loss = grads[0];
  var grad_grad = grads[1];
  var softmax_grad = op.outputs[1];
  var grad = _BroadcastMul(grad_loss, softmax_grad);

  var logits = op.inputs[0];
  if(grad_grad != null && !IsZero(grad_grad)) // <=
  {
    throw new NotImplementedException("_SoftmaxCrossEntropyWithLogitsGrad");
  }

  return new Tensor[] 
  {
    grad,
    _BroadcastMul(grad_loss, -nn_ops.log_softmax(logits))
  };
}

分析器警告V3022表达式'grad_grad!= Null &&!IsZero(grad_grad)'始终为false。nn_grad.cs 93

实际上,将永远不会引发NotImplementedException(“ _ SoftmaxCrossEntropyWithLogitsGrad”),因为此代码根本无法访问。为了阐明原因,您必须转到IsZero函数的代码

private static bool IsZero(Tensor g)
{
  if (new string[] { "ZerosLike", "Zeros" }.Contains(g.op.type))
    return true;

  throw new NotImplementedException("IsZero");
}

该方法返回true或引发异常。此代码不是错误-显然,此处的实现留待以后使用。最主要的是“那么”真的到了。好吧,事实证明,PVS-Studio不会让您忘记这里有这样的缺陷是非常成功的:)

张量是张量吗?


private static Tensor[] _ExtractInputShapes(Tensor[] inputs)
{
  var sizes = new Tensor[inputs.Length];
  bool fully_known = true;
  for(int i = 0; i < inputs.Length; i++)
  {
    var x = inputs[i];

    var input_shape = array_ops.shape(x);
    if (!(input_shape is Tensor) || input_shape.op.type != "Const")
    {
      fully_known = false;
      break;
    }

    sizes[i] = input_shape;
  }
  ....
}

分析器警告V3051类型检查过多。该对象已经是“张量”类型。array_grad.cs 154 shape

方法的返回类型Tensor所以input_shape是Tensor检查看起来至少很奇怪。也许,一旦方法返回了不同类型的值并且验证就有意义了,但条件也有可能代替该Tensor来指定此类的继承人。开发人员应以某种方式注意此片段。

彻底检查


public static Tensor[] _BaseFusedBatchNormGrad(....)
{
  ....
  if (data_format == "NCHW") // <=
    throw new NotImplementedException("");

  var results = grad_fun(new FusedBatchNormParams
  {
    YBackprop = grad_y,
    X = x,
    Scale = scale,
    ReserveSpace1 = pop_mean,
    ReserveSpace2 = pop_var,
    ReserveSpace3 = version == 2 ? op.outputs[5] : null,
    Epsilon = epsilon,
    DataFormat = data_format,
    IsTraining = is_training
  });

  var (dx, dscale, doffset) = (results[0], results[1], results[2]);
  if (data_format == "NCHW") // <=
    throw new NotImplementedException("");

  ....
}

分析仪警告

  • V3021有两个带有相同条件表达式的'if'语句。第一个'if'语句包含方法return。这意味着第二个“ if”语句是毫无意义的nn_grad.cs 230
  • V3022表达式'data_format ==“ NCHW”'始终为false。nn_grad.cs 247

与前面的一些示例不同,代码显然有问题。第二次检查没有任何意义,因为如果条件为真,那么程序的执行将根本无法实现。也许这里允许打错字,或者检查中的一项通常是多余的。

选择的幻想


public Tensor Activate(Tensor x, string name = null)
{
  ....
  Tensor negative_part;
  if (Math.Abs(_threshold) > 0.000001f)
  {
    negative_part = gen_ops.relu(-x + _threshold);
  } else
  {
    negative_part = gen_ops.relu(-x + _threshold);
  }
  ....
}

分析器警告V3004'then '语句等效于'else'语句。 gen_nn_ops.activations.cs 156

一个相当有趣的演示,说明了在开发中使用静态分析的有效性。很难找到一个合理的理由来说明开发人员编写此特定代码的原因-很可能是典型的复制粘贴错误(尽管这当然可能是另一个“稍后”)。

类似计划的其他部分,例如:

private static Operation _GroupControlDeps(
  string dev, Operation[] deps, string name = null
)
{
  return tf_with(ops.control_dependencies(deps), ctl =>
  {
    if (dev == null)
    {
      return gen_control_flow_ops.no_op(name);
    }
    else
    {
      return gen_control_flow_ops.no_op(name);
    }
  });
}

分析器警告V3004'then '语句等效于'else'语句。control_flow_ops.cs 135

也许一旦检查有意义,但随着时间的流逝,它就消失了,或者将来计划进行一些进一步的更改。但是,任何一个选项似乎都没有足够的理由在代码中留下类似内容,而无需以任何方式解释这种奇怪之处。极有可能以完全相同的方式犯下复制粘贴错误。

迟来的支票


public static Tensor[] Input(int[] batch_shape = null,
  TF_DataType dtype = TF_DataType.DtInvalid,
  string name = null,
  bool sparse = false,
  Tensor tensor = null)
{
  var batch_size = batch_shape[0];
  var shape = batch_shape.Skip(1).ToArray(); // <=

  InputLayer input_layer = null;
  if (batch_shape != null)                   // <=
    ....
  else
    ....

  ....
}

分析器警告V3095在对null进行验证之前,已使用'batch_shape'对象。检查行:39,42。keras.layers.cs 39

一个潜在的变量潜在使用的经典且相当危险的错误,它是无处链接。在这种情况下,代码明确暗示batch_shapenull的可能性-从参数列表和随后对同一变量的验证中都可以清楚地看出。因此,分析仪在此指示明显的错误。

另一个“待会儿”?


public MnistDataSet(
  NDArray images, NDArray labels, Type dataType, bool reshape // <=
) 
{
  EpochsCompleted = 0;
  IndexInEpoch = 0;

  NumOfExamples = images.shape[0];

  images = images.reshape(
    images.shape[0], images.shape[1] * images.shape[2]
  );
  images = images.astype(dataType);
  // for debug np.multiply performance
  var sw = new Stopwatch();
  sw.Start();
  images = np.multiply(images, 1.0f / 255.0f);
  sw.Stop();
  Console.WriteLine($"{sw.ElapsedMilliseconds}ms");
  Data = images;

  labels = labels.astype(dataType);
  Labels = labels;
}

分析器警告V3117未使用构造函数参数“ reshape”。MnistDataSet.cs 15

与其他一些奇怪现象一样,这很可能是由于该功能远未完全实现,并且将来很有可能在此构造函数中使用reshape参数但是就目前而言,似乎他正被这样扔在这里。如果确实做到“稍后”,那么最好在注释中加上一些标记。如果不是,那么事实证明构造该对象的代码将不得不向构造函数中添加一个额外的参数,并且很可能不这样做会更方便。

难以捉摸的null取消引用


public static Tensor[] _GatherV2Grad(Operation op, Tensor[] grads)
{
  ....
  if((int)axis_static == 0)
  {
    var params_tail_shape = params_shape.slice(new NumSharp.Slice(start:1));
    var values_shape = array_ops.concat(
      new[] { indices_size, params_tail_shape }, 0
    );
    var values = array_ops.reshape(grad, values_shape);
    indices = array_ops.reshape(indices, indices_size);
    return new Tensor[]
    {
      new IndexedSlices(values, indices, params_shape), // <=
      null,
      null
    };
  }
  ....
}

分析器警告V3146方法中的第一个参数'values'可能为空取消引用。 “ _outputs.FirstOrDefault()”可以返回默认的空值。 array_grad.cs 199

要了解问题所在,您应该首先查看indexedSlices构造函数代码

public IndexedSlices(
  Tensor values, Tensor indices, Tensor dense_shape = null
)
{
  _values = values;
  _indices = indices;
  _dense_shape = dense_shape;

  _values.Tag = this; // <=
}

显然,将null传递给此构造函数将引发异常。但是,为什么分析器认为values变量可能包含null

PVS-Studio使用数据流分析技术,该技术可让您在代码的不同部分中找到变量的可能值集。警告告诉您,可以通过以下行返回指定变量中的null:_outputs.FirstOrDefault()。但是,查看上面的代码,您可以发现values变量的通过调用array_ops.reshape(grad,values_shape)获得的。那么_outputs.FirstOrDefault()在哪里呢?

事实是,在分析数据流时,不仅要考虑当前函数,还要考虑所有函数;PVS-Studio可获取有关任何位置的任何变量的可能值集的信息。因此,警告表示array_ops.reshape(grad,values_shape)的实现包含对_outputs.FirstOrDefault()的调用,最终将返回其结果。

要验证这一点,请转到重塑实现

public static Tensor reshape<T1, T2>(T1 tensor, T2 shape, string name = null)
            => gen_array_ops.reshape(tensor, shape, null);

然后转到内部调用重塑方法
public static Tensor reshape<T1, T2>(T1 tensor, T2 shape, string name = null)
{
  var _op = _op_def_lib._apply_op_helper(
    "Reshape", name, new { tensor, shape }
  );
  return _op.output;
}

_apply_op_helper 函数返回包含输出属性Operation类的对象。在收到其值后,警告中描述的代码将被调用:

public Tensor output => _outputs.FirstOrDefault();

Tensor当然是引用类型,因此其默认值将为null从所有这些可以看出,PVS-Studio精心分析了代码的逻辑结构,深入到了调用的结构中。

分析仪完成工作,表明存在潜在问题的地方。程序员必须检查_outputs中的元素缺失时是否会出现这种情况

因此,静态分析将至少迫使开发人员注意可疑片段,以便评估错误是否可能实际上在那里发生。使用这种方法,可以迅速减少未被注意到的错误数量。

等待不可靠?


private (LoopVar<TItem>, Tensor[]) _BuildLoop<TItem>(
  ....
) where ....
{
  ....
  // Finds the closest enclosing non-None control pivot.
  var outer_context = _outer_context;
  object control_pivot = null;
  while (outer_context != null && control_pivot == null) // <=
  {

  }

  if (control_pivot != null)
  {

  }
  ....
}

分析器警告V3032等待该表达式不可靠,因为编译器可能会优化某些变量。使用易失性变量或同步原语可以避免这种情况。WhileContext.cs 212

分析器指示可以通过编译器优化这种等待的实现,但是我怀疑他们是否真的在此处尝试实现等待-很可能没有添加代码,并且将来会进一步开发。考虑到这种做法在项目的其他地方使用,可能值得在此处抛出NotImplementedException无论如何,我相信一些解释性评论显然不会受到伤害。

打破边界


public TensorShape(int[][] dims)
{
  if(dims.Length == 1)
  {
    switch (dims[0].Length)
    {
      case 0: shape = new Shape(new int[0]); break;
      case 1: shape = Shape.Vector((int)dims[0][0]); break;
      case 2: shape = Shape.Matrix(dims[0][0], dims[1][2]); break; // <=
      default: shape = new Shape(dims[0]); break;
    }
  }
  else
  {
    throw new NotImplementedException("TensorShape int[][] dims");
  }
}

分析器警告V3106可能索引超出范围。索引“ 1”指向“点心”范围之外。 TensorShape.cs 107

在我看到的奇怪代码段中,有一个真正的错误,很难注意到。以下片段在这里是错误的:dims [1] [2]。从一个元素的数组中获取索引为1的元素显然是错误的。同时,如果将片段更改为dims [0] [2],则会出现另一个错误-从dims [0]数组获取索引为2的元素,该数组的长度在这种情况下为2。因此,这个问题原来是好像是“双底”。

无论如何,开发人员应研究并纠正此代码片段。我认为,该示例很好地说明了PVS-Studio中数据流分析的性能。

Olepatka?


private void _init_from_args(object initial_value = null, ....) // <=
{
  var init_from_fn = initial_value.GetType().Name == "Func`1"; // <=
  ....
  tf_with(...., scope =>
  {
    ....
    tf_with(...., delegate
    {
      initial_value = ops.convert_to_tensor(  // <=
        init_from_fn ? (initial_value as Func<Tensor>)():initial_value,
        name: "initial_value",
        dtype: dtype
      );
    });
    _shape = shape ?? (initial_value as Tensor).TensorShape;
    _initial_value = initial_value as Tensor; // <=
    ....
    _dtype = _initial_value.dtype.as_base_dtype(); // <=

    if (_in_graph_mode)
    {
      ....

      if (initial_value != null) // <=
      {
        ....
      }

      ....
    }

    ....
  });
}

为了理解上面的代码,还值得介绍tf_with函数的实现:

[DebuggerStepThrough] // with "Just My Code" enabled this lets the 
[DebuggerNonUserCode()]  //debugger break at the origin of the exception
public static void tf_with<T>(
  T py, Action<T> action
) where T : ITensorFlowObject
{
  try
  {
    py.__enter__();
    action(py);
  }
  finally
  {
    py.__exit__();
    py.Dispose();
  }
}

分析器警告:V3019类型转换后使用'as'关键字可能会将不正确的变量与null进行比较。检查变量“ initial_value”,“ _ initial_value”。 ResourceVariable.cs 137

_init_from_args是相当庞大的函数,因此已省略了许多片段。您可以通过单击链接完全看到它。尽管起初警告对我来说似乎并不严重,但经过研究,我意识到代码肯定有问题。

首先,应注意,可以在不传递参数的情况下调用该方法,默认情况下,initial_value中的该方法null。在这种情况下,第一行将引发异常。

其次,验证initial_valuenull不等式看起来很奇怪:如果在调用ops.convert_to_tensor之后initial_value确实变为null,则_initial_value将为null,这意味着调用_initial_value.dtype.as_base_dtype()也将引发异常。 分析器提示您可能需要检查null是否为_initial_value,但如上所述,引用此变量发生在此测试之前,因此此选项也将不正确。 如果没有PVS-Studio,在如此庞大的功能中会注意到这个小错误吗?我非常怀疑。





结论


在一个包含许多奇怪代码示例的项目中,很多问题都可能被隐藏。程序员习惯于看到不可理解的事物,不再同时注意到错误。其后果可能是非常可悲的。的确,在分析器警告中也有错误的警告,但是,在大多数情况下,警告至少指示当人们查看时可能引起问题的代码片段。在故意编写奇怪的代码的情况下,值得留下一些解释,以便该片段对于以后将使用此代码的开发人员来说是清楚的(即使这意味着自己留下注释)。

同时,静态分析工具(例如PVS-Studio)可以极大地帮助您发现潜在的错误和奇怪之处,使它们可见而不会被遗忘,并且随后对所有临时解决方案进行完善并转化为干净,结构化和稳定的工作方式编码。


如果您想与说英语的人分享这篇文章,请使用以下链接:Nikita Lipilin。奇怪的代码如何隐藏错误?TensorFlow.NET分析

All Articles