Как выстрелить себе в ногу в C и C++. Сборник рецептов Haiku OS

    История встречи статического анализатора PVS-Studio с кодом операционной системы Haiku уходит в далёкий 2015-й год. Это был интереснейший эксперимент и полезный опыт для команд обоих проектов. Почему эксперимент? Анализатора для Linux тогда не было и не будет ещё полтора года. Но труды энтузиастов нашей команды были вознаграждены: мы познакомились с разработчиками Haiku и повысили качество кода, пополнили базу редкими ошибками программистов и доработали анализатор. Сейчас проверить код Haiku на наличие ошибок можно легко и быстро.

    Picture 3


    Введение


    Герои нашей истории — операционная система Haiku с открытым исходным кодом и статический анализатор PVS-Studio для языков C, C++, C# и Java. Когда 4.5 года назад мы взялись за анализ проекта, то пришлось работать только со скомпилированным исполняемым файлом анализатора. Вся инфраструктура для парсинга параметров компиляции, запуска препроцессора, распараллеливания анализа и т.д. была взята из утилиты Compiler Monitoring UI на языке C#, которая частями была портирована на платформу Mono, чтобы запускаться в Linux. Сам проект Haiku собирается с помощью кросс-компилятора под разными операционными системами, кроме Windows. Очередной раз хочу отметить удобство и полноту документации по сборке Haiku, а также поблагодарить разработчиков Haiku за помощь в сборке проекта.

    Сейчас выполнить анализ можно гораздо проще. Список всех команд для сборки и анализа проекта выглядит так:

    cd /opt
    git clone https://review.haiku-os.org/buildtools
    git clone https://review.haiku-os.org/haiku
    cd ./haiku
    mkdir generated.x86_64; cd generated.x86_64
    ../configure --distro-compatibility official -j12 \
      --build-cross-tools x86_64 ../../buildtools
    cd ../../buildtools/jam
    make all
    cd /opt/haiku/generated.x86_64
    pvs-studio-analyzer trace -- /opt/buildtools/jam/bin.linuxx86/jam \
      -q -j1 @nightly-anyboot
    pvs-studio-analyzer analyze -l /mnt/svn/PVS-Studio.lic -r /opt/haiku \
       -C x86_64-unknown-haiku-gcc -o /opt/haiku/haiku.log -j12

    Кстати, анализ проекта выполнялся в Docker-контейнере. Недавно мы подготовили новую документацию по этой теме: Запуск PVS-Studio в Docker. Это может сильно упростить некоторым компаниям применение методики статического анализа проектов.

    Неинициализированные переменные


    V614 Uninitialized variable 'rval' used. fetch.c 1727

    int
    auto_fetch(int argc, char *argv[])
    {
      volatile int  argpos;
      int    rval;                  // <=
      argpos = 0;
    
      if (sigsetjmp(toplevel, 1)) {
        if (connected)
          disconnect(0, NULL);
        if (rval > 0)               // <=
          rval = argpos + 1;
        return (rval);
      }
      ....
    }

    Переменная rval не была инициализирована при объявлении, поэтому её сравнение с нулевым значением приведёт к неопределённому результату. При неудачном стечении обстоятельств неопределённое значение переменной rval может стать возвращаемым значением функции auto_fetch.

    V614 Uninitialized pointer 'res' used. commands.c 2873

    struct addrinfo {
     int ai_flags;
     int ai_family;
     int ai_socktype;
     int ai_protocol;
     socklen_t ai_addrlen;
     char *ai_canonname;
     struct sockaddr *ai_addr;
     struct addrinfo *ai_next;
    };
    
    static int
    sourceroute(struct addrinfo *ai, char *arg, char **cpp,
                int *lenp, int *protop, int *optp)
    {
      static char buf[1024 + ALIGNBYTES];
      char *cp, *cp2, *lsrp, *ep;
      struct sockaddr_in *_sin;
    #ifdef INET6
      struct sockaddr_in6 *sin6;
      struct ip6_rthdr *rth;
    #endif
      struct addrinfo hints, *res;     // <=
      int error;
      char c;
    
      if (cpp == NULL || lenp == NULL)
        return -1;
      if (*cpp != NULL) {
        switch (res->ai_family) {      // <=
        case AF_INET:
          if (*lenp < 7)
            return -1;
          break;
          ....
        }
      }
      ....
    }

    Похожий случай использования неинициализированной переменной, только здесь имеет место неинициализированный указатель res.

    V506 Pointer to local variable 'normalized' is stored outside the scope of this variable. Such a pointer will become invalid. TextView.cpp 5596

    void
    BTextView::_ApplyStyleRange(...., const BFont* font, ....)
    {
      if (font != NULL) {
        BFont normalized = *font;
        _NormalizeFont(&normalized);
        font = &normalized;
      }
      ....
      fStyles->SetStyleRange(fromOffset, toOffset, fText->Length(), mode,
        font, color);
    }

    Вероятно, программисту понадобилось выполнить нормализацию объекта через промежуточную переменную. Но теперь в указатель font сохранили указатель на временный объект normalized, который будет разрушен после выхода из области видимости, в которой этот временный объект был создан.

    V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 27

    int8
    BUnicodeChar::Type(uint32 c)
    {
      BUnicodeChar();
      return u_charType(c);
    }

    Очень распространённая ошибка среди C++ программистов — использовать вызов конструктора якобы для инициализации/обнуления полей класса. В этом случае не происходит модификации полей класса, а создаётся новый неименованный объект этого класса, который тут же уничтожается. К сожалению, таких мест найдено очень много:

    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 37
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 49
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 58
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 67
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 77
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 89
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 103
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 115
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 126
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 142
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 152
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 163
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 186
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 196
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 206
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 214
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 222
    • V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 230

    V670 The uninitialized class member 'fPatternHandler' is used to initialize the 'fInternal' member. Remember that members are initialized in the order of their declarations inside a class. Painter.cpp 184

    Painter::Painter()
      :
      fInternal(fPatternHandler),
      ....
      fPatternHandler(),
      ....
    {
      ....
    };
    
    class Painter {
      ....
    private:
      mutable PainterAggInterface fInternal; // line 336
    
      bool fSubpixelPrecise : 1;
      bool fValidClipping : 1;
      bool fDrawingText : 1;
      bool fAttached : 1;
      bool fIdentityTransform : 1;
    
      Transformable fTransform;
      float fPenSize;
      const BRegion* fClippingRegion;
      drawing_mode fDrawingMode;
      source_alpha fAlphaSrcMode;
      alpha_function fAlphaFncMode;
      cap_mode fLineCapMode;
      join_mode fLineJoinMode;
      float fMiterLimit;
    
      PatternHandler fPatternHandler;        // line 355
      mutable AGGTextRenderer fTextRenderer;
    };

    Другой пример неправильной инициализации. Поля класса инициализируются в порядке их объявления в самом классе. В этом примере первым будет инициализировано поле fInternal с использованием неинициализированного значения fPatternHandler.

    Подозрительный #define


    V523 The 'then' statement is equivalent to the 'else' statement. subr_gtaskqueue.c 191

    #define  TQ_LOCK(tq)              \
      do {                \
        if ((tq)->tq_spin)          \
          mtx_lock_spin(&(tq)->tq_mutex);      \
        else              \
          mtx_lock(&(tq)->tq_mutex);      \
      } while (0)
    #define  TQ_ASSERT_LOCKED(tq)  mtx_assert(&(tq)->tq_mutex, MA_OWNED)
    
    #define  TQ_UNLOCK(tq)              \
      do {                \
        if ((tq)->tq_spin)          \
          mtx_unlock_spin(&(tq)->tq_mutex);    \
        else              \
          mtx_unlock(&(tq)->tq_mutex);      \
      } while (0)
    
    void
    grouptask_block(struct grouptask *grouptask)
    {
      ....
      TQ_LOCK(queue);
      gtask->ta_flags |= TASK_NOENQUEUE;
      gtaskqueue_drain_locked(queue, gtask);
      TQ_UNLOCK(queue);
    }

    Фрагмент кода не выглядит подозрительным, пока не взглянешь на результат работы препроцессора:

    void
    grouptask_block(struct grouptask *grouptask)
    {
      ....
      do { if ((queue)->tq_spin) mtx_lock(&(queue)->tq_mutex);
           else mtx_lock(&(queue)->tq_mutex); } while (0);
      gtask->ta_flags |= 0x4;
      gtaskqueue_drain_locked(queue, gtask);
       do { if ((queue)->tq_spin) mtx_unlock(&(queue)->tq_mutex);
            else mtx_unlock(&(queue)->tq_mutex); } while (0);
    }

    Анализатор действительно прав — ветви if и else идентичны. Но куда делись функции mtx_lock_spin и mtx_unlock_spin? Макросы TQ_LOCK, TQ_UNLOCK и функция grouptask_block объявлены в одном файле и практически рядом, тем не менее, где-то произошла подмена.

    Поиск по содержимому файлов нашёл только mutex.h со следующим содержимым:

    /* on FreeBSD these are different functions */
    #define mtx_lock_spin(x)   mtx_lock(x)
    #define mtx_unlock_spin(x) mtx_unlock(x)

    Правильная ли такая подмена или нет — стоит проверить разработчикам проекта. Я проверял проект в Linux, и такая подмена показалась мне подозрительной.

    Ошибки с функцией free


    V575 The null pointer is passed into 'free' function. Inspect the first argument. setmime.cpp 727

    void
    MimeType::_PurgeProperties()
    {
      fShort.Truncate(0);
      fLong.Truncate(0);
      fPrefApp.Truncate(0);
      fPrefAppSig.Truncate(0);
      fSniffRule.Truncate(0);
    
      delete fSmallIcon;
      fSmallIcon = NULL;
    
      delete fBigIcon;
      fBigIcon = NULL;
    
      fVectorIcon = NULL;            // <=
      free(fVectorIcon);             // <=
    
      fExtensions.clear();
      fAttributes.clear();
    }

    В функцию free передавать нулевой указатель можно, но такая запись явно вызывает подозрение. Так, анализатор нашёл перепутанные строки кода. Сначала надо было освободить память по указателю fVectorIcon, а только потом присвоить NULL.

    V575 The null pointer is passed into 'free' function. Inspect the first argument. driver_settings.cpp 461

    static settings_handle *
    load_driver_settings_from_file(int file, const char *driverName)
    {
      ....
      handle = new_settings(text, driverName);
      if (handle != NULL) {
        // everything went fine!
        return handle;
      }
    
      free(handle);           // <=
      ....
    }

    Ещё один пример явной передачи нулевого указателя в функцию free. Эту строку можно удалить, т.к. при успешном получении указателя выполняется выход из функции.

    V575 The null pointer is passed into 'free' function. Inspect the first argument. PackageFileHeapWriter.cpp 166

    void* _GetBuffer()
    {
      ....
      void* buffer = malloc(fBufferSize);
      if (buffer == NULL && !fBuffers.AddItem(buffer)) {
        free(buffer);
        throw std::bad_alloc();
      }
      return buffer;
    }

    Вот тут допущена ошибка. Вместо оператора && должен использоваться оператор ||. Только в этом случае будет брошено исключение std::bad_alloc(), если не удалось выделить память с помощью функции malloc.

    Ошибки с оператором delete


    V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] fMsg;'. Err.cpp 65

    class Err {
    public:
     ....
    private:
     char *fMsg;
     ssize_t fPos;
    };
    
    void
    Err::Unset() {
     delete fMsg;                                   // <=
     fMsg = __null;
     fPos = -1;
    }
    
    void
    Err::SetMsg(const char *msg) {
     if (fMsg) {
      delete fMsg;                                  // <=
      fMsg = __null;
     }
     if (msg) {
      fMsg = new(std::nothrow) char[strlen(msg)+1]; // <=
      if (fMsg)
       strcpy(fMsg, msg);
     }
    }

    Указатель fMsg используется для аллокации памяти под хранение массива символов, при этом для освобождения памяти используется оператор delete, вместо delete[].

    V611 The memory was allocated using 'new' operator but was released using the 'free' function. Consider inspecting operation logics behind the 'wrapperPool' variable. vm_page.cpp 3080

    status_t
    vm_page_write_modified_page_range(....)
    {
      ....
      PageWriteWrapper* wrapperPool
        = new(malloc_flags(allocationFlags)) PageWriteWrapper[maxPages + 1];
      PageWriteWrapper** wrappers
        = new(malloc_flags(allocationFlags)) PageWriteWrapper*[maxPages];
      if (wrapperPool == NULL || wrappers == NULL) {
        free(wrapperPool);                              // <=
        free(wrappers);                                 // <=
        wrapperPool = stackWrappersPool;
        wrappers = stackWrappers;
        maxPages = 1;
      }
      ....
    }

    Здесь malloc_flags – функция, которая делает malloc. И затем уже placement-new конструирует здесь объект. А так как класс PageWriteWrapper реализован следующим образом:

    class PageWriteWrapper {
    public:
     PageWriteWrapper();
     ~PageWriteWrapper();
     void SetTo(vm_page* page);
     bool Done(status_t result);
    
    private:
     vm_page* fPage;
     struct VMCache* fCache;
     bool fIsActive;
    };
    
    PageWriteWrapper::PageWriteWrapper()
     :
     fIsActive(false)
    {
    }
    
    PageWriteWrapper::~PageWriteWrapper()
    {
     if (fIsActive)
      panic("page write wrapper going out of scope but isn't completed");
    }

    то деструкторы объектов этого класса не будут вызваны из-за использования функции free для освобождения памяти.

    V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] fOutBuffer;'. Check lines: 26, 45. PCL6Rasterizer.h 26

    class PCL6Rasterizer : public Rasterizer
    {
    public:
      ....
      ~PCL6Rasterizer()
      {
        delete fOutBuffer;
        fOutBuffer = NULL;
      }
      ....
      virtual void InitializeBuffer()
      {
        fOutBuffer = new uchar[fOutBufferSize];
      }
    private:
      uchar* fOutBuffer;
      int    fOutBufferSize;
    };

    Использование оператора delete вместо delete[] — очень распространённая ошибка. Легче всего допустить ошибку при написании класса, т.к. код деструктора часто находится далеко от мест аллокации памяти. Здесь программист неправильно освобождает память в деструкторе по указателю fOutBuffer.

    V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. Hashtable.cpp 207

    void
    Hashtable::MakeEmpty(int8 keyMode,int8 valueMode)
    {
      ....
      for (entry = fTable[index]; entry; entry = next) {
        switch (keyMode) {
          case HASH_EMPTY_DELETE:
            // TODO: destructors are not called!
            delete (void*)entry->key;
            break;
          case HASH_EMPTY_FREE:
            free((void*)entry->key);
            break;
        }
        switch (valueMode) {
          case HASH_EMPTY_DELETE:
            // TODO: destructors are not called!
            delete entry->value;
            break;
          case HASH_EMPTY_FREE:
            free(entry->value);
            break;
        }
        next = entry->next;
        delete entry;
      }
      ....
    }

    Кроме неправильного выбора между delete/delete[] и free, можно добавить в программу неопределённое поведение ещё одним способом — попытаться очистить память по указателю на неопределённый тип (void*).

    Функции без возвращаемого значения


    V591 Non-void function should return a value. Referenceable.h 228

    BReference& operator=(const BReference<const Type>& other)
    {
      fReference = other.fReference;
    }

    В переопределённом операторе присваивания не хватает возвращаемого значения. В этом случае оператор будет возвращать случайное значение, что может приводить к странным ошибкам.

    Похожие проблемы в других фрагментах кода этого класса:

    • V591 Non-void function should return a value. Referenceable.h 233
    • V591 Non-void function should return a value. Referenceable.h 239

    V591 Non-void function should return a value. main.c 1010

    void errx(int, const char *, ...) ;
    
    char *
    getoptionvalue(const char *name)
    {
      struct option *c;
    
      if (name == NULL)
        errx(1, "getoptionvalue() invoked with NULL name");
      c = getoption(name);
      if (c != NULL)
        return (c->value);
      errx(1, "getoptionvalue() invoked with unknown option '%s'", name);
      /* NOTREACHED */
    }

    Пользовательский комментарий NOTREACHED здесь ничего не значит. Чтобы правильно написать код для таких сценариев, необходимо размечать функции как noreturn. Для этого существуют noreturn-атрибуты: стандартные и специфичные для компилятора. В первую очередь, такие атрибуты учитываются компиляторами для правильной кодогенерации или уведомления о некоторых типах ошибок с помощью варнингов. Различные инструменты статического анализа тоже учитывают атрибуты для повышения качества анализа.

    Работа с исключениями


    V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw ParseException(FOO); Response.cpp 659

    size_t
    Response::ExtractNumber(BDataIO& stream)
    {
      BString string = ExtractString(stream);
    
      const char* end;
      size_t number = strtoul(string.String(), (char**)&end, 10);
      if (end == NULL || end[0] != '\0')
        ParseException("Invalid number!");
    
      return number;
    }

    Здесь случайно забыто ключевое слово throw. Таким образом, исключение ParseException не будет сгенерировано, а объект этого класса будет просто уничтожен при выходе из области видимости. После чего функция продолжит свою работу как ни в чём не бывало, как будто введено корректное число.

    V1022 An exception was thrown by pointer. Consider throwing it by value instead. gensyscallinfos.cpp 316

    int
    main(int argc, char** argv)
    {
      try {
        return Main().Run(argc, argv);
      } catch (Exception& exception) {                                         // <=
        fprintf(stderr, "%s\n", exception.what());
        return 1;
      }
    }
    
    int Run(int argc, char** argv)
    {
      ....
      _ParseSyscalls(argv[1]);
      ....
    }
    
    void _ParseSyscalls(const char* filename)
    {
      ifstream file(filename, ifstream::in);
      if (!file.is_open())
        throw new IOException(string("Failed to open '") + filename + "'.");   // <=
      ....
    }

    Анализатор обнаружил исключение IOException, брошенное по указателю. Бросание указателя приводит к тому, что исключение не будет поймано, так исключение в итоге перехватывается по ссылке. Также использование указателя вынуждает перехватывающую сторону вызвать оператор delete для уничтожения созданного объекта, что тоже не сделано.

    Ещё два проблемных участка кода:

    • V1022 An exception was thrown by pointer. Consider throwing it by value instead. gensyscallinfos.cpp 347
    • V1022 An exception was thrown by pointer. Consider throwing it by value instead. gensyscallinfos.cpp 413

    Формальная безопасноcть


    V597 The compiler could delete the 'memset' function call, which is used to flush 'f_key' object. The memset_s() function should be used to erase the private data. dst_api.c 1018

    #ifndef SAFE_FREE
    #define SAFE_FREE(a) \
    do{if(a != NULL){memset(a,0, sizeof(*a)); free(a); a=NULL;}} while (0)
    ....
    #endif
    
    DST_KEY *
    dst_free_key(DST_KEY *f_key)
    {
      if (f_key == NULL)
        return (f_key);
      if (f_key->dk_func && f_key->dk_func->destroy)
        f_key->dk_KEY_struct =
          f_key->dk_func->destroy(f_key->dk_KEY_struct);
      else {
        EREPORT(("dst_free_key(): Unknown key alg %d\n",
           f_key->dk_alg));
      }
      if (f_key->dk_KEY_struct) {
        free(f_key->dk_KEY_struct);
        f_key->dk_KEY_struct = NULL;
      }
      if (f_key->dk_key_name)
        SAFE_FREE(f_key->dk_key_name);
      SAFE_FREE(f_key);
      return (NULL);
    }

    Анализатор обнаружил подозрительный код, предназначенный для безопасной очистки приватных данных. К сожалению, макрос SAFE_FREE, который раскрывается в вызовы memset, free и присваивание NULL, не делает код безопаснее, т.к. всё это удаляется компилятором уже при оптимизации O2.

    Кстати, это не что иное, как CWE-14: Compiler Removal of Code to Clear Buffers.

    Весь список мест, где очистка буферов по факту не выполняется:

    • V597 The compiler could delete the 'memset' function call, which is used to flush 'encoded_block' buffer. The memset_s() function should be used to erase the private data. dst_api.c 446
    • V597 The compiler could delete the 'memset' function call, which is used to flush 'key_st' object. The memset_s() function should be used to erase the private data. dst_api.c 685
    • V597 The compiler could delete the 'memset' function call, which is used to flush 'in_buff' buffer. The memset_s() function should be used to erase the private data. dst_api.c 916
    • V597 The compiler could delete the 'memset' function call, which is used to flush 'ce' object. The memset_s() function should be used to erase the private data. fs_cache.c 1078

    Сравнения с беззнаковыми переменными


    V547 Expression 'remaining < 0' is always false. Unsigned type value is never < 0. DwarfFile.cpp 1947

    status_t
    DwarfFile::_UnwindCallFrame(....)
    {
      ....
      uint64 remaining = lengthOffset + length - dataReader.Offset();
      if (remaining < 0)
        return B_BAD_DATA;
      ....
    }

    Анализатор обнаружил явное сравнение беззнаковой переменной с отрицательными значениями. Возможно, следует сравнить переменную remaining только с нулём или реализовать проверку на переполнение.

    V547 Expression 'sleep((unsigned) secs) < 0' is always false. Unsigned type value is never < 0. misc.cpp 56

    status_t
    snooze(bigtime_t amount)
    {
      if (amount <= 0)
        return B_OK;
    
      int64 secs = amount / 1000000LL;
      int64 usecs = amount % 1000000LL;
      if (secs > 0) {
        if (sleep((unsigned)secs) < 0)     // <=
          return errno;
      }
    
      if (usecs > 0) {
        if (usleep((useconds_t)usecs) < 0)
          return errno;
      }
    
      return B_OK;
    }

    Чтобы понять, в чём ошибка, обратимся к сигнатурам функций sleep и usleep:

    extern unsigned int sleep (unsigned int __seconds);
    extern int usleep (__useconds_t __useconds);

    Как мы видим, функция sleep возвращает значение беззнакового типа и её использование в коде является неправильным.

    Опасные указатели


    V774 The 'device' pointer was used after the memory was released. xhci.cpp 1572

    void
    XHCI::FreeDevice(Device *device)
    {
      uint8 slot = fPortSlots[device->HubPort()];
      TRACE("FreeDevice() port %d slot %d\n", device->HubPort(), slot);
    
      // Delete the device first, so it cleans up its pipes and tells us
      // what we need to destroy before we tear down our internal state.
      delete device;
    
      DisableSlot(slot);
      fDcba->baseAddress[slot] = 0;
      fPortSlots[device->HubPort()] = 0;            // <=
      delete_area(fDevices[slot].trb_area);
      delete_area(fDevices[slot].input_ctx_area);
      delete_area(fDevices[slot].device_ctx_area);
    
      memset(&fDevices[slot], 0, sizeof(xhci_device));
      fDevices[slot].state = XHCI_STATE_DISABLED;
    }

    Некий объект device очищается оператором delete. Это логичное действие для функции с названием FreeDevice. Но, по какой-то причине для освобождения других ресурсов в коде снова есть обращение к уже удалённому объекту.

    Такой код является крайне опасным и встречается ещё в нескольких местах:

    • V774 The 'self' pointer was used after the memory was released. TranslatorRoster.cpp 884
    • V774 The 'string' pointer was used after the memory was released. RemoteView.cpp 1269
    • V774 The 'bs' pointer was used after the memory was released. mkntfs.c 4291
    • V774 The 'bs' pointer was used after the memory was released. mkntfs.c 4308
    • V774 The 'al' pointer was used after the memory was reallocated. inode.c 1155

    V522 Dereferencing of the null pointer 'data' might take place. The null pointer is passed into 'malo_hal_send_helper' function. Inspect the third argument. Check lines: 350, 394. if_malohal.c 350

    static int
    malo_hal_fwload_helper(struct malo_hal *mh, char *helper)
    {
      ....
      /* tell the card we're done and... */
      error = malo_hal_send_helper(mh, 0, NULL, 0, MALO_NOWAIT); // <= NULL
      ....
    }
    
    static int
    malo_hal_send_helper(struct malo_hal *mh, int bsize,
        const void *data, size_t dsize, int waitfor)
    {
      mh->mh_cmdbuf[0] = htole16(MALO_HOSTCMD_CODE_DNLD);
      mh->mh_cmdbuf[1] = htole16(bsize);
      memcpy(&mh->mh_cmdbuf[4], data , dsize);                   // <= data
      ....
    }

    Межпроцедурный анализ позволил выявить ситуацию, когда в функцию передаётся NULL и указатель data с таким значением впоследствии разыменовывается в функции memcpy.

    V773 The function was exited without releasing the 'inputFileFile' pointer. A memory leak is possible. command_recompress.cpp 119

    int
    command_recompress(int argc, const char* const* argv)
    {
      ....
      BFile* inputFileFile = new BFile;
      error = inputFileFile->SetTo(inputPackageFileName, O_RDONLY);
      if (error != B_OK) {
        fprintf(stderr, "Error: Failed to open input file \"%s\": %s\n",
          inputPackageFileName, strerror(error));
        return 1;
      }
      inputFile = inputFileFile;
      ....
    }

    PVS-Studio умеет выявлять утечки памяти. Здесь не освобождается память для inputFileFile в случае какой-то ошибки. Кто-то считает, что в случае ошибок можно не заморачиваться с освобождением памяти — программа всё равно завершится. Но это не всегда так. Корректно обрабатывать ошибки и продолжать работу — требование к очень многим программам.

    V595 The 'fReply' pointer was utilized before it was verified against nullptr. Check lines: 49, 52. ReplyBuilder.cpp 49

    RPC::CallbackReply*
    ReplyBuilder::Reply()
    {
      fReply->Stream().InsertUInt(fStatusPosition, _HaikuErrorToNFS4(fStatus));
      fReply->Stream().InsertUInt(fOpCountPosition, fOpCount);
    
      if (fReply == NULL || fReply->Stream().Error() == B_OK)
        return fReply;
      else
        return NULL;
    }

    Как же часто разработчики разыменовывают указатели перед их проверкой. Диагностика V595 практически всегда лидер по количеству предупреждений в проекте. В этом фрагменте кода опасное использование указателя fReply.

    V595 The 'mq' pointer was utilized before it was verified against nullptr. Check lines: 782, 786. oce_queue.c 782

    static void
    oce_mq_free(struct oce_mq *mq)
    {
      POCE_SOFTC sc = (POCE_SOFTC) mq->parent;
      struct oce_mbx mbx;
      struct mbx_destroy_common_mq *fwcmd;
    
      if (!mq)
        return;
      ....
    }

    Похожий пример. Указатель mg разыменовывается на несколько строк ранее, чем проверяется на нулевое значение. Похожих мест в проекте много. В некоторых местах использование и проверка указателей значительно удалены друг от друга, поэтому в статью вошло два примера. Остальные смогут посмотреть разработчики в полном отчёте анализатора.

    Разные ошибки


    V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 101

    static void
    dump_acpi_namespace(acpi_ns_device_info *device, char *root, int indenting)
    {
      char output[320];
      char tabs[255] = "";
      ....
      strlcat(tabs, "|--- ", sizeof(tabs));
      ....
      while (....) {
        uint32 type = device->acpi->get_object_type(result);
        snprintf(output, sizeof(output), "%s%s", tabs, result + depth);
        switch(type) {
          case ACPI_TYPE_INTEGER:
            strncat(output, "     INTEGER", sizeof(output));
            break;
          case ACPI_TYPE_STRING:
            strncat(output, "     STRING", sizeof(output));
            break;
          ....
        }
        ....
      }
      ....
    }

    Разница между функциями strlcat и strncat не совсем очевидна человеку, плохо знакомому с описанием этих функций. Функция strlcat третьим аргументом принимает размер всего буфера, а функция strncat размер свободного места в буфере, что требует вычисления нужного значения перед вызовом функции. Но разработчики часто забывают или не знают об этом. Передача функции strncat размера всего буфера может привести к переполнению буфера, т.к. функция будет расценивать это значение как допустимое количество символов для копирования. Функция strlcat лишена этой проблемы, но для её корректной работы необходимо передавать строки, которые заканчиваются на терминальный ноль.

    Весь список опасных мест со строками:

    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 104
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 107
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 110
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 113
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 118
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 119
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 120
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 123
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 126
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 129
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 132
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 135
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 138
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 141
    • V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 144
    • V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 283
    • V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 284
    • V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 285

    V792 The 'SetDecoratorSettings' function located to the right of the operator '|' will be called regardless of the value of the left operand. Perhaps, it is better to use '||'. DesktopListener.cpp 324

    class DesktopListener : public DoublyLinkedListLinkImpl<DesktopListener> {
    public:
     ....
     virtual bool SetDecoratorSettings(Window* window,
             const BMessage& settings) = 0;
     ....
    };
    
    bool
    DesktopObservable::SetDecoratorSettings(Window* window,
      const BMessage& settings)
    {
      if (fWeAreInvoking)
        return false;
      InvokeGuard invokeGuard(fWeAreInvoking);
    
      bool changed = false;
      for (DesktopListener* listener = fDesktopListenerList.First();
        listener != NULL; listener = fDesktopListenerList.GetNext(listener))
        changed = changed | listener->SetDecoratorSettings(window, settings);
    
      return changed;
    }

    Скорее всего, в коде перепутали операторы '|' и '||'. Такая ошибка приводит к лишним вызовам функции SetDecoratorSettings.

    V627 Consider inspecting the expression. The argument of sizeof() is the macro which expands to a number. device.c 72

    #define PCI_line_size 0x0c /* (1 byte) cache line size in 32 bit words */
    
    static status_t
    wb840_open(const char* name, uint32 flags, void** cookie)
    {
      ....
      data->wb_cachesize = gPci->read_pci_config(data->pciInfo->bus,
        data->pciInfo->device, data->pciInfo->function, PCI_line_size,
        sizeof(PCI_line_size)) & 0xff;
      ....
    }

    Передача в оператор sizeof значения 0x0c выглядит подозрительно. Возможно, следовало посчитать размер какого-нибудь объекта, например, data.

    V562 It's odd to compare a bool type value with a value of 18: 0x12 == IsProfessionalSpdif(). CEchoGals_mixer.cpp 533

    typedef bool BOOL;
    
    virtual BOOL IsProfessionalSpdif() { ... }
    
    #define ECHOSTATUS_DSP_DEAD 0x12
    
    ECHOSTATUS CEchoGals::ProcessMixerFunction(....)
    {
      ....
      if ( ECHOSTATUS_DSP_DEAD == IsProfessionalSpdif() ) // <=
      {
        Status = ECHOSTATUS_DSP_DEAD;
      }
      else
      {
        pMixerFunction->Data.bProfSpdif = IsProfessionalSpdif();
      }
      ....
    }

    Функция IsProfessionalSpdif возвращает значение типа bool, при этом в условии результат функции сравнивается с числом 0x12.

    Заключение


    Мы пропустили выпуск первой беты Haiku прошлой осенью, т.к. были заняты выпуском PVS-Studio для языка Java. Но природа программистских ошибок такова, что они никуда не деваются, если их не искать и вообще не уделять внимание качеству кода. Разработчики пробовали использовать Coverity Scan, но последний запуск анализа был почти два года назад. Это должно огорчать пользователей Haiku. Хотя анализ с помощью Coverity был настроен в 2014 году, нам это не помешало в 2015 году написать две больших статьи с обзорами ошибок (часть 1, часть 2).

    Тех, кто дочитал до конца, ждёт ещё один обзор кода Haiku не меньшего объёма с новыми типами ошибок. Полный отчёт анализатора будет отправлен разработчикам до публикации обзоров кода, поэтому некоторые ошибки могут быть исправлены. Чтобы скоротать время между публикациями, предлагаю скачать и попробовать PVS-Studio на своём проекте.

    Хотите попробовать Haiku и у вас возникли вопросы? Разработчики Haiku приглашают вас в telegram-канал.



    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. How to shoot yourself in the foot in C and C++. Haiku OS Cookbook
    PVS-Studio
    539,98
    Static Code Analysis for C, C++, C# and Java
    Поделиться публикацией

    Похожие публикации

    Комментарии 45

      +4
      Интересно, они из принципа не пользуются статическим анализатором кода? Уже и не думал, что кто-то пишет на С/С++ без статического анализатора. Что стоит, сделал задачу (функцию, класс), проверь его статическим анализатором… Тем более, в таких больших проектах.
        0
        У них действительно маленькая команда разработчиков сейчас, чтобы за всем уследить, но с нашей подачи уже есть прогресс: git.haiku-os.org/haiku/log/?qt=grep&q=pvs
          +1
          Тем более, если маленькая команда разработки, то исправлять замечания после — прямой путь к бесконечной поддержке и отнимании ресурсов от добавления новых фич и развития проекта. Дешевле и быстрее проверить статическим анализатором маленькую задачку, на этапе её реализации сразу, чем потом, при нахождении ошибки, переделывать зависимые куски кода…
            +5
            чем потом при нахождении ошибки, переделывать зависимые куски кода…
            Кстати, на днях они решили удалить один компонент с найденными ошибками и просто взять другой)
          +2

          Мне больше интересно, что у них за компилятор такой. Предупреждения на "нет возврата в non-void функции" и "переменная используется неинициализированной" вроде бы уже любой нормальный компилятор сам выдает.

            +3
            Вы будете смеяться (плакать), но такое можно встретить даже в Clang:
            SequenceNumberManager &operator=(SequenceNumberManager &&Other) {
              NextSequenceNumber = std::move(Other.NextSequenceNumber);
              FreeSequenceNumbers = std::move(Other.FreeSequenceNumbers);
            }

            И неинициализированные переменные тоже:
            static Error mapNameAndUniqueName(....) {
              ....
              size_t BytesLeft = IO.maxFieldLength();
              if (HasUniqueName) {
                ....
                if (BytesNeeded > BytesLeft) {
                  size_t BytesToDrop = (BytesNeeded - BytesLeft);
                  size_t DropN = std::min(N.size(), BytesToDrop / 2);
                  size_t DropU = std::min(U.size(), BytesToDrop - DropN);
                  ....
                }
              } else {
                size_t BytesNeeded = Name.size() + 1;
                StringRef N = Name;
                if (BytesNeeded > BytesLeft) {
                  size_t BytesToDrop = std::min(N.size(), BytesToDrop); // <=
                  N = N.drop_back(BytesToDrop);
                }
                error(IO.mapStringZ(N));
              }
              ....
            }
              0

              Ндаа. Знаете, смеяться как-то не очень хочется.
              Но это как-то совсем странно. Предупреждения отключены что ли у них? Или их просто не читают?

                +2
                Причин несколько:
                1. Мы ведь не зря берём деньги за PVS-Studio :). Мы рассматриваем больше ошибочных паттернов и больше конструкций, в которых ошибка может проявить себя. Соответственно мы опережаем возможности компилятора. Компиляторы развиваются, и возможно сейчас Clang уже смог бы сам в себе обнаружить эту ошибку. Но ведь и мы на месте не стоим и за это время научились делать много всякого нового :). См. про технологии и развитие.
                2. Мы очень серьёзно работаем над сокращением количества ложных срабатываний. Многие анализаторы и компиляторы сильно не заморачиваются на эту тему. Вроде диагностика есть и ладно. Но фича в том, что если ложных срабатываний много, то сообщения этого типа просто перестают смотреть или отключают. Поэтому мы часто замечаем то, что просто не смотрят в силу неудобства. Эта мысль подробнее.
                3. Программисты считают, что не допускают таких ошибок :).
                  0

                  Ну, с первым можно слегка поспорить — ибо есть некоторые предупреждения, которые компиляторы выдают надежнее, чем PVS (например, "не все поля класса были проинициализированы в конструкторе").
                  Но в целом вы правы, конечно.

                    0
                    Ха. Здесь как раз та ситуация, что выдать много предупреждений дело не хитрое. Нет явной инициализации в конструкторе — ругаемся. Толку только от таких сообщений мало. В том смысле, что пользоваться нельзя. Слишком много шума. И мы немало потрудились, чтобы достичь разумного баланса между небольшим количеством сообщений и боязнью что-то пропустить. Заметка про то, как писалась эта диагностика: Поиск неинициализированных членов класса.
              0

              Если не ошибаюсь, Haiku до сих пор собирают компилятором GCC v2.9, чтобы иметь совместимость с BeOS. А когда она будет достигнута, будет совершен переход на что-то свежее (хотя бы не мертвое).

                0

                Боже. Полагаю, спрашивать, зачем им в 2019 году совместимость с BeOS, бесмысленно?

                  +1
                  Была изсчерпывающая статья на Хабре, в которой все расписано :)
                  Вкратце — хотят достичь 100% работы старых скомпилированных приложений, это цель номер один, а дальше уже можно будет развиваться в ином направлении.
                  0
                  А когда она будет достигнута, будет совершен переход на что-то свежее

                  Получится один из тех переездов, которые равны двум пожарам… Пока что-то отстроят на месте пепелища — пора будет гореть переезжать опять.
                0
                Работает=не трогай
                  0

                  Ну тот же Coverity Scan не умел в C++17 последние года два. Они только в июне вот выпустили новую версию, надо бы её будет потыкать, кстати.

                    0
                    Если не ошибаюсь, Haiku до сих пор собирают компилятором GCC v2.9, чтобы иметь совместимость с BeOS

                    Стандарт языка вряд ли повлиял на выбор инструмента статического анализа в данном случае)
                  0
                    delete fBigIcon;
                    fBigIcon = NULL;
                  
                    fVectorIcon = NULL;            // <=
                    free(fVectorIcon);             // <=

                  Из этого куска не ясно, но есть подозрение что вместо free нужно было вызывать delete.
                    +1
                    Это просто совпадение, что в проекте нашлось много ошибок с заменой delete/free, но прям тут просто перепутали строки местами. Сообщения у анализатора разные на этот счёт.
                    0
                    Скорее всего, в коде перепутали операторы '|' и '||'. Такая ошибка приводит к лишним вызовам функции SetDecoratorSettings.

                    А по-моему, это обычное применение паттерна Observer. Интересно, если бы запись была вида "changed |= listener->SetDecoratorSettings(window, settings);"
                    сработало бы предупреждение?

                      0
                      Паттерн такой есть. Но на каждый N-й такой паттерн программисты допускают ошибки, поэтому кое-что выписываем в статьях. Я подумал, что подозрительно дёргать сеттер настроек лишний раз.
                        +1

                        Возможно так сделали специально, чтобы уведомить об изменениях всех слушателей.

                          0

                          Ничего подозрительного: настройка изменилась, и надо изменить ее всюду, у всех слушателей. А переменная changed собирает информацию о том, было ли изменение на самом деле, или это был холостой цикл.

                            0
                            Ну ок. Осталось пройтись по 2999 другим предупреждениям уровня High) Сейчас в телеграмм-канале несколько человек вдохновились, чтобы мы помогли с исправлениями… вот поэтому мы не правим код и не отправляем PR после проверки проектов :-)
                        +3
                        У них там никакой политики работы с памятью нет что-ли? Адская каша из new/delete и malloc/free в коде C++ как бы намекает что в разработке имеет место определенный бардак.
                          +1
                          Скорее что в разработке С++ имеет место С и его библиотеки :)
                          Учитывая специфику проекта — что это ОС, которую пишут с 2002 года — это не так плохо.
                          За это время стандарт С++ неоднократно менялся.
                            0
                             case HASH_EMPTY_DELETE:
                                    // TODO: destructors are not called!
                                    delete entry->value;
                                    break;
                                  case HASH_EMPTY_FREE:
                                    free(entry->value);
                                    break;


                            Ну вот это же натуральный бардак, как-будто разные люди в рандомном порядке подходили к столу и просто дописывали куски кода как вздумается.

                            Или вот такое
                            void* _GetBuffer()
                            {
                              ....
                              void* buffer = malloc(fBufferSize);
                              if (buffer == NULL && !fBuffers.AddItem(buffer)) {
                                free(buffer);
                                throw std::bad_alloc();
                              }
                              return buffer;
                            }


                            malloc/free никаких о ctor/dtor не знают и они не вызываются, а это верный путь для того чтобы с разбегу запрыгнуть на грабли. Ну и печальный коммент "// TODO: destructors are not called!" как итог. И правда, почему же деструкторы не вызываются.
                            Никаким использованием C функций не оправдывается вообще.
                              0
                              Ну мало ли что там происходило в творческом процессе. Возможно автор пришёл к выводу, что уничтожение ключей и значений, помещённых в Хеш-таблицу, является ответственностью тех, кто их туда засовывал — и просто забил на этот кусок кода :)

                              Во втором примере кроме описанной в статье ошибки пара malloc-free вроде нормально используется.
                                0
                                C++ код с использованием malloc/free вместо штатных new/delete сам по себе выглядит странно.
                          +1

                          Вызов конструктора это жесть. Они из мира Джавы что ли пришли?

                            0
                            Каша из new/delete и malloc/free еще хуже.
                            +2
                            Переменная rval не была инициализирована при объявлении, поэтому её сравнение с нулевым значением приведёт к неопределённому результату.

                            Это при условии, если выполнится тело if, а выполнится оно только если кто-то вызовет longjmp(), но так как longjmp() явно не выполнится до того как rval инициализируется в следующем за if оператором for, то это и не ошибка вовсе:

                            ...
                            	if (sigsetjmp(toplevel, 1)) {
                            		if (connected)
                            			disconnect(0, NULL);
                            		if (rval > 0)
                            			rval = argpos + 1;
                            		return (rval);
                            	}
                            	(void)xsignal(SIGINT, intr);
                            	(void)xsignal(SIGPIPE, lostpeer);
                            
                            	/*
                            	 * Loop through as long as there's files to fetch.
                            	 */
                            	for (rval = 0; (rval == 0) && (argpos < argc); argpos++) {
                            ...
                            


                            Да, запутано и не «в лучших традициях», но всё же rval не будет иметь неопределенного значения. Видимо, ваш анализатор не учитывает как выполняется setjmp().

                            PS: Было бы очень здорово если бы при публикации примеров были ссылки на оригинальные файлы (хотя бы пути в исходниках), а то пойди найди этот fetch.c в коде haiku — в мастере его уже нет…
                              0
                              У нас бывают задачи, когда нужно найти очень старый код по предупреждению. Указание имени исходника и имени функции (+немного кода) вполне достаточно. Полный путь будет слишком длинно.

                              Я читал в переписке разработчиков, что ftp компонент они просто выкинут из-за ошибок и возьмут другой. К привязке к коду не могу сказать. Но вы можете легко восстановить файл по истории коммитов.
                                +2
                                Они то, может, и выкинут.

                                Но Вы свою ошибку анализа setjmp(), а, быть может, и иных методов обработки исключений, исправлять запланировали ли?
                                  0
                                  Мы провели работу по улучшению анализатора по результатам проверки ещё 2 недели назад, когда и выдали первый отчёт команде разработчиков.
                                  Да, запутано и не «в лучших традициях», но всё же

                                  Наверняка что-то осталось. Но если Ваш код нельзя описать этим предложением, то результаты анализа будут лучше. Как и мнение коллег по цеху)
                                    0
                                    Просто как-то странно, что статья «Как выстрелить себе в ногу...» начинается с выстрела в свою ногу, с явно ошибочного и ложного сообщения анализатора.

                                    Согласно стандартов C/C++, а setjmp() часть стандартов, этот фрагмент кода не содержит обращения к неинициализированной переменной и имеет вполне определённое поведение.

                                    Какая-то антиреклама получается.

                                    P.S. А, скажем, в таком коде:
                                    int cnt;
                                    try {
                                        for(cnt = 0; cnt < n; cnt++) {
                                            ...
                                        }
                                    } catch(...) {
                                        return(cnt);
                                    }
                                    

                                    Он тоже обнаружит неинициализированную переменную?!
                                      +3
                                      P.S. Извините, частично был гражданин соврамши, заглянул в стандарт следует читать: «и имел бы вполне определённое поведение при наличии квалификатора “volatile”» (C11, 7.13.2.1 The longjmp function).
                                0
                                В чём путаница и нарушение традиций? Некоторые, конечно, пишут иначе:
                                if(0 == setjmp(buf, 1)) {
                                    for(cnt = 0; cnt < n; cnt++) {
                                        ...
                                    }
                                } else {
                                    return(cnt);
                                }
                                
                                  0
                                  Я не зря поставил «традиции» в кавычки, но в целом ваш пример хорошо показывает как было бы лучше — всё же более логично если обработка исключений идёт после кода который может их вызывать, а не до (в пределах одной функции, по крайней мере).

                                  А путаница — setjmp()/longjmp() имеют массу подводных камней (да ещё и в сочетании с обработчиками сигналов), их использование как poor man's exception handling несколько опасно и чревато боком. Впрочем, если принять во внимание остальной код из Haiku, это наименьшая из их проблем…
                                –3

                                Вообще-то после вызова delete содержимое памяти по указанном адресу останется прежним, поэтому его еще можно безопасно использовать. Проблемы могут возникнуть только после следующего вызова new.
                                </irony-mode-off>

                                  0
                                  Это сильно зависит от деаллокатора и даже вполне вероятно
                                  содержимое памяти по указанном адресу останется прежним
                                  не факт, но скорее всего
                                  его еще можно безопасно использовать
                                  при некоторых условиях — да. Поскольку память выделяется страницами, если на той-же странице ещё остались выделенные блоки то ко всему содержимому страницы ещё можно «безопасно» обращаться. Увы это не долго продержится, и если это был последний блок в странице, то первое же обращение выдаст ошибку
                                  Проблемы могут возникнуть только после следующего вызова new
                                  не только, если страницу освободят то возникнут до этого, если удаляли ещё несколько объектов, то и после new могут не возникнуть.

                                  Самое плохое, что в таком «особом» режиме обращения к освобождённой памяти программа может очень долго работать без ошибок. А когда ошибка возникнет — то её не так просто повторить. Есть способы отловить такую ошибку — как простые, например менеджер памяти, который выделяет по странице на объект (считаем что объект меньше страницы), так и более продвинутые.
                                    0

                                    Большое спасибо за развернутое разъяснение. Абсолютно согласен с вами насчет того, что это не просто выстрел в ногу, это скорее мина в рюкзаке, готовая взорваться от любой встряски.


                                    Помнится, когда еще не было таких хороших компиляторов и статических анализаторов, я писал свои кастомные free, malloc, delete и new, где принудительно после выделения памяти, заполнял ее символами ‘+’, а после освобождения — символами ‘-‘, и это позволяло быстро обнаружить такие косяки, как и выход за границы буфера.


                                    Потом, когда я все эти косяки отловил, то переписал весь свой код на смартпоинтерах, чтобы вообще нигде явно не вызывать ни delete, ни free.

                                    0
                                    Это хорошо, но не очень интересно читать как анализатор отпределеяет разименование NULL.
                                    А есть сложные, интересные ошибки? Такие что с первого взгляда и не кажутся ошибками, а скорее ложными срабатываниями анализатора, или например ошибки которые сложно обнаружить «методом пристального взгляда» но анализатор их находит? Конечно такую подборку сделать сложно, но кажется, она была бы полезна всем. В том числе и в качестве качественной рекламы.
                                    Заранее прошу прощения если такая подборка уже есть, за всем не уследить.

                                  Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

                                  Самое читаемое