{"id":395984,"date":"2024-06-29T12:33:40","date_gmt":"2024-06-29T12:33:40","guid":{"rendered":"http:\/\/savepearlharbor.com\/?p=395984"},"modified":"-0001-11-30T00:00:00","modified_gmt":"2024-06-29T12:33:40","slug":"","status":"publish","type":"post","link":"https:\/\/savepearlharbor.com\/?p=395984","title":{"rendered":"Checking the Code of DeepSpeech, or Why You Shouldn’t Write in namespace std<\/span>"},"content":{"rendered":"
<\/div>\n
\n
\n
\n
DeepSpeech is an open-source speech recognition engine developed by Mozilla. It’s pretty fast and ranks high, which makes its source code an interesting target for static analysis. In this article, I’ll show you some of the bugs found in DeepSpeech’s C++ code. <\/p>\n
\"image1.png\"<\/div>\n

<\/p>\n

Introduction<\/h2>\n

We have already scanned a few projects based on machine learning before, so there was nothing new about DeepSpeech<\/a> to us in this respect. No wonder since the project is quite popular: as of this writing, it has 15k stars on GitHub.
<\/a>
As usual, the bugs discussed in this article have been found using the static code analyzer PVS-Studio.<\/p>\n

DeepSpeech employs the TensorFlow library. I excluded the library’s code from analysis because we already made a separate article<\/a> about it, but I left analysis enabled for all the other libraries used by DeepSpeech. Why? Because any bugs that sit in any of the libraries included into your project become bugs in your project. That’s why it makes sense to scan not only your own code but also any third-party code that you include. We gave a detailed argument for this approach in our recent article<\/a>.<\/p>\n

That’s all for the introduction \u2013 let’s move on to the bugs. By the way, if you are here to find out the answer to the question mentioned in the title (why you shouldn’t write in namespace std), you can skip straight over to the end of the article. There you’ll find an especially tasty example!<\/p>\n

10 interesting warnings by PVS-Studio<\/h2>\n

Warning 1<\/b><\/p>\n

V773<\/a> The function was exited without releasing the ‘data’ pointer. A memory leak is possible. edit-fst.h 311<\/p>\n

\/\/ EditFstData method implementations: just the Read method. template <typename A, typename WrappedFstT, typename MutableFstT> EditFstData<A, WrappedFstT, MutableFstT> * EditFstData<A, WrappedFstT, MutableFstT>::Read(std::istream &strm,                                                const FstReadOptions &opts) {   auto *data = new EditFstData<A, WrappedFstT, MutableFstT>();   \/\/ next read in MutabelFstT machine that stores edits   FstReadOptions edits_opts(opts);    ....      std::unique_ptr<MutableFstT> edits(MutableFstT::Read(strm, edits_opts));   if (!edits) return nullptr; \/\/ <=    .... }<\/code><\/pre>\n
  This snippet is a classic example of a memory leak: the Read<\/i> function calls ‘return nullptr<\/i>‘ without having first freed the memory allocated earlier using the ‘new EditFstData<\/i>‘ expression. When the function returns in a way like that (without calling delete data<\/i>), only the pointer itself will be deleted, without calling the destructor of the object that it points at. Thus, the object will stay in memory and you won’t be able to delete or use it. <\/p>\n
  Besides the bug, this snippet also uses another poor practice: one function handles both smart and regular pointers at the same time. If, for example, data<\/i> were a smart pointer too, no such error would have occurred: when necessary, smart pointers will automatically call the destructor of the allocated object upon leaving the scope.<\/p>\n
  Warning 2<\/b><\/p>\n
  V1062<\/a> The ‘DfsState’ class defines a custom ‘new’ operator. The ‘delete’ operator must also be defined. dfs-visit.h 62<\/p>\n
\/\/ An FST state's DFS stack state. template <class FST> struct DfsState { public:   ....   void *operator new(size_t size,                       MemoryPool<DfsState<FST>> *pool) {     return pool->Allocate();   }   .... }<\/code><\/pre>\n
  PVS-Studio never stops evolving and acquiring new diagnostics. The code above is a perfect example for showcasing one such new diagnostic, V1062<\/a>.<\/p>\n
  The rule behind it is simple: if you define your own ‘new’ operator, you must also define your own ‘delete’ operator. Likewise, if you define your own ‘delete’ operator, you must also define your own ‘new’.<\/p>\n
  This rule was broken in this example: an object is created using a user-defined ‘new’ operator but deleted using the standard ‘delete’. Let’s see what the Allocate<\/i> function of the class MemoryPool<\/i> does when it’s called by the user-defined ‘new’:<\/p>\n
void *Allocate() {   if (free_list_ == nullptr) {     auto *link = static_cast<Link *>(mem_arena_.Allocate(1));     link->next = nullptr;     return link;   } else {     auto *link = free_list_;     free_list_ = link->next;     return link;   } }<\/code><\/pre>\n
  This function creates an element and adds it to a linked list. Implementing such allocation in your own ‘new’ makes sense. <\/p>\n
  But wait! Just a few lines later, you see the following function:<\/p>\n
void Free(void *ptr) {   if (ptr) {     auto *link = static_cast<Link *>(ptr);     link->next = free_list_;     free_list_ = link;   } }<\/code><\/pre>\n
  So, they already have ready-made functions both for allocation and deallocation. The programmer probably intended to write their own ‘delete’ operator using this Free()<\/i> function for deallocation.<\/p>\n
  The analyzer found at least three more errors of this type:<\/p>\n