PVS-Studio в гостях у Apache Hive

15 ИЮНЯ 2022

------------------------------------------------------------------------

Наш телеграм-канал: t.me/@mailsgun_online

Youtube: youtube.com/@mailsgun_online

------------------------------------------------------------------------

Последние десять лет движение open source является одним из ключевых факторов развития IT-отрасли и важной ее составной частью. Роль и место open source не только усиливается в виде роста количественных показателей, но происходит и изменение его качественного позиционирования на IT-рынке в целом. Не сидя сложа руки, бравая команда PVS-Studio активно способствует закреплению позиций open source проектов, находя затаившиеся баги в огромных толщах кодовых баз и предлагая для таких проектов бесплатные лицензии. Эта статья не исключение! Сегодня речь пойдет об Apache Hive! Отчет получен — есть на что посмотреть!

О PVS-Studio

Статический анализатор кода PVS-Studio существует более 10 лет на IT рынке и представляет собой многофункциональное и легко внедряемое программное решение. На данный момент анализатор поддерживает языки C, C++, C#, Java и работает на платформах Windows, Linux и macOS.

PVS-Studio является платным B2B-решением и используется большим количеством команд в различных компаниях. Если хотите посмотреть, на что способен анализатор, то скачать дистрибутив и запросить триальный ключ можно здесь.

Если Вы гик open source или, например, являетесь студентом, то вы можете воспользоваться одним из бесплатных вариантов лицензирования PVS-Studio.

Об Apache Hive

Объем данных в последние годы растет с большой скоростью. Стандартные базы данных больше не могут поддерживать работоспособность при таком темпе роста объёма информации, что послужило появлению термина Big Data и всего, что с ним связано (обработка, хранение и все вытекающие действия с такими объемами данных).

На данный момент Apache Hadoop считается одной из основополагающих технологий Big Data. Главные задачи этой технологии — хранение, обработка и управление большими объемами данных. Основными составляющими фреймворка являются Hadoop Common, , Hadoop MapReduce, Hadoop YARN. Со временем вокруг Hadoop образовалась целая экосистема из связанных проектов и технологий, многие из которых развивались изначально в рамках проекта, а впоследствии стали самостоятельными. Одним из таких проектов и является Apache Hive.

Apache Hive представляет собой распределенное хранилище данных. Оно управляет данными, которые хранятся в HDFS, и предоставляет язык запросов на базе SQL(HiveQL) для работы с этими данными. Для подробного ознакомления с этим проектом можно изучить информацию здесь.

Об анализе

Последовательность действий для анализа достаточно проста и не потребовала много времени:

  • Получил Apache Hive с ;
  • Воспользовался инструкцией по запуску Java-анализатора и запустил анализ;
  • Получил отчет анализатора, проанализировал его и выделил интересные случаи.

Результаты анализа: 1456 предупреждений уровня достоверности High и Medium (602 и 854, соответственно) были выданы для 6500+ файлов.

Не все предупреждения являются ошибками. Это нормальная ситуация и перед регулярным использованием анализатора требуется его настройка. После чего можно ожидать достаточно низкий процент ложных срабатываний (пример).

Среди предупреждений не рассматривались 407 предупреждений (177 High и 230 Medium), приходящихся на файлы с тестами. Не рассматривалось диагностическое правило V6022 (сложно отделить ошибочные ситуации от корректных в незнакомом коде), у которого было аж 482 предупреждения. V6021 c 179 предупреждениями тоже не рассматривалось.

В конечном итоге все равно осталось достаточное количество предупреждений. А поскольку я не настраивал анализатор, то среди них опять-таки есть ложные срабатывания. Нет смысла описывать большое количество предупреждений в статье :). Рассмотрим то, что мне попалось на глаза и показалось интересным.

Предопределенность условий

Диагностическое правило V6007 — рекордсмен среди всех оставшихся предупреждений анализатора. Чуть более 200 предупреждений!!! Какие-то, вроде, безобидные, некоторые — подозрительные, а другие и вовсе реальные ошибки! Давайте разберем некоторые из них.
V6007 Expression ‘key.startsWith(«hplsql.»)’ is always true. Exec.java(675)

Довольно продолжительная конструкция if-else-if! Анализатор ругается на последний if (key.startsWith(«hplsql.»)), указывая на его истинность в случае достижения программой этого фрагмента кода. Ведь правда, если глянуть в самое начало конструкции if-else-if, то проверка уже была осуществлена. И в случае, если наша строка не начиналась с подстроки «hplsql.», то выполнение кода сразу же перескакивало на следующую итерацию.
V6007 Expression ‘columnNameProperty.length() == 0’ is always false. OrcRecordUpdater.java(238)

Сравнение длины строки columnNameProperty с нулем всегда нам будет возвращать false. Это происходит из-за того, что наше сравнение находится под проверкой !Strings.isNullOrEmpty(columnNameProperty). Если состояние программы дойдет до нашего рассматриваемого условия, то строка columnNameProperty гарантированно будет ненулевой и не пустой.

Это актуально и для строки columnTypeProperty. Предупреждение строкой ниже:
V6007 Expression ‘colOrScalar1.equals(«Column»)’ is always false. GenVectorCode.java(3469)
}

Здесь банальный copy-paste. Получилось так, что строка colOrScalar1 должна одновременно равняться разным значениям, а это невозможно. По всей видимости слева должна проверяться переменная colOrScalar1, а справа colOrScalar2.

Еще схожие предупреждения строками ниже:
Как итог, никакие действия в конструкции if-else-if никогда выполняться не будут.

Некоторые другие предупреждения V6007:

  • и так далее…

V6008 Potential null dereference of ‘dagLock’. QueryTracker.java(557), QueryTracker.java(553)

Отловили нулевой объект, залогировали и… продолжили работать. Это приводит к тому, что следом за проверкой объекта происходит разыменование нулевого объекта. Печалька!

Скорее всего в случае нулевой ссылки следовало сразу выйти из функции или сгенерировать какое-то специальное исключение.
V6008 Null dereference of ‘buffer’ in function ‘unlockSingleBuffer’. MetadataCache.java(410), MetadataCache.java(465)

И снова потенциальный NPE. Если выполнение программы достигнет метода unlockSingleBuffer, то объект buffer будет нулевым. Допустим, это произошло! Заглянем в метод unlockSingleBuffer и сразу на первой строчке видим, что происходит разыменование нашего объекта. Вот мы и попались!

Не уследили за сдвигом

V6034 Shift by the value of ‘bitShiftsInWord — 1’ could be inconsistent with the size of type: ‘bitShiftsInWord — 1’ = [-1… 30]. UnsignedInt128.java(1791)

Возможное смещение на -1. В случае, если на вход рассматриваемого метода придут, например, wordShifts == 3 и bitShiftsInWord == 0, то в указанной строке произойдет 1 << -1. Запланировано ли это?
V6034 Shift by the value of ‘j’ could be inconsistent with the size of type: ‘j’ = [0… 63]. IoTrace.java(272)

В указанной строке переменная j может принимать значение в диапазоне [0… 63]. Из-за этого вычисление значения val в цикле может происходить не так, как задумывал разработчик. В выражении (1 << j) единица имеет тип int, и, смещая ее от 32 и более, мы выходим за рамки допустимого. Для исправления ситуации необходимо написать .

Увлеклись логированием

V6046 Incorrect format. A different number of format items is expected. Arguments not used: 1, 2. StatsSources.java(89)

При форматировании строки через String.format() разработчик спутал синтаксис. Итог: в результирующую строку переданные параметры так и не попали. Могу предположить, что в предыдущей задаче разработчик работал над логированием, откуда и позаимствовал синтаксис.

Украли исключение

V6051 The use of the ‘return’ statement in the ‘finally’ block can lead to the loss of unhandled exceptions. ObjectStore.java(9080)

Возвращать что-либо из блока finally — очень плохая практика, и на этом примере мы в этом убедимся.

В блоке try происходит формирование запроса и обращение к хранилищу. Переменная committed по умолчанию имеет значение false и меняет свое состояние только после всех успешно выполненных действий в try блоке. Это означает, что в случае возникновения исключения наша переменная будет всегда false. Catch блок словил исключение, немного скорректировал и пробросил далее. И когда приходит время блока finally, выполнение заходит в условие, из которого возвращаем наружу пустой список. Чего стоит нам это возвращение? А стоит это того, что все пойманные исключения никогда не будут выброшены наружу и обработаны соответствующим способом. Все те исключения, что указаны в сигнатуре метода, никогда не будут выброшены и просто сбивают с толку.

Подобное предупреждение:

… прочее

V6009 Function ‘compareTo’ receives an odd argument. An object ‘o2.getWorkerIdentity()’ is used as an argument to its own method. LlapFixedRegistryImpl.java(244)

Copy-paste, невнимательность, спешка и много других причин сделать эту глупую ошибку. При проверках open source проектов ошибки такого рода встречаются довольно часто. Есть даже целая статья про это.
V6020 Divide by zero. The range of the ‘divisor’ denominator values includes zero. SqlMathUtil.java(265)

Здесь достаточно все просто. Ряд проверок не предостерегли от деления на 0.

Еще предупреждения:
V6030 The method located to the right of the » operator will be called regardless of the value of the left operand. Perhaps, it is better to use ». OperatorUtils.java(573)

Вместо логического оператора написали битовый оператор . Это означает, что правая часть будет выполнена независимо от результата левой части. Такая опечатка, в случае parents == null, сразу же приведет к NPE в следующем логическом подвыражении.
V6042 The expression is checked for compatibility with type ‘A’ but is cast to type ‘B’. VectorColumnAssignFactory.java(347)

Рассматриваемые классы LongColumnVector extends ColumnVector и TimestampColumnVector extends ColumnVector. Проверка нашего объекта destCol на принадлежность LongColumnVector явно нам указывает, что внутри условного оператора будет именно объект этого класса. Несмотря на это, у нас происходит каст к TimestampColumnVector! Как видно, эти классы разные, не считая их общего родителя. Как итог — ClassCastException.

Все то же самое можно сказать и про приведение типа к IntervalDayTimeColumnVector:
V6060 The ‘var’ reference was utilized before it was verified against null. Var.java(402), Var.java(395)

Странное сравнение объекта var с null после того, как произошло разыменование. В данном контексте var и obj — один и тот же объект (var = (Var)obj). Проверка на null подразумевает, что может прийти нулевой объект. И в случае вызова equals(null) мы получим сразу же на первой строке NPE вместо ожидаемого false. Увы, проверка есть, но не там.

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

  • и так далее…

Заключение

Кто хоть чуть-чуть интересовался Big Data, тот вряд ли пропустил мимо своих ушей значимость Apache Hive. Проект популярный и достаточно большой, и в своем составе имеет более 6500 файлов исходного кода (*.java). Код писался многими разработчиками долгие годы и, как следствие, статическому анализатору есть что находить. Это еще раз подтверждает то, что статический анализ крайне важен и полезен при разработке средних и больших проектов!

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

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

Невозможно представить Apache Hive без Apache Hadoop, поэтому вполне вероятно, что единорог от PVS-Studio заглянет и туда. Но на сегодня это все, а пока скачивайте анализатор и проверяйте собственные проекты.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Maxim Stefanov. PVS-Studio Visits Apache Hive.

Source: habr1

Ну и напоследок напоминаю вам о нашем новом проекте vkserfing bot.