Toп 10 ошибок в C++ проектах за 2017 год

    Picture 1


    За окном уже почти как 3 месяца стоит 2018 год, а это значит, что пришло время (пусть и немного запоздало) составить топ 10 ошибок, найденных анализатором PVS-Studio в C++ проектах за прошедший год. Итак, начнём!

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

    Десятое место

    Источник: Notepad++: проверка кода пять лет спустя

    Ошибка была обнаружена при проверке одного из самых известных текстовых редакторов — Notepad++.

    Фрагмент кода, содержащий ошибку:

    TCHAR GetASCII(WPARAM wParam, LPARAM lParam)
    {
      int returnvalue;
      TCHAR mbuffer[100];
      int result;
      BYTE keys[256];
      WORD dwReturnedValue;
      GetKeyboardState(keys);
      result = ToAscii(static_cast<UINT>(wParam),
        (lParam >> 16) && 0xff, keys, &dwReturnedValue, 0);
      returnvalue = (TCHAR) dwReturnedValue;
      if(returnvalue < 0){returnvalue = 0;}
      wsprintf(mbuffer, TEXT("return value = %d"), returnvalue);
      if(result!=1){returnvalue = 0;}
      return (TCHAR)returnvalue;
    }

    Предупреждение PVS-Studio: V560 A part of conditional expression is always true: 0xff. babygrid.cpp 711

    Анализатор счёл подозрительным выражение (lParam >> 16) && 0xff. Второй аргумент, передаваемый в функцию ToAscii, всегда будет иметь значение 0 или 1, причём результирующее значение зависит только от левого подвыражения — (lParam >> 16). Очевидно, что вместо оператора && должен был использоваться оператор &.

    Девятое место

    Источник: Передаю привет разработчикам компании Yandex

    На девятом месте расположилась ошибка из проекта ClickHouse, разрабатываемого компанией Yandex.

    bool executeForNullThenElse(....)
    {
      ....
      const ColumnUInt8 * cond_col =
        typeid_cast<const ColumnUInt8 *>(arg_cond.column.get());
      ....
      if (cond_col)
      {
        ....
      }
      else if (cond_const_col)
      {
        ....
      }
      else
        throw Exception(
          "Illegal column " + cond_col->getName() +
          " of first argument of function " + getName() +
          ". Must be ColumnUInt8 or ColumnConstUInt8.",
          ErrorCodes::ILLEGAL_COLUMN);
      ....
    }

    Предупреждение PVS-Studio: V522 Dereferencing of the null pointer 'cond_col' might take place. FunctionsConditional.h 765

    В данном коде неправильно обрабатывается ошибочная ситуация, когда необходимо сгенерировать исключение. Обратите внимание на указатель cond_col. Для него в операторе if выполняется проверка, что указатель — ненулевой. Если управление дошло до else ветви, где генерируется исключение, указатель cond_col точно нулевой. Однако при формировании сообщения исключения, cond_col разыменовывается в выражении cond_col->getName().

    Восьмое место

    Источник: Сравнение качества кода Firebird, MySQL и PostgreSQL

    На восьмом месте расположилась одна из ошибок, найденных в проекте MySQL во время сравнения качества кода Firebird, MySQL и PostgreSQL.

    Код метода, содержащего ошибку:

    mysqlx::XProtocol* active()
    {
      if (!active_connection)
        std::runtime_error("no active session");
      return active_connection.get();
    }

    Предупреждение PVS-Studio: V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw runtime_error(FOO); mysqlxtest.cc 509

    В случае отсутствия активного соединения (!active_connection) создаётся объект исключения типа std::runtime_error и… на этом всё. После создания он будет попросту удалён, при этом продолжится выполнение метода. Очевидно, что разработчик забыл ключевое слово throw, чтобы сгенерировать исключение.

    Седьмое место

    Источник: Как найти 56 потенциальных уязвимостей в коде FreeBSD за один вечер

    Как найти 56 потенциальных уязвимостей за вечер? С помощью статического анализа, конечно же!

    Одна из проблем, обнаруженных в коде FreeBSD:

    int mlx5_core_create_qp(struct mlx5_core_dev *dev,
          struct mlx5_core_qp *qp,
          struct mlx5_create_qp_mbox_in *in,
          int inlen)
    {
      ....
      struct mlx5_destroy_qp_mbox_out dout;
      ....
    err_cmd:
      memset(&din, 0, sizeof(din));
      memset(&dout, 0, sizeof(dout));
      din.hdr.opcode = cpu_to_be16(MLX5_CMD_OP_DESTROY_QP);
      din.qpn = cpu_to_be32(qp->qpn);
      mlx5_cmd_exec(dev, &din, sizeof(din), &out, sizeof(dout));
    
      return err;
    }

    Предупреждение PVS-Studio: V597 The compiler could delete the 'memset' function call, which is used to flush 'dout' object. The memset_s() function should be used to erase the private data. mlx5_qp.c 159

    Обратите внимание на выражение memset(&dout, 0, sizeof(dout)). Разработчик хотел «затереть» данные в блоке памяти, соответствующем dout, выставив нулевые значения. Обычно такой подход используется, когда необходимо очистить какие-то приватные данные, чтобы они не «висели» в памяти.

    Однако дальше dout не используется (sizeof(dout) не в счёт), что позволит компилятору удалить приведённый выше вызов функции memset, т.к. подобная оптимизация не влияет на поведение программы с точки зрения C/C++. Как следствие — данные, которые должны были быть зачищены, могут так и остаться в памяти.

    Чтобы глубже погрузиться в тему, я рекомендую для прочтения следующие статьи:


    Шестое место

    Источник: Долгожданная проверка CryEngine V

    Первый игровой движок, к коду которого мы обратимся в этом топе — CryEngine V.

    int CTriMesh::Slice(....)
    {
      ....
      bop_meshupdate *pmd = new bop_meshupdate, *pmd0;
      pmd->pMesh[0]=pmd->pMesh[1] = this;  AddRef();AddRef();
      for(pmd0=m_pMeshUpdate; pmd0->next; pmd0=pmd0->next);
        pmd0->next = pmd;
      ....
    }

    Предупреждение PVS-Studio: V529 Odd semicolon ';' after 'for' operator. boolean3d.cpp 1314

    Согласитесь, что если бы этот фрагмент не был выписан вот так вот — будучи сокращённым и отделённым от остального кода, не так легко было бы обнаружить в нём тот подозрительный участок, который нашёл анализатор — символ ';', завершающий цикл for. При этом форматирование кода (сдвиг следующего выражения) также наводит на мысли о том, что символ ';' тут лишний, а выражение pmd0->next = pmd; должно быть телом цикла. Но судя по логике цикла for, здесь именно неправильное форматирование кода, сбивающее с толку, а не логическая ошибка. В коде CryEngine форматирование кода, кстати, поправили.

    Пятое место

    Источник: Статический анализ как часть процесса разработки Unreal Engine

    Следующая ошибка была обнаружена в ходе работы над исправлением ошибок, найденных PVS-Studio в коде игрового движка Unreal Engine.

    for(int i = 0; i < SelectedObjects.Num(); ++i)
    {
      UObject* Obj = SelectedObjects[0].Get();
      EdObj = Cast<UEditorSkeletonNotifyObj>(Obj);
      if(EdObj)
      {
        break;
      }
    }

    Предупреждение PVS-Studio: V767 Suspicious access to element of 'SelectedObjects' array by a constant index inside a loop. skeletonnotifydetails.cpp 38

    В цикле хотели обойти все элементы и найти среди них первый, имеющий тип UEditorSkeletonNotifyObj. Но допустили досадную опечатку, использовав в выражении SelectedObjects[0].Get() константный индекс 0 вместо счётчика цикла i. Как результат — всегда будет проверяться только первый элемент.

    Четвёртое место

    Источник: 27000 ошибок в операционной системе Tizen

    Ошибка была найдена при проверке операционной системы Tizen, а также сторонних компонентов, используемых в ней. Статья большая, и в ней выписано много интересных примеров ошибок — настоятельно рекомендую ознакомиться.

    Но вернёмся к конкретному предупреждению:

    int _read_request_body(http_transaction_h http_transaction,
                           char **body)
    {
      ....
      *body = realloc(*body, new_len + 1);
      ....
      memcpy(*body + curr_len, ptr, body_size);
      body[new_len] = '\0';
      curr_len = new_len;
      ....
    }

    Предупреждение PVS-Studio: V527 It is odd that the '\0' value is assigned to 'char' type pointer. Probably meant: *body[new_len] = '\0'. http_request.c 370

    Ошибка кроется в выражении body[new_len] = '\0'. Обратите внимание, что параметр body имеет тип char**, соответственно, тип выражения body[new_len]char*. Но разработчик дал маху, и, забыв ещё одно разыменование, попытался записать в указатель значение '\0' (будет преобразовано в нулевой указатель).

    Это приводит к двум проблемам:

    • Куда-то (body[new_len]) будет записан нулевой указатель.
    • Терминальный ноль не будет записан в конец строки.

    Корректный код:

    (*body)[new_len] = '\0';

    Третье место

    Источник: Как PVS-Studio может помочь в поиске уязвимостей?

    Вот мы и подобрались к тройке лидеров. Приведённый ниже код был обнаружен в ходе поиска ответа на вопрос: «Как PVS-Studio справляется с поиском CVE?» (ответ смотрите в указанной выше статье). Код из проекта illumos-gate.

    static int devzvol_readdir(....)
    {
      ....
      char *ptr;
      ....
      ptr = strchr(ptr + 1, '/') + 1;
      rw_exit(&sdvp->sdev_contents);
      sdev_iter_datasets(dvp, ZFS_IOC_DATASET_LIST_NEXT, ptr);
      ....
    }

    Предупреждение PVS-Studio: V769 The 'strchr(ptr + 1, '/')' pointer in the 'strchr(ptr + 1, '/') + 1' expression could be nullptr. In such case, resulting value will be senseless and it should not be used.

    Функция strchr возвращает указатель на первое вхождение символа, заданного вторым аргументом, в строке, заданной первым аргументом. Если же такого символа не находится, strchr возвращает NULL. Однако этот факт не учитывается, и к возвращаемому значению всегда прибавляется значение '1'. Как результат, указатель ptr всегда будет ненулевым, а значит, дальнейшие проверки вида ptr != NULL не дадут информации о валидности указателя. В итоге, при определённых условиях данный код приводил к возникновению kernel panic.

    Данной ошибке был присвоен идентификатор CVE-2014-9491: The devzvol_readdir function in illumos does not check the return value of a strchr call, which allows remote attackers to cause a denial of service (NULL pointer dereference and panic) via unspecified vectors.

    Несмотря на то, что сама CVE была обнаружена в 2014 году, в ходе собственных исследований мы обнаружили эту ошибку в 2017, поэтому она попала в этот топ.

    Второе место

    Источник: Статический анализ как часть процесса разработки Unreal Engine

    Ошибка, расположившаяся на втором месте, была обнаружена… да, опять в Unreal Engine. Мне она показалась очень интересной, так что я не смог удержаться и не выписать её.

    Примечание. На самом деле, я бы выписал ещё парочку ошибок из приведённой выше статьи про Unreal Engine, но всё же не хочется так часто обращаться к одному и тому же проекту. Поэтому я настоятельно рекомендую посмотреть упомянутую выше статью самостоятельно, в частности — предупреждения V714 и V709.

    Дальше будет много кода, но он необходим, чтобы понять суть проблемы.

    bool FCreateBPTemplateProjectAutomationTests::RunTest(
      const FString& Parameters)
    {
      TSharedPtr<SNewProjectWizard> NewProjectWizard;
      NewProjectWizard = SNew(SNewProjectWizard);
    
      TMap<FName, TArray<TSharedPtr<FTemplateItem>> >& Templates =
        NewProjectWizard->FindTemplateProjects();
      int32 OutMatchedProjectsDesk = 0;
      int32 OutCreatedProjectsDesk = 0;
      GameProjectAutomationUtils::CreateProjectSet(Templates, 
        EHardwareClass::Desktop, 
        EGraphicsPreset::Maximum, 
        EContentSourceCategory::BlueprintFeature,
        false,
        OutMatchedProjectsDesk,
        OutCreatedProjectsDesk);
    
      int32 OutMatchedProjectsMob = 0;
      int32 OutCreatedProjectsMob = 0;
      GameProjectAutomationUtils::CreateProjectSet(Templates, 
        EHardwareClass::Mobile,
        EGraphicsPreset::Maximum,
        EContentSourceCategory::BlueprintFeature,
        false,
        OutMatchedProjectsMob,
        OutCreatedProjectsMob);
    
      return ( OutMatchedProjectsDesk == OutCreatedProjectsDesk ) &&
             ( OutMatchedProjectsMob  == OutCreatedProjectsMob  );
    }

    Отметим следующий важный момент, необходимый для понимания проблемы. Переменные OutMatchedProjectsDesk, OutCreatedProjectsDesk и OutMatchedProjectsMob, OutCreatedProjectsMob при объявлении инициализируются нулями, после чего передаются в качестве аргументов методу CreateProjectSet.

    После этого переменные сравниваются в выражении оператора return. Следовательно, метод CreateProjectSet должен выполнять инициализацию последних двух аргументов.

    Теперь же обратимся к методу CreateProjectSet, который и содержит ошибки.

    static void CreateProjectSet(.... int32 OutCreatedProjects,
                                      int32 OutMatchedProjects)
    {
      ....
      OutCreatedProjects = 0;
      OutMatchedProjects = 0;
      ....
      OutMatchedProjects++;
      ....
      OutCreatedProjects++;
      ....
    }

    Предупреждения PVS-Studio:
    • V763 Parameter 'OutCreatedProjects' is always rewritten in function body before being used. gameprojectautomationtests.cpp 88
    • V763 Parameter 'OutMatchedProjects' is always rewritten in function body before being used. gameprojectautomationtests.cpp 89

    Параметры OutCreatedProjects и OutMatchedProjects забыли сделать ссылками, и в итоге значения соответствующих аргументов просто копируются. Как результат — возвращаемое значение метода RunTest, приведённого выше, всегда true, так как все сравниваемые переменные имеют одно и то же значение, заданное при инициализации — 0.

    Правильный вариант кода:

    static void CreateProjectSet(.... int32 &OutCreatedProjects,
                                      int32 &OutMatchedProjects)

    Первое место

    Источник: Любите статический анализ кода!

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

    PUGI__FN bool set_value_convert(
      char_t*& dest,
      uintptr_t& header,
      uintptr_t header_mask,
      int value)
    {
      char buf[128];
      sprintf(buf, "%d", value);
    
      return set_value_buffer(dest, header, header_mask, buf);
    }

    Ну что, как успехи с поиском ошибки? :)

    Предупреждение PVS-Studio: V614 Uninitialized buffer 'buf' used. Consider checking the first actual argument of the 'printf' function. pugixml.cpp 3362

    Наверняка у вас возник вопрос: "printf? Откуда в предупреждении анализатора printf, если в коде есть вызов только функции sprintf?"

    В этом-то и суть! sprintf — это макрос, который раскрывается в (!) std::printf!

    #define sprintf std::printf

    В итоге неинициализированный буфер buf используется как строка форматирования. Здорово, не так ли? Я думаю, эта ошибка вполне себе заслуженно заняла первое место.

    Ссылка на заголовочный файл с объявлением макроса.

    Заключение


    Надеюсь, вам понравились собранные ошибки. Лично мне они показались достаточно интересными. Но, конечно, ваше видение может отличаться от моего, поэтому вы можете составить свой «Tоп 10...», почитав статьи из нашего блога или посмотрев список ошибок, найденных PVS-Studio в open source проектах.

    Picture 2


    Также напоминаю, что все приведённые в статье ошибки (а также множество других) были найдены с помощью анализатора PVS-Studio, который советую попробовать и на вашем проекте: ссылка на страницу загрузки.



    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Sergey Vasiliev. Top 10 Bugs in the C++ Projects of 2017

    Прочитали статью и есть вопрос?
    Часто к нашим статьям задают одни и те же вопросы. Ответы на них мы собрали здесь: Ответы на вопросы читателей статей про PVS-Studio, версия 2015. Пожалуйста, ознакомьтесь со списком.
    PVS-Studio
    Статический анализ кода для C, C++, C# и Java

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

      +8
      М--даааа… Это что, кто-то #define true false забабахал уходя?!
      Как, ну КААК иначе такие дефайны появляются, блин?
        +5
        Напомнило недавнее из javascript
        function if (x) {return true;}

        Здесь после if неразрывный пробел &nbsp;, который будет считаться частью идентификатора, плюс любовь js к автоматической расстановке точек с запятой, что позволяет этому работать.
        Конечно, не так злобно, как в примере в статье, зато если хочется нагадить — можно применять избирательно, только по особо сложным местам.
        +3
        Пример с CryEngine некорректен. Там, очевидно, всё правильно — новый элемент добавляется в конец односвязного списка. Цикл ищет последний элемент, довольно рапространённый паттерн. Предложенное вами «исправление» не имеет смысла.
          +1
          Только я не предлагал 'исправлений'. :)
          Возможно, недостоточно ясно выразился в статье.

          В коде CryEngine форматирование поправили, к слову.
            +1
            Если всё правильно, то форматирование плохое. Отступ намекает, что оператор принадлежит циклу, а если всё правильно, то это не так.
            –1
            Больше половины ошибок GCC выдаст в режиме -Wall -Wextra.

            Ошибку с sprintf, я думаю, он тоже покажет. Да даже если не покажет, то тут уж надо разуть глаза и посмотреть на код в IDE. Макросы подсвечиваются другим цветом, нежели функции. Ну, а если ты сделал одним цветом, то, как обычно, ССЗБ.
              +7
              Прошу продемонстрировать. Очень часто слышу про "-Wall -Wextra", но никто из авторов не удосуживается попробовать проверить свои предположения про магическую силу gcc/clang :).
                +1
                godbolt.org/g/4p3ZGg

                -Wall и -Wextra не помогают конечно, но помогает -Weverything
                  0
                  Хорошо, одна есть. Что касаемо «больше половины ошибок GCC выдаст в режиме ....»? :)
                    0
                    Это не моё утверждение. Я просто показал, что, при желании, компилятор кое-что может отловить.

                    Естественно тюнинг флагов компилятора под проект (чтобы не втыкать -Weverything) — это отдельная задача.
                      0
                      На if(); gcc ругается, а вот на for(); нет. Да, не много чего он ловит, но и на том спасибо (у меня была такая ошибка, а еще if(a=b)).
                      +1
                      Можно уточнить, а какое количество всего варнингов данный режим компилирования выдаст в сумме для протестированных исходников, чтоб реально понимать, можно ли там человеку будет увидеть конкретно эту ошибку?
                        –1
                        Можно не только уточнить, но и самостоятельно проверить. Заодно и сравнить с тем, сколько на указанных исходниках будет сообщений от, скажем, PVS-Studio, Clang Static Analyzer.

                        Любопытно будет прочитать про результаты такого теста и сравнения.
                  0
                  У второго места (ошибка в Unreal Engine) ещё перепутаны местами два последних аргумента в вызове, судя по их названиям. Впрочем, на работу функции это влияния не оказывает.

                  Объявление и вызов соответственно:
                  static void CreateProjectSet(… int32 OutCreatedProjects, int32 OutMatchedProjects);
                  GameProjectAutomationUtils::CreateProjectSet(… OutMatchedProjectsMob, OutCreatedProjectsMob);
                  • НЛО прилетело и опубликовало эту надпись здесь
                      0
                      А в примере из FreeBSD (седьмое место) действительно «повезло», что где-то выше объявлена переменная out, и вот эта строка
                      mlx5_cmd_exec(dev, &din, sizeof(din), &out, sizeof(dout));

                      компилируется без ошибки?
                      • НЛО прилетело и опубликовало эту надпись здесь
                          0
                          Тогда прошу вас выкинуть всю технику из дома имеющую больше 3 кнопок, ведь её мозги тоже написаны на крестах
                          0

                          В коде, который на пятом месте, IDE должна была подсветить неиспользуемую переменную. И банальный линтер в pre-commit hook выловил бы такое. Хорошо хоть, что разработчики решили использовать статический анализатор (как я понимаю, постфактум, то есть, на коде из репозитория).

                            0
                            for(int i = 0; i < SelectedObjects.Num(); ++i)
                            {
                              UObject* Obj = SelectedObjects[0].Get();
                              EdObj = Cast<UEditorSkeletonNotifyObj>(Obj);
                              if(EdObj)
                              {
                                break;
                              }
                            }

                            А где здесь неиспользуемая переменная?
                              0

                              Извиняюсь, протупил: i не используется только в теле цикла, но используется в объявлении.

                            0
                            executeForNullThenElse

                            На первый взгляд какой-то вывих мозга. Но полез в гит… И мозг свихнулся окончательно :) Особенно вот тут:

                            if (cond_col)
                                    {
                                        if (!( executeLeftType<UInt8>(cond_col, block, arguments, result)
                                            || executeLeftType<UInt16>(cond_col, block, arguments, result)
                                            || executeLeftType<UInt32>(cond_col, block, arguments, result)
                                            || executeLeftType<UInt64>(cond_col, block, arguments, result)
                                            || executeLeftType<Int8>(cond_col, block, arguments, result)
                                            || executeLeftType<Int16>(cond_col, block, arguments, result)
                                            || executeLeftType<Int32>(cond_col, block, arguments, result)
                                            || executeLeftType<Int64>(cond_col, block, arguments, result)
                                            || executeLeftType<Float32>(cond_col, block, arguments, result)
                                            || executeLeftType<Float64>(cond_col, block, arguments, result)
                                            || executeString(cond_col, block, arguments, result)
                                            || executeGenericArray(cond_col, block, arguments, result)
                                            || executeTuple(block, arguments, result)))
                                            throw Exception("Illegal columns " + arg_then.column->getName()
                                                + " and " + arg_else.column->getName()
                                                + " of second (then) and third (else) arguments of function " + getName(),
                                                ErrorCodes::ILLEGAL_COLUMN);
                                    }
                              0
                              Шутка с «не переходите к описанию проблемы», всё же, не очень хороша — кто-нибудь с хорошим знанием синтаксиса С++ может и спятить, как географ, не нашедший на карте Берингова пролива.
                                –1

                                И после этого кто-то говорит о том, что Rust не нужен… 8 из 10 таких ошибок в Rust просто не скомпилируются.

                                  0
                                  8 из 10 приведенных участков написаны на диалекте с++, который старше раста
                                    0

                                    То есть в современном C++ компиляция данного кода упадёт с ошибкой? Или вы имеете ввиду "старость надо уважать"?

                                      0
                                      Может и не собраться, да. Но суть скорее в размере кодовой базы которую нельзя просто взять и RIIRнуть
                                        0
                                        сегодня на с++ так уже не пишут, а когда писали — не было раста. Код на современном с++ в свою очередь менее подвержен ошибкам.
                                  • НЛО прилетело и опубликовало эту надпись здесь
                                    • НЛО прилетело и опубликовало эту надпись здесь
                                        0

                                        Не согласен по поводу CryEngine.


                                          for(pmd0=m_pMeshUpdate; pmd0->next; pmd0=pmd0->next);
                                            pmd0->next = pmd;

                                        Очевидно тут ищется конец списка и вставляется новый элемент в конец. В противном случае получится бесконечный цикл, поскольку после первого прохода pmd0 = pmd0->next <=> pmd0 = pmd, затем второй проход в теле цикла pmd0->next = pmd <=> pmd->next = pmd, привет infinite loop.


                                        Так что это баг автоматического форматирования, а не программиста.

                                          0
                                          Выше обсуждали.
                                          0
                                          Наверное, я отстал от моды, но как такое возможно?
                                          sprintf — это макрос, который раскрывается в (!) std::printf!

                                          Вот пишут, что, как и раньше,
                                          int sprintf ( char * str, const char * format,… );

                                          Иначе весь «legacy»-код можно выбросить на помойку. Разве нет?
                                            +1
                                            Вы же понимаете, что делает эта команда?
                                            #define sprintf std::printf
                                              –1
                                              Разумеется.
                                              Но кто, где и зачем такое написал, мне непонятно?
                                              Или это где-то в том же исходнике было, но автор умолчал?
                                              Что-то в стиле
                                              #define TRUE FALSE

                                                +2
                                                Это define из исходников.
                                                Мне показалось это очевидным, поэтому я не стал писать этот факт прямым текстом.
                                            0
                                            Подозреваю, что в примере с mlx5_core_create_qp() просто опечатка при вызове mlx5_cmd_exec(): если предпоследний аргумент &dout, всё встаёт на свои места (и sizeof(dout)). Иначе зачем dout, вообще, нужен?
                                            Приведённого куска кода явно недостаточно, чтобы сделать правильный вывод…
                                              0
                                              mlx5_core_create_qp()…
                                              т.к. подобная оптимизация не влияет на поведение программы с точки зрения C/C++

                                              Да ну, не может быть. Откуда компилятору знать, что memset не имеет побочных эффектов? Это можно понять только в рантайме. А если memset'у скормить адрес переменной из стека, а размер указать побольше, что бы записалась и следующая? А её уже использовать в коде? Понятно, что это страшный костыль, и того кто так делает к клавиатуре вообще подпускать нельзя, но тем не менее, так сделать можно. И компилятор не имеет права предполагать отстутствие побочных эффектов.
                                                +1
                                                Это будет undefined behavior.
                                                  0
                                                  Ну behavior, к сожалению, вполне себе defined, если известна целевая архитектура, а компилятору она известна. Иначе не работали бы так любимые всеми атаки на переполнение стекового буфера. Но не суть. Даже если и undefined — это все равно побочный эффект. А раз вызов функции может иметь побочные эффекты — удалять вызов нельзя. Компилятор может определить отсутствие побочных эффектов по синтаксическому дереву функции. Очевидно, что если, например, это const функция, внутри которой нет ничего подозрительного, типа обращения к mutable членам, или вызова внешних функций, то вызов можно вырезать. Но в случае memset, очевидно, присутствуют команды записи данных по адресу. Что бы понять, что такую запись можно удалить, компилятор по сути должен понять, что не важно, была сделана запись, или нет, программа дальше будет работать одинаково. Т.е. надо понимать, что после записи, этот адрес обязательно будет еще раз перезаписан, до попытки повторно из него прочитать. А еще есть такие вещи, как отображаемые в память файлы, порты ввода вывода. Т.е. если есть только код, который в память только пишет, но нет кода, который из неё обратно читает, это еще не значит, что запись можно не делать. Задача определения отсутствия побочных эффектов при наличии операций записи по адресу — очень крутая, в complie time её решить крайне трудно.
                                                    0
                                                    Это не так работает. Компилятор делает оптимизации исходя из того, что код написан правильно и UB не может возникнуть ни при каких входных данных.
                                                    То есть если написан memset локальной переменной, которая потом не читается, а размер приходит извне, то такой memset можно удалить.
                                                0

                                                Пардон, но:


                                                #define sprintf std::printf

                                                это в чьём коде? Без этих данных решить задачу нет возможности. Тем более, что в Gcc актуальных версий такого определения в cstdio нет, там сделано так:


                                                #undef sprintf
                                                namespace std
                                                {
                                                  using ::sprintf;
                                                }
                                                  0
                                                  видимо, этот дефайн был добавлен для отладки и его забыли убрать
                                                    0
                                                    Зачем фантазировать… В статье же есть отсылка к первоисточнику: "Любите статический анализ кода". Там показано, что это просто такой дурацкий стиль именования функций, с помощью буквы 's'. Примеры: #define sfstream std::fstream. Злополучный заголовочный файл: definesTypes.h.
                                                      0

                                                      Просто если лезть в статью, то как быть с пожеланием:


                                                      Ни за что не переходите к описанию проблемы, пока не найдёте ошибку в приведённом фрагменте кода.

                                                      ;-)

                                                        +2
                                                        Правильное пожелание, для демонстрации того, что ошибка может быть в другом месте, а конкретное место будет казаться правильным на 100%…
                                                        Они ж демонстрируют возможности анализатора.

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

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