War of the Machines: PVS-Studio vs. TensorFlow

TensorFlow

TensorFlow is a machine learning library, developed by Google corporation and available as an open-source project since November, 9th, 2015 year. At the moment it is actively used in research work and in dozens of commercial products of Google, including Google Search, Gmail, YouTube, Photos, Translate, Assistant, etc. The source code is available at the repository on GitHub and on the Google Open Source platform.

  • Machine learning. Nowadays, this subject is gaining more and more popularity. For good reason, some of the results are truly impressive! I won’t bring the examples here, you may easily find them yourselves.
  • Statistics on GitHub. This is also quite an important criterion, because the more popular is the project, the better. TensorFlow is breaking all the possible and impossible records! It takes one of the top places among C++ projects, has more than 50 000 stars and over 20 000 forks! It’s amazing!

What was the tool of the analysis?

If you know what PVS-Studio is, then you know the answer. In case you are still not aware, please don’t rush reading on. For example, it could be interesting to know that we have a C# analyzer for more than a year, and a Linux version for roughly half a year.

The article format

This time I want to divert from a traditional flow of the analysis: Downloaded the project — checked — wrote about the found bugs. I also want to tell about some analyzer settings and the ways they can be useful. In particular I will show how to fight against false positives, how to benefit from disabling certain diagnostics and excluding particular files from the analysis. Of course, we will have a look at the errors that PVS-Studio managed to find in the source code TensorFlow.

Preparation for the analysis

Now that PVS-Studio is also available under Linux, we have a choice of how to perform the analysis: under Linux or Windows. Quite recently I checked one project under openSUSE, which was quite simple and convenient, but still I decided to check TensorFlow under Windows. It was more familiar for me. What’s more, it can be built using CMake which presupposes further work in the Visual Studio IDE, for which we have a special plugin (the latest version obtained code highlighting of erroneous fragments).

False positives: arithmetic and fun

Why false positives are important (and frustrating)

False positives — a headache for everybody: for us, the developers of a static code analyzer and for the users because they clutter up useful output. A large number of false positives may repel people from using the tool. In addition, people usually judge the analyzer based on the criteria of the percentage of false positives. It is not that easy as it may seem, and this topic is for another article and discussion. My colleague has recently written an article about this, I recommend having a look at it.

How to fight against false positives?

Our task is to try getting rid of the false positives on the analysis stage, so that the users never see them. To do this, we add exceptions to the diagnostic rules, i.e. special cases, when the analyzer shouldn’t issue warnings at all. The number of these exceptions can vary greatly from the diagnostic to diagnostic: for some diagnostics we don’t have to write exceptions at all and sometimes we may have dozens of such exceptions implemented.

False positives and TensorFlow

Why have I started speaking about false positives in general? Firstly, because it is very important to fight against false positives, secondly, because of what I saw when I checked TensorFlow and filtered and output by the diagnostic rule V654 (the image is clickable).

false && expr
DCHECK(v);
DCHECK(stream != nullptr);
DCHECK(result != nullptr);
#ifndef NDEBUG
....
#define DCHECK(condition) CHECK(condition)
....
#else
....
#define DCHECK(condition) \
while (false && (condition)) LOG(FATAL)
....
#endif
//-V:DCHECK:654 
#define DCHECK(condition) \
while (false && (condition)) LOG(FATAL)

A couple more settings

Perhaps, you can’t wait to take a look at the bugs we found, but please, be patient and read about couple more settings that will make the life easier during the work with the analysis results.

Warnings in automatically generated files

During the analysis we checked not only the code, which was written manually by the programmers, but the automatically generated. It won’t be interesting for us to warning for such code, that’s why we will exclude them from the analysis. ‘Don’t check files’ settings are coming to aid here. Specifically for this project, I specified the following file names:

pywrap_*
*.pb.cc

Disabling specific diagnoses

One more analyzer setting that turned out to be very useful — disabling groups of diagnostic rules. Why can it be relevant? For example, there were about 70 warnings V730 (not all the class members are initialized in the constructor). These warnings really need reviewing, because they might signal about hard-to-detect bugs. Nevertheless, it may not be clear to a person, who is not much familiar with the code, if the uninitialized member will lead to problems or there is another tricky way of it further initialization. For an article, these errors aren’t much interesting too. That’s why, the developers should really review them and we won’t focus on it here. Therefore, we have a goal — to disable a whole group of diagnostic rules. It can be easily done: in settings of the PVS-Studio plugin you should just uncheck the necessary diagnostic.

The analyzer warnings issued for the project

Well, now let’s move on to the most intriguing part — those code fragments that the analyzer found suspicious.

void ToGraphDef(const Graph* g, GraphDef* gdef, bool pretty) {
....
gtl::InlinedVector<const Edge*, 4> inputs;
....
for (const Edge* e : inputs) {
const string srcname = NewName(e->src(), pretty);
if (e == nullptr) {
ndef->add_input("unknown");
} else if (!e->src()->IsOp()) {
} else if (e->IsControlEdge()) {
ndef->add_input(strings::StrCat("^", srcname));
} else if (e->src_output() == 0) {
ndef->add_input(srcname);
} else {
ndef->add_input(strings::StrCat(srcname, ":", e->src_output()));
}
}
....
}
for (const Edge* e : n->in_edges()) {
if (e->IsControlEdge()) {
inputs.push_back(e);
} else {
if (inputs[e->dst_input()] == nullptr) {
inputs[e->dst_input()] = e;
} else {
LOG(WARNING) << "Malformed graph node. multiple input edges: "
<< n->DebugString();
}
}
}
Status MasterSession::StartStep(const BuildGraphOptions& opts, 
int64* count,
ReffedClientGraph** rcg,
bool is_partial) {
....
ReffedClientGraph* to_unref = nullptr;
....
if (to_unref) to_unref->Unref();
....
}
struct LSTMBlockCellBprop ....
{
....
void operator()(...., bool use_peephole, ....) {
....
if (use_peephole) {
cs_prev_grad.device(d) =
cs_prev_grad +
di * wci.reshape(p_shape).broadcast(p_broadcast_shape) +
df * wcf.reshape(p_shape).broadcast(p_broadcast_shape);
}
if (use_peephole) {
wci_grad.device(d) =
(di * cs_prev).sum(Eigen::array<int, 1>({0}));
wcf_grad.device(d) =
(df * cs_prev).sum(Eigen::array<int, 1>({0}));
wco_grad.device(d) =
(do_ * cs).sum(Eigen::array<int, 1>({0}));
}
....
}
};
struct CompressFlags {
....
Format format;
....
int quality = 95;
....
bool progressive = false;
....
bool optimize_jpeg_size = false;
....
bool chroma_downsampling = true;
....
int density_unit = 1;
int x_density = 300;
int y_density = 300;
....
StringPiece xmp_metadata;
....
int stride = 0;
};
class EncodeJpegOp : public OpKernel {
....
explicit EncodeJpegOp(OpKernelConstruction* context) :
OpKernel(context) {
....
OP_REQUIRES_OK(context,
context->GetAttr("quality", &flags_.quality));
OP_REQUIRES(context,
0 <= flags_.quality && flags_.quality <= 100,
errors::InvalidArgument("quality must be in [0,100], got ",
flags_.quality));
OP_REQUIRES_OK(context,
context->GetAttr("progressive",
&flags_.progressive));
OP_REQUIRES_OK(context,
context->GetAttr("optimize_size",
&flags_.optimize_jpeg_size));
OP_REQUIRES_OK(context,
context->GetAttr("chroma_downsampling", // <=
&flags_.chroma_downsampling));
OP_REQUIRES_OK(context,
context->GetAttr("chroma_downsampling", // <=
&flags_.chroma_downsampling));
....
}
....
jpeg::CompressFlags flags_;
}
class InferenceContext {
....
inline int64 Value(DimensionOrConstant d) const {
return d.dim.IsSet() ? d.dim->value_ : d.val;
}
....
}
REGISTER_OP("UnpackPath")
.Input("path: int32")
.Input("path_values: float")
.Output("unpacked_path: float")
.SetShapeFn([](InferenceContext* c) {
....
int64 num_nodes = InferenceContext::kUnknownDim;
if (c->ValueKnown(tree_depth)) {
num_nodes = (1 << c->Value(tree_depth)) - 1; // <=
}
....
})
....;
AlphaNum::AlphaNum(Hex hex) {
....
uint64 value = hex.value;
uint64 width = hex.spec;
// We accomplish minimum width by OR'ing in 0x10000 to the user's
// value,
// where 0x10000 is the smallest hex number that is as wide as the
// user
// asked for.
uint64 mask = ((static_cast<uint64>(1) << (width - 1) * 4)) | value;
....
}
uint64 mask = (static_cast<uint64>(1) << ((width - 1) * 4)) | value;
void Compute(OpKernelContext* context) override {
....
int64 v = floor(in_x);
....
v = ceil(in_x1);
x_interp.end = ceil(in_x1);
v = x_interp.end - 1;
....
}
void Compute(OpKernelContext* context) override {
....
int64 sparse_input_start; // <=
....
if (sparse_input) {
num_total_features += GetNumSparseFeatures(
sparse_input_indices, *it, &sparse_input_start); // <=
}
if (num_total_features == 0) {
LOG(WARNING) << "num total features is zero.";
break;
}
if (rand_feature < input_spec_.dense_features_size()) {
....
} else {
....
const int32 sparse_index = sparse_input_start + // <=
rand_feature - input_spec_.dense_features_size();
....
}
....
}

Is that all?

Well, yes and no. To be honest, I was hoping to find more defects and write an article in the style of the articles Qt, Mono, Unreal Engine 4 and similar to them, but it didn’t work. The project authors did a great job, there were not so many errors found. I was also hoping that the project would of a bigger size, but there were only 700 files checked in the chosen configuration, including the auto generated files.

  • we did not review the warnings of the 3 (Low) level of certainty;
  • the analyzer issued several dozens of V730 warnings, but it’s hard to judge about their criticality, so it’s up to developers to decide;
  • and many more.

Summing up

TensorFlow turned out to be quite an interesting and high-quality project in terms of code, but, as we saw, not without flaws. At the same time PVS-Studio proved once more that it is able to find errors even in the code of well-known developers.

--

--

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store
Unicorn Developer

Unicorn Developer

The developer, the debugger, the unicorn. I know all about static analysis and how to find bugs and errors in C, C++, C#, and Java source code.