n0mo Jan 25 2022 at 12:54

PascalABC.NET, повторная проверка

8 min

PVS-Studio corporate blog.NET*Visual Studio*C#*Development for Windows*

Comments 10

VBKesha Jan 25 2022 at 15:23

Начну с классических ошибок типа copy-paste. Невероятно, но программисты раз за разом допускают эти досадные ошибки

На днях я видел как такую ошибку допустил схемотехник, там всё веселей получилось и с дымом :)

pankraty Jan 26 2022 at 06:14

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

Как-то это плохо стыкуется с утверждениями из статей про внедрение PVS Studio на больших проектах, о том, что можно запустить анализ, пометить всё ошибки как "базовый уровень", и в последующих анализах видеть только новые ошибки. Я думал, это делается в несколько кликов.

n0mo Jan 26 2022 at 09:19

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

Вот только я не вхожу в группу разработки PascalABC.NET. Если бы было по-другому, то конечно, ещё в 2017 году те старые сообщения можно было бы подавить и уже не отвлекаться на них сегодня. А ещё лучше - исправить самые очевидные ошибки, а потом подавить оставшееся как "технический долг".

В статье же я как раз описал неэффективный сценарий использования анализатора время от времени. Причем в моем случае перерыв составил несколько лет :)

pankraty Jan 26 2022 at 09:36

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

n0mo Jan 26 2022 at 10:29

Честно говоря, я даже не подумал про такой сценарий, так как был уверен, что за 5 лет те старые и такие очевидные ошибки будут исправлены.

LifeSweetener Jan 26 2022 at 08:58

Хорошая статья про статический анализ, Сергей!) И ещё Вы анализировали Паскаль - это круто! Старый добрый школьный ПаскальABC)

miksmiks Jan 28 2022 at 14:54

В своё время проект PascalABC.NET запомнился мне интересным общением с разработчиками, с которыми мы случайно пересеклись на одной из конференций сразу после выхода статьи

Приятно встретиться с Вами снова на страницах Хабра! Действительно, пять лет прошло с момента конференции "Языки программирования и компиляторы", которую мы организовывали в 2017 году. Я помню тот замечательный мастер-класс, который проводили представители PVS Studio для участников конференции и студентов ЮФУ. Кажется, что это было вчера.

Помню, что тогда PVS-анализатор для C# только только возник, а сейчас через 5 лет наверняка произошло много изменений.

Наибольший интерес ваша сегодняшняя статья для нас - конечно в анализе новых ошибок и неточностей, сделанных за эти 5 лет. Сконцентрируемся на тех, которые описаны в статье.

1. Действительно, там должен быть флаг dotnet5macos - исправили.
2.

if (.... && (ctn2.compiled_type == TypeFactory.ObjectType ||
      (ctn2.compiled_type == TypeFactory.ObjectType ||
       ctn2.compiled_type.IsInterface)))

Здесь явная ошибка. Отлично, что PVS её нашёл. Сходу не стали исправлять - будем разбираться.
3. Лишний SymKind.field. Да, убрали, но там код, нужный только для отображения при трассировке - он используется раз в 2 года и абсолютно незначим.
4.

if(scope is ClassMethodScope)
    scope = scope.TopScope;
else
    scope = scope.TopScope;

Это забавная неточность. Когда-то у нас были два вида TopScope, потом решили обойтись одним, а код остался исторически. Даже жалко убирать - закомментировали на память )
5.

catch(Exception e)
  {
    e = e;
  }

Ну, это немного грязно - чтобы посмотреть тип исключения при трассировке ) Закомментировали. В Release всё равно оптимизатор это обрежет.
6. Код:

if (t == null)
  {
    ....
    if (t != null)
      t.Nodes.Add(tn);
    else
      nodes.Add(tn);
    ....
  }

это - вы правы - результат рефакторинга. Исправили - убрали невыполняющуюся ветвь.
7. Вот здесь

 if (.... || fn.return_value_type.original_generic == to || ....
      && fn.return_value_type != null && ....)

здорово, что PVS это нашёл - глазом бы не заметили - выражение сложное. Подумаем, как аккуратно это исправить
8. volatile - абсолютно верно - исправили
9. Форматирование - неверное. Код - верный. VS так выравнивает по предыдущей строчке. При быстром наборе не уследили. Но код верный
10. Нетривиально. Я не могу пока сказать, ошибка это или нет. Код сля лямбда выражений сложен. Поразбираемся.
11. Possible null dereference

symbolInfo.FirstOrDefault().sym_info

Да, отлично! Там этот код в нескольких местах. Скорее всего, результат неверного рефакторинга. А студент 4 года назад за эту работу получил "отлично" :)

По поводу старых ошибок - посмотрим обязательно. Сейчас краски стёрлись, но помню, что мы их обсуждали и практически все отметили как незначительные неточности. Помнится, много ошибок нашлось в OpenSource компонентах, которые писали не мы. Например, в ICSharpCode :) Там мы не знаем, как исправлять ))

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

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

n0mo Jan 28 2022 at 17:41

Спасибо и Вам за тёплые слова и развёрнутый ответ с комментариями к ошибкам.

А с .NET и его версиями мы тоже регулярно сражаемся :) Вот, например, небольшая статья из нашего блога про поддержку .NET5.

Firensis Jan 28 2022 at 18:11

Не могли бы вы, пожалуйста, привести пример ошибки, связанной с учётом особенностей .NET Framework? К примеру, в анализаторе есть правило V5614, которое учитывает версию .NET Framework при исследовании кода на уязвимость к XXE. Вы имеете ввиду подобные случаи или что-то другое?

miksmiks Jan 28 2022 at 22:17

Прочитал вашу статью про версии .NET. Проблемы у нас немного другие. Но ощущения похожие )

Поскольку PascalABC.NET частично ориентирован на школы, а там старая техника с Windows XP, приходится использовать возможности версии .NET 4.0. Иногда мы про это забывали, и делали перегруженные вызовы методов, в которых в NET 4.0 не было тех параметров, которые добавили в старщих версиях. В результате код, связанный с некоторыми возможностями языка или библиотеками, переставал работать. Наиболее показательна была ошибка, когда мы при инициализации системного модуля делали вызов, которого нет в NET 4.0. И всё - весь PascalABC.NET переставал работать на части компьютеров
Чтобы учесть отсутствие различных контант в перечислимых типах NET 4.0, приходится в инициализирующей части системного модуля писать такой код:
var defaultCulture := typeof(System.Globalization.CultureInfo).GetProperty('DefaultThreadCurrentCulture');
if defaultCulture <> nil then
defaultCulture.SetValue(nil, new System.Globalization.CultureInfo('en-US'), nil);
На этот код почему-то реагируют некоторые антивирусы, ложно показывая немыслимые вирусы в пустой откомпилированной программе ) Мы с этим боремся по-другому...
Известный всем пример - замена типа System.Tuple на System.ValueTuple в какой-то версии .NET привела нас в уныние. Дело в том, что наши кортежи были заточены именно под System.Tuple. Промучившись месяц, мы приняли решение не переводить кортежи на System.ValueTuple, обеспечить совместимость со старыми версиями .NET и с гордостью говорить, что кортежи появились у нас раньше чем в C# ))
Мы сделали много методов расширения последовательностей, а потом некоторые из них стали появляться в NET 5.0. Когда мы таки перейдём на неё, код стандартного модуля вступит в конфликт с этими методами расширения, поскольку методы расширения действуют глобально, минуя пространства имён. Мы конечно этого ждём, и пушки не зачехлены, но бой может оказаться неравным.
Под Mono некоторые вещи не компилируются в принципе - например, вся WPF. Так вот - в один прекрасный момент мы переходили на HighDPI и добавили в наш графический модуль GraphABC один единственный вызов из WPF, связанный с проверкой коэффициента масштабирования экрана. И не проверили это под Linux. И всё. Вся графика под Linux перестала работать.

Вот такие вот проблемы. Вообщем, как говорится, зануда это тот, кто на вопрос "как дела?" начинает отвечать, как у него дела ))