Алексей Горшков

04 Мар 2024

Теги:

#Cpp

Проверяем Blender

04 Мар 2024

Автор: Алексей Горшков

Несколько слов о проекте
Результаты проверки
Прозрачные баги
Проблемы при разыменовании
Условие... результат никогда не меняется...
Я хочу сыграть с вами в одну игру... На внимательность
Больше неопределённого поведения богу неопределённого поведения!
Использование неинициализированных данных
Очень странные дела
Потерянные в пространстве имён
Заключение

Командный центр PVS-Studio: "Как быстро летит время... А ведь в этом году, второго января, Blender исполнилось 30 лет! Как будто ещё вчера мы публиковали статью с разбором ошибок... Как 8 лет назад? Надо срочно исправлять ситуацию!".

Несколько слов о проекте

Blender — проект, безусловно, культовый, и заслужил с течением лет всеобщий интерес и признание. Случилось это благодаря тому факту, что с 2000 года, существуя как Open Source решение, он позволяет любому художнику, хотя бы малость знакомому с инструментом, воплотить в X-Y-Z-axis-реальность свои самые смелые фантазии.

Также у него очень приятный и удобный интерфейс, с которым хочется работать. В наличии такие рендеры как Eevee и Cycles, которые могут сделать красивое. А если хочется ещё больше и разнообразнее, много чего можно загуглить и установить. Вообще, для Blender существует категорически много разнообразных аддонов, которыми можно полностью закрыть все потребности.

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

Да, давным-давно пару раз у меня Blender отваливался, благо присутствует автосохранение. Именно тогда я понял: чтобы сюрпризов не случалось, надо как следует научиться пользоваться этим инструментом. Благо YouTube содержит кучу бесплатных курсов и отдельных видео о том, как сделать ту или иную, э... штуку. Как уже было сказано, проект культовый.

Так, о чём это я? Баги! Не то чтобы всё постоянно отваливалось. Нет! Работает всё стабильно, и работать можно по 12-14 часов в день, и всё хорошо! Но время от времени ваша искорёженная во все стороны анимация как бы намекааает, что-то не так. Думаю, любой, кто когда-либо пользовался или пользуется Blender, со мной согласится. Особенно если не уследил за параметрами Transform, в частности Scale — это база, это знать надо!

Коммит, на котором я клонировал проект: 76092ac.

Дисклеймер open icon

Написание и публикация этой статьи не имеют цели обесценить труд программистов, занимающихся разработкой рассматриваемого продукта. Мне он очень нравится! Цель — популяризация статического анализатора, нахождение ошибок и передача отчёта разработчикам для улучшения качества кода проекта.

Результаты проверки

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

Так что далее, в присущей мне манере, представляю вам наиболее интересные участки кода Blender, в которых, по мнению статического анализатора PVS-Studio, есть подозрительные места.

Пристёгивайтесь, и да начнётся разбор!

Прозрачные баги

Вдохновляясь идеей прозрачного кода, я разобрал некоторые проблемные места кода Blender и открыл для себя концепцию прозрачных багов: "Идея прозрачных багов проста. Такого рода система прозрачна тогда, когда вы можете взглянуть на её код и понять, где баги, что они делают и как".

Фрагмент N1

void Cryptomatte::begin_sync()
{
  const eViewLayerEEVEEPassType enabled_passes = 
                                               
    static_cast<eViewLayerEEVEEPassType>(
      inst.film.enabled_passes_get() &
      (  EEVEE_RENDER_PASS_CRYPTOMATTE_OBJECT | 
         EEVEE_RENDER_PASS_CRYPTOMATTE_ASSET  |
         EEVEE_RENDER_PASS_CRYPTOMATTE_ASSET ) );
  ....
  
}

Предупреждение анализатора: V501 There are identical sub-expressions 'EEVEE_RENDER_PASS_CRYPTOMATTE_ASSET' to the left and to the right of the '|' operator. eevee_cryptomatte.cc 18

Очевидный баг: два раза используется одна и та же константа EEVEE_RENDER_PASS_CRYPTOMATTE_ASSET при получении побитовой маски. Скорее всего, должна использоваться другая константа — EEVEE_RENDER_PASS_CRYPTOMATTE_MATERIAL = (1 << 18).

Фрагмент N2

void MeshFromGeometry::create_edges(Mesh *mesh)
{
  ....
  for (int i = 0; i < tot_edges; ++i) 
  {
    ....
    dst_edge[0] = mesh_geometry_.global_to_local_vertices_
                                .lookup_default(src_edge[0], 0);
    dst_edge[1] = mesh_geometry_.global_to_local_vertices_
                                .lookup_default(src_edge[1], 0);
    BLI_assert(   dst_edge[0] < total_verts
               && dst_edge[0] < total_verts); 
  }
  ....
}

Предупреждение анализатора: V501 There are identical sub-expressions 'dst_edge[0] < total_verts' to the left and to the right of the '&&' operator. obj_import_mesh.cc 266

Левое и правое подвыражения логического "И" в BLI_assert абсолютно одинаковы. Скорее всего, во втором подвыражении должен был проверяться dst_edge[1] вместо dst_edge[0].

Фрагмент N3

static void get_nearest_fcurve_verts_list (bAnimContext *ac, 
                                           const int mval[2],
                                           ListBase *matches)
{
  ....
  filter = (ANIMFILTER_DATA_VISIBLE  |
            ANIMFILTER_CURVE_VISIBLE |         
            ANIMFILTER_FCURVESONLY   |     
            ANIMFILTER_NODUPLIS      | 
            ANIMFILTER_FCURVESONLY);       
  ....
}

Предупреждение анализатора: V501 There are identical sub-expressions 'ANIMFILTER_FCURVESONLY' to the left and to the right of the '|' operator. graph_select.cc 178

Форматирование кода таблицей — как одна из форм искусства. В этом примере всё очевидно!

Концепция прозрачности багов работает.

Ничего не надо объяснять.

Это прекрасно!

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

Проблемы при разыменовании

"Интересным примером разыменования нулевых указателей в случае срабатывания диагностического правила V522 является тот факт, что разыменование — лишь последствие ошибки, а не причина возникновения".

Разыменование нулевых указателей — проблема довольно частая. Особенно из-за логических ошибок в условиях кода программы. Как эти баги устроены? Как их найти? У нас с анализатором есть ответы на эти вопросы, а также парочка интересных примеров для нашего разбора.

Фрагмент N4

static bool gpencil_stroke_eraser_is_occluded (tGPsdata *p, bGPDlayer *gpl,
                                               bGPDspoint *pt, const int x,
                                               const int y)
{
  Object *obact = (Object *)p->ownerPtr.data;
  Brush *brush = p->brush;
  Brush *eraser = p->eraser;
  BrushGpencilSettings *gp_settings = nullptr;

  if (brush->gpencil_tool == GPAINT_TOOL_ERASE) 
  {
    gp_settings = brush->gpencil_settings;
  }
  else 
  if ((eraser != nullptr) &                               // <=
           (eraser->gpencil_tool == GPAINT_TOOL_ERASE)) 
  {
    gp_settings = eraser->gpencil_settings;
  }

  if ((gp_settings != nullptr) && 
      (gp_settings->flag & GP_BRUSH_OCCLUDE_ERASER) ) {
    RegionView3D *rv3d = static_cast<RegionView3D *>(p->region->regiondata);

    ....
  return false;
}

Предупреждение анализатора: V522 Dereferencing of the null pointer 'eraser' might take place. Check the bitwise operation. gpencil_paint.cc 1429

Как мы можем заметить, с проверкой указателя и его разыменовыванием проблем нет. Если, конечно, не обращать внимание на символ &, гордо разделяющий два подвыражения в скобках. Конечно же, здесь должен быть использован двойной амперсант && как в условии ниже. Однако код именно таков, каков он и есть: оператор побитового "И" исполнит также и правое подвыражение, которое разыменует потенциальный нулевой указатель.

Кстати, в этом же файле присутствует ещё одно такое же срабатывание:

V522 Dereferencing of the null pointer 'eraser' might take place. Check the bitwise operation. gpencil_paint.cc 1821

Фрагмент N5

void ui_draw_popover_back (ARegion *region, uiStyle * /*style*/, 
                           uiBlock *block,  rcti *rect          )
{
  ....
  if (block) 
  {
    float mval_origin[2] = {float(block->bounds_offset[0]), 
                            float(block->bounds_offset[1])};
    ui_window_to_block_fl (region, block, &mval_origin[0], &mval_origin[1]);
    ui_draw_popover_back_impl (wt->wcol_theme, rect, block->direction,
                               U.widget_unit / block->aspect, mval_origin);
  }
  else 
  {
    const float zoom = 1.0f / block->aspect;               // <=
    wt->state (wt, &STATE_INFO_NULL, UI_EMBOSS_UNDEFINED);
    wt->draw_block (&wt->wcol, rect, 0, 0, zoom);
  }
  ....
}

Предупреждение анализатора: V522 Dereferencing of the null pointer 'block' might take place. interface_widgets.cc 5294

В этом кусочке кода есть определённая проблема в логике. Если block == nullptr, то выполнится ветка else, в которой произойдёт гарантированное разыменование нулевого указателя block.

И такое срабатывание не одно:

V522 Dereferencing of the null pointer 'em' might take place. transform.cc 2117
V522 Dereferencing of the null pointer 'mesh' might take place. MOD_cloth.cc 108
V522 Dereferencing of the null pointer 'data.mval_fl' might take place. editmesh_select.cc 801

Условие... результат никогда не меняется...

"Безумие — это точное повторение одного и того же действия. Раз за разом, в надежде на изменение. Это есть безумие", — Ваас, FarCry 3.

Довольно символично: у нас есть баг, мы пытаемся вызвать функцию, а она не вызывается, код правильный, что же не так? Бегаем по одному и тому же коду уже полчаса и не понимаем, в чём дело. Чудеса, да и только! А потом, неожиданно для самих себя, находим проблему в том месте, где и не думали её встретить. Ошибки в логике условий. Потому что часто логику переусложняют, а в голову и мысли не приходит о том, что мы сделали что-то не так.

Фрагмент N6

void BLI_threadpool_init(ListBase *threadbase,
                         void *(*do_thread)(void *),
                         int tot)
{
  ....
  if (threadbase != nullptr && tot > 0) 
  {
    ....
    if (tot > RE_MAX_THREAD) 
    {
      tot = RE_MAX_THREAD;
    }
    else if (tot < 1)    // <=
    {
      tot = 1;
    }
    ....
  }
  ....
}

Предупреждение анализатора: V547 Expression 'tot < 1' is always false. threads.cc 131

Тут всё просто: если заглянуть в первое условие, то мы найдём проверку tot > 0. При этом tot имеет тип int, и если оно больше 0, то меньше 1 уже быть не может. Соответственно, условие tot < 1 всегда будет false. В итоге код можно упростить, полностью убрав эту проверку.

Фрагмент N7

enum
{
  ALIGN_WORLD = 0,
  ALIGN_VIEW,
  ALIGN_CURSOR,
};

bool ED_object_add_generic_get_opts(bool *r_is_view_aligned, ....)
{
  ....
    if (RNA_struct_property_is_set(op->ptr, "rotation")) 
    {
      ....
    }
    else 
    {
      int alignment = ALIGN_WORLD;
      PropertyRNA *prop = RNA_struct_find_property(op->ptr, "align");

      if (RNA_property_is_set(op->ptr, prop)) 
      {
        *r_is_view_aligned = alignment == ALIGN_VIEW; 
        alignment = RNA_property_enum_get(op->ptr, prop);
      }
    }
  ....
}

Оставлю вам возможность самостоятельно догадаться, в чём здесь проблема.

Предупреждение анализатора: V547 Expression 'alignment == ALIGN_VIEW' is always false. object_add.cc 544

Так, о чём это я? Поводом, возможно, послужила опечатка, а убийцей был дворецкий. Переменная r_is_view_aligned имеет тип bool *, и проверка действительно всегда равна false. Причём, скорее всего, выражение справа от оператора сравнения == должно быть другим, или всё же так и задумано? Абсолютно непонятная ситуация, но главное, что проблему нашли, а там уже и до исправления недалеко. Мелочь, а приятно!

Дополнительные срабатывания:

V547 Expression 'changed == false' is always true. editmesh_tools.cc 1456
V547 Expression 'ob->type == OB_GPENCIL_LEGACY' is always true. gpencil_edit.cc 130
V547 Expression 'closure_count > i' is always true. eevee_raytrace.cc 371
V547 Expression 'retval == OPERATOR_FINISHED' is always false. wm_gizmo_group.cc 490

Я хочу сыграть с вами в одну игру... На внимательность

Фрагмент N8

void BKE_gpencil_stroke_copy_settings(const bGPDstroke *gps_src,
                                      bGPDstroke *gps_dst)
{
  gps_dst->thickness = gps_src->thickness;
  gps_dst->flag = gps_src->flag;
  gps_dst->inittime = gps_src->inittime;
  gps_dst->mat_nr = gps_src->mat_nr;
  copy_v2_v2_short(gps_dst->caps, gps_src->caps);
  gps_dst->hardness = gps_src->hardness;
  copy_v2_v2(gps_dst->aspect_ratio, gps_src->aspect_ratio);
  gps_dst->fill_opacity_fac = gps_dst->fill_opacity_fac; // <=
  copy_v3_v3(gps_dst->boundbox_min, gps_src->boundbox_min);
  copy_v3_v3(gps_dst->boundbox_max, gps_src->boundbox_max);
  gps_dst->uv_rotation = gps_src->uv_rotation;
  copy_v2_v2(gps_dst->uv_translation, gps_src->uv_translation);
  gps_dst->uv_scale = gps_src->uv_scale;
  gps_dst->select_index = gps_src->select_index;
  copy_v4_v4(gps_dst->vert_color_fill, gps_src->vert_color_fill);
}

Предупреждение анализатора: V570 The 'gps_dst->fill_opacity_fac' variable is assigned to itself. gpencil_legacy.cc 1029

Переменная gps_dst->fill_opacity_fac присваивается сама себе. Может, так и было задумано? Нет. Скорее всего, имеет место быть опечатка, и в правой части присвоения должно быть другое выражение. Что-то типа gps_src->fill_opacity_fac. Проблема Copy/Paste?

Фрагмент N9

static int gizmo_cage2d_modal(....)
{
  ....
  if ((transform_flag & ED_GIZMO_CAGE_XFORM_FLAG_SCALE_UNIFORM) == 0) 
  {
    const bool use_temp_uniform = (event->modifier & KM_SHIFT) != 0;
    const bool changed = data->use_temp_uniform != use_temp_uniform;
    data->use_temp_uniform = data->use_temp_uniform;
    ....
  }
  ....
}

Предупреждение анализатора: V570 The 'data->use_temp_uniform' variable is assigned to itself. cage2d_gizmo.cc 1094

И снова проблема Copy/Paste. Скорее всего, в итоге всё должно выглядеть так:

data->use_temp_uniform = use_temp_uniform;

Фрагмент N10

static void space_text_update_drawcache(SpaceText *st,
                                        const ARegion *region)
{
  ....
  if (st->wordwrap) 
  {
    ....
    if (drawcache->update) 
    {
      drawcache->valid_tail = drawcache->valid_head = 0;
      ....
      memmove(new_tail, old_tail, drawcache->valid_tail);
      ....
    }
    ....
  }
  ....
}

V575 The 'memmove' function processes '0' elements. Inspect the third argument. text_draw.cc 673

Странно, что мы пытаемся копировать из одной области памяти в другую область памяти 0 байт информации.

Фрагмент N11

void ui_but_value_set(uiBut *but, double value)
{
  ....
    if (but->editval) {
      value = *but->editval = value;
    }
    else
    ....
  ui_but_update_select_flag(but, &value);
}

Предупреждение анализатора: V570 The same value is assigned twice to the 'value' variable. interface.cc 2676

Как вы думаете, так было задумано, или это тоже опечатка? Оставлю вопрос открытым, знаете, как в плохих фильмах ужасов.

Больше неопределённого поведения богу неопределённого поведения!

"Ктулху фхтагн, дорогие мои! И правда, кто ещё, как ни спящий, может подойти на роль бога неопределённого поведения? Однако будьте осторожнее в своих действиях, вы же не хотите его пробуждения раньше назначенного часа? В коде уже и так много хаоса!"

Фрагмент N12

static void rigidbody_update_ob_array(RigidBodyWorld *rbw)
{
  if (rbw->group == nullptr) 
  {
    rbw->numbodies = 0;
    rbw->objects = static_cast<Object **>(realloc(rbw->objects, 0)); // <=
    return;
  }
  ....
}

V575 The 'realloc' function processes '0' elements. Inspect the second argument. rigidbody.cc 1696

Начиная со стандарта С23, если размер памяти для перераспределённого участка памяти равен нулю, то поведение считается неопределённым. Кстати, с С17 такой паттерн воспринимается устаревшим.

Фрагмент N13

void BKE_vfont_build_char(....)
{
  ....
  BezTriple *bezt2 = (BezTriple *)MEM_malloc_arrayN(u, 
                                                    sizeof(BezTriple),
                                                    "duplichar_bezt2" );
  ....
  for (int i = nu2->pntsu; i > 0; i--) 
  {
    float *fp = bezt2->vec[0];    
    fp[0] = (fp[0] + ofsx) * fsize;
    fp[1] = (fp[1] + ofsy) * fsize;
    fp[3] = (fp[3] + ofsx) * fsize;
    fp[4] = (fp[4] + ofsy) * fsize;
    fp[6] = (fp[6] + ofsx) * fsize;
    fp[7] = (fp[7] + ofsy) * fsize;
    bezt2++;
  } 
  ....
}

Предупреждение анализатора: V557 Array overrun is possible. The '7' index is pointing beyond array bound. vfont.cc 612

Также представляю вашему вниманию сам массив:

typedef struct BezTriple 
{
  float vec[3][3];
  ....
}

В этом примере диагностическое правило выдало достаточно много срабатываний на каждое присвоение, начиная с fp[3]. С двумерным массивом bezt2->vec[3][3] производятся операции как будто с одномерным. До сих пор некоторые программисты считают, что они при этом делают всё правильно, и проблем с доступом последовательно размещённых в памяти элементов двумерного массива не бывает.

Однако новейшие и мощнейшие технологии компиляторов для языков С и С++, называемые оптимизациями, дарят высшую награду в номинации "Программист года в области неведения неопределённого поведения" верующим в истину о правильности использования двумерных массивов как одномерных.

Как-то мой коллега Антон Третьяков опубликовал статью "Ква! Как писали код во времена Quake", в которой был подобный пример, который как раз объяснял, почему использовать многомерные массивы как одномерные неправильно. Советую всем прочесть. Также мой коллега Филипп Хандельянц — он же один из моих наставников и дядька, который знает почти всё — воспроизвёл проблему неопределённого поведения. Также рекомендую ознакомиться.

В общем-то, всё доказано, всё проверено. Неопределённое поведение имеет место быть из-под различных оптимизаций. Всем советую так не делать. Идём дальше.

Использование неинициализированных данных

"Основная проблематика поиска неинициализированных переменных часто состоит в том, что кода много, ход мыслей относительно того, где используются переменная и её данные теряется на фоне происходящих параллельно вычислений и изменений в коде, а если эти данные ещё и постоянно передаются из функции в функцию, не меняя своего внутреннего состояния и уже потом где-то там возможно что-то с ними происходит, и переменная все же инициализируется... или нет."

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

Фрагмент N14

static void gpencil_convert_spline(....)
{
  ....
  float init_co[3];

  switch (nu->type) {
    case CU_POLY: 
    {
      ....
    }
    case CU_BEZIER: 
    {
      ....
    }
    case CU_NURBS: 
    {
      if (nu->pntsv == 1) 
      {
        ....
        gpencil_add_new_points (gps, coord_array, 1.0f, 1.0f, 0, 
                                gps->totpoints, init_co, false); // <=
        ....
    }
    default: 
    {
      break;
    }
}

Предупреждение анализатора: V614 Uninitialized buffer 'init_co' used. Consider checking the seventh actual argument of the 'gpencil_add_new_points' function. gpencil_curve_legacy.cc 439

Так вот, если быстро: нашлась init_co, она же float init_co[3].

Если проверить всю функцию, в которой эта переменная-массив объявлена, до того места, где она передаётся в gpencil_add_new_points(), никакой инициализации её каким-либо значением мы не увидим. Рандомные данные. Окей, возможно, она инициализируется внутри этой функции? Смотрим:

static void gpencil_add_new_points(....,
                                   const float init_co[3],
                                   const bool last)
{
  BLI_assert(totpoints > 0);
  ....
  for (int i = 0; i < totpoints; i++) 
  {
    ....
    if ((last) && (i > 0) && (i == totpoints - 1)) 
    {
      float dist = len_v3v3(init_co, &pt->x);
      ....
    }
    ....
  }
}

Наш мини-массив теперь уже передаётся в функцию len_v3v3, и какими-либо значениями до этого его не заполнили. Опять рандом.

Прыгаем внутрь len_v3v3 и ищем init_co, которая теперь a[3]:

static __forceinline float len_v3v3(const float a[3], const float b[3])
{
  float d[3];

  sub_v3_v3v3(d, b, a);
  return len_v3(d);
}

И ещё глубже в sub_v3_v3v3 за a[3], которая уже b[3]:

Static __forceinline void sub_v3_v3v3(float r[3], const float a[3], 
                                      const float b[3] )
{
  r[0] = a[0] - b[0];
  r[1] = a[1] - b[1];
  r[2] = a[2] - b[2];
}

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

Дополнительные срабатывания:

V614 Uninitialized variable 'efd.distance' used. boids.cc 133
V614 Potentially uninitialized pointer 'g_prev' used. Consider checking the third actual argument of the 'blf_font_width_to_strlen_glyph_process' function. blf_font.cc 784
V614 Uninitialized variable 'dummy_matrix[0][0]' used. Consider checking the first actual argument of the 'GPU_uniform' function. node_shader_tex_coord.cc 43

Очень странные дела

Фрагмент N15

typedef struct Point2Struct
{
  double coordinates[2];

  Point2Struct() { .... }
  ....
} Point2;

typedef Point2 Vector2;

using BezierCurve = Vector2 *;

static BezierCurve GenerateBezier(Vector2 *d,
                                  int first, int last,
                                  double *uPrime,
                                  Vector2 tHat1, Vector2 tHat2)
{
  ....
  BezierCurve bezCurve; /* RETURN bezier curve control points. */
  ....
  bezCurve = (Vector2 *)malloc(4 * sizeof(Vector2)); // <=
  ....
  if (alpha_l < 1.0e-6 || alpha_r < 1.0e-6)
  {
    ....
    bezCurve[0] = d[first]; // <=
    bezCurve[3] = d[last];  // <=
    ....
  }
  ....
}

Предупреждение анализатора: V630 The 'malloc' function is used to allocate memory for an array of objects which are classes containing constructors. FitCurve.cpp 129

Функция malloc используется для создания массива четырёх объектов типа Point2 (Vector2 есть псевдоним Point2). Однако класс Point2 содержит нетривиальный конструктор по умолчанию, который зануляет массив coordinates. Следовательно, в коде содержится неопределённое поведение: к моменту присвоения время жизни этих объектов ещё не стартовало.

На текущий момент компиляторы не пользуются такой возможностью для хитрых оптимизаций. Однако всё может измениться в будущем :).

Как это можно исправить? Самым правильным вариантом будет использование оператора new[]. Однако код состоит из смеси языков C и С++, и его логика благополучно сломается, если мы начнём использовать оператор new[].

К тому же использовать операторы new и delete уже давно не модно и даже опасно ввиду такой человеческой черты как невнимательность. Очевидный вариант — умные указатели. И вроде бы проблема решена, и можно на этом закончить. Но использование умных указателей также приведёт к тому, что код придётся переписывать.

Какие ещё варианты? Есть способ починить всё, ничего не переписывая:

static BezierCurve GenerateBezier(Vector2 *d,
                                  int first, int last,
                                  double *uPrime,
                                  Vector2 tHat1, Vector2 tHat2)
{
  ....
  BezierCurve bezCurve; /* RETURN bezier curve control points. */
  ....
  bezCurve = (Vector2 *)malloc(4 * sizeof(Vector2));
  std::uninitialized_default_construct_n(bezCurve, 4); // <=
  ....
  if (alpha_l < 1.0e-6 || alpha_r < 1.0e-6)
  {
    ....
    bezCurve[0] = d[first]; 
    bezCurve[3] = d[last]; 
    ....
  }
  ....
}

Проблема неопределённого поведения относительно времени жизни объектов решена. Подробнее об этом функционале можно почитать здесь.

Касательно C++20. Внимательный читатель может знать о проползале P0593R6, благодаря которому поведение программы, начиная с C++20, будет строго определено.

Потерянные в пространстве имён

Фрагмент N16

Но сначала, так как случай непростой и очень запутанный, предупреждение анализатора:

V762 It is possible a virtual function was overridden incorrectly. See first argument of function 'preferred_domain' in derived class 'HandlePositionFieldInput' and base class 'CurvesFieldInput'. node_geo_input_curve_handles.cc 95

Итак, из текста предупреждения видно, что проблема у нас в неправильном переопределении виртуальной функции в произвольных классах. Что не так? Давайте разбираться.

Начнём с базового класса:

typedef struct CurvesGeometry { .... };

namespace bke
{
  ....
  class CurvesGeometry : public ::CurvesGeometry { .... };

  class CurvesFieldInput : public fn::FieldInput 
  {
    ....
    virtual std::optional<AttrDomain> preferred_domain(
      const CurvesGeometry &curves) const;
  };
  ....
}

Виртуальная функция preferred_domain принимает параметр типа bke::CurvesGeometry. Запомним.

Теперь посмотрим на наследника:

namespace blender::nodes::node_geo_input_curve_handles_cc
{
  class HandlePositionFieldInput final : public bke::CurvesFieldInput 
  {
    ....
    std::optional<AttrDomain> preferred_domain(
      const CurvesGeometry & /*curves*/) const;
  };
}

Нашли проблему? Если нет, то не расстраивайтесь, я тоже сначала не понял :). Будем разбираться.

В базовом классе виртуальная функция принимает параметр с неквалифицированным именем CurvesGeometry. Когда компилятор будет осуществлять поиск этого типа, он начнёт с области видимости класса CurvesFieldInput и будет заглядывать во все обрамляющие области видимости, пока не встретит этот тип. В итоге будет найден тип bke::CurvesGeometry.

Теперь посмотрим на производные классы. Они определены в пространстве имён, отличном от того, где располагается базовый класс. Компилятор также начнёт поиск нужного имени CurvesGeometry, не встретит его в обрамляющих областях видимости и дойдёт до глобального. А в глобальном пространстве имён тоже есть CurvesGeometry, только не тот, что нам нужен для переопределения функции :).

Для исправления надо всего лишь указать квалифицированное имя типа. Ну и воспользуемся возможностями C++11 (override), защитив будущие поколения от ошибок:

namespace blender::nodes::node_geo_input_curve_handles_cc
{
  class HandlePositionFieldInput final : public bke::CurvesFieldInput 
  {
    ....
    std::optional<AttrDomain> preferred_domain(
      const bke::CurvesGeometry & /*curves*/) const override;
  };
}

По соседству располагаются также другие два класса, в которых содержится та же ошибка:

V762 It is possible a virtual function was overridden incorrectly. See first argument of function 'preferred_domain' in derived class 'IndexOnSplineFieldInput' and base class 'CurvesFieldInput'. node_geo_curve_spline_parameter.cc 277
V762 It is possible a virtual function was overridden incorrectly. See first argument of function 'preferred_domain' in derived class 'EndpointFieldInput' and base class 'CurvesFieldInput'. node_geo_curve_endpoint_selection.cc 105

Заключение

Ощущение от проверки проекта осталось двоякое. С одной стороны, встретилось много дополнительных мер защиты: часто умных, хитрых и вообще красивых. С другой стороны, даже анализатор PVS-Studio время от времени не может понять причинно-следственных связей этого лабиринта кода, где пол — это лава.

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

Традиционно предлагаю попробовать наш анализатор PVS-Studio. Для Open Source проектов у нас предоставляется бесплатная лицензия.

Берегите себя и всего доброго!

Бонус для тех, кто дочитал до конца =)

Смотреть картинки: open icon

"Изгоняем бога неопределённого поведения (Ктулху)"